refactor(components): decompose DocumentViewer into document-viewer/ modules (maturity X3)#1025
Merged
Merged
Conversation
…modules (maturity X3) DocumentViewer.tsx was a 3,164-line monolith the maintainability ratchet caps but never reduces. Extract its self-contained presentational leaf clusters into a document-viewer/ sibling directory (matching the pdf-canvas-viewer / non-pdf-source-preview modules already there), leaving the container composition-only: - document-viewer/types.ts — shared row shapes (pages, images, table facts, chunks, full-document search hits, index health) - document-viewer/source-panels.tsx — clinical summary profile, high-yield summary, source images/tables, the table-review queue, section anchors and summaries, pinned source evidence, and the indexed source-text search panel - document-viewer/manual-tag-editor.tsx — the curated manual-tag editor - document-viewer/document-overview-landing.tsx — the overview landing card, quick actions, and page-jump chips All pure moves — component bodies are verbatim and the container imports and composes them exactly as before. DocumentViewer.tsx 3,164 -> 1,733 lines; the maintainability budget is ratcheted 3,166 -> 1,733 to lock in the reduction. The rendered-text-formatting guard now scans the extracted modules alongside the container (its established "raw-render guards travel with the code" pattern) so the image-caption formatter assertion follows DocumentImage into source-panels. Verified: typecheck, lint, the full unit suite (3,027 passing; only the pre-existing container-only pdf-extraction-budget flake fails, identical on the base), knip, maintainability budgets, the isolated production build, and 7/8 targeted document-viewer Chromium specs. The one UI failure is pdf.js canvas paint under the container's mismatched Playwright browser build (1194 vs the pinned 1228), in the untouched pdf-canvas-viewer path — CI runs the UI gate with the correct browser. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Jc1ZYHFjXjn6mE6U6riVU
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe document viewer is split into typed source-panel and overview modules, while ChangesDocument viewer decomposition
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
BigSimmo
marked this pull request as ready for review
July 21, 2026 00:59
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
BigSimmo
enabled auto-merge (squash)
July 21, 2026 00:59
![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: 2
🧹 Nitpick comments (2)
src/components/document-viewer/manual-tag-editor.tsx (2)
103-105: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNo confirmation before deleting a manual tag.
deleteManualTagfires theDELETErequest immediately on click (Line 244), with no confirmation step for what is an irreversible action. A single misclick removes a curated label.Also applies to: 242-254
🤖 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/components/document-viewer/manual-tag-editor.tsx` around lines 103 - 105, Update deleteManualTag to request explicit user confirmation before calling submitManualTag with the DELETE operation. If confirmation is declined, return without submitting; preserve the existing deletion payload and request key when confirmed.
57-80: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win401 handling falls through to the generic error path.
Line 70 calls
onUnauthorized()on a 401 but doesn't return, so execution still hits Line 71'sthrow(since a 401 response is!response.ok), which then setserrorto the server message viasetErrorin thecatchblock. This means both the unauthorized callback and the inline error banner fire together, which can leave a stale/confusing error message rendered underneath whatever unauthorized-state UI the parent shows.🩹 Proposed fix
const payload = await response.json().catch(() => ({})); - if (response.status === 401) onUnauthorized(); + if (response.status === 401) { + onUnauthorized(); + return false; + } if (!response.ok) throw new Error(typeof payload?.error === "string" ? payload.error : "Tag update failed.");🤖 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/components/document-viewer/manual-tag-editor.tsx` around lines 57 - 80, Update submitManualTag so the 401 branch invokes onUnauthorized() and exits before the generic !response.ok error path. Preserve existing error handling for all other unsuccessful responses and ensure busy-state cleanup still occurs through finally.
🤖 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/maturity-backlog-workorders.md`:
- Around line 84-89: Update the DocumentViewer progress entry to remove the
“Pure moves” and “composition-only” claims, describing the work instead as
composition-focused/presentational extraction. Acknowledge that
manual-tag-editor.tsx contains behavior-bearing tag-editing logic and that
DocumentViewer.tsx retains orchestration such as dynamic PDF loading, without
changing the listed decomposition targets.
In `@src/components/document-viewer/manual-tag-editor.tsx`:
- Around line 119-157: Align manual tag client validation with the API’s 2–64
character label constraint in the add and edit flows. Update the add input and
submit button plus the edit-mode input and save button near the existing
draftLabel/edit label handling to enforce a maximum length of 64 and disable
submission when the trimmed value is shorter than 2 characters; preserve the
existing permission and busy-state checks.
---
Nitpick comments:
In `@src/components/document-viewer/manual-tag-editor.tsx`:
- Around line 103-105: Update deleteManualTag to request explicit user
confirmation before calling submitManualTag with the DELETE operation. If
confirmation is declined, return without submitting; preserve the existing
deletion payload and request key when confirmed.
- Around line 57-80: Update submitManualTag so the 401 branch invokes
onUnauthorized() and exits before the generic !response.ok error path. Preserve
existing error handling for all other unsuccessful responses and ensure
busy-state cleanup still occurs through finally.
🪄 Autofix (Beta)
❌ Autofix failed (check again to retry)
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: d2c573a6-8641-4614-a2d9-fc4efa00f17b
📒 Files selected for processing (8)
docs/maturity-backlog-workorders.mdscripts/check-maintainability-budgets.mjssrc/components/DocumentViewer.tsxsrc/components/document-viewer/document-overview-landing.tsxsrc/components/document-viewer/manual-tag-editor.tsxsrc/components/document-viewer/source-panels.tsxsrc/components/document-viewer/types.tstests/rendered-text-formatting.test.ts
"Presentational leaf clusters"/"composition-only" overstated it: manual-tag-editor.tsx is behaviour-bearing (add/rename/delete label API calls) and the container retains its detail fetch, dynamic PDF loading, and state orchestration. Reword to "cohesive leaf modules" and "composition-focused" while keeping the accurate verbatim-move claim. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Jc1ZYHFjXjn6mE6U6riVU
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Autofix skipped. No unresolved CodeRabbit review comments with fix instructions found. |
This was referenced Jul 21, 2026
BigSimmo
added a commit
that referenced
this pull request
Jul 21, 2026
… confirm, 401) (#1035) * fix(document-viewer): harden manual-tag editor UX (validation, delete confirm, 401) Address the three pre-existing manual-tag-editor findings deferred from #1025 — they predate the decomposition, so folding them into that pure-move PR would have broken its no-behaviour-change contract. Client-side only; the server already enforces the label contract and auth. - Match the server's 2-64 char label rule (manualLabelSchema in src/app/api/documents/[id]/labels/route.ts) in the field: maxLength={64} on the add and edit inputs, and disable submit/save until the trimmed value reaches the 2-char minimum, so a too-short/too-long tag no longer round-trips to a generic 400. - Require an explicit second click to confirm a manual-tag delete (inline two-click confirm with a Cancel, no native window.confirm) so a single misclick can't remove a curated label. - Return after onUnauthorized() on a 401 instead of falling through to the generic throw, so the unauthorized handoff no longer also raises a stale inline error banner. Adds tests/manual-tag-editor.dom.test.tsx covering all three behaviours. Verified: typecheck, lint, and the full unit suite (3,065 passing incl. the 3 new tests; only the pre-existing container-only pdf-extraction-budget flake fails). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Jc1ZYHFjXjn6mE6U6riVU * fix(document-viewer): make manual-tag delete confirm double-click-safe (CodeRabbit) The first version rendered the "Confirm remove" button at the same fragment index as the "Remove" button, so React reused the Remove button's DOM node for the destructive confirm — a rapid double-click on Remove could land its second click on the in-place delete control. Fix it two ways: - Give the Rename/Remove and confirm/cancel controls distinct `key`s so React never reuses the Remove node for the destructive confirm — the control the user pressed is never turned into "delete" in place. - Render a non-action "Remove this tag?" prompt in the position the Remove button was clicked from, shifting the confirm control off that hit target, and label the confirm button "Confirm remove". Adds a regression test: after one Remove click the original Remove control is gone (replaced by the prompt) and re-clicking it issues no DELETE. Verified: typecheck, lint, the manual-tag-editor dom suite (4 passing), and the full unit suite (3,066 passing; only the pre-existing pdf-extraction-budget flake fails). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Jc1ZYHFjXjn6mE6U6riVU --------- Co-authored-by: Claude <noreply@anthropic.com>
This was referenced Jul 22, 2026
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
2 participants
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.
Summary
Maturity X3 — decompose the
DocumentViewer.tsxmonolith. It was 3,164 lines, cappedbut never reduced by the maintainability ratchet. Its self-contained presentational leaf
clusters move into a
src/components/document-viewer/sibling directory (the same home as theexisting
pdf-canvas-viewer/non-pdf-source-previewmodules), leaving the container as acomposition-only shell.
document-viewer/types.ts— shared row shapes (pages, images, table facts, chunks,full-document search hits, index health)
document-viewer/source-panels.tsx— clinical summary profile, high-yield summary, sourceimages/tables, the table-review queue, section anchors and summaries, pinned source
evidence, and the indexed source-text search panel
document-viewer/manual-tag-editor.tsx— the curated manual-tag editordocument-viewer/document-overview-landing.tsx— the overview landing card, quick actions,and page-jump chips
All pure moves — the component bodies are verbatim and the container imports and composes
them exactly as before.
DocumentViewer.tsx3,164 → 1,733 lines (−45%); themaintainability budget is ratcheted 3,166 → 1,733 to lock in the reduction.
knipconfirms no orphaned exports.
The
rendered-text-formattingguard now scans the extracted modules alongside the container —following that file's own established "raw-render guards travel with the code" pattern — so the
image-caption formatter assertion follows
DocumentImageintosource-panels.This is off the RAG-ranking lane: no
src/lib/rag/**, retrieval, ranking, selection, eval, orfixture files are touched — purely a UI component split.
Verification
npm run typecheck— 0 errorsnpm run lint— 0 warningsnpm run test— 3,027 passing. The only failure is the pre-existing container-onlypdf-extraction-budgetflake (Python OCR stack absent in this VM), confirmed failingidentically on the base checkout and touching no DocumentViewer code.
npm run check:maintainability-budgets— ratcheted to 1,733, passesnpm run check:knip— no dead code / orphaned exportsverify:ui(Chromium) — 7/8 document-viewer specs pass, including both@criticalspecs and the dashboard/document-viewer screenshot capture.UI verification note: the container's Playwright browser is build 1194 but the project
pins build 1228 (
chrome-headless-shell-1228is absent), so the fullverify:uirunerrors at browser launch for every spec — a documented container limitation, not a code
issue. Pointing Playwright at the installed Chromium (
chromium-1194) and running thedocument-viewer specs gives 7/8 green. The 1 failure is
pdf-canvas-scroll canvasvisibility —pdf.js canvas paint under the mismatched browser, in the
pdf-canvas-viewerpath that thisdiff never touches (the dynamic import/render wiring is 0-diff), and the spec passed every
assertion on the extracted components (pinned evidence, highlighted passage, viewer nav)
before hitting that unrelated canvas check. CI's Production UI job runs the gate with the
correct browser binaries.
Risk and rollout
the full unit suite, the production build, and targeted Chromium specs. No behaviour,
props, or render structure changed.
Clinical Governance Preflight
Not applicable — per
scripts/pr-policy.mjs, presentation components are clinical-risk onlywhen they touch auth/upload/download/privacy/patient surfaces, which this does not. No
ingestion, answer generation, retrieval/ranking, source-governance, document-access, privacy,
schema, or production behaviour changed; the rendered clinical output is byte-identical.
Notes
docs/maturity-backlog-workorders.md, X3), updatedon-branch: X3 →
IN PROGRESS.ClinicalDashboard.tsx(4,271) andrag.tsremain the opendecomposition targets;
rag.tsstays deferred until the RAG lane is confirmed quiet.🤖 Generated with Claude Code
Generated by Claude Code
Summary by CodeRabbit
New Features
Documentation
Refactor / Tests / Chores