Skip to content

Remove the tag kind, now implied by the tag's collection - #2758

Open
gabrielfruet wants to merge 2 commits into
gabriel-lig-10097-rehome-tag-linksfrom
gabriel-lig-10097-drop-tag-kind
Open

gabrielfruet wants to merge 2 commits into
gabriel-lig-10097-rehome-tag-linksfrom
gabriel-lig-10097-drop-tag-kind

Conversation

@gabrielfruet

@gabrielfruet gabrielfruet commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

What has changed and why?

Stacked on #2757. A tag now holds only samples of its own collection, so kind repeats the collection's sample type. Creating a tag takes only {name}, and the PostHog samples_tagged event sends grid_type instead of tag_kind.

The GUI looks the same: unlike #2737, which this replaces, image and annotation views keep separate tags. The annotations page now clears its own tag selection on entry, not the root collection's.

The Postgres downgrade restores kind from each tag's collection, and sample for a tag whose collection was deleted. DuckDB has no migrations: an existing .db keeps a NOT NULL kind column and cannot create tags until re-indexed.

How has it been tested?

  • uv run pytest -n 8: 3246 passed. 3 failures and 4 collection errors need s3fs, adlfs or a built webapp.
  • uv run pytest tests/database --postgres: 169 passed, including a downgrade case for a tag of a deleted collection.
  • Frontend make static-checks: 0 errors. make test: 3336 passed. Not checked in a running GUI.

Did you update CHANGELOG.md?

  • Yes
  • Not needed (internal change)

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: lightly-ai/lightly-studio/.coderabbit.yaml
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: f1f37d89-9467-4f65-802f-d042a41046cb
📥 Commits

Reviewing files that changed from the base of the PR and between e11b4bb and 2484d70.

📒 Files selected for processing: 67
  • CHANGELOG.md
  • lightly_studio/src/lightly_studio/migrations/versions/1791482578_3220ed4b892e_drop_tag_kind.py
  • lightly_studio/src/lightly_studio/models/image.py
  • lightly_studio/src/lightly_studio/models/tag.py
  • lightly_studio/src/lightly_studio/resolvers/image_resolver/annotation_count_helpers/get_and_validate_sample_tags.py
  • lightly_studio/src/lightly_studio/resolvers/tag_resolver/get_or_create_sample_tag_by_name.py
  • lightly_studio/src/lightly_studio/resolvers/tag_resolver/rename.py
  • lightly_studio/src/lightly_studio/resolvers/tag_resolver/split_samples.py
  • lightly_studio/src/lightly_studio/services/sampling_service/validate_preselection.py
  • lightly_studio/tests/api/routes/api/test_collection_tag.py
  • lightly_studio/tests/api/routes/api/test_embeddings2d.py
  • lightly_studio/tests/api/routes/api/test_embeddings2d__color_by.py
  • lightly_studio/tests/api/routes/api/test_image_count_by_sample_tags.py
  • lightly_studio/tests/api/routes/api/test_sampling.py
  • lightly_studio/tests/benchmarks/tag_assignment_benchmark.py
  • lightly_studio/tests/conftest.py
  • lightly_studio/tests/core/dataset_query/test_dataset_query__add_tag.py
  • lightly_studio/tests/core/mcap/test_write_labels.py
  • lightly_studio/tests/database/test_db_migrations.py
  • lightly_studio/tests/helpers_resolvers.py
  • lightly_studio/tests/models/test_image.py
  • lightly_studio/tests/resolvers/annotations/test_delete_annotation.py
  • lightly_studio/tests/resolvers/group_resolver/test_get_all.py
  • lightly_studio/tests/resolvers/image_resolver/annotation_count_helpers/test_get_and_validate_sample_tags.py
  • lightly_studio/tests/resolvers/image_resolver/test_count_image_annotations_by_sample_tags.py
  • lightly_studio/tests/resolvers/image_resolver/test_get_all_by_dataset_id.py
  • lightly_studio/tests/resolvers/image_resolver/test_get_dimension_bounds.py
  • lightly_studio/tests/resolvers/sample_resolver/test_get_filtered_samples.py
  • lightly_studio/tests/resolvers/sample_resolver/test_sample_filter.py
  • lightly_studio/tests/resolvers/tag_resolver/test_add_sample_ids_to_tag_id.py
  • lightly_studio/tests/resolvers/tag_resolver/test_add_samples_to_tag_from_query.py
  • lightly_studio/tests/resolvers/tag_resolver/test_add_tag_to_sample.py
  • lightly_studio/tests/resolvers/tag_resolver/test_create.py
  • lightly_studio/tests/resolvers/tag_resolver/test_get_or_create_sample_tag_by_name.py
  • lightly_studio/tests/resolvers/tag_resolver/test_remove_tag_from_sample.py
  • lightly_studio/tests/resolvers/tag_resolver/test_rename.py
  • lightly_studio/tests/resolvers/test_annotation_resolver.py
  • lightly_studio/tests/resolvers/test_image_filter.py
  • lightly_studio/tests/sampling/test_sampling_via_db.py
  • lightly_studio_view/src/lib/components/AnnotationsGrid/AnnotationsGrid.svelte
  • lightly_studio_view/src/lib/components/Images/Images.svelte
  • lightly_studio_view/src/lib/components/Operator/OperatorDialog.svelte
  • lightly_studio_view/src/lib/components/PlotPanel/PlotPanel.svelte
  • lightly_studio_view/src/lib/components/PlotPanel/PlotPanel.test.ts
  • lightly_studio_view/src/lib/components/Sampling/PreselectedTagField/PreselectedTagField.test.ts
  • lightly_studio_view/src/lib/components/Sampling/SamplingCombinationDialog.test.ts
  • lightly_studio_view/src/lib/components/Sampling/useSamplingCombinationDialog/useSamplingCombinationDialog.ts
  • lightly_studio_view/src/lib/components/SegmentTags/SegmentTags.svelte
  • lightly_studio_view/src/lib/components/SegmentTags/SegmentTags.test.ts
  • lightly_studio_view/src/lib/components/TagsMenu/TagAssignInput.test.ts
  • lightly_studio_view/src/lib/components/TagsMenu/TagsMenu.svelte
  • lightly_studio_view/src/lib/components/TagsMenu/TagsMenu.test.ts
  • lightly_studio_view/src/lib/hooks/useAddTagToSample/useAddTagToSample.ts
  • lightly_studio_view/src/lib/hooks/useAnnotationsFilter/useAnnotationsFilter.ts
  • lightly_studio_view/src/lib/hooks/useCreateSampling/useCreateSampling.test.ts
  • lightly_studio_view/src/lib/hooks/useSubmitCombinationSelection/useSubmitCombinationSelection.test.ts
  • lightly_studio_view/src/lib/hooks/useTags/toggleTagSelection.test.ts
  • lightly_studio_view/src/lib/hooks/useTags/useTags.test.ts
  • lightly_studio_view/src/lib/hooks/useTags/useTags.ts
  • lightly_studio_view/src/lib/services/types.ts
  • lightly_studio_view/src/routes/datasets/[dataset_id]/[collection_type]/[collection_id]/ImageDistributionPanel/ImageDistributionPanel.svelte
  • lightly_studio_view/src/routes/datasets/[dataset_id]/[collection_type]/[collection_id]/annotations/+layout.ts
  • lightly_studio_view/src/routes/datasets/[dataset_id]/[collection_type]/[collection_id]/annotations/+page.svelte
  • lightly_studio_view/src/routes/datasets/[dataset_id]/[collection_type]/[collection_id]/annotations/page.test.ts
  • lightly_studio_view/src/routes/datasets/[dataset_id]/[collection_type]/[collection_id]/frames/+page.svelte
  • lightly_studio_view/src/routes/datasets/[dataset_id]/[collection_type]/[collection_id]/images/+layout.ts
  • lightly_studio_view/src/routes/datasets/[dataset_id]/[collection_type]/[collection_id]/videos/+page.svelte
💤 Files with no reviewable changes: 23
  • lightly_studio/tests/resolvers/annotations/test_delete_annotation.py
  • lightly_studio/tests/resolvers/test_image_filter.py
  • lightly_studio_view/src/lib/components/SegmentTags/SegmentTags.test.ts
  • lightly_studio_view/src/lib/hooks/useCreateSampling/useCreateSampling.test.ts
  • lightly_studio/tests/core/dataset_query/test_dataset_query__add_tag.py
  • lightly_studio/tests/conftest.py
  • lightly_studio_view/src/lib/hooks/useSubmitCombinationSelection/useSubmitCombinationSelection.test.ts
  • lightly_studio/tests/resolvers/group_resolver/test_get_all.py
  • lightly_studio/tests/resolvers/tag_resolver/test_rename.py
  • lightly_studio/tests/api/routes/api/test_sampling.py
  • lightly_studio/tests/resolvers/image_resolver/test_get_dimension_bounds.py
  • lightly_studio/tests/resolvers/test_annotation_resolver.py
  • lightly_studio/tests/resolvers/tag_resolver/test_get_or_create_sample_tag_by_name.py
  • lightly_studio/tests/models/test_image.py
  • lightly_studio_view/src/lib/components/TagsMenu/TagAssignInput.test.ts
  • lightly_studio/tests/resolvers/image_resolver/test_get_all_by_dataset_id.py
  • lightly_studio_view/src/lib/services/types.ts
  • lightly_studio/tests/benchmarks/tag_assignment_benchmark.py
  • lightly_studio/tests/core/mcap/test_write_labels.py
  • lightly_studio/src/lightly_studio/resolvers/tag_resolver/rename.py
  • lightly_studio_view/src/lib/hooks/useTags/toggleTagSelection.test.ts
  • lightly_studio/tests/resolvers/tag_resolver/test_add_sample_ids_to_tag_id.py
  • lightly_studio/tests/resolvers/tag_resolver/test_create.py

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review.

📝 Walkthrough

The change removes tag kind from the API models and database schema. Backend validation and frontend tag selection no longer filter by kind. PostgreSQL migration coverage checks column removal and downgrade backfills; the changelog notes that DuckDB users must re-index.

Changes: Tag kind removal

Layer / File(s) Summary
Remove kind from tag contracts and storage
CHANGELOG.md, lightly_studio/src/lightly_studio/migrations/versions/*, lightly_studio/src/lightly_studio/models/{tag,image}.py, lightly_studio/tests/database/test_db_migrations.py
Tag and image-view models no longer include kind. The migration drops tag.kind; its downgrade restores the column and backfills values based on collection sample type. Migration tests cover both directions, including orphaned tags. The changelog records the database scope and DuckDB re-index requirement.
Update backend tag operations and validation
lightly_studio/src/lightly_studio/resolvers/*, lightly_studio/src/lightly_studio/services/sampling_service/validate_preselection.py, lightly_studio/tests/api/*, lightly_studio/tests/resolvers/*, lightly_studio/tests/sampling/*, lightly_studio/tests/conftest.py, lightly_studio/tests/helpers_resolvers.py, other updated backend tests
Resolvers no longer set or check tag kind. Sample-tag validation accepts tags in the requested collection and retains collection-ownership checks. Backend fixtures and tests remove kind-specific setup and assertions; tests now cover foreign-collection tags where relevant.
Update frontend tag selection and creation
lightly_studio_view/src/lib/hooks/useTags/*, lightly_studio_view/src/lib/hooks/useAddTagToSample/*, lightly_studio_view/src/lib/components/*, lightly_studio_view/src/lib/services/types.ts, lightly_studio_view/src/routes/datasets/*
useTags returns all tags for a collection and no longer accepts a kind filter. Components and routes remove kind filters and creation fields. TagsMenu uses grid type for annotation selection, and the annotations page passes its collection ID to useTags. Frontend fixtures and tests remove kind fields and cover annotation-selection behavior.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Merge Risk: ⚪ Minimal · up to 2484d

No merge-blocking issue is established. Existing DuckDB databases require re-indexing before tags can be created.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 45 functions across 33 files. (11 skipped… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description check Passed The description explains the removal of tag kind, the collection-based behavior, migration differences, GUI impact, testing results, known test limitations, and changelog status. It follows the requir…
Title check Passed The title clearly and concisely describes the primary change: removing tag kind because the collection now implies it.
Full details: Docstring Coverage

Explanation

Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 45 functions across 33 files. (11 skipped: 11 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
📝 Generate docstrings: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-08T18:34:24.785029Z dcb397e PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@lightly-fast-track-bot

lightly-fast-track-bot Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

❌  Fast Track: checks did not pass

Guardrail Result Message
frontend/complexity ❌ lightly_studio_view/src/lib/components/Operator/OperatorDialog.svelte:115 — Async function 'handleExecute' has a complexity of 14. Maximum allowed is 10.
backend/complexity ✅ 38 file(s) checked, no violations.
backend/coverage ✅ 6 file(s) checked at 90%.
[PASS] lightly_studio/src/lightly_studio/models/image.py: 100.0%
[PASS] lightly_studio/src/lightly_studio/models/tag.py: 100.0%
[PASS] lightly_studio/src/lightly_studio/resolvers/image_resolver/annotation_count_helpers/get_and_validate_sample_tags.py: 100.0%
[PASS] lightly_studio/src/lightly_studio/resolvers/tag_resolver/get_or_create_sample_tag_by_name.py: 100.0%
[PASS] lightly_studio/src/lightly_studio/resolvers/tag_resolver/split_samples.py: no executable added lines
[PASS] lightly_studio/src/lightly_studio/services/sampling_service/validate_preselection.py: 100.0%
diff-size ❌ PR adds 321 line(s), which exceeds the limit of 215.
frontend/coverage ❌ 16 file(s) checked at 90%.
[PASS] lightly_studio_view/src/lib/components/AnnotationsGrid/AnnotationsGrid.svelte: 100.0%
[FAIL] lightly_studio_view/src/lib/components/Images/Images.svelte: 0.0% coverage (required 90%)
[FAIL] lightly_studio_view/src/lib/components/Operator/OperatorDialog.svelte: 0.0% coverage (required 90%)
[PASS] lightly_studio_view/src/lib/components/PlotPanel/PlotPanel.svelte: 100.0%
[PASS] lightly_studio_view/src/lib/components/Sampling/useSamplingCombinationDialog/useSamplingCombinationDialog.ts: 100.0%
[PASS] lightly_studio_view/src/lib/components/SegmentTags/SegmentTags.svelte: 100.0%
[PASS] lightly_studio_view/src/lib/components/TagsMenu/TagsMenu.svelte: 100.0%
[FAIL] lightly_studio_view/src/lib/hooks/useAddTagToSample/useAddTagToSample.ts: 0.0% coverage (required 90%)
[PASS] lightly_studio_view/src/lib/hooks/useAnnotationsFilter/useAnnotationsFilter.ts: 100.0%
[PASS] lightly_studio_view/src/lib/hooks/useTags/useTags.ts: 100.0%
[PASS] lightly_studio_view/src/routes/datasets/[dataset_id]/[collection_type]/[collection_id]/ImageDistributionPanel/ImageDistributionPanel.svelte: 100.0%
[FAIL] lightly_studio_view/src/routes/datasets/[dataset_id]/[collection_type]/[collection_id]/annotations/+layout.ts: 0.0% coverage (required 90%)
[PASS] lightly_studio_view/src/routes/datasets/[dataset_id]/[collection_type]/[collection_id]/annotations/+page.svelte: 100.0%
[FAIL] lightly_studio_view/src/routes/datasets/[dataset_id]/[collection_type]/[collection_id]/frames/+page.svelte: 0.0% coverage (required 90%)
[FAIL] lightly_studio_view/src/routes/datasets/[dataset_id]/[collection_type]/[collection_id]/images/+layout.ts: 0.0% coverage (required 90%)
[FAIL] lightly_studio_view/src/routes/datasets/[dataset_id]/[collection_type]/[collection_id]/videos/+page.svelte: 0.0% coverage (required 90%)

View the guardrail run

To run the guardrails locally, from fast_track/ run make install once, then make run-guardrails (or GUARDRAILS=<name1>,<name2> make run-guardrails for some guardrails).

Reflects f453afb.

@gabrielfruet
gabrielfruet added this pull request to stack #2781 October 13, 2026 19:27
gabrielfruet and others added 2 commits October 13, 2026 16:28
A tag holds only samples of its own collection, so the sample type of
the collection gives the kind. The downgrade restores kind from it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The page still cleared the root collection's selection, left over from
when annotation tags lived on the root collection.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@gabrielfruet
gabrielfruet force-pushed the gabriel-lig-10097-drop-tag-kind branch from f453afb to 2484d70 Compare October 13, 2026 19:29
@gabrielfruet gabrielfruet changed the title Remove the tag kind field Remove the tag kind, now implied by the tag's collection Oct 13, 2026
@gabrielfruet

Copy link
Copy Markdown
Contributor Author

/review

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant