Split marm-console's Knowledge.tsx into per-component files#100
Conversation
Pure refactor, no behavior changes. Extracted via exact index-based line-slicing (all 6 moved bodies verified byte-identical to the original before any import cleanup) -- same methodology as the Memory.tsx split. - src/pages/Knowledge.tsx keeps only the KnowledgePage shell: 871 -> 45 lines. - src/components/knowledge/shared.tsx (new, 41 lines): DEFAULT_HIDDEN_PREDICATES, TYPE_COLORS, OTHER_TYPE_COLOR, typeColor, nodeRadius, mergeNeighborhoods. - src/components/knowledge/GraphViz.tsx (new, 291 lines): the canvas/react-force-graph-2d visualization component alone -- the largest and most self-contained piece. - src/components/knowledge/ExplorerTab.tsx (new, 340 lines): ProvenancePanel + ExplorerTab -- kept together since ProvenancePanel is only ever rendered by ExplorerTab, receiving all its data as props. - src/components/knowledge/BuildAndDuplicates.tsx (new, 172 lines): BuildConceptsDialog + DuplicatesTab -- functionally unrelated but both small and both used directly by the page shell, grouped for file-count balance rather than functional coupling. Split shape differs from the Memory.tsx split (flat sibling tabs) since this file has real parent-child coupling (GraphViz/ProvenancePanel are children of ExplorerTab, not sibling tabs) -- documented in docs/current/knowledge-tsx-module-split.md. Each extracted component/const referenced across file boundaries gained `export` (mechanical TS requirement). One real gap caught by tsc --noEmit during implementation: ExplorerTab.tsx's import list initially omitted DEFAULT_HIDDEN_PREDICATES from shared.tsx despite using it -- fixed before commit. Validated: tsc --noEmit clean, npm run build succeeds, and a real Playwright smoke pass against the running dev server confirmed both tabs render and the Build Concepts dialog opens with a working scope selector, zero JS console errors.
|
Warning Review limit reached
Next review available in: 16 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughChangesKnowledge console
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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
`@marm-console/artifacts/marm-console/src/components/knowledge/BuildAndDuplicates.tsx`:
- Around line 29-38: Update the build dialog lifecycle around isRunning and the
existing dialog open/close state: treat an active job whose initial jobStatus
fetch is still loading as running so the Close action is unavailable until
status is known. Add a close-state effect that clears jobId and resets all build
form/status state when the dialog closes, while preserving state during an open
dialog and allowing a fresh build on the next opening.
In
`@marm-console/artifacts/marm-console/src/components/knowledge/ExplorerTab.tsx`:
- Around line 304-330: Update the render condition around GraphViz in
ExplorerTab so loadError is checked before graph, preventing a retained previous
graph from rendering after a failed seed or direction request. Keep the existing
graph rendering for successful loads and preserve the current loadError message
and UI.
🪄 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
Run ID: 0c367b22-fd64-48d9-92e1-2a0de852ade4
📒 Files selected for processing (5)
marm-console/artifacts/marm-console/src/components/knowledge/BuildAndDuplicates.tsxmarm-console/artifacts/marm-console/src/components/knowledge/ExplorerTab.tsxmarm-console/artifacts/marm-console/src/components/knowledge/GraphViz.tsxmarm-console/artifacts/marm-console/src/components/knowledge/shared.tsxmarm-console/artifacts/marm-console/src/pages/Knowledge.tsx
📜 Review details
🔇 Additional comments (4)
marm-console/artifacts/marm-console/src/components/knowledge/shared.tsx (1)
1-41: LGTM!marm-console/artifacts/marm-console/src/components/knowledge/GraphViz.tsx (1)
1-291: LGTM!marm-console/artifacts/marm-console/src/components/knowledge/ExplorerTab.tsx (1)
1-303: LGTM!Also applies to: 331-340
marm-console/artifacts/marm-console/src/pages/Knowledge.tsx (1)
1-45: LGTM!
Both findings were confirmed real, pre-existing bugs in the original Knowledge.tsx (carried over verbatim by the split). Fixing here since this branch is the only place with easy access to the touched code right now, same reasoning as PR #99's follow-up fixes. - BuildAndDuplicates.tsx: isRunning now also covers the initial poll gap (jobId set but jobStatus not yet loaded), so the Close button can't appear before the job's real status is known. Added a close-effect that resets jobId/scope/scopeValue/confirmAll when the dialog closes, so reopening starts a fresh build instead of showing the previous job's finished state. - ExplorerTab.tsx: loadError is now checked before the stale-graph branch, so a failed seed/direction request shows the error state instead of silently re-rendering the previous successful graph. Verified: tsc --noEmit clean, npm run build succeeds, Playwright smoke pass against the live dev server confirmed no JS errors and the dialog reset behavior works end-to-end (open -> close -> reopen shows a fresh scope selector, not the prior job's status view). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011s8tBNnQTmb1q5nRAKwhvv
All 5 findings were confirmed real, pre-existing bugs in the original Memory.tsx (carried over verbatim by the split). Fixing here since this branch is the only place with easy access to the touched code right now, same reasoning as PRs #99/#100's follow-up fixes. - MemoriesTab.tsx: selectedIds now drops IDs no longer in the visible result set whenever search/filter results change, so bulk delete can't act on hidden stale selections. toggleAll's 'all selected' check now verifies every displayed ID is selected instead of just comparing set sizes. - MemoriesTab.tsx: clearing the context type field now sends null to the server instead of falling back to the memory's previous value. - NotebookAndCompactionTabs.tsx: name/project/platform are the notebook entry's identity key, so they're now locked once editing an existing entry -- editing them on save previously created a duplicate record instead of updating the original. - SessionsAndLogsTabs.tsx + NotebookAndCompactionTabs.tsx: session, log, notebook, and compaction mutations now all route their success/error outcomes through the single actionNotice state instead of independent per-mutation .error checks that could mask a newer result behind a stale one. - shared.tsx: deleteNotice now derives its base message from the actual deletedCount (including zero) and returns 'warning' whenever any requested ID was missing, instead of always reporting success. Verified: tsc --noEmit clean, npm run build succeeds, Playwright smoke pass against the live dev server hit all 5 tabs with zero JS errors. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011s8tBNnQTmb1q5nRAKwhvv
Pure refactor, no behavior changes. Extracted via exact index-based line-slicing (all 6 moved bodies verified byte-identical to the original before any import cleanup) -- same methodology as the Memory.tsx split.
Split shape differs from the Memory.tsx split (flat sibling tabs) since this file has real parent-child coupling (GraphViz/ProvenancePanel are children of ExplorerTab, not sibling tabs) -- documented in docs/current/knowledge-tsx-module-split.md.
Each extracted component/const referenced across file boundaries gained
export(mechanical TS requirement). One real gap caught by tsc --noEmit during implementation: ExplorerTab.tsx's import list initially omitted DEFAULT_HIDDEN_PREDICATES from shared.tsx despite using it -- fixed before commit.Validated: tsc --noEmit clean, npm run build succeeds, and a real Playwright smoke pass against the running dev server confirmed both tabs render and the Build Concepts dialog opens with a working scope selector, zero JS console errors.
Summary by CodeRabbit