not ready to merge - This prototype adds categorical metadata distributions and filtering to the dataset distribution panel.#1687
Open
ikondrat wants to merge 20 commits into
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>
Contributor
📝 WalkthroughWalkthroughAdds categorical metadata value-count APIs, validated ChangesCategorical metadata workflow
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant DistributionPanel
participant CategoricalQuery
participant MetadataAPI
participant GlobalStorage
participant ImagesQuery
User->>DistributionPanel: Select categorical value
DistributionPanel->>GlobalStorage: Store selected value
GlobalStorage->>CategoricalQuery: Update active filter
CategoricalQuery->>MetadataAPI: POST metadata/value-counts
MetadataAPI-->>CategoricalQuery: Return categorical buckets
CategoricalQuery-->>DistributionPanel: Render updated counts
GlobalStorage->>ImagesQuery: Build categorical in-filter
ImagesQuery-->>User: Refresh filtered samples
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 |
ikondrat
changed the title
Contributor
Author
|
/review |
![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: 8ef4bc588a
ℹ️ 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".
| export const useCategoricalMetadataDistribution = ( | ||
| getOptions: () => CategoricalMetadataDistributionOptions & { enabled?: boolean } | ||
| ) => | ||
| createQuery(() => { |
There was a problem hiding this comment.
The Frontend guidelines require hooks that call TanStack Query's createQuery/createInfiniteQuery to live in .svelte.ts files. This new hook wraps createQuery from a plain .ts module, so please rename the module/export to .svelte.ts before building more reactive query behavior on it.
Useful? React with 👍 / 👎.
| import { Button } from '$lib/components/ui/button'; | ||
| import { Checkbox } from '$lib/components/ui/checkbox'; | ||
| import { Input } from '$lib/components/ui/input'; | ||
| import type { CategoricalMetadataBucket } from '$lib/hooks/useCategoricalMetadataDistribution/useCategoricalMetadataDistribution'; |
There was a problem hiding this comment.
The Frontend import guidelines ask shared modules to be imported from their module/barrel API rather than deep paths. This component reaches into the hook implementation just for CategoricalMetadataBucket; export the type from the hook module's public API (or move it to a shared types file) and import from there instead.
Useful? React with 👍 / 👎.
Comment on lines
+49
to
+51
| schema = metadata_info_resolver._get_merged_schema( # noqa: SLF001 | ||
| session=session, collection_id=collection_id | ||
| ) |
There was a problem hiding this comment.
This resolver calls _get_merged_schema (and below, other underscored helpers) from get_metadata_info while suppressing SLF001. That couples the new value-counts resolver to private histogram internals, so a local refactor there can silently break this endpoint; move the shared schema/filter helpers to a public resolver utility instead.
Useful? React with 👍 / 👎.
| @@ -0,0 +1,163 @@ | |||
| <script lang="ts"> | |||
There was a problem hiding this comment.
The Frontend guidelines ask components to stay under about 100 lines and split logic into focused, testable pieces. This new component is roughly 150 lines and combines bucket retention, label disambiguation, search state, and popover rendering; extracting the option/summary logic or a list subcomponent would keep the UI easier to maintain.
Useful? React with 👍 / 👎.
![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: 1
🧹 Nitpick comments (1)
lightly_studio_view/src/lib/components/ClassSetConfig/ClassSetConfigDialog.svelte (1)
28-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive
itemNounfrom an explicit prop instead of a hardcoded string match.
itemNoun={itemNounPlural === 'classes' ? 'class' : 'value'}only works for the two current callers ('classes'/'values'). Any future caller passing a different plural (e.g. 'samples', 'tags') silently gets the wrong singular noun ('value').♻️ Proposed fix: add an explicit `itemNoun` prop
/** Singular/plural labels for the configured chart items. */ itemNounPlural?: string; + /** Singular label for the configured chart items (default 'class'). */ + itemNoun?: string;showAllButton = false, itemNounPlural = 'classes', + itemNoun = 'class',{items} {itemNounPlural} - itemNoun={itemNounPlural === 'classes' ? 'class' : 'value'} + {itemNoun}Callers (e.g.
DistributionConfigDialog) would then passitemNoun="value"alongsideitemNounPlural="values"explicitly.Also applies to: 39-50, 133-135
🤖 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/ClassSetConfig/ClassSetConfigDialog.svelte` around lines 28 - 29, Replace the hardcoded singular-noun derivation in ClassSetConfigDialog with an explicit itemNoun prop, and update the component’s prop definition and all callers, including DistributionConfigDialog, to pass the correct singular alongside itemNounPlural. Remove the classes-specific conditional so arbitrary plural labels such as samples or tags retain their intended singular noun.
🤖 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/src/lightly_studio/resolvers/metadata_resolver/sample/categorical_value_counts.py`:
- Around line 19-21: The categorical value counts resolver directly depends on
private helpers from metadata_info_resolver. Move _get_merged_schema,
_without_metadata_key_filter, and _apply_image_filters into a shared module with
non-private names, then update both get_metadata_info and
categorical_value_counts to import and use the shared helpers, removing the
SLF001 suppressions.
---
Nitpick comments:
In
`@lightly_studio_view/src/lib/components/ClassSetConfig/ClassSetConfigDialog.svelte`:
- Around line 28-29: Replace the hardcoded singular-noun derivation in
ClassSetConfigDialog with an explicit itemNoun prop, and update the component’s
prop definition and all callers, including DistributionConfigDialog, to pass the
correct singular alongside itemNounPlural. Remove the classes-specific
conditional so arbitrary plural labels such as samples or tags retain their
intended singular noun.
🪄 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: fc5c8556-a56e-4f32-91aa-85c42f66fc1f
📒 Files selected for processing (64)
AGENTS.mddocs/features/lig-9584-categorical-metadata-value-counts.mddocs/features/lig-9585-categorical-metadata-distribution-filter.mdlightly_studio/e2e-tests/index_distribution.pylightly_studio/src/lightly_studio/api/routes/api/metadata.pylightly_studio/src/lightly_studio/database/db_json.pylightly_studio/src/lightly_studio/models/metadata.pylightly_studio/src/lightly_studio/resolvers/metadata_resolver/metadata_filter.pylightly_studio/src/lightly_studio/resolvers/metadata_resolver/sample/categorical_value_counts.pylightly_studio/src/lightly_studio/type_definitions.pylightly_studio/tests/api/routes/api/test_metadata.pylightly_studio/tests/database/test_db_json.pylightly_studio/tests/metadata/test_get_metadata_value_counts.pylightly_studio/tests/resolvers/metadata_resolvers/sample/test_sample_metadata_filtering.pylightly_studio/tests/resolvers/metadata_resolvers/test_metadata_filter.pylightly_studio_view/src/lib/components/BarChart/BarChart.sveltelightly_studio_view/src/lib/components/BarChart/buildEchartsOption.test.tslightly_studio_view/src/lib/components/BarChart/buildEchartsOption.tslightly_studio_view/src/lib/components/BarChart/types.tslightly_studio_view/src/lib/components/ClassSetConfig/ClassSetConfigDialog.sveltelightly_studio_view/src/lib/components/ClassSetConfig/ClassSetConfigDialog.test.tslightly_studio_view/src/lib/components/ClassSetConfig/ManualClassSelector.sveltelightly_studio_view/src/lib/components/ClassSetConfig/ManualClassSelector.test.tslightly_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/index.tslightly_studio_view/src/lib/hooks/useCategoricalMetadataDistribution/useCategoricalMetadataDistribution.test.tslightly_studio_view/src/lib/hooks/useCategoricalMetadataDistribution/useCategoricalMetadataDistribution.tslightly_studio_view/src/lib/hooks/useFramesFilter/frameFilter.test.tslightly_studio_view/src/lib/hooks/useFramesFilter/frameFilter.tslightly_studio_view/src/lib/hooks/useGlobalStorage.tslightly_studio_view/src/lib/hooks/useImageFilters/useImageFilters.test.tslightly_studio_view/src/lib/hooks/useImageFilters/useImageFilters.tslightly_studio_view/src/lib/hooks/useImagesInfinite/buildRequestBody.test.tslightly_studio_view/src/lib/hooks/useImagesInfinite/buildRequestBody.tslightly_studio_view/src/lib/hooks/useImagesInfinite/createImagesInfiniteOptions.test.tslightly_studio_view/src/lib/hooks/useImagesInfinite/createImagesInfiniteOptions.tslightly_studio_view/src/lib/hooks/useImagesInfinite/types.tslightly_studio_view/src/lib/hooks/useMetadataFilters/useMetadataFilters.test.tslightly_studio_view/src/lib/hooks/useMetadataFilters/useMetadataFilters.tslightly_studio_view/src/lib/hooks/useVideoFilters/useVideoFilters.test.tslightly_studio_view/src/lib/hooks/useVideoFilters/useVideoFilters.tslightly_studio_view/src/lib/services/types.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
Comment on lines
+19
to
+21
| from lightly_studio.resolvers.metadata_resolver.sample import ( | ||
| get_metadata_info as metadata_info_resolver, | ||
| ) |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Cross-module reliance on get_metadata_info's private helpers.
_get_merged_schema, _without_metadata_key_filter, and _apply_image_filters are all underscore-prefixed (private) in get_metadata_info, yet this module imports and calls them directly with # noqa: SLF001 suppressions in three places. This couples the resolver to internal implementation details that carry no API stability guarantee — a future rename/refactor of those helpers (which look "private" and safe to change) could silently break this file.
Consider promoting these three helpers to a shared, non-underscored module (e.g. a small metadata_query_helpers module imported by both get_metadata_info and categorical_value_counts), or exposing them as an explicit internal API on get_metadata_info without the underscore prefix.
Also applies to: 49-58, 119-121, 147-149
🤖 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/src/lightly_studio/resolvers/metadata_resolver/sample/categorical_value_counts.py`
around lines 19 - 21, The categorical value counts resolver directly depends on
private helpers from metadata_info_resolver. Move _get_merged_schema,
_without_metadata_key_filter, and _apply_image_filters into a shared module with
non-private names, then update both get_metadata_info and
categorical_value_counts to import and use the shared helpers, removing the
SLF001 suppressions.
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>
❌ Fast Track: checks did not passFailed guardrails: frontend/complexity (Guardrail threw: Cannot find module 'eslint'
Reflects |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
lightly_studio_view/src/lib/components/BarChart/buildEchartsOption.test.ts (1)
64-93: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSolid coverage of the overlay/no-overlay paths; consider a combined selection+filter case.
These new cases correctly pin down the background/foreground overlay behavior and the no-op single-series fallback. One gap: no test exercises
hasAnySelected && hasActiveFiltertogether (e.g., a selected item with a reducedfilteredCount), which is the most complex branch combination inbuildEchartsOption.🤖 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/BarChart/buildEchartsOption.test.ts` around lines 64 - 93, Add a test for buildEchartsOption covering both a selected item and an active filter, using input where at least one item is selected and its filteredCount is lower than count. Assert the combined branch produces the expected selection and filtered overlay series behavior, including relevant series count and values.
🤖 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/BarChart/buildEchartsOption.ts`:
- Around line 127-157: The foregroundSeries and backgroundSeries type literals
are inferred too broadly, and the series construction should use the background
value as the condition. Add as const to both type: 'bar' literals, then build
series with backgroundSeries ? [backgroundSeries, foregroundSeries] :
[foregroundSeries] so null is excluded.
---
Nitpick comments:
In `@lightly_studio_view/src/lib/components/BarChart/buildEchartsOption.test.ts`:
- Around line 64-93: Add a test for buildEchartsOption covering both a selected
item and an active filter, using input where at least one item is selected and
its filteredCount is lower than count. Assert the combined branch produces the
expected selection and filtered overlay series behavior, including relevant
series count and values.
🪄 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: 96659d3f-72da-45b0-a542-6f64217a708e
📒 Files selected for processing (7)
lightly_studio_view/src/lib/components/BarChart/buildEchartsOption.test.tslightly_studio_view/src/lib/components/BarChart/buildEchartsOption.tslightly_studio_view/src/lib/components/BarChart/types.tslightly_studio_view/src/lib/components/DatasetDistributionPanel/DatasetDistributionPanel.sveltelightly_studio_view/src/lib/components/DatasetDistributionPanel/DatasetDistributionPanel.test.tslightly_studio_view/src/lib/components/DatasetDistributionPanel/types.tslightly_studio_view/src/routes/datasets/[dataset_id]/[collection_type]/[collection_id]/+layout.svelte
🚧 Files skipped from review as they are similar to previous changes (5)
- lightly_studio_view/src/lib/components/BarChart/types.ts
- lightly_studio_view/src/lib/components/DatasetDistributionPanel/types.ts
- lightly_studio_view/src/routes/datasets/[dataset_id]/[collection_type]/[collection_id]/+layout.svelte
- lightly_studio_view/src/lib/components/DatasetDistributionPanel/DatasetDistributionPanel.test.ts
- lightly_studio_view/src/lib/components/DatasetDistributionPanel/DatasetDistributionPanel.svelte
Comment on lines
+127
to
+157
| const foregroundSeries = { | ||
| type: 'bar', | ||
| data: foregroundData, | ||
| itemStyle: { color: BAR_COLOR }, | ||
| barCategoryGap: '25%', | ||
| // Overlay on top of the background series (barGap: '-100%' makes the bar | ||
| // occupy the same slot as the preceding series instead of shifting right). | ||
| ...(hasActiveFilter ? { barGap: '-100%' } : {}), | ||
| emphasis: CHART_EMPHASIS | ||
| }; | ||
|
|
||
| // Background series: full (unfiltered) count, always grey — only rendered | ||
| // when the filter is active and at least one bar is visibly reduced. | ||
| const backgroundSeries = hasActiveFilter | ||
| ? { | ||
| type: 'bar', | ||
| data: data.map((item) => ({ | ||
| value: item.count, | ||
| itemStyle: { | ||
| color: BAR_COLOR_BACKGROUND, | ||
| opacity: item.selectable === false ? 0.45 : 1 | ||
| } | ||
| })), | ||
| barCategoryGap: '25%', | ||
| // Suppress hover highlight on the background so emphasis styling | ||
| // (colour change, scale) only fires on the foreground bars. | ||
| emphasis: { disabled: true } | ||
| } | ||
| : null; | ||
|
|
||
| const series = hasActiveFilter ? [backgroundSeries, foregroundSeries] : [foregroundSeries]; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the target file and inspect the relevant section.
sed -n '1,240p' lightly_studio_view/src/lib/components/BarChart/buildEchartsOption.ts
# Find the option type definitions used by this component.
rg -n "EChartsCoreOption|ECBasicOption|series:" lightly_studio_view/src -g '!**/node_modules/**'Repository: lightly-ai/lightly-studio
Length of output: 10223
🏁 Script executed:
#!/bin/bash
set -euo pipefail
nl -ba lightly_studio_view/src/lib/components/BarChart/buildEchartsOption.ts | sed -n '1,220p'Repository: lightly-ai/lightly-studio
Length of output: 203
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the bar chart tests and the local type imports around ECharts usage.
sed -n '1,220p' lightly_studio_view/src/lib/components/BarChart/buildEchartsOption.test.ts
printf '\n---\n'
sed -n '1,220p' lightly_studio_view/src/lib/components/Histogram/buildHistogramOption.tsRepository: lightly-ai/lightly-studio
Length of output: 14747
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'JS'
for (const mod of ['typescript', 'ts-node']) {
try {
const path = require.resolve(mod);
console.log(mod + ': ' + path);
} catch (e) {
console.log(mod + ': not found');
}
}
JSRepository: lightly-ai/lightly-studio
Length of output: 249
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n '"echarts"|echarts/core|EChartsCoreOption' lightly_studio_view/package.json lightly_studio_view/pnpm-lock.yaml lightly_studio_view/package-lock.json lightly_studio_view/yarn.lock lightly_studio_view/src -g '!**/node_modules/**'Repository: lightly-ai/lightly-studio
Length of output: 3346
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n "type: 'bar' as const|type: 'custom' as const|series:" lightly_studio_view/src/lib/components -g '!**/node_modules/**'Repository: lightly-ai/lightly-studio
Length of output: 2038
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' lightly_studio_view/package.json
printf '\n--- TS probe ---\n'
node - <<'JS'
const ts = require('/usr/local/lib/node_modules/typescript');
const source = `
const hasActiveFilter = true as boolean;
const foregroundData = [] as number[];
const data = [] as { count: number }[];
const foregroundSeries = {
type: 'bar',
data: foregroundData,
itemStyle: { color: 'x' },
barCategoryGap: '25%',
...(hasActiveFilter ? { barGap: '-100%' } : {}),
emphasis: {}
};
const backgroundSeries = hasActiveFilter
? {
type: 'bar',
data: data.map((item) => ({ value: item.count })),
barCategoryGap: '25%',
emphasis: { disabled: true }
}
: null;
const series = hasActiveFilter ? [backgroundSeries, foregroundSeries] : [foregroundSeries];
`;
const fileName = 'probe.ts';
const sf = ts.createSourceFile(fileName, source, ts.ScriptTarget.Latest, true, ts.ScriptKind.TS);
const host = {
fileExists: (f) => f === fileName,
readFile: (f) => (f === fileName ? source : undefined),
getSourceFile: (f, lang) => (f === fileName ? sf : undefined),
getDefaultLibFileName: () => ts.getDefaultLibFileName({ target: ts.ScriptTarget.Latest }),
writeFile: () => {},
getCurrentDirectory: () => process.cwd(),
getDirectories: () => [],
getCanonicalFileName: (f) => f,
useCaseSensitiveFileNames: () => true,
getNewLine: () => '\n',
readDirectory: () => [],
};
const program = ts.createProgram([fileName], { noResolve: true, target: ts.ScriptTarget.Latest }, host);
const checker = program.getTypeChecker();
function findVar(name) {
let found;
function visit(node) {
if (ts.isVariableDeclaration(node) && node.name.kind === ts.SyntaxKind.Identifier && node.name.text === name) {
found = node;
}
ts.forEachChild(node, visit);
}
visit(sf);
return found;
}
for (const name of ['foregroundSeries', 'backgroundSeries', 'series']) {
const decl = findVar(name);
const type = checker.getTypeAtLocation(decl.name);
console.log(name + ': ' + checker.typeToString(type));
}
JSRepository: lightly-ai/lightly-studio
Length of output: 4827
Keep the bar series types narrow. Add as const to both type: 'bar' literals, and branch on backgroundSeries itself so null doesn’t flow into series:
backgroundSeries ? [backgroundSeries, foregroundSeries] : [foregroundSeries].
🤖 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/BarChart/buildEchartsOption.ts` around
lines 127 - 157, The foregroundSeries and backgroundSeries type literals are
inferred too broadly, and the series construction should use the background
value as the condition. Add as const to both type: 'bar' literals, then build
series with backgroundSeries ? [backgroundSeries, foregroundSeries] :
[foregroundSeries] so null is excluded.
Source: Learnings
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.
Important
Functional prototype — not ready to merge.
This PR implements the feature end to end so we can evaluate the UI/UX, collect feedback,
and identify quick fixes. A production-ready implementation will require code polishing
and splitting this work into smaller, focused PRs.
What has changed and why?
This prototype adds categorical metadata distributions and filtering to the dataset
distribution panel.
The goal is to validate whether users can:
The PR combines LIG-9584
and LIG-9585
to provide a functional end-to-end prototype.
Included in the prototype
Feedback requested
Please focus feedback on the product behavior and UI/UX rather than detailed code quality:
Not production-ready
This PR is intentionally broad and is not intended to be merged as-is. Production readiness
will require:
A likely split is:
How has it been tested?
Did you update CHANGELOG.md?
Summary by CodeRabbit
Summary by CodeRabbit