refactor(web): split the chat attachment views out of chat-entries.tsx (#tech-debt) - #1081
Merged
Merged
Conversation
…x (#tech-debt) chat-entries.tsx was 1325 lines, the largest web component file in the repo. The Attachments section is a self-contained cluster of presentational leaves — pure move, no behavior change. chat-entries.tsx 1325 -> 1044; new chat-attachment-views.tsx at 304. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
What
apps/web/src/components/app/chat/chat-entries.tsxwas 1325 lines — the largest web component file in the repo, and the standing tech-debt complexity hotspot for the chat surface (backlog entry from the 2026-09-05 audit, which recorded it at 1180; it has grown 145 lines since).This moves the file's Attachments section — 269 lines, one contiguous block — into a new
chat-attachment-views.tsx. Pure move, no behavior change.chat-entries.tsx: 1325 → 1044chat-attachment-views.tsx: 304What moved, and what the moved set has in common
AttachmentBlock,LinkAttachment,FileAttachment,CodeAttachment,LivePin,PinAttachment,AttachmentView,AttachmentList, plus the two helpers only they use (mediaFileUrl,hostOf).Precisely: every one is a presentational leaf that takes a
ChatAttachment(or a pin id) and renders it, and none of them reads the feed beyond three fields —agentId,agentName,onOpenMedia. That is what the new module'sAttachmentCtx = Pick<FeedContext, "agentId" | "agentName" | "onOpenMedia">states;FeedContextstill lives inchat-entries.tsxand is imported back type-only, so nothing is restated and there is no runtime cycle.chat-entries.tsxre-imports the four symbols it still composes with:AttachmentBlockandmediaFileUrl(used byMediaEntryView),LivePin(byPinEntryView),AttachmentList(byChatMessageView). Six imports became unused inchat-entries.tsxand were dropped (ChatAttachment,ExternalLink,FileText,GitPullRequest,PinItem,usePinShortcuts).Deliberately NOT in scope
The backlog entry and the run state both warn against trying to take this file from 1325 to 300 in one PR —
agents-view.tsxhas had four slicing runs with recorded diminishing returns. So:QuestionOptions+DeliveryMeta(lines ~470–615 now): the next-best seam, but they are not leaves —QuestionOptionstakesonAnswer/answering/answersDisabledstraight offChatMessageView's props and is part of the message's own interaction contract, not a self-contained renderer. Queued as a follow-up backlog entry, not bundled here.Post/Avatar/POST_TINT/SIDE_POST_INDENT— the post shell. Extracting it would move the file's public surface (chat-feed.tsxandchat-feed.test.tsximportPOST_TINT,POST_BODY_MEASURE,SIDE_POST_INDENTfromchat-entries). Larger blast radius for the same line count; rejected.ChatMessageView,StatusLine,AgentMessageView,MediaEntryView,PinEntryView,ReviewEntryView) — these are the module's exported API; splitting them is a consumer-facing change, not housekeeping.chat-composer.tsx(818) andapps/server/src/chat/service.ts(997) — named as siblings in the backlog entry, untouched.Why it's tech debt
Nothing here is broken. The attachment renderers had simply accreted into the same file as the post shell, the author model, the question/delivery UI and six entry-kind views, so any change to one meant paging through 1300 lines of unrelated concerns.
Validation
pnpm run check— cleanpnpm run finalize:web— clean (type check + production build)cd apps/web && pnpm vitest run— 1911/1911 passed, 130 filespnpm run test:e2e— 198 passed on the second run. The first run had one failure ine2e/agent-surfaces.spec.ts:680("nested action menus claim Escape and restore confirm focus"), which is unrelated to this diff (no chat files) and passed on re-run; recorded as a pre-existing flake.Proving the move is non-vacuous
Reverting is degenerate here — the new module would cease to exist — so per the standing method rule I probed defects into the new module instead, each reverted after, running
chat-feed.test.tsx(46 tests) each time. All five went red:mediaFileUrlpath +encodeURIComponentbrokenhostOfalways returns""FileAttachmentmimeType image detection disabledLivePinmissing-pin branch made unreachableAttachmentListalways returnsnullNo green probes, so nothing to report as an uncovered branch.
Queued next
The
QuestionOptions+DeliveryMetafollow-up above, recorded on the backlog.One question for the reviewer
The only arguable choice in this diff is how the new leaf module learns what it needs from the feed context. I picked A; please confirm or overturn so the next run can cite it rather than re-decide:
type AttachmentCtx = Pick<FeedContext, "agentId" | "agentName" | "onOpenMedia">, withFeedContextstaying inchat-entries.tsxand imported back type-only. Single source of truth, no restatement, no runtime cycle — but the module graph does have a type-level child→parent edge.AttachmentCtxwith no import at all. Truly dependency-free leaf, zero cycle of any kind — but it restates three fields that must stay structurally compatible withFeedContextforever, which is the exact drift class this job exists to remove.A third option — moving
FeedContext/PeerInfo/PeerDirectoryinto a separate leaf type module per standing ruling 872 — was rejected as scope creep: it would touchuse-chat-feed-context.ts,chat-feed.tsxandchat-feed.test.tsxfor no line-count win in the hotspot itself. Say so if you disagree and I'll queue it.🤖 Generated with Claude Code