Change foreign key in AnnotationLabelTable#883
Open
lukas-lightly wants to merge 3 commits intomainfrom
Open
Conversation
Contributor
📝 WalkthroughWalkthroughMigrates annotation-label scoping from collection-root ( Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API_Route as API Route
participant CollectionResolver as Collection Resolver
participant LabelResolver as AnnotationLabel Resolver
participant DB as Database
Client->>API_Route: POST/GET /annotation-label
API_Route->>CollectionResolver: get_root_collection(root_collection_id)
CollectionResolver-->>API_Route: root_collection (includes dataset_id)
API_Route->>LabelResolver: get/create label using dataset_id
LabelResolver->>DB: SELECT/INSERT AnnotationLabel WHERE dataset_id=...
DB-->>LabelResolver: rows / created record
LabelResolver-->>API_Route: AnnotationLabel(s)
API_Route-->>Client: response (labels)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8878c87218
ℹ️ 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".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
lightly_studio/src/lightly_studio/resolvers/annotation_label_resolver/get_all.py
Outdated
Show resolved
Hide resolved
lightly_studio/src/lightly_studio/resolvers/annotation_label_resolver/get_by_label_name.py
Outdated
Show resolved
Hide resolved
lightly_studio/src/lightly_studio/resolvers/dataset_resolver/delete_dataset.py
Outdated
Show resolved
Hide resolved
Base automatically changed from
lukas-lig-9013-change-the-foreing-key-in-annotationlabeltable-and
to
main
April 2, 2026 12:04
lukas-lightly
force-pushed
the
lukas-lig-9013-change-the-foreing-key-in-annotationlabeltable-and-3
branch
from
8878c87 to
3282e14
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
lightly_studio/src/lightly_studio/export/lightly_studio_label_input.py (1)
50-53: Prefer explicit null-check on root collection lookup before dereferencing.This keeps error handling clearer than relying on attribute access failure.
🛠️ Proposed hardening
- dataset_id = collection_resolver.get_root_collection( - session=session, collection_id=root_collection_id - ).dataset_id + root_collection = collection_resolver.get_root_collection( + session=session, collection_id=root_collection_id + ) + if root_collection is None: + raise ValueError(f"Collection {root_collection_id} does not exist.") + dataset_id = root_collection.dataset_id🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lightly_studio/src/lightly_studio/export/lightly_studio_label_input.py` around lines 50 - 53, The code dereferences the result of collection_resolver.get_root_collection(...) to access .dataset_id without checking for None; modify the block using collection_resolver.get_root_collection(session=session, collection_id=root_collection_id) to assign the result to a local (e.g., root_collection), explicitly check if root_collection is None, and raise a clear exception or handle the error (with a descriptive message referencing root_collection_id and session context) instead of letting an AttributeError occur when extracting dataset_id.lightly_studio/tests/helpers_resolvers.py (1)
153-169: Align helper signature with dataset-scoped API to remove indirection.Now that label creation is dataset-scoped, this helper can accept
dataset_iddirectly and avoid the extra collection lookup + legacy naming.♻️ Proposed refactor
-# TODO(lukas 04/2026): change the signature to accept `dataset_id` def create_annotation_label( session: Session, - root_collection_id: UUID, + dataset_id: UUID, label_name: str = "cat", ) -> AnnotationLabelTable: """Helper function to insert an annotation label.""" - collection = collection_resolver.get_by_id(session=session, collection_id=root_collection_id) - if collection is None: - raise ValueError(f"Collection {root_collection_id} doesn't exist") - dataset_id = collection.dataset_id - return annotation_label_resolver.create( session=session, label=AnnotationLabelCreate( dataset_id=dataset_id, annotation_label_name=label_name, ), )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lightly_studio/tests/helpers_resolvers.py` around lines 153 - 169, The helper create_annotation_label currently takes root_collection_id and looks up collection_resolver.get_by_id to extract dataset_id; change its signature to accept dataset_id: UUID (keep session and label_name) and remove the collection lookup and ValueError path; pass the dataset_id directly into annotation_label_resolver.create (via AnnotationLabelCreate.dataset_id) and update any tests or callers that invoked create_annotation_label with a collection id to pass the dataset id instead.lightly_studio/src/lightly_studio/resolvers/dataset_resolver/delete_dataset.py (1)
225-229: Docstring is slightly stale.The docstring says "Delete annotation labels for the root collection" but the function now operates on
dataset_id. Consider updating it for clarity.def _delete_annotation_labels(session: Session, dataset_id: UUID) -> None: - """Delete annotation labels for the root collection.""" + """Delete annotation labels for the given dataset.""" session.exec( delete(AnnotationLabelTable).where(col(AnnotationLabelTable.dataset_id) == dataset_id) )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lightly_studio/src/lightly_studio/resolvers/dataset_resolver/delete_dataset.py` around lines 225 - 229, Update the stale docstring on _delete_annotation_labels to accurately describe its behavior: mention that it deletes all AnnotationLabelTable rows for the given dataset_id (not just "root collection"). Keep reference to the function name _delete_annotation_labels and the parameter dataset_id so the docstring clearly states it deletes annotation labels by dataset_id using the session.exec(delete(AnnotationLabelTable).where(col(AnnotationLabelTable.dataset_id) == dataset_id)).lightly_studio/src/lightly_studio/resolvers/dataset_resolver/deep_copy.py (1)
239-261: Docstring is slightly stale.Line 245 says "Copy annotation labels (belong to root collection only)" but labels now belong to datasets. Consider updating.
def _copy_annotation_labels( session: Session, old_dataset_id: UUID, new_dataset_id: UUID, ctx: DeepCopyContext, ) -> None: - """Copy annotation labels (belong to root collection only).""" + """Copy annotation labels for the dataset."""🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lightly_studio/src/lightly_studio/resolvers/dataset_resolver/deep_copy.py` around lines 239 - 261, Update the stale docstring for _copy_annotation_labels: change "Copy annotation labels (belong to root collection only)" to reflect that annotation labels belong to datasets (e.g., "Copy annotation labels for a dataset") and ensure it mentions old_dataset_id/new_dataset_id usage; no code changes needed beyond editing the docstring in the _copy_annotation_labels function to accurately describe that labels are dataset-scoped.lightly_studio/src/lightly_studio/selection/select_via_db.py (1)
77-91: Docstring parameter description is inconsistent with the new parameter name.Line 87 still references "root collection ID" but the parameter has been renamed to
dataset_id. Update for clarity.Args: session: The SQLAlchemy session. - dataset_id: The dataset ID to look for annotation labels. + dataset_id: The dataset ID used to scope annotation label lookups.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lightly_studio/src/lightly_studio/selection/select_via_db.py` around lines 77 - 91, The docstring for _process_explicit_target_distribution still refers to "root collection ID" while the parameter was renamed to dataset_id; update the Args section to mention dataset_id (e.g., "dataset_id: The dataset ID to look up annotation labels" or similar) and remove any "root collection ID" wording so the parameter description matches the actual function signature and intent.lightly_studio/src/lightly_studio/core/annotation/annotation_create.py (1)
59-76: Incremental migration approach adds DB round-trips per annotation.The
_get_label_idmethod now callscollection_resolver.get_root_collection()to derivedataset_idfromroot_collection_id. For each annotation creation, this triggers hierarchy traversal queries (as shown inget_collection.py:14-51). In bulk annotation scenarios, this could multiply DB round-trips.The TODO comments indicate this is an intentional stepping stone before the full API migration. If bulk annotation performance is critical, consider caching the
dataset_idlookup per batch operation in the caller, or completing the API migration to passdataset_iddirectly.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lightly_studio/src/lightly_studio/core/annotation/annotation_create.py` around lines 59 - 76, The _get_label_id method currently calls collection_resolver.get_root_collection(session, collection_id=root_collection_id) to derive dataset_id for every annotation, causing extra DB round-trips in bulk operations; fix by either (A) changing the caller to pass dataset_id directly and update _get_label_id signature to accept dataset_id instead of root_collection_id (and then use annotation_label_resolver.get_by_label_name/create with that dataset_id), or (B) if immediate API changes are too large, add a per-batch caching mechanism in the caller that resolves dataset_id once (via collection_resolver.get_root_collection) and reuses it for repeated calls to _get_label_id/annotation_label_resolver.get_by_label_name/create to avoid repeated hierarchy lookups.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lightly_studio/src/lightly_studio/models/annotation_label.py`:
- Around line 17-18: The field dataset_id in the AnnotationLabel model is
misnamed and points to the wrong FK; rename dataset_id to root_collection_id,
update its Field(foreign_key=...) to reference the collection table/column that
holds root_collection_id, and update the model's
UniqueConstraint("annotation_label_name", "dataset_id") to use
"root_collection_id" instead; also update any referenced symbols in resolvers
such as get_by_label_name and delete_annotation_labels to use
root_collection_id; ensure all usages of AnnotationLabel.dataset_id are replaced
with AnnotationLabel.root_collection_id and add appropriate DB migration steps
separately to preserve existing data.
In
`@lightly_studio/src/lightly_studio/resolvers/annotation_label_resolver/get_all.py`:
- Around line 30-45: Update the docstring for get_all_sorted_alphabetically to
use consistent terminology (dataset_id) and fix the "to to" typo: change the
Args description for dataset_id to something like "dataset_id (UUID): The
dataset ID to which the labels belong." Also update the top-line description if
needed to reference "dataset" rather than "root collection" so the docstring
consistently refers to dataset_id.
---
Nitpick comments:
In `@lightly_studio/src/lightly_studio/core/annotation/annotation_create.py`:
- Around line 59-76: The _get_label_id method currently calls
collection_resolver.get_root_collection(session,
collection_id=root_collection_id) to derive dataset_id for every annotation,
causing extra DB round-trips in bulk operations; fix by either (A) changing the
caller to pass dataset_id directly and update _get_label_id signature to accept
dataset_id instead of root_collection_id (and then use
annotation_label_resolver.get_by_label_name/create with that dataset_id), or (B)
if immediate API changes are too large, add a per-batch caching mechanism in the
caller that resolves dataset_id once (via
collection_resolver.get_root_collection) and reuses it for repeated calls to
_get_label_id/annotation_label_resolver.get_by_label_name/create to avoid
repeated hierarchy lookups.
In `@lightly_studio/src/lightly_studio/export/lightly_studio_label_input.py`:
- Around line 50-53: The code dereferences the result of
collection_resolver.get_root_collection(...) to access .dataset_id without
checking for None; modify the block using
collection_resolver.get_root_collection(session=session,
collection_id=root_collection_id) to assign the result to a local (e.g.,
root_collection), explicitly check if root_collection is None, and raise a clear
exception or handle the error (with a descriptive message referencing
root_collection_id and session context) instead of letting an AttributeError
occur when extracting dataset_id.
In `@lightly_studio/src/lightly_studio/resolvers/dataset_resolver/deep_copy.py`:
- Around line 239-261: Update the stale docstring for _copy_annotation_labels:
change "Copy annotation labels (belong to root collection only)" to reflect that
annotation labels belong to datasets (e.g., "Copy annotation labels for a
dataset") and ensure it mentions old_dataset_id/new_dataset_id usage; no code
changes needed beyond editing the docstring in the _copy_annotation_labels
function to accurately describe that labels are dataset-scoped.
In
`@lightly_studio/src/lightly_studio/resolvers/dataset_resolver/delete_dataset.py`:
- Around line 225-229: Update the stale docstring on _delete_annotation_labels
to accurately describe its behavior: mention that it deletes all
AnnotationLabelTable rows for the given dataset_id (not just "root collection").
Keep reference to the function name _delete_annotation_labels and the parameter
dataset_id so the docstring clearly states it deletes annotation labels by
dataset_id using the
session.exec(delete(AnnotationLabelTable).where(col(AnnotationLabelTable.dataset_id)
== dataset_id)).
In `@lightly_studio/src/lightly_studio/selection/select_via_db.py`:
- Around line 77-91: The docstring for _process_explicit_target_distribution
still refers to "root collection ID" while the parameter was renamed to
dataset_id; update the Args section to mention dataset_id (e.g., "dataset_id:
The dataset ID to look up annotation labels" or similar) and remove any "root
collection ID" wording so the parameter description matches the actual function
signature and intent.
In `@lightly_studio/tests/helpers_resolvers.py`:
- Around line 153-169: The helper create_annotation_label currently takes
root_collection_id and looks up collection_resolver.get_by_id to extract
dataset_id; change its signature to accept dataset_id: UUID (keep session and
label_name) and remove the collection lookup and ValueError path; pass the
dataset_id directly into annotation_label_resolver.create (via
AnnotationLabelCreate.dataset_id) and update any tests or callers that invoked
create_annotation_label with a collection id to pass the dataset id instead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 59c28a60-97b7-4118-bd85-d0603d5314dc
📒 Files selected for processing (22)
lightly_studio/src/lightly_studio/api/routes/api/annotation_label.pylightly_studio/src/lightly_studio/core/annotation/annotation_create.pylightly_studio/src/lightly_studio/core/labelformat_helpers.pylightly_studio/src/lightly_studio/examples/coco_plugins_demo/lightly_train_inference_operator.pylightly_studio/src/lightly_studio/export/lightly_studio_label_input.pylightly_studio/src/lightly_studio/export/youtube_vis_label_input.pylightly_studio/src/lightly_studio/few_shot_classifier/classifier_manager.pylightly_studio/src/lightly_studio/models/annotation_label.pylightly_studio/src/lightly_studio/resolvers/annotation_label_resolver/get_all.pylightly_studio/src/lightly_studio/resolvers/annotation_label_resolver/get_by_label_name.pylightly_studio/src/lightly_studio/resolvers/dataset_resolver/deep_copy.pylightly_studio/src/lightly_studio/resolvers/dataset_resolver/delete_dataset.pylightly_studio/src/lightly_studio/selection/select_via_db.pylightly_studio/src/lightly_studio/services/annotations_service/update_annotation_label.pylightly_studio/tests/api/routes/api/test_annotation_label.pylightly_studio/tests/api/routes/api/test_export.pylightly_studio/tests/conftest.pylightly_studio/tests/helpers_resolvers.pylightly_studio/tests/resolvers/annotation_label_resolver/test_get_all.pylightly_studio/tests/resolvers/annotation_label_resolver/test_get_by_label_name.pylightly_studio/tests/resolvers/annotations/test_annotations_base_delete_annotations.pylightly_studio/tests/selection/test_select_via_db.py
lightly_studio/src/lightly_studio/resolvers/annotation_label_resolver/get_all.py
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lightly_studio/tests/helpers_resolvers.py (1)
153-157: Consider completing the helper signature migration todataset_id.
create_annotation_labelstill takesroot_collection_idand performs an extra lookup (Line 160) only to derivedataset_id. Acceptingdataset_iddirectly would simplify tests and remove the extra query.♻️ Suggested refactor
-# TODO(lukas, 04/2026): change the signature to accept `dataset_id` def create_annotation_label( session: Session, - root_collection_id: UUID, + dataset_id: UUID, label_name: str = "cat", ) -> AnnotationLabelTable: """Helper function to insert an annotation label.""" - collection = collection_resolver.get_by_id(session=session, collection_id=root_collection_id) - if collection is None: - raise ValueError(f"Collection {root_collection_id} doesn't exist") - dataset_id = collection.dataset_id - return annotation_label_resolver.create( session=session, label=AnnotationLabelCreate( dataset_id=dataset_id, annotation_label_name=label_name, ), )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lightly_studio/tests/helpers_resolvers.py` around lines 153 - 157, Refactor the helper create_annotation_label to accept dataset_id (UUID) instead of root_collection_id: change its signature from (session: Session, root_collection_id: UUID, ...) to (session: Session, dataset_id: UUID, ...), remove the extra lookup that queries the collection to derive dataset_id, and use the provided dataset_id where the function previously used the derived value; update any tests or callers to pass dataset_id rather than root_collection_id and keep the rest of the function behavior unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@lightly_studio/tests/helpers_resolvers.py`:
- Around line 153-157: Refactor the helper create_annotation_label to accept
dataset_id (UUID) instead of root_collection_id: change its signature from
(session: Session, root_collection_id: UUID, ...) to (session: Session,
dataset_id: UUID, ...), remove the extra lookup that queries the collection to
derive dataset_id, and use the provided dataset_id where the function previously
used the derived value; update any tests or callers to pass dataset_id rather
than root_collection_id and keep the rest of the function behavior unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: cade174b-19ee-48ff-8d07-5539f2b815bd
📒 Files selected for processing (4)
lightly_studio/src/lightly_studio/resolvers/annotation_label_resolver/get_all.pylightly_studio/src/lightly_studio/resolvers/annotation_label_resolver/get_by_label_name.pylightly_studio/src/lightly_studio/resolvers/dataset_resolver/delete_dataset.pylightly_studio/tests/helpers_resolvers.py
✅ Files skipped from review due to trivial changes (1)
- lightly_studio/src/lightly_studio/resolvers/annotation_label_resolver/get_all.py
🚧 Files skipped from review as they are similar to previous changes (1)
- lightly_studio/src/lightly_studio/resolvers/dataset_resolver/delete_dataset.py
lukas-lightly
force-pushed
the
lukas-lig-9013-change-the-foreing-key-in-annotationlabeltable-and-3
branch
from
9c1ea51 to
c055872
Compare
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lightly_studio_view/src/lib/schema.d.ts (1)
1892-1918:⚠️ Potential issue | 🟠 MajorMigrate all
createLabelcallsites fromroot_collection_idtodataset_id.The schema contract at lines 1897 and 1918 now expects
dataset_id, but four locations still passroot_collection_id:
SampleDetailsClassificationSegment.svelte:92SampleObjectDetectionRect.svelte:162useInstanceSegmentationBrush.ts:147useInstanceSegmentationBrush.test.ts:271Update all four to pass
dataset_id(asDatasetTable.dataset_id, not collection ID).Pattern
- label = await createLabel({ - root_collection_id: collectionId, - annotation_label_name: 'DEFAULT' - }); + label = await createLabel({ + dataset_id: datasetId, + annotation_label_name: 'DEFAULT' + });- expect(createLabel).toHaveBeenCalledWith({ - root_collection_id: 'c1', - annotation_label_name: 'DEFAULT' - }); + expect(createLabel).toHaveBeenCalledWith({ + dataset_id: 'd1', + annotation_label_name: 'DEFAULT' + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lightly_studio_view/src/lib/schema.d.ts` around lines 1892 - 1918, Several createLabel callsites are still passing root_collection_id; update them to pass dataset_id taken from DatasetTable.dataset_id. In SampleDetailsClassificationSegment, SampleObjectDetectionRect, useInstanceSegmentationBrush and its test (useInstanceSegmentationBrush.test) find the createLabel invocation and replace the root_collection_id field with dataset_id: DatasetTable.dataset_id (ensure the payload key is dataset_id per the AnnotationLabelCreate/AnnotationLabelCreateRequest contract). Keep all other payload fields unchanged and adjust any test fixtures/mocks to supply DatasetTable.dataset_id instead of a collection id.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@lightly_studio_view/src/lib/schema.d.ts`:
- Around line 1892-1918: Several createLabel callsites are still passing
root_collection_id; update them to pass dataset_id taken from
DatasetTable.dataset_id. In SampleDetailsClassificationSegment,
SampleObjectDetectionRect, useInstanceSegmentationBrush and its test
(useInstanceSegmentationBrush.test) find the createLabel invocation and replace
the root_collection_id field with dataset_id: DatasetTable.dataset_id (ensure
the payload key is dataset_id per the
AnnotationLabelCreate/AnnotationLabelCreateRequest contract). Keep all other
payload fields unchanged and adjust any test fixtures/mocks to supply
DatasetTable.dataset_id instead of a collection id.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d3282f26-ef3c-4cc4-9231-7f8ce40d5e7d
📒 Files selected for processing (5)
lightly_studio/src/lightly_studio/resolvers/annotation_label_resolver/get_all.pylightly_studio/src/lightly_studio/resolvers/annotation_label_resolver/get_by_label_name.pylightly_studio/src/lightly_studio/resolvers/dataset_resolver/delete_dataset.pylightly_studio/tests/helpers_resolvers.pylightly_studio_view/src/lib/schema.d.ts
✅ Files skipped from review due to trivial changes (1)
- lightly_studio/src/lightly_studio/resolvers/annotation_label_resolver/get_by_label_name.py
🚧 Files skipped from review as they are similar to previous changes (2)
- lightly_studio/tests/helpers_resolvers.py
- lightly_studio/src/lightly_studio/resolvers/annotation_label_resolver/get_all.py
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lightly_studio_view/src/lib/hooks/useInstanceSegmentationBrush.test.ts (1)
116-121:⚠️ Potential issue | 🟠 MajorAdd
datasetIdto alluseInstanceSegmentationBrushinvocations in this test file.The hook signature now requires
datasetId: string(see line 21 of useInstanceSegmentationBrush.ts). Only the test case at line 263 was updated; the invocations at lines 116, 135, 159, 187, 209, and 235 still omit this required parameter, causing TypeScript type-checking failures.Suggested fix pattern
const { finishBrush } = useInstanceSegmentationBrush({ collectionId: 'c1', + datasetId: 'some-dataset-id', sampleId: 's1', sample, refetch });Apply this to each remaining invocation in the file.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lightly_studio_view/src/lib/hooks/useInstanceSegmentationBrush.test.ts` around lines 116 - 121, The test calls to useInstanceSegmentationBrush are missing the new required datasetId parameter; update every invocation that currently passes { collectionId, sampleId, sample, refetch } to include a datasetId string (e.g., datasetId: 'd1') so the hook signature matches the implementation. Search for all useInstanceSegmentationBrush(...) usages in the test file (the ones returning finishBrush or similar) and add datasetId to the argument object for each invocation to satisfy TypeScript and mirror the already-updated test case.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@lightly_studio_view/src/lib/hooks/useInstanceSegmentationBrush.test.ts`:
- Around line 116-121: The test calls to useInstanceSegmentationBrush are
missing the new required datasetId parameter; update every invocation that
currently passes { collectionId, sampleId, sample, refetch } to include a
datasetId string (e.g., datasetId: 'd1') so the hook signature matches the
implementation. Search for all useInstanceSegmentationBrush(...) usages in the
test file (the ones returning finishBrush or similar) and add datasetId to the
argument object for each invocation to satisfy TypeScript and mirror the
already-updated test case.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 183a43b6-2cab-4c92-87f5-8fc8c3f9c04b
📒 Files selected for processing (5)
lightly_studio_view/src/lib/components/SampleDetails/SampleDetailsClassificationSegment/SampleDetailsClassificationSegment.sveltelightly_studio_view/src/lib/components/SampleDetails/SampleInstanceSegmentationRect/SampleInstanceSegmentationRect.sveltelightly_studio_view/src/lib/components/SampleDetails/SampleObjectDetectionRect/SampleObjectDetectionRect.sveltelightly_studio_view/src/lib/hooks/useInstanceSegmentationBrush.test.tslightly_studio_view/src/lib/hooks/useInstanceSegmentationBrush.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What has changed and why?
(Delete this: Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.)
How has it been tested?
(Delete this: Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration.)
Did you update CHANGELOG.md?
Summary by CodeRabbit
Refactor
Tests