refactor(chat): move ChatPage message-content helpers to their own module - #8978
Conversation
|
Local gate run for
|
First Principles Review (Fable 5) — ✅ PASSPremise-level review of All claims verified. The three newly exported symbols each have a real consumer inside First-Principles-Verdict: PASS A byte-preserving slice of the ChatPage monolith, motivated by a measured CI limit, with every new export consumed and every side edit mandated or declared. What this change shipsIntent: land the ChatPage split (#7255) in review-sized slices by moving its pure message-content helpers into their own module — a refactor (a MOVE, not a capability change).
Nothing is undeclared: the description accounts for [FIRST-PRINCIPLES-REVIEWED] c5e170e |
Design Review (Fable 5) — ✅ PASSDesign-level review of The change verifies as described: ~730 lines of self-contained helpers moved from the 10k-line ChatPage into Design-Verdict: PASS A byte-preserving, review-sized slice of a tracked monolith split, at the established Suggestions
[DESIGN-REVIEWED] c5e170e |
Opus 4.8 Review — ✅ no blocking findingsReviewed Review detailsAll moved symbols used in No findings. [OPUS-REVIEWED] c5e170e Verdict parsed from the review's SHA-scoped output markers for commit False positive or not applicable? A repository writer can comment: |
GPT 5.6 Review — ✅ no blocking findingsGPT 5.6 completed its review of This comment is updated in place on each push. Review detailsNo findings. False positive or not applicable? A repository writer can comment: |
UX Review (Fable 5) — ✅ PASSUX-level review of The check is clean: every unbalanced literal is an import path ( UX-Verdict: PASS Pure code move — no string, control, state, or pixel changes; nothing a user could experience differently. [UX-REVIEWED] c5e170e |
…dule ChatPage.tsx is a 10k-line page. The header menu, the row-key helpers (msgIdentityKey, turnLeadKey, anchorAltIdFor, stableAnchorIdFor, virtualKeyFor, uniqueRowKeys, messageRowKey), mintSendId, the knowledge bubble chip and the user-bubble content renderers (renderUserContent and its file/dir segment helpers) do not read page state. They move byte-for- byte into pages/chat/ChatPageMessageContent.tsx; the page imports them back. No behaviour, string, or wire format changes. The page no longer re-exports the moved names. Every consumer of those names is a test in this repository, so each one now imports the owning module directly. The one remaining default export and the two pre- existing re-exports (isBrowseCommand, PREFILL_STORAGE_KEY) are untouched. First slice of the ChatPage split proposed in #7255, cut small enough for one review pass and re-based on main's chat-core transport work. Co-authored-by: Kiro Crew <noreply@kiro.dev>
299601b to
c5e170e
Compare
|
Rebased onto Local gates on the new head: full website vitest suite with coverage 1886 files, 29703 passed, 1 expected fail, 2 skipped, exit 0; frontend line-rate 92.54%; |
|
The module is grouped by "doesn't touch the send path," not by theme —
Agreed on the diagnosis. The two symbols have owners further up the stack, and each moves when its owner lands, so nothing is left to a rename later:
Each of those PRs updates the feature-map Sessions row for the symbol it takes, so the entry added here is transitional, not cemented. Tracked in the stack plan on #7255 (comment linked from each stack PR); the current stack (#9072 → #9078 → #9079) does not touch either symbol, so the re-homing is not in scope there. |
Problem / Motivation
website/src/pages/ChatPage.tsxis a 10,169-line page. Its header menu, row-key helpers,mintSendId, the knowledge chip and the user-bubble content renderers sit at the top of that file even though none of them read page state. Finding them means scrolling through a monolith.Why it matters
The whole-page split in #7255 (92 files, +12.6k/-8.7k) grew past the point where CI's Opus review lane can finish one pass (it fails closed above 15k reviewable lines), and it collided with
main's chat-core transport work. Cutting the split into slices that each fit one review pass is the only way that refactor lands. This is the first slice: the part with zero dependency on the send path.What changed (motivation → approach → change)
The moved code is pure: components and functions that take their inputs as arguments. So it can leave the page without changing who calls it.
ChatPage.tsxlines 387–1117 move byte-for-byte intopages/chat/ChatPageMessageContent.tsx. The only edits to the moved text are three addedexportkeywords (mintSendId,msgIdentityKey,KnowledgeBubbleChip) so the page can import them back.NO_AUTO_DENIED, which sat between two moved blocks, stays in the page because only the page reads it.The page imports the eleven names it still uses from the new module and drops the imports that only the moved code needed. It does not re-export the moved names: every consumer is a test in this repo, so each of the 13 tests now imports
pages/chat/ChatPageMessageContentdirectly. The feature map's Sessions row names the new module the same way it already namesTranscriptScrollShell.tsx.main's recentDirChipchange (theuseRevealFailure+ErrorNoticereveal-failure notice) moves with the code it belongs to.Tests
website/src/test/ChatPageMessageContent.test.ts(new): pinsmintSendId()to the legacys-<ts36>-<nonce36>wire format with mockedDate.nowandMath.random.tsc -b,eslint(0 warnings),jscpd(0 clones),i18n:check,lint:phantom-classes,check_feature_map.py,check_brand_name.py,docs-lint.shall exit 0.Manual verification
N/A — this is a byte-preserving move of pure functions and components; the existing renderUserContent / ChatHeaderMenu / row-key tests exercise every moved export.
Screenshots / video
Why no screenshot: pure move of functions and components; nothing renders differently.
Related Issues
Part of the split of #7255. no linked issue: the parent PR has no tracking issue.
Checklist
feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)