Skip to content

fix(knowledge-base): align Filter by metadata pickers with app dropdowns#13117

Merged
keval718 merged 4 commits into
release-1.10.0from
fix/chunks-metadata-filter-combobox
May 13, 2026
Merged

fix(knowledge-base): align Filter by metadata pickers with app dropdowns#13117
keval718 merged 4 commits into
release-1.10.0from
fix/chunks-metadata-filter-combobox

Conversation

@keval718
Copy link
Copy Markdown
Collaborator

@keval718 keval718 commented May 13, 2026

Summary

Knowledge-base UX polish bundle for 1.10.0: brings the chunks browser's Filter by metadata popover in line with the rest of the app, slims the component along single-responsibility lines, and closes a bug where the Ingest Files modal silently dropped invalid Custom Fields after letting the user click Next Step.

Three logical changes

  1. Combobox parity for the Filter by metadata key/value pickers.
  2. Refactor of ChunksMetadataFilter into focused modules (SOLID-aligned).
  3. Bug fix: gate Next Step on metadata-pair validity so the modal stops lying about what will be ingested.

1. Combobox parity for Filter by metadata

The popover used plain <Input> fields backed by a native HTML <datalist> for key/value suggestions. The browser-native dropdown looked nothing like the shadcn Select used right next to it (e.g. All sources) — QA flagged the inconsistency.

Both pickers now use a shadcn-style combobox so the popover matches every other dropdown in the app:

  • Each picker (key + value) is Popover + Command (cmdk):
    • Outline button trigger with chevron, visually identical to SelectTrigger.
    • Suggestion list rendered as CommandItem rows with a check on the current selection.
    • When the typed query is not in the suggestions, a Use "foo" item appears so users can still commit a custom key/value (preserves the free-text path the old datalist offered).
    • Closing a picker clears the search query; selecting an item commits and closes.
  • Wrapper popover state (open/close), KEY_PATTERN validation, refetch-on-open, error message, "Showing first 50 values per key" hint, and submit flow are unchanged.

2. ChunksMetadataFilter refactor

The original file held data fetching, derived option lists, validation rules, popover orchestration, and the combobox UI all in one place. Split along single-responsibility lines so each piece changes for its own reason:

  • MetadataCombobox.tsx — presentational Popover + Command combobox with custom-entry fallback. Generic over options / testId; reusable.
  • hooks/useChunksMetadataFilter.ts — wraps useGetKbMetadataKeys, derives availableKeys / valueSuggestions / flags / refetch. Encapsulates data plumbing.
  • metadataFilterValidation.ts — pure KEY_PATTERN + validateMetadataFilter returning { ok, key, value } | { ok: false, error }. Testable in isolation, no DOM.
  • ChunksMetadataFilter.tsx — orchestration only: outer popover, form state (key/value/error), submit. Composes the three above.

3. Bug fix: gate Next Step on metadata-pair validity

Symptom — In the Ingest Files modal → Custom Fields, typing an invalid key like Year showed the red "Keys must be 1-32 lowercase letters, digits, or underscores" message, but clicking Next Step still advanced to Review & Build. The summary even rendered Metadata: Year: 2020, suggesting the row would be persisted.

Root cause — three permissive layers:

  1. MetadataEditor validated key-on-blur and stored the error in component-local state; the parent never heard about it.
  2. useKnowledgeBaseForm.getValidationErrors validated name / embedding / backend / file size but did not check metadataPairs. So handleNext proceeded.
  3. StepReview rendered raw pairs, while metadataPairsToFormValue silently filtered the invalid row out at submit — backend never received it. The UI lied to the user.

Fix — single source of truth in metadataValidation.ts:

  • validateMetadataPair — per-row (empty, regex, value length).
  • validateMetadataPairs — cross-row (duplicates, max count).
  • filterValidMetadataPairs — clean trimmed set for display/submit.

Wired through every surface that used to disagree:

  • MetadataEditor derives errors from the prop list via the shared validator, dropping the blur-only local state. Errors always reflect the current input.
  • useKnowledgeBaseForm.getValidationErrors now checks run-level and per-file metadata. handleNext blocks while any row is invalid and surfaces a single validationErrors.metadata key.
  • StepConfiguration renders that error next to the Custom Fields block.
  • StepReview filters the summary through filterValidMetadataPairs so it only shows pairs the backend will actually persist.

Tests

  • New metadataFilterValidation.test.ts — covers chunks-browser key/value validator (regex, empty, uppercase, happy path).
  • New metadataValidation.test.ts — covers Ingest Files validator (per-row rules, duplicates, max-count cap, filterValidMetadataPairs).
  • Rewritten ChunksMetadataFilter.test.tsx — new combobox testids (chunks-metadata-filter-{key,value,input,option-*,custom}), plus a jsdom shim for Element.prototype.scrollIntoView because cmdk calls it on selection.
  • Full knowledgePage jest suite: 130/130 ✅
  • Full knowledgeBaseUploadModal jest suite: 91/91 ✅
  • make format_frontend clean.

Test plan

Filter by metadata combobox

  • Open a KB → chunks browser → click + Filter by metadata.
  • Key picker opens with same styling as All sources dropdown (outline trigger + chevron, item list with hover/selected check).
  • Picking a suggested key populates the value picker with the matching values.
  • Typing a key not in the suggestions shows Use "xyz" — selecting it commits the typed value.
  • Uppercase key still triggers the existing validation error.
  • Add Filter chains: popover closes, chip appears, popover reopens with cleared inputs.
  • Refetch fires every time the outer popover reopens (cached + fresh ingestions both visible).
  • truncated: true from the server still shows the "Showing first 50 values per key" hint.

Ingest Files → Custom Fields validation gating

  • Type an invalid key (uppercase, punctuation, blank value) → Next Step is blocked and the error renders next to the editor.
  • Duplicate keys: both rows marked, Next Step still blocked until resolved.
  • Add >16 rows: set-level error shows; Next blocked.
  • With only valid rows, Next proceeds; Review & Build summary shows only the valid set (no phantom invalid pair).
  • Per-file metadata override with an invalid key also blocks Next.

Screenshots

Before
image

image

After
image

image

The "+ Filter by metadata" popover used plain <Input> fields backed by a
native HTML <datalist> for key/value suggestions. The browser-native
dropdown looked nothing like the shadcn `Select` used right next to it
(e.g. "All sources") — QA flagged the inconsistency.

Replace each free-text input with a shadcn-style combobox built on
Popover + Command (cmdk):
  - Outline button trigger with chevron, matching SelectTrigger.
  - Suggestion list rendered as CommandItem rows with a check on the
    current selection.
  - When the typed query is not in the suggestions, a "Use \"foo\"" item
    appears so users can still commit a custom key/value.
  - Closing the picker clears the search query; selecting an item
    commits and closes.

Validation, refetch-on-open, error messaging, and submit flow are
unchanged. Tests rewritten against the new combobox testids and a
jsdom shim for Element.prototype.scrollIntoView (cmdk calls it).
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 13, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: c5ee4cde-6436-461c-8fa9-32e440a2db1a

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/chunks-metadata-filter-combobox

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added the bug Something isn't working label May 13, 2026
@keval718 keval718 requested a review from erichare May 13, 2026 21:22
@github-actions github-actions Bot added bug Something isn't working and removed bug Something isn't working labels May 13, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 13, 2026

Codecov Report

❌ Patch coverage is 95.20154% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.74%. Comparing base (ea29bcc) to head (9a853e0).
⚠️ Report is 1 commits behind head on release-1.10.0.

Files with missing lines Patch % Lines
...ledgeBaseUploadModal/hooks/useKnowledgeBaseForm.ts 40.00% 9 Missing ⚠️
...ledgeBaseUploadModal/components/MetadataEditor.tsx 91.42% 6 Missing ⚠️
...geBaseUploadModal/components/StepConfiguration.tsx 25.00% 6 Missing ⚠️
...e/sourceChunksPage/components/MetadataCombobox.tsx 97.51% 4 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@                Coverage Diff                 @@
##           release-1.10.0   #13117      +/-   ##
==================================================
+ Coverage           54.59%   54.74%   +0.14%     
==================================================
  Files                2140     2144       +4     
  Lines              199789   200145     +356     
  Branches            30123    30188      +65     
==================================================
+ Hits               109084   109562     +478     
+ Misses              89523    89402     -121     
+ Partials             1182     1181       -1     
Flag Coverage Δ
backend 59.88% <ø> (-0.05%) ⬇️
frontend 54.77% <95.20%> (+0.21%) ⬆️
lfx 49.30% <ø> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...knowledgeBaseUploadModal/components/StepReview.tsx 85.07% <100.00%> (-0.23%) ⬇️
...geBaseUploadModal/components/metadataValidation.ts 100.00% <100.00%> (ø)
...urceChunksPage/components/ChunksMetadataFilter.tsx 97.07% <100.00%> (+3.01%) ⬆️
...ksPage/components/hooks/useChunksMetadataFilter.ts 100.00% <100.00%> (ø)
...eChunksPage/components/metadataFilterValidation.ts 100.00% <100.00%> (ø)
...e/sourceChunksPage/components/MetadataCombobox.tsx 95.45% <97.51%> (ø)
...ledgeBaseUploadModal/components/MetadataEditor.tsx 91.74% <91.42%> (+1.18%) ⬆️
...geBaseUploadModal/components/StepConfiguration.tsx 95.30% <25.00%> (-1.26%) ⬇️
...ledgeBaseUploadModal/hooks/useKnowledgeBaseForm.ts 83.10% <40.00%> (-0.92%) ⬇️

... and 39 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions github-actions Bot added bug Something isn't working and removed bug Something isn't working labels May 13, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 13, 2026

Frontend Unit Test Coverage Report

Coverage Summary

Lines Statements Branches Functions
Coverage: 38%
38.38% (47838/124623) 67.87% (6533/9625) 38.22% (1097/2870)

Unit Test Results

Tests Skipped Failures Errors Time
4295 0 💤 0 ❌ 0 🔥 9m 14s ⏱️

keval718 added 2 commits May 13, 2026 17:35
…ules

ChunksMetadataFilter was holding data fetching, derived option lists,
validation rules, popover orchestration, and the combobox UI all in one
file. Split along single-responsibility lines so each piece changes for
its own reason:

  - MetadataCombobox.tsx — presentational Popover+Command combobox with
    custom-entry fallback. Generic over options/testId; reusable.
  - hooks/useChunksMetadataFilter.ts — wraps useGetKbMetadataKeys and
    derives availableKeys / valueSuggestions / flags / refetch.
  - metadataFilterValidation.ts — pure KEY_PATTERN + validateMetadataFilter
    returning { ok, key, value } | { ok: false, error }. Testable in
    isolation; no DOM.
  - ChunksMetadataFilter.tsx — orchestration only: outer popover, form
    state, submit. Composes the three above.

Adds focused unit tests for the validator (regex + happy path + empty
+ uppercase). Existing integration test for the filter itself is left
untouched.
Custom Fields validation only fired on blur and lived in MetadataEditor's
local state, so an invalid key like "Year" still let the user advance to
Review & Build. StepReview rendered the raw pairs (showing "Year: 2020"
in the summary) but ``metadataPairsToFormValue`` silently filtered the
invalid row out at submit — the backend never received that metadata.
The UI was effectively lying.

Single source of truth in metadataValidation.ts:

  - validateMetadataPair (per-row: empty, regex, value length)
  - validateMetadataPairs (cross-row: duplicates, max count)
  - filterValidMetadataPairs (clean trimmed set for display/submit)

Wired through:

  - MetadataEditor derives errors from the prop list via the shared
    validator, dropping the blur-only local state. Errors always
    reflect the current input.
  - useKnowledgeBaseForm.getValidationErrors now checks run-level and
    per-file metadata. handleNext blocks while any row is invalid and
    surfaces a single error key under validationErrors.metadata.
  - StepConfiguration renders that error next to the Custom Fields
    block.
  - StepReview filters the summary through filterValidMetadataPairs so
    it only shows pairs the backend will actually persist.

Adds focused unit tests for the new validator (per-row rules, dupes,
max-count, filter behavior).
@github-actions github-actions Bot added bug Something isn't working and removed bug Something isn't working labels May 13, 2026
@github-actions github-actions Bot added bug Something isn't working and removed bug Something isn't working labels May 13, 2026
Copy link
Copy Markdown
Collaborator

@erichare erichare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@github-actions github-actions Bot added lgtm This PR has been approved by a maintainer bug Something isn't working and removed bug Something isn't working labels May 13, 2026
@keval718 keval718 enabled auto-merge May 13, 2026 21:43
@github-actions github-actions Bot added bug Something isn't working and removed bug Something isn't working labels May 13, 2026
@keval718 keval718 self-assigned this May 13, 2026
@github-actions github-actions Bot added bug Something isn't working and removed bug Something isn't working labels May 13, 2026
@keval718 keval718 added this pull request to the merge queue May 13, 2026
Merged via the queue into release-1.10.0 with commit 0d80a36 May 13, 2026
104 of 105 checks passed
@keval718 keval718 deleted the fix/chunks-metadata-filter-combobox branch May 13, 2026 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working lgtm This PR has been approved by a maintainer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants