Feature/lig 9585 categorical metadata distribution - #1793
Conversation
Adds an 'in' operator that accepts a list of string/boolean/null values and builds an OR predicate, using a LEFT OUTER JOIN when null is present to match samples with no metadata entry. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Introduces the CategoricalMetadataValue/CategoricalMetadataValues types and a session-storage-backed store for selected categorical filter values, alongside an export for the upcoming useCategoricalMetadataDistribution hook. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
createMetadataFilters now accepts an optional categoricalMetadataValues map and appends one 'in' filter per non-empty key; also clears the categorical store when the collection changes. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Fetches value-counts for all categorical metadata fields and shapes the response into typed CategoricalMetadataBucket arrays, disambiguating literal 'Missing'/'Other' values from the sentinel missing/other buckets. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds the optional categorical_metadata_values param to useImageFilters, useVideoFilters, useFramesFilter, and useImagesInfinite so that active categorical selections are included in every sample filter request. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CategoryCount gains optional selected and selectable fields; buildEchartsOption renders a bright border on selected bars and dims unselectable ones. Clicks on bars with selectable=false are suppressed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Renders a checkbox list for selecting categorical metadata values, with a 'Clear' action and a missing-value entry as a first-class option. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds a 'categorical' slot to DistributionSourceGroup; the panel renders the MetadataCategoricalFilter plus a horizontal BarChart with selection visuals and loading/error/retry states when a group carries categorical data. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
useMetadataFilterChips now tracks categorical selections and produces chips with a disambiguated label format for literal 'Missing'/'Other' values vs the sentinel missing bucket. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…pages The layout fetches categorical distributions, exposes toggle/clear handlers to DatasetDistributionPanel, and passes categoricalMetadataValues into the shared metadataFilters. Frames and videos pages forward the same values to their respective filter hooks. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds weather (skewed string), reviewed (boolean), camera_id (25 values, exercises the "Other" bucket), and split (~20% missing, exercises the "Missing" bucket) alongside the existing numeric fields. location now uses weighted sampling instead of uniform. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
When the user selects one or more categorical metadata values the remaining bars now render in grey (#4b5563), matching the existing behaviour of the numeric Histogram where bins outside the selected range are dimmed. The white border on the selected bar has been removed because the green/grey colour contrast already communicates selection clearly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Metadata distribution plots previously passed the full imageAnnotationCountsFilter to the histogram and categorical queries, causing bar heights to shrink as the user applied annotation, dimension, or metadata filters from the left sidebar. This made it hard to understand what was being filtered away because the original distribution context was lost. Introduce distributionBaseFilter — a derived value that omits the analysis filters (metadataFilters, annotationFilter, dimensionsValues) while retaining the collection-scoping context (tagIds, sampleIds, confusionCell, queryExpr). Both distribution queries now use this filter so bar heights always reflect the full dataset; the active selection is communicated through bar colour alone (green = in selection, grey = outside). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…al distributions When sidebar filters (annotation class, image dimensions, metadata of other fields) are active, categorical metadata bars previously shrank to show only the filtered counts, losing the original distribution as context. This change fetches a second distribution using the full sidebar filter (imageAnnotationCountsFilter) alongside the existing base-filter query, then passes the filtered counts as `filteredBuckets` to each categorical group. The bar chart renders two ECharts series when any bar's filteredCount differs from its full count: - Background series (grey, #374151): full unfiltered count — stable height - Foreground series (green/grey): filtered count — shows filter effect The tooltip updates to "Total / In filter" for bars where the filter actually reduces the count, falling back to the single-value "Count" line otherwise. When no filter is active the filtered query returns the same counts as the base query, so hasActiveFilter is false and a single series is rendered — no visual change from the pre-filter state. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…reconciled with main's independent reimplementation Main independently reimplemented the categorical metadata distribution/filter feature (categorical_value_counts.py with fields param, MetadataCategoricalFilter.svelte, useCategoricalMetadataDistribution hook, DatasetDistributionPanel categorical support) in a more complete form than pr-1687. Conflicts were resolved by preferring main's version in nearly all cases since it fully supersedes pr-1687's implementation; a few small pieces (usePostHog tracking + categorical distribution import in +layout.svelte, trackEvent wiring in useMetadataFilterChips) were folded in from both sides where they were complementary rather than duplicative.
|
Kondrat seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
📝 WalkthroughWalkthroughAdds categorical metadata value-count specifications and integrates categorical filtering into dataset distribution panels, shared filter chips, query construction, and image/video/frame loading. The UI supports typed values, Missing and Other buckets, selection retention, retry states, configurable sorting, and categorical chart rendering. ChangesCategorical metadata distribution
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant DatasetLayout
participant DistributionQuery
participant DatasetDistributionPanel
participant MetadataCategoricalFilter
DatasetLayout->>DistributionQuery: request categorical bucket counts
DistributionQuery-->>DatasetLayout: return bucket data and status
DatasetLayout->>DatasetDistributionPanel: provide categorical distribution state
DatasetDistributionPanel->>MetadataCategoricalFilter: render selectable values
MetadataCategoricalFilter->>DatasetLayout: update or clear categorical filters
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 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 |
❌ Fast Track: checks did not pass
To run the guardrails locally, from Reflects |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
lightly_studio_view/src/lib/components/DatasetDistributionPanel/MetadataCategoricalFilter.svelte (1)
79-87: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winAvoid the non-null assertion in
summary.The invariant (every selected value is either returned or synthesized as retained) holds today, but a single unresolvable selection would make
optionLabel(undefined!)throw and blank the panel. A fallback keeps it safe.🛡️ Proposed change
- : optionLabel( - options.find(({ bucket }) => Object.is(bucket.value, selectedValues[0]))! - ) + : (() => { + const option = options.find(({ bucket }) => + Object.is(bucket.value, selectedValues[0]) + ); + return option ? optionLabel(option) : '1 selected'; + })()🤖 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.svelte` around lines 79 - 87, Update the selected-value branch of the summary derived value to avoid the non-null assertion on the options.find result. Handle an unresolved selection by supplying a safe fallback label/value to optionLabel so summary computation never throws, while preserving the existing “All values” and “N selected” behavior.lightly_studio_view/src/routes/datasets/[dataset_id]/[collection_type]/[collection_id]/+layout.svelte (1)
705-721: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDeselecting the last value leaves an empty array in the filter state.
handleCategoricalValueTogglestores[]whileclearCategoricalValuesdeletes the key. Both mean "no predicate", but the empty entry still changes the serialized filter state used for query keys/filter hashes, causing avoidable cache misses and non-idempotent state. Prune the key whennextis empty.♻️ Proposed change
- updateCategoricalMetadataValues({ - ...$categoricalMetadataValues, - [metadataKey]: next - }); + if (next.length === 0) { + clearCategoricalValues(metadataKey); + return; + } + updateCategoricalMetadataValues({ + ...$categoricalMetadataValues, + [metadataKey]: next + });(requires moving
clearCategoricalValuesabove the toggle handler)🤖 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/routes/datasets/`[dataset_id]/[collection_type]/[collection_id]/+layout.svelte around lines 705 - 721, Update handleCategoricalValueToggle so that when removing the final selected value, it prunes metadataKey from the filter state instead of storing an empty array; preserve the existing update behavior when values remain or are newly selected, reusing clearCategoricalValues if appropriate.
🤖 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 `@docs/features/lig-9585-categorical-metadata-distribution-filter.md`:
- Line 3: Update the status and acceptance-criteria section of the categorical
metadata distribution filter specification so they agree: mark each delivered
criterion in lines 139-156 as checked when the feature is implemented, or
downgrade the status from “Implemented; QA ready” if those criteria are not
delivered. Use the neighboring categorical metadata value-counts specification
as the formatting reference.
In
`@lightly_studio_view/src/lib/components/MetadataFilterChips/useMetadataFilterChips.svelte.ts`:
- Around line 115-121: Update the categorical branches in the metadata filter
change handler to invoke trackFilterChanged before each early return, including
the branch around lastCategoricalValues and the additional branch at the
referenced later range. Ensure categorical enable, disable, and clear actions
emit metadata_filter_changed consistently with the numeric path.
In
`@lightly_studio_view/src/routes/datasets/`[dataset_id]/[collection_type]/[collection_id]/+layout.svelte:
- Around line 386-402: Update the filtered distribution query alongside
distributionBaseFilter so each metadata key uses a faceted filter: apply all
active predicates except the predicate for the key currently being rendered,
while preserving tag, sample, confusion-cell, query, and other metadata filters.
Ensure the edited field’s non-selected buckets retain their actual counts
instead of collapsing to zero; keep distributionBaseFilter’s full-dataset
behavior unchanged.
- Around line 664-676: Update the categorical distribution state construction
around categoricalMetadataFilteredQuery so loading reflects either metadata
query and error surfaces failures from either query, including the filtered
query’s error. Update the retry handler near the existing
categoricalMetadataQuery refetch to refetch both queries together, preserving
the current unfiltered-query behavior.
---
Nitpick comments:
In
`@lightly_studio_view/src/lib/components/DatasetDistributionPanel/MetadataCategoricalFilter.svelte`:
- Around line 79-87: Update the selected-value branch of the summary derived
value to avoid the non-null assertion on the options.find result. Handle an
unresolved selection by supplying a safe fallback label/value to optionLabel so
summary computation never throws, while preserving the existing “All values” and
“N selected” behavior.
In
`@lightly_studio_view/src/routes/datasets/`[dataset_id]/[collection_type]/[collection_id]/+layout.svelte:
- Around line 705-721: Update handleCategoricalValueToggle so that when removing
the final selected value, it prunes metadataKey from the filter state instead of
storing an empty array; preserve the existing update behavior when values remain
or are newly selected, reusing clearCategoricalValues if appropriate.
🪄 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: 54cf9f14-8a3c-4ed1-b5c1-e016fe591c7f
⛔ Files ignored due to path filters (1)
lightly_studio_view/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (35)
AGENTS.mddocs/features/lig-9584-categorical-metadata-value-counts.mddocs/features/lig-9585-categorical-metadata-distribution-filter.mdlightly_studio_view/src/lib/components/BarChart/BarChart.sveltelightly_studio_view/src/lib/components/DatasetDistributionPanel/DatasetDistributionPanel.sveltelightly_studio_view/src/lib/components/DatasetDistributionPanel/DatasetDistributionPanel.test.tslightly_studio_view/src/lib/components/DatasetDistributionPanel/DistributionConfigDialog/DistributionConfigDialog.sveltelightly_studio_view/src/lib/components/DatasetDistributionPanel/DistributionConfigDialog/DistributionConfigDialog.test.tslightly_studio_view/src/lib/components/DatasetDistributionPanel/ExpandDialog/ExpandDialog.sveltelightly_studio_view/src/lib/components/DatasetDistributionPanel/ExpandDialog/ExpandDialog.test.tslightly_studio_view/src/lib/components/DatasetDistributionPanel/MetadataCategoricalFilter.sveltelightly_studio_view/src/lib/components/DatasetDistributionPanel/MetadataCategoricalFilter.test.tslightly_studio_view/src/lib/components/DatasetDistributionPanel/PanelHeader/PanelHeader.sveltelightly_studio_view/src/lib/components/DatasetDistributionPanel/PanelHeader/PanelHeader.test.tslightly_studio_view/src/lib/components/DatasetDistributionPanel/selectVisibleCounts.test.tslightly_studio_view/src/lib/components/DatasetDistributionPanel/selectVisibleCounts.tslightly_studio_view/src/lib/components/DatasetDistributionPanel/types.tslightly_studio_view/src/lib/components/Histogram/buildHistogramOption.test.tslightly_studio_view/src/lib/components/Histogram/buildHistogramOption.tslightly_studio_view/src/lib/components/Images/Images.sveltelightly_studio_view/src/lib/components/MetadataFilterChips/MetadataFilterChips.sveltelightly_studio_view/src/lib/components/MetadataFilterChips/MetadataFilterChips.test.tslightly_studio_view/src/lib/components/MetadataFilterChips/useMetadataFilterChips.svelte.tslightly_studio_view/src/lib/components/MetadataFilterChips/useMetadataFilterChips.test.tslightly_studio_view/src/lib/hooks/useFramesFilter/frameFilter.test.tslightly_studio_view/src/lib/hooks/useFramesFilter/frameFilter.tslightly_studio_view/src/lib/hooks/useImageFilters/useImageFilters.test.tslightly_studio_view/src/lib/hooks/useImagesInfinite/buildRequestBody.test.tslightly_studio_view/src/lib/hooks/useImagesInfinite/createImagesInfiniteOptions.test.tslightly_studio_view/src/lib/hooks/useMetadataFilters/useMetadataFilters.test.tslightly_studio_view/src/lib/hooks/useVideoFilters/useVideoFilters.test.tslightly_studio_view/src/lib/hooks/useVideoFilters/useVideoFilters.tslightly_studio_view/src/routes/datasets/[dataset_id]/[collection_type]/[collection_id]/+layout.sveltelightly_studio_view/src/routes/datasets/[dataset_id]/[collection_type]/[collection_id]/frames/+page.sveltelightly_studio_view/src/routes/datasets/[dataset_id]/[collection_type]/[collection_id]/videos/+page.svelte
| @@ -0,0 +1,218 @@ | |||
| # Feature: Categorical Metadata Distribution and Filter | |||
|
|
|||
| **Status:** Implemented; QA ready | |||
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Status says implemented, but no acceptance criteria are checked.
Lines 139-156 are all [ ], unlike docs/features/lig-9584-categorical-metadata-value-counts.md which marks delivered items [x]. Tick the delivered criteria (or downgrade the status) so the spec stays a reliable QA reference.
🤖 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 `@docs/features/lig-9585-categorical-metadata-distribution-filter.md` at line
3, Update the status and acceptance-criteria section of the categorical metadata
distribution filter specification so they agree: mark each delivered criterion
in lines 139-156 as checked when the feature is implemented, or downgrade the
status from “Implemented; QA ready” if those criteria are not delivered. Use the
neighboring categorical metadata value-counts specification as the formatting
reference.
| if (lastCategoricalValues[key]) { | ||
| updateCategoricalMetadataValues({ | ||
| ...categoricalStore.current, | ||
| [key]: checked ? lastCategoricalValues[key] : [] | ||
| }); | ||
| return; | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Track categorical chip changes.
These branches return before trackFilterChanged, so categorical enable, disable, and clear actions are absent from metadata_filter_changed analytics. Invoke it before each return, matching the numeric path.
Proposed fix
if (lastCategoricalValues[key]) {
updateCategoricalMetadataValues({
...categoricalStore.current,
[key]: checked ? lastCategoricalValues[key] : []
});
+ trackFilterChanged(key, checked ? 'enabled' : 'disabled');
return;
}
@@
lastCategoricalValues = Object.fromEntries(
Object.entries(lastCategoricalValues).filter(([valueKey]) => valueKey !== key)
);
+ trackFilterChanged(key, 'disabled');
return;
}Also applies to: 133-141
🤖 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.svelte.ts`
around lines 115 - 121, Update the categorical branches in the metadata filter
change handler to invoke trackFilterChanged before each early return, including
the branch around lastCategoricalValues and the additional branch at the
referenced later range. Ensure categorical enable, disable, and clear actions
emit metadata_filter_changed consistently with the numeric path.
| // Distribution queries always show the full dataset so bar heights stay | ||
| // stable as sidebar filters are applied. Filtering is communicated through | ||
| // bar colour (green = in selection, grey = out of selection) rather than | ||
| // through shrinking bars, which loses the original distribution context. | ||
| // Tag / sample / confusion-cell / query context is kept so the distributions | ||
| // stay scoped to the collection the user is currently exploring. | ||
| const distributionBaseFilter = $derived( | ||
| buildImageFilter({ | ||
| dimensionsValues: null, | ||
| annotationFilter: undefined, | ||
| metadataFilters: undefined, | ||
| sampleIds: isAnnotations ? [] : plotFilterImageSampleIds, | ||
| tagIds: isAnnotations ? [] : plotFilterTagIds, | ||
| confusionCell: isAnnotations ? null : plotFilterConfusionCell, | ||
| queryExpr: isAnnotations ? null : plotFilterQueryExpr | ||
| }) | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Base bars drop faceting, so the edited field's foreground bars collapse to a single value.
distributionBaseFilter strips all metadata filters, and the second query applies the full filter including the edited field's own in predicate. For the key currently being filtered, every non-selected bucket therefore renders filteredCount: 0, which contradicts the agreed contract in this PR's specs (docs/features/lig-9585-categorical-metadata-distribution-filter.md line 86 and docs/features/lig-9584-categorical-metadata-value-counts.md lines 131-133: own-field predicate excluded, all other predicates applied). Consider building the filtered query's filter with the active key's own predicate removed (per-key faceting), or update the spec to document the intentional deviation.
Also applies to: 631-643
🤖 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/routes/datasets/`[dataset_id]/[collection_type]/[collection_id]/+layout.svelte
around lines 386 - 402, Update the filtered distribution query alongside
distributionBaseFilter so each metadata key uses a faceted filter: apply all
active predicates except the predicate for the key currently being rendered,
while preserving tag, sample, confusion-cell, query, and other metadata filters.
Ensure the edited field’s non-selected buckets retain their actual counts
instead of collapsing to zero; keep distributionBaseFilter’s full-dataset
behavior unchanged.
| ...categoricalKeys.map((key) => ({ | ||
| id: key, | ||
| label: key, | ||
| categorical: { | ||
| buckets: categoricalMetadataDistributions[key] ?? [], | ||
| // undefined until the filtered query has returned so the | ||
| // distribution panel waits before showing background bars. | ||
| filteredBuckets: categoricalMetadataFilteredDistributions?.[key], | ||
| selectedValues: $categoricalMetadataValues[key] ?? [], | ||
| loading: categoricalMetadataQuery.isFetching, | ||
| error: categoricalMetadataQuery.error?.message | ||
| } | ||
| })) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Filtered query's loading/error state is never surfaced.
loading/error come only from categoricalMetadataQuery; if categoricalMetadataFilteredQuery fails, filteredBuckets silently stays undefined with no error and no retry path (line 929 refetches only the unfiltered query). Fold both queries into the reported state.
♻️ Suggested wiring
- loading: categoricalMetadataQuery.isFetching,
- error: categoricalMetadataQuery.error?.message
+ loading:
+ categoricalMetadataQuery.isFetching ||
+ categoricalMetadataFilteredQuery.isFetching,
+ error:
+ categoricalMetadataQuery.error?.message ??
+ categoricalMetadataFilteredQuery.error?.messageand retry both:
- onCategoricalRetry={() =>
- categoricalMetadataQuery.refetch()}
+ onCategoricalRetry={() => {
+ categoricalMetadataQuery.refetch();
+ categoricalMetadataFilteredQuery.refetch();
+ }}🤖 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/routes/datasets/`[dataset_id]/[collection_type]/[collection_id]/+layout.svelte
around lines 664 - 676, Update the categorical distribution state construction
around categoricalMetadataFilteredQuery so loading reflects either metadata
query and error surfaces failures from either query, including the filtered
query’s error. Update the retry handler near the existing
categoricalMetadataQuery refetch to refetch both queries together, preserving
the current unfiltered-query behavior.
![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
Revert unrelated package-lock regeneration
No dependency manifest changed, but this commit rewrites platform libc metadata throughout the lockfile and adds a nested optional picomatch entry. This appears to be lockfile regeneration with a different npm version rather than part of the feature; revert it to keep the PR focused and avoid changing installation resolution without an intentional dependency update.
AGENTS.md reference: AGENTS.md:L43-L45
ℹ️ 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".
| buckets: categoricalMetadataDistributions[key] ?? [], | ||
| // undefined until the filtered query has returned so the | ||
| // distribution panel waits before showing background bars. | ||
| filteredBuckets: categoricalMetadataFilteredDistributions?.[key], |
There was a problem hiding this comment.
When another sidebar filter changes the value ranking, the unfiltered and filtered endpoint responses can contain different top-20 sets. Passing the filtered list here and matching it to base buckets by ID makes an omitted base value appear as zero and compares two incompatible Other aggregates, so foreground counts can be incorrect. Display the filtered bucket set directly or request counts for a fixed set of base values. This conflicts with the endpoint-ordered, live-count behavior specified in docs/features/lig-9585-categorical-metadata-distribution-filter.md.
AGENTS.md reference: AGENTS.md:L52-L54
Useful? React with 👍 / 👎.
| const metadataHistogramsQuery = useNumericMetadataDistribution(() => ({ | ||
| collectionId: collectionId, | ||
| filter: imageAnnotationCountsFilter, | ||
| filter: distributionBaseFilter, |
There was a problem hiding this comment.
When dimension, annotation, or metadata filters are active, distributionBaseFilter explicitly removes them at lines 394-396, so using it here changes numeric histograms from the current view to the full collection while the grid and class distributions remain filtered. There is no filtered histogram overlay to communicate that difference. Restore imageAnnotationCountsFilter; the feature specification explicitly lists changing numeric histogram behavior as a non-goal.
AGENTS.md reference: AGENTS.md:L52-L54
Useful? React with 👍 / 👎.
| loading: categoricalMetadataQuery.isFetching, | ||
| error: categoricalMetadataQuery.error?.message |
There was a problem hiding this comment.
When only the sidebar-filtered query refetches or fails, these fields continue reporting the unchanged base query's state. The panel therefore shows stale foreground counts without an updating/error message, and its Retry callback refetches only the base query. Combine both queries' loading/error state and retry the failed filtered query so filter changes cannot silently leave stale counts.
AGENTS.md reference: AGENTS.md:L52-L54
Useful? React with 👍 / 👎.
| @@ -0,0 +1,163 @@ | |||
| <script lang="ts"> | |||
There was a problem hiding this comment.
This adds a second MetadataCategoricalFilter beside the already tested and barrel-exported MetadataCategoricalFilter/MetadataCategoricalFilter.svelte. The duplicate also inlines 163 lines of option and label logic that the existing module has already split into helpers. Import the existing component module instead, preserving the frontend guideline that each component and its related files live in one scoped folder.
AGENTS.md reference: AGENTS.md:L43-L45
Useful? React with 👍 / 👎.
| const categoricalData = $derived<CategoryCount[]>( | ||
| (activeCategorical?.buckets ?? []).map((bucket) => { |
There was a problem hiding this comment.
The categorical mapping, configuration state, loading/error UI, selector, chart accessibility markup, and callbacks add over 200 lines to an already large component. Move this behavior into a focused categorical-distribution component or component-specific hook so DatasetDistributionPanel remains an orchestrator and follows the frontend guideline to keep components under 100 lines.
AGENTS.md reference: AGENTS.md:L43-L45
Useful? React with 👍 / 👎.
| // selectDistributions internally via the TanStack Query `select` option. | ||
| const metadataDistributions = $derived(metadataHistogramsQuery.data ?? {}); | ||
|
|
||
| const categoricalMetadataQuery = useCategoricalMetadataDistribution(() => ({ |
There was a problem hiding this comment.
The route now owns two categorical queries, response normalization, source construction, selection mutation, and retry wiring, adding roughly 100 lines of feature-specific business logic to the layout. Extract a small hook returning the distribution groups and actions, consistent with the frontend guideline that hooks own data-fetching state and components avoid mixing business logic with presentation.
AGENTS.md reference: AGENTS.md:L43-L45
Useful? React with 👍 / 👎.
2 participants
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