refactor: extract shared DataTableTab shell for files and knowledge tabs - #14249
refactor: extract shared DataTableTab shell for files and knowledge tabs#14249tarciorodrigues wants to merge 7 commits into
Conversation
FilesTab had zero test files. Per the ticket, coverage must exist BEFORE the shared dataTableTabComponent shell extraction: rows render (date-sorted), quick filter narrows (input wiring + forward to the grid), selection enables bulk delete, delete confirms through the modal. Also covers loading state, empty state and the selection reset on data refresh, which the shell will absorb. 13 tests, green against the untouched component, mirroring the KnowledgeBasesTab.test.tsx mocking pattern. Refs: LE-1736 W06
Extracts the shell that FilesTab and KnowledgeBasesTab duplicate today: search Input wired to quickFilterText, toolbar actions slot, the shared TableComponent configuration (row heights, multi selection, pagination, shared grid options) and the line-identical selection-changed handler (row propagation + delayed quantity clear). Divergent behavior stays behind props: table passthroughs (editable, onCellKeyDown, onRowClicked, getRowId, extra gridOptions, className), search input identity, a table wrapper hook for the files drop zone, and raw loadingState/emptyState slots so both tabs keep their exact current DOM for those states. No consumer is rewired yet; the tabs adopt the shell in follow-up commits. Refs: LE-1736 W07
FilesTab keeps only its column defs, data hooks, upload/rename/delete wiring and the drop-zone wrapper; the duplicated shell (search input, toolbar slot, shared table configuration, selection handler) now comes from dataTableTabComponent. Loading and empty states are passed as raw slots carrying the exact previous DOM. The 13-test behavior suite from the previous commit passes unchanged against the rebased component. tsc reports zero new errors on touched files (the two TS2322 hits pre-exist on the untouched baseline at the same code sites). Refs: LE-1736 W08
KnowledgeBasesTab keeps its data hooks (polling, actions, optimistic update), column factory, row/cell keyboard handlers, focus management and the three controlled modals (single delete, bulk delete, upload — now passed as shell children); the duplicated shell markup and the selection handler come from dataTableTabComponent. Loading and empty states are raw slots preserving the exact previous DOM (both were early returns without wrappers). KB-specific grid behavior stays behind shell props: getRowId, isRowSelectable/paginationAutoPageSize gridOptions, ag-knowledge-table class, onRowClicked. All 15 knowledgePage test suites pass unchanged (130 tests, zero assertion changes). tsc reports zero errors on touched files. Refs: LE-1736 W09
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughChangesShared data table tab migration
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
actor User
participant FilesTab
participant DataTableTab
participant TableComponent
User->>FilesTab: enter search text or select rows
FilesTab->>DataTableTab: pass filter, row data, toolbar, and selection setters
DataTableTab->>TableComponent: render filtered, selectable table
TableComponent-->>DataTableTab: emit selection change
DataTableTab-->>FilesTab: update selected rows and quantity
FilesTab-->>User: show upload or bulk-delete action
Suggested reviewers: 🚥 Pre-merge checks | ✅ 8 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (8 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 |
✅ Test Coverage AdvisorNo source changes detected without accompanying tests. Thanks for keeping coverage up! 🎉
|
github-actions
Bot
added
refactor
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/frontend/src/pages/MainPage/pages/filesPage/components/__tests__/FilesTab.test.tsx (1)
18-38: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffReal grid selection/pagination behavior stays untested behind the table mock.
Replacing
TableComponentwith adivthat captures props means selection, quick-filter matching, and pagination are only asserted at the prop-wiring level (e.g.mockLatestTableProps.quickFilterText), not against actual ag-grid behavior. The unit tests here are reasonable, but consider an integration/Playwright test that exercises the real grid so the migration's selection and filtering are verified end-to-end.As per coding guidelines: "prefer integration tests when unit tests are overly mocked" and "use Playwright where appropriate".
🤖 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 `@src/frontend/src/pages/MainPage/pages/filesPage/components/__tests__/FilesTab.test.tsx` around lines 18 - 38, add an integration or Playwright test for FilesTab that renders the real TableComponent/ag-grid instead of the mocked div and verifies row selection, quick-filter matching, and pagination through user-visible grid behavior. Keep the existing unit tests for prop wiring, while covering the migration’s end-to-end behavior without relying on mockLatestTableProps.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.
Inline comments:
In `@src/frontend/src/components/core/dataTableTabComponent/index.tsx`:
- Around line 72-82: Update handleSelectionChanged to store the deferred reset
timer in a ref, clear any existing timer on every selection change before
applying the new selection count, and clear the timer during component unmount
via the component’s effect lifecycle. Preserve the 300ms delayed reset only when
no rows are selected.
In `@src/frontend/src/pages/MainPage/pages/filesPage/components/FilesTab.tsx`:
- Around line 368-376: Update the sortedFiles computation in FilesTab to clone
files before sorting, mirroring the non-mutating pattern used by
KnowledgeBasesTab. Preserve the existing comparator and loading behavior while
ensuring the React Query cache array is never modified in place.
---
Nitpick comments:
In
`@src/frontend/src/pages/MainPage/pages/filesPage/components/__tests__/FilesTab.test.tsx`:
- Around line 18-38: add an integration or Playwright test for FilesTab that
renders the real TableComponent/ag-grid instead of the mocked div and verifies
row selection, quick-filter matching, and pagination through user-visible grid
behavior. Keep the existing unit tests for prop wiring, while covering the
migration’s end-to-end behavior without relying on mockLatestTableProps.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1d2848d4-9be5-4361-b36d-ce87d9697a83
📒 Files selected for processing (4)
src/frontend/src/components/core/dataTableTabComponent/index.tsxsrc/frontend/src/pages/MainPage/pages/filesPage/components/FilesTab.tsxsrc/frontend/src/pages/MainPage/pages/filesPage/components/__tests__/FilesTab.test.tsxsrc/frontend/src/pages/MainPage/pages/knowledgePage/components/KnowledgeBasesTab.tsx
| const handleSelectionChanged = (event: SelectionChangedEvent) => { | ||
| const selectedRows = event.api.getSelectedRows(); | ||
| setSelectedRows(selectedRows); | ||
| if (selectedRows.length > 0) { | ||
| setQuantitySelected(selectedRows.length); | ||
| } else { | ||
| setTimeout(() => { | ||
| setQuantitySelected(0); | ||
| }, 300); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Deferred reset can clobber a fresh selection and leaks past unmount.
handleSelectionChanged schedules setQuantitySelected(0) after 300ms but never tracks/clears the timer. If the user re-selects rows within that window, the immediate setQuantitySelected(count) is later overwritten by the stale timeout (resetting the toolbar to 0 while rows remain selected). The timer can also fire after the tab unmounts. Track the handle and clear it on the next selection change and on unmount.
🛡️ Suggested guard
+ const resetTimerRef = useRef<ReturnType<typeof setTimeout>>();
const handleSelectionChanged = (event: SelectionChangedEvent) => {
const selectedRows = event.api.getSelectedRows();
setSelectedRows(selectedRows);
+ if (resetTimerRef.current) clearTimeout(resetTimerRef.current);
if (selectedRows.length > 0) {
setQuantitySelected(selectedRows.length);
} else {
- setTimeout(() => {
+ resetTimerRef.current = setTimeout(() => {
setQuantitySelected(0);
}, 300);
}
};(add useEffect(() => () => clearTimeout(resetTimerRef.current), []) for unmount)
🤖 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 `@src/frontend/src/components/core/dataTableTabComponent/index.tsx` around
lines 72 - 82, Update handleSelectionChanged to store the deferred reset timer
in a ref, clear any existing timer on every selection change before applying the
new selection count, and clear the timer during component unmount via the
component’s effect lifecycle. Preserve the 300ms delayed reset only when no rows
are selected.
| const isLoadingFiles = !files || !Array.isArray(files); | ||
| const sortedFiles = isLoadingFiles | ||
| ? [] | ||
| : files.sort((a, b) => { | ||
| return sortByDate( | ||
| a.updated_at ?? a.created_at, | ||
| b.updated_at ?? b.created_at, | ||
| ); | ||
| }); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Avoid mutating the React Query cache with in-place .sort().
files.sort(...) sorts the array returned by useGetFilesV2() in place, mutating the cached query data and any other consumers holding that reference. KnowledgeBasesTab already does this correctly via [...knowledgeBases].sort(...); mirror that here.
🐛 Proposed fix
const sortedFiles = isLoadingFiles
? []
- : files.sort((a, b) => {
+ : [...files].sort((a, b) => {
return sortByDate(
a.updated_at ?? a.created_at,
b.updated_at ?? b.created_at,
);
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const isLoadingFiles = !files || !Array.isArray(files); | |
| const sortedFiles = isLoadingFiles | |
| ? [] | |
| : files.sort((a, b) => { | |
| return sortByDate( | |
| a.updated_at ?? a.created_at, | |
| b.updated_at ?? b.created_at, | |
| ); | |
| }); | |
| const isLoadingFiles = !files || !Array.isArray(files); | |
| const sortedFiles = isLoadingFiles | |
| ? [] | |
| : [...files].sort((a, b) => { | |
| return sortByDate( | |
| a.updated_at ?? a.created_at, | |
| b.updated_at ?? b.created_at, | |
| ); | |
| }); |
🤖 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 `@src/frontend/src/pages/MainPage/pages/filesPage/components/FilesTab.tsx`
around lines 368 - 376, Update the sortedFiles computation in FilesTab to clone
files before sorting, mirroring the non-mutating pattern used by
KnowledgeBasesTab. Preserve the existing comparator and loading behavior while
ensuring the React Query cache array is never modified in place.
Frontend Unit Test Coverage ReportCoverage Summary
Unit Test Results
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## release-1.12.0 #14249 +/- ##
==================================================
+ Coverage 61.35% 62.00% +0.65%
==================================================
Files 2345 2340 -5
Lines 238012 236327 -1685
Branches 35528 33322 -2206
==================================================
+ Hits 146021 146530 +509
+ Misses 90181 88034 -2147
+ Partials 1810 1763 -47
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
Cristhianzl
left a comment
Cristhianzl
left a comment
There was a problem hiding this comment.
⚠️ Important (preferably this PR)
I1 — The new shared component has zero direct tests, and the added suite mocks away everything it owns
File: src/frontend/src/components/core/dataTableTabComponent/index.tsx (no __tests__/)
Issue: DataTableTab is now on the render path of both /assets/files and /assets/knowledge-bases, and it owns the loading/empty short-circuit, the shared gridOptions merge, the selection handler, the class-name merge and the renderTableWrapper hook. There is no test file for it. The new FilesTab.test.tsx mocks TableComponent wholesale (FilesTab.test.tsx:39-59), so the props the shell now composes — getRowId, isRowSelectable, paginationAutoPageSize, editable, the ag-knowledge-table class, the stopEditingWhenCellsLoseFocus / ensureDomOrder / colResizeDefault defaults — are asserted by nothing.
Why it matters: The whole value proposition of this PR is "one implementation instead of two". That only holds if the one implementation is guarded. Today a regression in the gridOptions spread order or a dropped passthrough breaks both pages at once and no unit test fails — the 130 knowledge-page tests pass because they mock the same layer. The evidence screenshots cover it once, manually; nothing covers it on the next change.
Suggested fix: Add dataTableTabComponent/__tests__/index.test.tsx with a mocked TableComponent that captures props, and assert: (a) isLoading renders loadingState and nothing else, (b) empty rowData renders emptyState and nothing else, (c) consumer gridOptions merge on top of the shared defaults rather than replacing them, (d) tableClassName is merged into the base classes, (e) renderTableWrapper wraps the table, (f) the selection handler forwards rows and defers the zero.
I2 — Configuration over composition: 24 props for two consumers
File: src/frontend/src/components/core/dataTableTabComponent/index.tsx:16-49
Issue: DataTableTabProps has 24 members. Roughly half are pure AG Grid passthroughs (editable, onCellKeyDown, onRowClicked, getRowId, gridOptions, tableRef, tableClassName), three are raw ReactNode slots that each tab fills with its exact previous DOM, and one is a render prop. .claude/rules/frontend.md is explicit: "Composition over configuration; prefer explicit variant components over boolean-flag combinations."
Why it matters: With loadingState, emptyState and toolbarActions passed as opaque nodes, the shell isn't really sharing behavior — it's sharing a <div className="flex h-full flex-col"> plus an <Input> plus a 10-line handler. Every future divergence adds another prop, and the two call sites are now coupled through an interface neither of them fully uses (Files passes no onRowClicked/getRowId; Knowledge passes no editable/renderTableWrapper). This is the rule-of-three problem: two consumers rarely justify a parameterized shell.
Suggested fix: Not necessarily a rewrite. Either (a) narrow the surface by exporting the two genuinely shared pieces — a <TableTabHeader search toolbar> and a useTableSelection() hook returning the selection handler — and let each tab keep its own table, or (b) keep the shell but collapse the AG Grid passthroughs into a single tableProps?: Partial<TableComponentProps> so the interface stops growing one prop per divergence. If it stays as-is, note in the module docblock that the slot props exist to freeze current DOM, not as a design goal.
Code reference
export interface DataTableTabProps<TData> {
columnDefs: ColDef[];
rowData: TData[];
isLoading: boolean;
loadingState: ReactNode;
emptyState: ReactNode;
// …19 more
}I3 — children are silently dropped in the loading and empty branches
File: src/frontend/src/components/core/dataTableTabComponent/index.tsx:84-96, 146
Issue: The shell early-returns <>{loadingState}</> and <>{emptyState}</> before it ever renders {children}. KnowledgeBasesTab passes three modals as children — two DeleteConfirmationModals and KnowledgeBaseUploadModal — so none of them mount while the list is loading or empty. This matches the pre-refactor early returns exactly, so it is not a regression today; the problem is that the contract is now invisible at the call site.
Why it matters: children is documented as "Feature-specific modals rendered inside the shell root" (line 48), which reads as unconditional. The next person who adds a modal that must survive an empty list — e.g. a "create first knowledge base" dialog, or the upload modal once it can be opened from the empty state — will get a dialog that silently never opens, and the failure mode (nothing happens on click) is hard to trace back to a rowData.length === 0 check two components away.
Suggested fix: Render children in all three branches (<>{loadingState}{children}</> etc.) — mounting three closed dialogs costs nothing — or, if the current mounting behavior is deliberate, change the JSDoc to say so: "Rendered only when the table is shown; not mounted during loading/empty."
Code reference
if (isLoading) {
return <>{loadingState}</>;
}
if (rowData.length === 0) {
return <>{emptyState}</>;
}💡 Recommended (can ship as a follow-up)
R1 — files.sort() mutates the React Query cache array in place
File: src/frontend/src/pages/MainPage/pages/filesPage/components/FilesTab.tsx:368-375
Issue: files.sort(...) sorts the array returned by useGetFilesV2 in place. This is pre-existing (the base did the same inline in JSX), and the PR explicitly preserves it — but the refactor moved it from a JSX expression to a named sortedFiles binding, which is the natural moment to fix it.
Why it matters: Mutating cached query data means any other consumer of the same query key observes a different order than the server returned, and React Query's structural-sharing equality checks can miss the change. It also makes sortedFiles === files true, so the shell's rowData prop is referentially the cache array.
Suggested fix: [...files].sort(...). One character of allocation, removes a whole class of cross-component surprise. Add an assertion to the new suite that the hook's data array is not reordered.
Code reference
const isLoadingFiles = !files || !Array.isArray(files);
const sortedFiles = isLoadingFiles
? []
: files.sort((a, b) => { … });R2 — isLoading conflates "still fetching" with "payload is not an array"
File: src/frontend/src/pages/MainPage/pages/filesPage/components/FilesTab.tsx:368
Issue: isLoadingFiles = !files || !Array.isArray(files) maps three distinct states — query pending, query errored (data === undefined), and a malformed non-array payload — onto one spinner. The PR note calls out the third case as unreachable with the real API, which is fair, but the errored case is very reachable and shows an infinite spinner with no message and no retry.
Why it matters: A 500 or a network drop on /api/v2/files leaves the user staring at a spinner forever. useGetFilesV2 exposes isError/error; nothing reads them.
Suggested fix: Follow-up: add an errorState?: ReactNode slot to the shell (it already has the loading/empty slot pattern) and wire isError from both tabs. Keep isLoading meaning isLoading.
R3 — The Files search input still ships without an accessible name
File: src/frontend/src/pages/MainPage/pages/filesPage/components/FilesTab.tsx (searchInputAriaLabel not passed) · shell at dataTableTabComponent/index.tsx:135
Issue: The shell added an optional searchInputAriaLabel, Knowledge passes one, Files leaves it undefined. The input has a placeholder but no label, so screen readers announce it as an unlabeled textbox. .claude/rules/frontend.md: "Every control has a label."
Why it matters: Pre-existing gap, but the PR just built the exact seam that fixes it in one line, and D1 in the description records the missing aria-label as a divergence to preserve rather than a defect to close.
Suggested fix: Pass searchInputAriaLabel={t("files.searchFiles")} (or a dedicated key) and make the prop required on the shell so the next consumer can't skip it.
R4 — handleSelectionChanged is recreated on every render
File: src/frontend/src/components/core/dataTableTabComponent/index.tsx:72-82
Issue: The handler is a plain function declared in the component body and passed to TableComponent → AG Grid on every render. Same as before the refactor, but it is now on the shared path for both pages.
Why it matters: AG Grid re-binds the listener each render; with paginationAutoPageSize on the knowledge table and frequent polling-driven re-renders, this is measurable churn on large lists.
Suggested fix: Wrap in useCallback with [setSelectedRows, setQuantitySelected]. Also worth capturing the setTimeout(…, 300) handle and clearing it on unmount — right now a fast unmount after deselection fires setQuantitySelected(0) on an unmounted tree.
🔹 Nice-to-have
N1 — The TData generic isn't actually enforced
File: src/frontend/src/components/core/dataTableTabComponent/index.tsx:72-74
event.api.getSelectedRows() returns any[], and it flows straight into setSelectedRows(rows: TData[]). The generic gives callers type-safe rowData, but the selection path is an unchecked widening — DataTableTab<FileType> would happily hand KnowledgeBaseInfo rows to setSelectedFiles. Consider event.api.getSelectedRows<TData>() if the AG Grid typings allow it, or an explicit as TData[] with a Why: comment so the hole is visible.
N2 — Search input class order changed (cosmetic, no action needed)
FilesTab went from className="mr-2 w-full" to cn("w-full", "mr-2"). cn/tailwind-merge handles it and the S1 screenshots confirm no visual delta — noting it only so a reviewer diffing class strings doesn't stop on it.
…intent The shell carries 24 props for two consumers. Per review I2, add a module docblock noting that the ReactNode slots and AG-Grid passthroughs exist to freeze each tab's current DOM during the two-into-one extraction, not as a design goal, and that a third consumer should narrow the surface rather than grow it. Refs: LE-1736
…tates The shell early-returned loadingState/emptyState before rendering children, so the feature modals (Knowledge's upload + delete dialogs) never mounted while the list was loading or empty — a modal opened from the empty state (e.g. "create the first knowledge base") set its open flag against a dialog that was not in the tree. Render children in all three branches (review I3); the dialogs are controlled and closed by default, so mounting them costs nothing. Refs: LE-1736
The shared shell had zero direct tests and the tab suites mock TableComponent wholesale, so the props the shell composes were asserted by nothing (review I1). Add dataTableTabComponent/__tests__/index.test.tsx with a prop-capturing TableComponent mock, covering: the loading/empty short-circuits render nothing else, children mount in all three states, consumer gridOptions merge on top of the shared defaults, tableClassName is merged into the base classes, renderTableWrapper wraps the table, and the selection handler forwards rows immediately while deferring the zero by 300ms. Refs: LE-1736
github-actions
Bot
added
refactor
2 participants
Why
FilesTab(501 lines, zero tests) andKnowledgeBasesTab(533 lines) duplicate the same table-tab shell: search Input wired toquickFilterText, a conditional primary/delete toolbar, the sameTableComponentconfiguration (row heights, multi selection, pagination, shared grid options) and a line-identical selection-changed handler including the delayed quantity clear. Part of the Tier B consolidations (LE-1736, WP3a).What
filesPage/components/__tests__/FilesTab.test.tsx, 13 tests) — the ticket-mandated missing coverage, committed BEFORE any refactor and green against the untouched component: rows render date-sorted, quick filter narrows (input wiring + forward to the grid), selection enables bulk delete, delete confirms through the modal, plus loading/empty states and the selection reset on data refresh. It mirrors the existingKnowledgeBasesTab.test.tsxmocking pattern.components/core/dataTableTabComponent/— single implementation of the shared shell. Divergent behavior stays behind props: search-input identity,toolbarActionsslot, rawloadingState/emptyStateslots (each tab keeps its exact current DOM for those states), table passthroughs (editable,onCellKeyDown,onRowClicked,getRowId, extragridOptions,tableClassName), and arenderTableWrapperhook for the files drop zone.filesPage/index.tsx,knowledgePage/index.tsx) are untouched, andgit diff --name-onlyagainst the base shows only the four expected files.Divergence list (every shell difference → where it lives now)
search-store-input, files placeholder,mr-2, no aria-labelsearch-kb-input, aria-label,w-fullsearchInputTestId/searchPlaceholder/searchInputAriaLabel/searchInputClassNameasChild)toolbarActionsslot — each tab passes its exact current nodesetSelectedRows/setQuantitySelected)py-4+ centered spinnerflex-1centered spinner + captionloadingStateraw slotpy-4+ drop-zone upload cardKnowledgeBaseEmptyStateearly returnemptyStateraw slotonCellKeyDown, drop-zone wrapperonRowClicked,getRowId,isRowSelectable+paginationAutoPageSize,ag-knowledge-tableclass, ownonCellKeyDownrowDataprop — each tab keeps its own sortuseEffectresets selection when file data changesHow to validate
Run the numbered script below on EACH tab — Files at
/assets/filesand Knowledge at/assets/knowledge-bases. The evidence screenshots are numbered 1:1 with these steps (compare step Sn with image Sn):Plus: devtools console shows zero new errors vs baseline throughout.
Evidence — Files (S1–S9, before | after)
Evidence — Knowledge (S1–S9, before | after)
Tests
npx jest src/pages/MainPage/pages/filesPage— 13/13, committed FIRST and green against the untouched FilesTab, then unchanged and green against the rebased one.npx jest src/pages/MainPage/pages/knowledgePage— all 15 suites, 130/130, zero assertion changes.make test_frontend— full run 475 suites / 5,494 tests, 100% green.npx playwright test --list— 437 tests / 152 files collected clean.npx biome checkclean on all touched files;tsc --noEmitadds zero new errors on touched files (the two TS2322 hits on FilesTab pre-exist on the untouched baseline at the same code sites; repo-wide error count unchanged at 272).Note
onSelectionChangedandonDeleteas shell props. In today's code the selection handler is line-identical in both tabs, so the shell absorbs it (setSelectedRows/setQuantitySelected); the delete confirmations differ structurally (files: trigger-based modal wrapping the toolbar button; knowledge: two controlled modals with focus restore), so they stay in each tab's slot/children and anonDeleteshell prop would be dead API. Same intent, current-code shape — the same kind of deviation the PR-A strategy object documented.Refs LE-1736
Summary by CodeRabbit
New Features
Bug Fixes
Tests