Skip to content

refactor(web): split the chat attachment views out of chat-entries.tsx (#tech-debt) - #1081

Merged
selfcontained merged 1 commit into
mainfrom
tech-debt/chat-attachment-views
Sep 11, 2026
Merged

refactor(web): split the chat attachment views out of chat-entries.tsx (#tech-debt)#1081
selfcontained merged 1 commit into
mainfrom
tech-debt/chat-attachment-views

Conversation

@selfcontained

Copy link
Copy Markdown
Owner

What

apps/web/src/components/app/chat/chat-entries.tsx was 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 → 1044
  • new chat-attachment-views.tsx: 304
  • net diff: +304 / −288, two files

What 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 fieldsagentId, agentName, onOpenMedia. That is what the new module's AttachmentCtx = Pick<FeedContext, "agentId" | "agentName" | "onOpenMedia"> states; FeedContext still lives in chat-entries.tsx and is imported back type-only, so nothing is restated and there is no runtime cycle.

chat-entries.tsx re-imports the four symbols it still composes with: AttachmentBlock and mediaFileUrl (used by MediaEntryView), LivePin (by PinEntryView), AttachmentList (by ChatMessageView). Six imports became unused in chat-entries.tsx and 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.tsx has had four slicing runs with recorded diminishing returns. So:

  • QuestionOptions + DeliveryMeta (lines ~470–615 now): the next-best seam, but they are not leaves — QuestionOptions takes onAnswer/answering/answersDisabled straight off ChatMessageView'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.tsx and chat-feed.test.tsx import POST_TINT, POST_BODY_MEASURE, SIDE_POST_INDENT from chat-entries). Larger blast radius for the same line count; rejected.
  • The per-entry-kind views (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) and apps/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 — clean
  • pnpm run finalize:web — clean (type check + production build)
  • cd apps/web && pnpm vitest run1911/1911 passed, 130 files
  • pnpm run test:e2e198 passed on the second run. The first run had one failure in e2e/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:

probe result
mediaFileUrl path + encodeURIComponent broken 2 failed
hostOf always returns "" 1 failed
FileAttachment mimeType image detection disabled 1 failed
LivePin missing-pin branch made unreachable 2 failed
AttachmentList always returns null 3 failed

No green probes, so nothing to report as an uncovered branch.

Queued next

The QuestionOptions + DeliveryMeta follow-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:

  • A (shipped)type AttachmentCtx = Pick<FeedContext, "agentId" | "agentName" | "onOpenMedia">, with FeedContext staying in chat-entries.tsx and 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.
  • B (rejected) — the new module declares its own standalone three-field AttachmentCtx with no import at all. Truly dependency-free leaf, zero cycle of any kind — but it restates three fields that must stay structurally compatible with FeedContext forever, which is the exact drift class this job exists to remove.

A third option — moving FeedContext/PeerInfo/PeerDirectory into a separate leaf type module per standing ruling 872 — was rejected as scope creep: it would touch use-chat-feed-context.ts, chat-feed.tsx and chat-feed.test.tsx for no line-count win in the hotspot itself. Say so if you disagree and I'll queue it.

🤖 Generated with Claude Code

…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>
@selfcontained
selfcontained merged commit b49900e into main Sep 11, 2026
1 check passed
@selfcontained
selfcontained deleted the tech-debt/chat-attachment-views branch September 11, 2026 09:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant