feat: introduce MetadataCategoricalFilter component - #1787
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughAdds categorical bucket types and a Svelte metadata filter with searchable values, missing/other handling, retained selections, callback behavior, unit tests, component tests, and Storybook coverage. ChangesCategorical metadata filter
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant MetadataCategoricalFilter
participant buildOptions
participant onToggle
User->>MetadataCategoricalFilter: Open values filter
MetadataCategoricalFilter->>buildOptions: Build selectable and retained options
User->>MetadataCategoricalFilter: Toggle an option
MetadataCategoricalFilter->>onToggle: Send selected categorical value
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
|
/review |
❌ Fast Track: checks did not pass
To run the guardrails locally, from Reflects |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7c5aff6a9f
ℹ️ 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".
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/DatasetDistributionPanel/MetadataCategoricalFilter/helpers/buildOptions.ts`:
- Around line 13-33: Deduplicate selectedValues before filtering and mapping
retained options in buildOptions.ts, preserving the existing selectable
exclusion and bucket construction behavior. In buildOptions.test.ts, add
regression coverage asserting duplicate absent selections produce exactly one
retained option.
In
`@lightly_studio_view/src/lib/components/DatasetDistributionPanel/MetadataCategoricalFilter/helpers/getCheckboxLabel.ts`:
- Around line 3-6: Update getCheckboxLabel so a non-retained option with
bucket.count equal to one uses “sample” instead of “samples,” while preserving
the plural form for all other counts and the existing missing/retained behavior.
Add a one-sample expectation in getCheckboxLabel.test.ts covering this singular
label.
🪄 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: 258b787a-e066-4131-b30a-8efd2e243a37
📒 Files selected for processing (9)
lightly_studio_view/src/lib/components/DatasetDistributionPanel/MetadataCategoricalFilter/MetadataCategoricalFilter.sveltelightly_studio_view/src/lib/components/DatasetDistributionPanel/MetadataCategoricalFilter/MetadataCategoricalFilterPopover/MetadataCategoricalFilterPopover.sveltelightly_studio_view/src/lib/components/DatasetDistributionPanel/MetadataCategoricalFilter/helpers/buildOptions.test.tslightly_studio_view/src/lib/components/DatasetDistributionPanel/MetadataCategoricalFilter/helpers/buildOptions.tslightly_studio_view/src/lib/components/DatasetDistributionPanel/MetadataCategoricalFilter/helpers/getCheckboxLabel.test.tslightly_studio_view/src/lib/components/DatasetDistributionPanel/MetadataCategoricalFilter/helpers/getCheckboxLabel.tslightly_studio_view/src/lib/components/DatasetDistributionPanel/MetadataCategoricalFilter/helpers/getOptionLabel.test.tslightly_studio_view/src/lib/components/DatasetDistributionPanel/MetadataCategoricalFilter/helpers/getOptionLabel.tslightly_studio_view/src/lib/components/DatasetDistributionPanel/MetadataCategoricalFilter/helpers/index.ts
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/components/DatasetDistributionPanel/MetadataCategoricalFilter/MetadataCategoricalFilter.svelte (1)
29-37: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winCover retained boolean filter values.
buildOptionscreates a retained option for stale selected values as{ kind: 'value', value, ... }, so boolean selections are supported by the implementation.options.find(...)!is type-safe becauseFilterOption.bucketis narrowed toSelectableBucket, which excludeskind: 'other'. Add abuildOptions.test.tscase forbuildOptions([], [true])/[false]to cover this retained boolean path.🤖 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/components/DatasetDistributionPanel/MetadataCategoricalFilter/MetadataCategoricalFilter.svelte` around lines 29 - 37, The retained-value path in buildOptions needs test coverage for boolean selections. Add buildOptions.test.ts cases exercising buildOptions with an empty options list and selected values [true] and [false], verifying each boolean is retained as a value option with the expected label/value.
🧹 Nitpick comments (1)
lightly_studio_view/src/lib/components/DatasetDistributionPanel/MetadataCategoricalFilter/MetadataCategoricalFilter.svelte (1)
78-89: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRedundant
optionLabel(option)computation per row (P2 style).
optionLabel(option)is invoked separately forcheckboxLabel(line 81) and twice more for thetitle/text span (lines 84-85), recomputinggetOptionLabelup to three times per rendered row. Cache it once per iteration with{@const}.♻️ Suggested refactor
{`#each` visible as option (option.bucket.id)} + {`@const` label = optionLabel(option)} <label class="flex min-h-8 cursor-pointer items-center gap-2 rounded px-2 text-sm hover:bg-accent max-sm:min-h-11" > <Checkbox checked={isSelected(option.bucket.value)} onCheckedChange={() => onToggle(option.bucket.value)} - aria-label={checkboxLabel(option)} + aria-label={getCheckboxLabel(option, label)} /> - <span class="min-w-0 flex-1 truncate" title={optionLabel(option)} - >{optionLabel(option)}</span + <span class="min-w-0 flex-1 truncate" title={label} + >{label}</span >As per coding guidelines: "focus on code style, use succinct comments, assign style comments priority P2..." — treating this as a P2 style/perf nit.
🤖 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/components/DatasetDistributionPanel/MetadataCategoricalFilter/MetadataCategoricalFilter.svelte` around lines 78 - 89, Within the option-row iteration containing Checkbox and the label spans, compute optionLabel(option) once with a Svelte {`@const`} binding and reuse that value for checkboxLabel, the title attribute, and displayed text. Preserve the existing labels and rendering behavior while eliminating repeated optionLabel calls.Source: Coding guidelines
🤖 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.
Outside diff comments:
In
`@lightly_studio_view/src/lib/components/DatasetDistributionPanel/MetadataCategoricalFilter/MetadataCategoricalFilter.svelte`:
- Around line 29-37: The retained-value path in buildOptions needs test coverage
for boolean selections. Add buildOptions.test.ts cases exercising buildOptions
with an empty options list and selected values [true] and [false], verifying
each boolean is retained as a value option with the expected label/value.
---
Nitpick comments:
In
`@lightly_studio_view/src/lib/components/DatasetDistributionPanel/MetadataCategoricalFilter/MetadataCategoricalFilter.svelte`:
- Around line 78-89: Within the option-row iteration containing Checkbox and the
label spans, compute optionLabel(option) once with a Svelte {`@const`} binding and
reuse that value for checkboxLabel, the title attribute, and displayed text.
Preserve the existing labels and rendering behavior while eliminating repeated
optionLabel calls.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ec51145f-d10f-446f-86ff-d8fcb07ae089
📒 Files selected for processing (2)
lightly_studio_view/src/lib/components/DatasetDistributionPanel/MetadataCategoricalFilter/MetadataCategoricalFilter.sveltelightly_studio_view/src/lib/hooks/useCategoricalMetadataDistribution/useCategoricalMetadataDistribution.svelte.ts
What has changed and why?
This PR introduces component to use as part of the dataset distribution panel to select values for categorical metadata values
How has it been tested?
Accompanied with unit test and storybook story for visual testing
Screen.Recording.2026-07-28.at.11.13.59.mov
Did you update CHANGELOG.md?
Summary by CodeRabbit
null) values and retained selections not in the latest top results, including clearer “Other” and “Missing” labeling and “not in top” indicators.