feat: adjust function to render filter chip - #1785
Conversation
📝 WalkthroughWalkthroughThe metadata filter chip hook is implemented in a nested module with a re-exported entrypoint. It derives remembered numeric and categorical chips, supports toggling and clearing filters, formats values, and resets categorical filter state in tests. ChangesMetadata filter chips
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant MetadataFilterChips
participant useMetadataFilterChips
participant useMetadataFilters
MetadataFilterChips->>useMetadataFilterChips: request chips and handlers
useMetadataFilterChips->>useMetadataFilters: read filter bounds and selections
useMetadataFilters-->>useMetadataFilterChips: return metadata filter state
useMetadataFilterChips-->>MetadataFilterChips: return active or remembered chips
MetadataFilterChips->>useMetadataFilterChips: toggle or clear chip
useMetadataFilterChips->>useMetadataFilters: update or reset filter values
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 |
![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: 3e30b63c45
ℹ️ 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".
| const formatCategoricalValues = (values: CategoricalMetadataValue[] = []): string => { | ||
| const hasMissingValue = values.includes('Missing'); | ||
| const hasNoValue = values.includes(null); | ||
| return values | ||
| .map((value) => { | ||
| if (value === null) return hasMissingValue ? 'Missing (no value)' : 'Missing'; | ||
| if (value === 'Missing' && hasNoValue) return 'Missing (value)'; | ||
| if (value === 'Other') return 'Other (value)'; |
There was a problem hiding this comment.
When a categorical filter contains the supported __missing__ or __other__ sentinel, this formatter falls through to String(value) and exposes the raw token in the chip. CategoricalMetadataValue excludes null, so the null branches cannot handle missing values; compare against MISSING_CATEGORICAL_VALUE and OTHER_CATEGORICAL_VALUE instead.
Useful? React with 👍 / 👎.
| kind: 'numeric' | 'categorical'; | ||
| range?: Range; | ||
| values?: CategoricalMetadataValue[]; |
There was a problem hiding this comment.
Define numeric and categorical chip variants so kind: 'numeric' requires range and kind: 'categorical' requires values. Making both payloads optional permits invalid states and forces consumers to dereference possibly undefined fields instead of narrowing on kind.
AGENTS.md reference: AGENTS.md:L43-L45
Useful? React with 👍 / 👎.
| values?: CategoricalMetadataValue[]; | ||
| } | ||
|
|
||
| export function useMetadataFilterChips(collectionId: string | undefined) { |
There was a problem hiding this comment.
This hook now spans nearly 150 lines and combines numeric state, categorical state, chip construction, toggling, clearing, and formatting. Extract the numeric and categorical chip logic into focused helpers to follow the repository guideline that functions remain small and single-purpose.
AGENTS.md reference: AGENTS.md:L43-L45
Useful? React with 👍 / 👎.
| beforeEach(() => { | ||
| storage.updateMetadataBounds({}); | ||
| storage.updateMetadataValues({}); | ||
| storage.updateCategoricalMetadataValues({}); |
There was a problem hiding this comment.
The only categorical-related test change clears the store; no test seeds a categorical selection or verifies its subtitle, toggle, remembered value, or clear behavior. Add behavior-focused tests for the new branch so its user-visible contract is covered as required by the frontend TDD guidance.
AGENTS.md reference: AGENTS.md:L43-L45
Useful? React with 👍 / 👎.
| // One chip per key that is narrowed now or has a remembered range: active | ||
| // chips show the current range, disabled ones the remembered range. |
There was a problem hiding this comment.
This comment now describes every chip as carrying a remembered range, although categorical chips carry value arrays instead. Update it to document both variants so the newly added categorical behavior is not contradicted by the surrounding explanation.
AGENTS.md reference: AGENTS.md:L43-L45
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/MetadataFilterChips/useMetadataFilterChips/useMetadataFilterChips.svelte.ts`:
- Around line 9-15: Update the MetadataFilterChip model and MetadataFilterChips
rendering to discriminate categorical chips from numeric chips, ensuring
categorical entries do not access chip.range and instead use
hook.formatCategoricalValues(chip.values) for their subtitle. Add coverage for
rendering, toggling, and clearing categorical selections, then run make
static-checks and make test from lightly_studio_view.
🪄 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: 53dd3105-ef6e-4894-804e-658fdd581e2d
📒 Files selected for processing (4)
lightly_studio_view/src/lib/components/MetadataFilterChips/useMetadataFilterChips.svelte.tslightly_studio_view/src/lib/components/MetadataFilterChips/useMetadataFilterChips/index.tslightly_studio_view/src/lib/components/MetadataFilterChips/useMetadataFilterChips/useMetadataFilterChips.svelte.tslightly_studio_view/src/lib/components/MetadataFilterChips/useMetadataFilterChips/useMetadataFilterChips.test.ts
💤 Files with no reviewable changes (1)
- lightly_studio_view/src/lib/components/MetadataFilterChips/useMetadataFilterChips.svelte.ts
| export interface MetadataFilterChip { | ||
| key: string; | ||
| active: boolean; | ||
| kind: 'numeric' | 'categorical'; | ||
| range?: Range; | ||
| values?: CategoricalMetadataValue[]; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Find relevant files:"
fd -a 'useMetadataFilterChips\.svelte\.ts|MetadataFilterChips\.svelte|ai_guidelines/frontend\.md' . | sed 's#^\./##'
echo
echo "Inspect target files around relevant content:"
for f in \
lightly_studio_view/src/lib/components/MetadataFilterChips/useMetadataFilterChips/useMetadataFilterChips.svelte.ts \
lightly_studio_view/src/lib/components/MetadataFilterChips/MetadataFilterChips.svelte
do
if [ -f "$f" ]; then
echo "===== $f ($(wc -l < "$f") lines) ====="
nl -ba "$f" | sed -n '1,180p'
else
echo "MISSING: $f"
fi
done
echo
echo "Search categorical/numeric chips usage:"
rg -n "MetadataFilterChip|formatCategoricalValues|chip\.range|kind:|CategoricalMetadataValue" lightly_studio_view/src/lib/components/MetadataFilterChips lightly_studio_view/src/lib || true
echo
echo "Check frontend guideline existence/content:"
if [ -f ai_guidelines/frontend.md ]; then
nl -ba ai_guidelines/frontend.md | sed -n '1,220p'
else
echo "MISSING ai_guidelines/frontend.md"
fi
echo
echo "Check Makefile/package for static/tests:"
if [ -f lightly_studio_view/Makefile ]; then
nl -ba lightly_studio_view/Makefile | sed -n '1,180p'
fi
if [ -f lightly_studio_view/package.json ]; then
node - <<'JS'
const fs=require('fs');
const p=JSON.parse(fs.readFileSync('lightly_studio_view/package.json','utf8'));
console.log(JSON.stringify({scripts:p.scripts, dependencies:p.dependencies, devDependencies:p.devDependencies}, null, 2));
JS
fiRepository: lightly-ai/lightly-studio
Length of output: 650
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect deterministic Svelte/template expressions involving chip.range without running repo code.
python3 - <<'PY'
from pathlib import Path
p = Path('lightly_studio_view/src/lib/components/MetadataFilterChips/MetadataFilterChips.svelte')
text = p.read_text() if p.exists() else ''
terms = [
('chip.range.min', text.find('chip.range.min')),
('chip.range.max', text.find('chip.range.max')),
('values?: CategoricalMetadataValue', text.find('values?: CategoricalMetadataValue')),
('formatCategoricalValues', text.find('formatCategoricalValues')),
('chip.kind', text.find('chip.kind')),
]
for term, pos in terms:
print(f'{term}: {pos}')
PYRepository: lightly-ai/lightly-studio
Length of output: 285
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "===== lightly_studio_view/src/lib/components/MetadataFilterChips/useMetadataFilterChips/useMetadataFilterChips.svelte.ts ====="
awk '{printf "%5d\t%s\n", NR, $0}' lightly_studio_view/src/lib/components/MetadataFilterChips/useMetadataFilterChips/useMetadataFilterChips.svelte.ts
echo
echo "===== lightly_studio_view/src/lib/components/MetadataFilterChips/MetadataFilterChips.svelte ====="
awk '{printf "%5d\t%s\n", NR, $0}' lightly_studio_view/src/lib/components/MetadataFilterChips/MetadataFilterChips.svelte
echo
echo "===== related usage search ====="
rg -n "MetadataFilterChip|formatCategoricalValues|chip\.range|chip\.kind|CategoricalMetadataValue|Categorical|numeric" lightly_studio_view/src/lib/components/MetadataFilterChips lightly_studio_view/src/lib || true
echo
echo "===== frontend guidelines ====="
if [ -f ai_guidelines/frontend.md ]; then
awk '{printf "%5d\t%s\n", NR, $0}' ai_guidelines/frontend.md
else
echo "MISSING ai_guidelines/frontend.md"
fi
echo
echo "===== lightly_studio_view build/test config ====="
if [ -f lightly_studio_view/Makefile ]; then
awk '{printf "%5d\t%s\n", NR, $0}' lightly_studio_view/Makefile
fi
if [ -f lightly_studio_view/package.json ]; then
node - <<'JS'
const fs=require('fs');
const p=JSON.parse(fs.readFileSync('lightly_studio_view/package.json','utf8'));
console.log(JSON.stringify({scripts:p.scripts, dependencies:p.dependencies, devDependencies:p.devDependencies}, null, 2));
JS
fiRepository: lightly-ai/lightly-studio
Length of output: 50383
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "===== MetadataFilterChips tests ====="
awk '{printf "%5d\t%s\n", NR, $0}' lightly_studio_view/src/lib/components/MetadataFilterChips/MetadataFilterChips.test.ts
echo
echo "===== make/package availability ====="
for f in lightly_studio_view/Makefile lightly_studio_view/package.json; do
echo "--- $f ---"
if [ -f "$f" ]; then
awk '{printf "%5d\t%s\n", NR, $0}' "$f"
else
echo "MISSING"
fi
done
echo
echo "===== static-check command file references ====="
rg -n "static-checks|type-check|eslint|tsc|vitest|svelte-check" -g 'Makefile' -g 'package.json' -g '*.md' .Repository: lightly-ai/lightly-studio
Length of output: 11471
Make MetadataFilterChips branch on categorical chips.
Categorical chips are emitted without range, but this component reads chip.range.min/max unconditionally in the subtitle snippet. Make the chip model discriminated and render categorical subtitles with hook.formatCategoricalValues(chip.values) instead. Add a render/toggle/clear test for categorical selections, then run make static-checks and make test from lightly_studio_view.
🤖 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/MetadataFilterChips/useMetadataFilterChips/useMetadataFilterChips.svelte.ts`
around lines 9 - 15, Update the MetadataFilterChip model and MetadataFilterChips
rendering to discriminate categorical chips from numeric chips, ensuring
categorical entries do not access chip.range and instead use
hook.formatCategoricalValues(chip.values) for their subtitle. Add coverage for
rendering, toggling, and clearing categorical selections, then run make
static-checks and make test from lightly_studio_view.
Source: Coding guidelines
1 participant
What has changed and why?
This PR contains changes for left side bar filtering chips to support categorical metadata filters
How has it been tested?
Accompanied wtih unit test
Did you update CHANGELOG.md?
Summary by CodeRabbit
New Features
Tests