Merge pull request #2052 from transformerlab/add/experiment-tags-design

Add tags for experiments

Author: deep1401

.agents/skills/transformerlab-cli/references/commands.md

@@ -76,6 +76,20 @@ Show current user, team, and server.
 
 ---
 
+## Experiment Commands
+
+### `experiment list [--tag <tag>]...`
+
+List experiments. Pass `--tag` one or more times to filter to experiments that have **all** of the given tags (AND semantics). Filtering is client-side.
+
+```bash
+lab experiment list --tag fine-tuning            # all experiments tagged fine-tuning
+lab experiment list --tag fine-tuning --tag llama  # must have both tags
+lab --format json experiment list --tag llama    # JSON output includes `tags` field
+```
+
+---
+
 ## Task Commands
 
 **All task commands require `current_experiment` to be set.**

api/test/test_experiment_tags_service.py

@@ -0,0 +1,159 @@
+import pytest
+from unittest.mock import AsyncMock, MagicMock, patch
+
+from transformerlab.services import experiment_service as svc
+
+
+def test_normalize_tags_lowercases_and_trims():
+    assert svc.normalize_tags(["  Foo ", "BAR"]) == ["foo", "bar"]
+
+
+def test_normalize_tags_deduplicates_preserving_order():
+    assert svc.normalize_tags(["foo", "Bar", "FOO", "bar"]) == ["foo", "bar"]
+
+
+def test_normalize_tags_allows_dot_dash_underscore_digits():
+    assert svc.normalize_tags(["fine-tune", "v1.0", "exp_2"]) == [
+        "fine-tune",
+        "v1.0",
+        "exp_2",
+    ]
+
+
+def test_normalize_tags_rejects_spaces_inside_tag():
+    with pytest.raises(ValueError, match="hello world"):
+        svc.normalize_tags(["hello world"])
+
+
+def test_normalize_tags_rejects_punctuation():
+    with pytest.raises(ValueError, match="bad!"):
+        svc.normalize_tags(["bad!"])
+
+
+def test_normalize_tags_rejects_unicode():
+    with pytest.raises(ValueError, match="café"):
+        svc.normalize_tags(["café"])
+
+
+def test_normalize_tags_rejects_over_32_chars():
+    long_tag = "a" * 33
+    with pytest.raises(ValueError, match="32"):
+        svc.normalize_tags([long_tag])
+
+
+def test_normalize_tags_accepts_exactly_32_chars():
+    long_tag = "a" * 32
+    assert svc.normalize_tags([long_tag]) == [long_tag]
+
+
+def test_normalize_tags_rejects_empty_after_strip():
+    with pytest.raises(ValueError):
+        svc.normalize_tags(["   "])
+
+
+def test_normalize_tags_empty_input_returns_empty_list():
+    assert svc.normalize_tags([]) == []
+
+
+def _mock_experiment(current_tags=None):
+    """Build a mock Experiment whose get_json_data returns config.tags=current_tags."""
+    mock_exp = MagicMock()
+    mock_exp.get_json_data = AsyncMock(return_value={"config": {"tags": list(current_tags) if current_tags else []}})
+    mock_exp.update_config_field = AsyncMock()
+    return mock_exp
+
+
+@patch("transformerlab.services.experiment_service.cache.invalidate", new_callable=AsyncMock)
+@patch("transformerlab.services.experiment_service.Experiment.get", new_callable=AsyncMock)
+async def test_experiment_add_tags_merges_with_existing(mock_get, _mock_cache):
+    mock_exp = _mock_experiment(current_tags=["foo"])
+    mock_get.return_value = mock_exp
+
+    result = await svc.experiment_add_tags("exp1", ["Bar", "BAZ"])
+
+    assert result == ["foo", "bar", "baz"]
+    mock_exp.update_config_field.assert_awaited_once_with("tags", ["foo", "bar", "baz"])
+
+
+@patch("transformerlab.services.experiment_service.cache.invalidate", new_callable=AsyncMock)
+@patch("transformerlab.services.experiment_service.Experiment.get", new_callable=AsyncMock)
+async def test_experiment_add_tags_is_idempotent(mock_get, _mock_cache):
+    mock_exp = _mock_experiment(current_tags=["foo", "bar"])
+    mock_get.return_value = mock_exp
+
+    result = await svc.experiment_add_tags("exp1", ["FOO", "bar"])
+
+    assert result == ["foo", "bar"]
+    mock_exp.update_config_field.assert_awaited_once_with("tags", ["foo", "bar"])
+
+
+@patch("transformerlab.services.experiment_service.cache.invalidate", new_callable=AsyncMock)
+@patch("transformerlab.services.experiment_service.Experiment.get", new_callable=AsyncMock)
+async def test_experiment_add_tags_handles_missing_tags_field(mock_get, _mock_cache):
+    mock_exp = MagicMock()
+    mock_exp.get_json_data = AsyncMock(return_value={"config": {}})
+    mock_exp.update_config_field = AsyncMock()
+    mock_get.return_value = mock_exp
+
+    result = await svc.experiment_add_tags("exp1", ["foo"])
+
+    assert result == ["foo"]
+
+
+@patch("transformerlab.services.experiment_service.Experiment.get", new_callable=AsyncMock)
+async def test_experiment_add_tags_enforces_max_cap(mock_get):
+    existing = [f"t{i}" for i in range(19)]
+    mock_exp = _mock_experiment(current_tags=existing)
+    mock_get.return_value = mock_exp
+
+    with pytest.raises(ValueError, match="20"):
+        await svc.experiment_add_tags("exp1", ["new1", "new2"])
+    mock_exp.update_config_field.assert_not_awaited()
+
+
+@patch("transformerlab.services.experiment_service.cache.invalidate", new_callable=AsyncMock)
+@patch("transformerlab.services.experiment_service.Experiment.get", new_callable=AsyncMock)
+async def test_experiment_remove_tags_removes_present(mock_get, _mock_cache):
+    mock_exp = _mock_experiment(current_tags=["foo", "bar", "baz"])
+    mock_get.return_value = mock_exp
+
+    result = await svc.experiment_remove_tags("exp1", ["Bar"])
+
+    assert result == ["foo", "baz"]
+    mock_exp.update_config_field.assert_awaited_once_with("tags", ["foo", "baz"])
+
+
+@patch("transformerlab.services.experiment_service.cache.invalidate", new_callable=AsyncMock)
+@patch("transformerlab.services.experiment_service.Experiment.get", new_callable=AsyncMock)
+async def test_experiment_remove_tags_absent_is_noop(mock_get, _mock_cache):
+    mock_exp = _mock_experiment(current_tags=["foo"])
+    mock_get.return_value = mock_exp
+
+    result = await svc.experiment_remove_tags("exp1", ["missing"])
+
+    assert result == ["foo"]
+    mock_exp.update_config_field.assert_awaited_once_with("tags", ["foo"])
+
+
+def test_aggregate_tags_dedupes_and_sorts():
+    experiments = [
+        {"id": "a", "config": {"tags": ["foo", "bar"]}},
+        {"id": "b", "config": {"tags": ["bar", "baz"]}},
+        {"id": "c", "config": {}},
+        {"id": "d", "config": {"tags": []}},
+    ]
+    assert svc.aggregate_tags(experiments) == ["bar", "baz", "foo"]
+
+
+def test_aggregate_tags_handles_string_config_blob():
+    experiments = [{"id": "a", "config": '{"tags": ["zeta", "alpha"]}'}]
+    assert svc.aggregate_tags(experiments) == ["alpha", "zeta"]
+
+
+def test_aggregate_tags_skips_non_string_entries():
+    experiments = [{"id": "a", "config": {"tags": ["foo", 42, None, "bar"]}}]
+    assert svc.aggregate_tags(experiments) == ["bar", "foo"]
+
+
+def test_aggregate_tags_empty_input():
+    assert svc.aggregate_tags([]) == []

api/transformerlab/routers/experiment/experiment.py

@@ -161,6 +161,15 @@ async def experiments_create(
     return newid
 
 
+@router.get("/tags", summary="List all distinct tags across permitted experiments", tags=["experiment"])
+async def experiments_list_all_tags(
+    session: AsyncSession = Depends(get_async_session),
+    user_and_team: dict = Depends(get_user_and_team),
+):
+    permitted = await experiments_get_all(session=session, user_and_team=user_and_team)
+    return {"tags": experiment_service.aggregate_tags(permitted)}
+
+
 @router.get("/{id}", summary="Get Experiment by ID", tags=["experiment"])
 async def experiment_get(
     id: str,
@@ -297,3 +306,39 @@ async def experiment_get_file_contents(
         return ""
 
     return file_contents
+
+
+@router.post("/{id}/tags/add", summary="Add tags to an experiment", tags=["experiment"])
+async def experiments_tags_add(
+    id: str,
+    body: Annotated[dict, Body()],
+    _: None = Depends(require_permission("experiment", "write")),
+):
+    tags_input = body.get("tags") or []
+    if not isinstance(tags_input, list):
+        raise HTTPException(status_code=422, detail="'tags' must be a list of strings")
+    try:
+        merged = await experiment_service.experiment_add_tags(id, tags_input)
+    except ValueError as e:
+        raise HTTPException(status_code=422, detail=str(e)) from e
+    except FileNotFoundError as e:
+        raise HTTPException(status_code=404, detail=f"Experiment {id} not found") from e
+    return {"tags": merged}
+
+
+@router.post("/{id}/tags/remove", summary="Remove tags from an experiment", tags=["experiment"])
+async def experiments_tags_remove(
+    id: str,
+    body: Annotated[dict, Body()],
+    _: None = Depends(require_permission("experiment", "write")),
+):
+    tags_input = body.get("tags") or []
+    if not isinstance(tags_input, list):
+        raise HTTPException(status_code=422, detail="'tags' must be a list of strings")
+    try:
+        kept = await experiment_service.experiment_remove_tags(id, tags_input)
+    except ValueError as e:
+        raise HTTPException(status_code=422, detail=str(e)) from e
+    except FileNotFoundError as e:
+        raise HTTPException(status_code=404, detail=f"Experiment {id} not found") from e
+    return {"tags": kept}

api/transformerlab/services/experiment_service.py

@@ -2,6 +2,7 @@
 import logging
 import json
 import os
+import re
 
 from sqlalchemy import delete
 from lab import Experiment
@@ -15,6 +16,36 @@
 logger = logging.getLogger(__name__)
 EXPERIMENT_LIST_CONCURRENCY = max(1, int(os.getenv("TLAB_EXPERIMENT_LIST_CONCURRENCY", "24")))
 
+_TAG_PATTERN = re.compile(r"^[a-z0-9._-]{1,32}$")
+TAG_MAX_LEN = 32
+TAGS_MAX_PER_EXPERIMENT = 20
+
+
+def normalize_tags(raw):
+    """Lowercase, trim, validate charset (a-z0-9._-, max 32 chars), and dedupe.
+
+    Raises ValueError on the first invalid tag.
+    """
+    seen = set()
+    result = []
+    for item in raw or []:
+        if not isinstance(item, str):
+            raise ValueError(f"Tag must be a string, got {type(item).__name__}: {item!r}")
+        normalized = item.strip().lower()
+        if not normalized:
+            raise ValueError("Tag is empty after trimming whitespace")
+        if len(normalized) > TAG_MAX_LEN:
+            raise ValueError(f"Tag {normalized!r} exceeds max length {TAG_MAX_LEN}")
+        if not _TAG_PATTERN.match(normalized):
+            raise ValueError(
+                f"Tag {normalized!r} contains invalid characters (allowed: lowercase a-z, 0-9, '.', '-', '_')"
+            )
+        if normalized in seen:
+            continue
+        seen.add(normalized)
+        result.append(normalized)
+    return result
+
 
 @cached(key="experiments:list", ttl="2m", tags=["experiments"])
 async def experiment_get_all():
@@ -188,3 +219,62 @@ async def experiment_update_configs(id, updates: dict):
         print(f"Experiment with id '{id}' not found")
     except Exception as e:
         print(f"Error updating experiment config: {e}")
+
+
+async def _read_current_tags(experiment_id):
+    exp = await Experiment.get(experiment_id)
+    data = await exp.get_json_data()
+    config = data.get("config", {})
+    if isinstance(config, str):
+        try:
+            config = json.loads(config)
+        except json.JSONDecodeError:
+            config = {}
+    raw = config.get("tags", []) or []
+    if not isinstance(raw, list):
+        return exp, []
+    return exp, [t for t in raw if isinstance(t, str)]
+
+
+async def experiment_add_tags(experiment_id, tags):
+    """Union-merge ``tags`` into the experiment's existing tags. Returns the full list."""
+    new_tags = normalize_tags(tags)
+    exp, current = await _read_current_tags(experiment_id)
+    merged = list(current)
+    for t in new_tags:
+        if t not in merged:
+            merged.append(t)
+    if len(merged) > TAGS_MAX_PER_EXPERIMENT:
+        raise ValueError(f"Cannot exceed {TAGS_MAX_PER_EXPERIMENT} tags per experiment (would be {len(merged)})")
+    await exp.update_config_field("tags", merged)
+    await cache.invalidate("experiments")
+    return merged
+
+
+async def experiment_remove_tags(experiment_id, tags):
+    """Set-difference ``tags`` from the experiment's existing tags. Returns the full list."""
+    to_remove = set(normalize_tags(tags))
+    exp, current = await _read_current_tags(experiment_id)
+    kept = [t for t in current if t not in to_remove]
+    await exp.update_config_field("tags", kept)
+    await cache.invalidate("experiments")
+    return kept
+
+
+def aggregate_tags(experiments):
+    """Return a sorted, deduped list of all tags across ``experiments``."""
+    bag = set()
+    for exp in experiments or []:
+        config = exp.get("config", {}) if isinstance(exp, dict) else {}
+        if isinstance(config, str):
+            try:
+                config = json.loads(config)
+            except json.JSONDecodeError:
+                config = {}
+        tags = config.get("tags", []) if isinstance(config, dict) else []
+        if not isinstance(tags, list):
+            continue
+        for t in tags:
+            if isinstance(t, str) and t:
+                bag.add(t)
+    return sorted(bag)