FE: thread categorical metadata values through all filter hooks - #1747
Conversation
📝 WalkthroughWalkthroughImage, frame, video, and infinite image filtering now accepts categorical metadata values, passes them to metadata filter creation, and includes them in infinite-query cache keys. ChangesCategorical metadata filtering
Estimated code review effort: 2 (Simple) | ~15 minutes Sequence Diagram(s)sequenceDiagram
participant FilterHook
participant createMetadataFilters
participant buildRequestBody
participant InfiniteQuery
FilterHook->>createMetadataFilters: provide metadata_values and categorical_metadata_values
createMetadataFilters-->>FilterHook: return metadata_filters
buildRequestBody->>createMetadataFilters: provide metadata_values and categorical_metadata_values
createMetadataFilters-->>buildRequestBody: return metadata_filters
InfiniteQuery->>InfiniteQuery: include categorical_metadata_values in queryKey
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8788416d40
ℹ️ 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".
❌ Fast Track: checks did not pass
To run the guardrails locally, from Reflects |
d54a61f to
8d4e752
Compare
8788416 to
aaeaf8a
Compare
71797b1 to
c7d0aab
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lightly_studio_view/src/lib/hooks/useImagesInfinite/buildRequestBody.ts (1)
17-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression coverage for categorical filters.
Please add tests for categorical-only and combined numeric/categorical metadata values in
buildRequestBody.test.ts, asserting that the generated request contains the expectedinfilters and preserves range filters.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lightly_studio_view/src/lib/hooks/useImagesInfinite/buildRequestBody.ts` around lines 17 - 23, Add regression tests in buildRequestBody.test.ts for categorical-only metadata values and combined numeric/categorical values. Assert categorical filters produce the expected “in” conditions, and combined input preserves the existing numeric range filters alongside them.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@lightly_studio_view/src/lib/hooks/useImagesInfinite/buildRequestBody.ts`:
- Around line 17-23: Add regression tests in buildRequestBody.test.ts for
categorical-only metadata values and combined numeric/categorical values. Assert
categorical filters produce the expected “in” conditions, and combined input
preserves the existing numeric range filters alongside them.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 8654c8e9-d6d4-47cb-a4ca-95bbf07a735c
📒 Files selected for processing (5)
lightly_studio_view/src/lib/hooks/useImageFilters/useImageFilters.tslightly_studio_view/src/lib/hooks/useImagesInfinite/buildRequestBody.tslightly_studio_view/src/lib/hooks/useImagesInfinite/createImagesInfiniteOptions.tslightly_studio_view/src/lib/hooks/useImagesInfinite/types.tslightly_studio_view/src/routes/datasets/[dataset_id]/[collection_type]/[collection_id]/videos/+page.svelte
🚧 Files skipped from review as they are similar to previous changes (4)
- lightly_studio_view/src/routes/datasets/[dataset_id]/[collection_type]/[collection_id]/videos/+page.svelte
- lightly_studio_view/src/lib/hooks/useImagesInfinite/createImagesInfiniteOptions.ts
- lightly_studio_view/src/lib/hooks/useImagesInfinite/types.ts
- lightly_studio_view/src/lib/hooks/useImageFilters/useImageFilters.ts
|
/review |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@lightly_studio_view/src/lib/components/Images/Images.svelte`:
- Line 70: Update the filterHash derivation used by initialScrollPosition and
scrollResetKey to include $categoricalMetadataValues, ensuring categorical
selection changes produce a distinct scroll-restoration hash and reset behavior.
🪄 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 Plus
Run ID: 3c7ed7e9-522f-4277-8722-4a57b3cfbe05
📒 Files selected for processing (3)
lightly_studio_view/src/lib/components/Images/Images.sveltelightly_studio_view/src/lib/hooks/useImageFilters/useImageFilters.tslightly_studio_view/src/lib/hooks/useImagesInfinite/buildRequestBody.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- lightly_studio_view/src/lib/hooks/useImageFilters/useImageFilters.ts
- lightly_studio_view/src/lib/hooks/useImagesInfinite/buildRequestBody.ts
6268968 to
6ba47ca
Compare
…ta-bar-chart-filter-ui-8-sp.p6
There was a problem hiding this comment.
🧹 Nitpick comments (1)
lightly_studio_view/src/lib/hooks/useFramesFilter/frameFilter.ts (1)
65-69: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression coverage for categorical metadata filtering.
lightly_studio_view/src/lib/hooks/useFramesFilter/frameFilter.ts#L65-L69: test categorical-only and combined metadata inputs.lightly_studio_view/src/lib/hooks/useVideoFilters/useVideoFilters.ts#L74-L81: test categorical-only and combined metadata inputs.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@lightly_studio_view/src/lib/hooks/useFramesFilter/frameFilter.ts` around lines 65 - 69, The metadata filtering logic around createMetadataFilters in lightly_studio_view/src/lib/hooks/useFramesFilter/frameFilter.ts (lines 65-69) requires regression tests for categorical-only and combined metadata_values/categorical_metadata_values inputs. Add equivalent coverage around the metadata handling in lightly_studio_view/src/lib/hooks/useVideoFilters/useVideoFilters.ts (lines 74-81), verifying both input combinations produce the expected filters; no production logic change is requested.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@lightly_studio_view/src/lib/hooks/useFramesFilter/frameFilter.ts`:
- Around line 65-69: The metadata filtering logic around createMetadataFilters
in lightly_studio_view/src/lib/hooks/useFramesFilter/frameFilter.ts (lines
65-69) requires regression tests for categorical-only and combined
metadata_values/categorical_metadata_values inputs. Add equivalent coverage
around the metadata handling in
lightly_studio_view/src/lib/hooks/useVideoFilters/useVideoFilters.ts (lines
74-81), verifying both input combinations produce the expected filters; no
production logic change is requested.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3401a257-0d3a-42a8-b84a-986d73c5ebc5
📒 Files selected for processing (6)
lightly_studio_view/src/lib/hooks/useFramesFilter/frameFilter.tslightly_studio_view/src/lib/hooks/useImageFilters/useImageFilters.tslightly_studio_view/src/lib/hooks/useImagesInfinite/buildRequestBody.tslightly_studio_view/src/lib/hooks/useImagesInfinite/createImagesInfiniteOptions.tslightly_studio_view/src/lib/hooks/useImagesInfinite/types.tslightly_studio_view/src/lib/hooks/useVideoFilters/useVideoFilters.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- lightly_studio_view/src/lib/hooks/useImageFilters/useImageFilters.ts
- lightly_studio_view/src/lib/hooks/useImagesInfinite/types.ts
- lightly_studio_view/src/lib/hooks/useImagesInfinite/buildRequestBody.ts
- lightly_studio_view/src/lib/hooks/useImagesInfinite/createImagesInfiniteOptions.ts
Summary
Stacked on p5.
Adds the optional
categorical_metadata_valuesparameter to the four filter hooks so that active categorical selections are included in every sample-list request:useImageFiltersuseVideoFiltersuseFramesFilteruseImagesInfinite(viabuildRequestBodyandcreateImagesInfiniteOptions)No UI changes — the parameter defaults to
undefinedso existing callers are unaffected.Test plan
make static-checks(frontend) passesnpm run test:unitpasses (new parametrised tests in all four hooks)Part of LIG-9585.
Summary by CodeRabbit