Skip to content

feat(chat-core): P5-e — one virtualized transcript for every chat surface - #10146

Merged
bolichen97 merged 1 commit into
mainfrom
feat/chat-core-p5e-transcript
Sep 12, 2026
Merged

feat(chat-core): P5-e — one virtualized transcript for every chat surface#10146
bolichen97 merged 1 commit into
mainfrom
feat/chat-core-p5e-transcript

Conversation

@CrysisDeu

@CrysisDeu CrysisDeu commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

Summary

chat-core P5-e: ChatPage's private virtualized transcript becomes a reusable unit, website/src/chat-core/transcript/, and ChatMessageList mounts it behind a new transcript prop — so the Crew Members DM / split panes (ChatPane), the side panel (SideChat), the app-SDK embed (ChatEmbed) and the spec-builder column (through ChatEmbed) render only their viewport window and gain the earlier-history bar. ChatEmbed's poll is bounded to one page (?limit=200) and the bar widens it up to the handler's 500-row ceiling. Second PR of #10005; independent of the stopgap in #10026 (both branch from main).

Closes nothing on its own; #10005 closes when both PRs land.

Why now

#10005: a running Crew Members DM over a ~3000-row / 10 MB thread froze the tab. #10026 stops the fetch from being unbounded; this PR stops the render from being unbounded on every non-page host — a thread's DOM cost is now its viewport (overscan + window), not its history — and closes the same fetch hole in the embed, whose 1 s poll re-read the whole slot while a turn ran.

What changed

  • chat-core/transcript/VirtualTranscript.tsx (new): useVirtualChat over grouped DisplayItems + the shared TranscriptScrollShell + EarlierMessagesBar, with an imperative handle (scrollToBottom / scrollToIndex / mountIndex / getFollow / scroller), aboveRows / belowRows slots, an optional external scrollerRef (for usePinnedPrompt), onAtBottomChange, and isRowHidden. Same tuning as the page (estimated 100 px, overscan 6, eager first measure, width-bucketed height scope). Height/anchor caches are partitioned per host (pane: / side: / embed: prefixes) so a split pane never restores the main page's anchor for the same slot.
  • chat-core/transcript/rowKeys.ts: msgIdentityKey, turnLeadKey, virtualKeyFor, uniqueRowKeys, stableAnchorIdFor, anchorAltIdFor MOVED here byte-for-byte; ChatPageMessageContent re-exports them, so ChatPage, the store and the tests keep their import path. useStableMessageKey hoists the page's clientTs → ts → minted-id rule.
  • app-sdk/ChatMessageList.tsx: new transcript?: TranscriptMount prop. With it the list OWNS its scroller through VirtualTranscript; without it, the bare fragment the SDK shipped (unchanged, pinned by the existing tests). forwardRef exposes the handle. streamingIndex is derived from the last row's tail ROLE (streaming), not the run flag.
  • Hosts: ChatPane, SideChat, ChatEmbed drop their overflow-y-auto div and useChatScrollFollow; empty states, the hydrate error + retry, the "earlier messages → open session" row, ChatFooter and the thinking dots move into the slots. ChatPane passes running || paneSlot.running (a remotely driven turn still follows, like its footer already assumed) and its pinned prompt waits for mounted rows at the hand-off line (requiresMountedHandoff), as the page does. The jump pill / edge fades are unchanged.
  • ChatEmbed: GET /api/chat/slots/{slot}?limit=200 (the handler's own page size); has_more drives the bar; each press widens by 200 to 500; the widening is remembered per slot so a new slot's FIRST read is one page; a same-slot widen keeps the rows on screen as placeholder (never across slots). A REJECTED widen keeps the last settled page on screen (the transcript never blanks on a failed history read), the bar shows the failure, and Retry re-reads the limit that failed; the bar's loading / failed are the widen's, not the ambient poll's. The top-anchored mode's "smooth-scroll on new reply" is keyed on the tail row's identity, so a prepended page does not yank the reader. startAtBottomfollowOutput + initialPlacement: 'bottom'; otherwise top.
  • TranscriptScrollShell: headerSpacer opt-out (default true; the page passes nothing; the recipe tokens are intact and ChatPage.scrollShell.recipe.test.tsx is untouched).
  • RFC docs/request-for-change/rfc-chat-core-extraction.md: §4 P5-e note, §4.2 P5-e (this PR) + P5-f (ChatPage switch, before-cursor pager for the pane, MeasureFarm for hosts, incremental embed poll) rows, §2.1 embed note.
  • Capture: website/scripts/capture-chat-core-p5e.mjs (+ openMembersDm harness helper) — real-pod screenshots that also assert the row counts; the images are PR attachments (gh pr edit --attach, per chore: upload PR evidence with gh --attach instead of committing it #10010), not committed files.
  • Deleted: app-sdk/useChatScrollFollow.ts and its test — this PR removed its last consumers; FollowController stays (the virtualizer uses it).

Screenshots (real pod, kirocrew pod up wt-p5e-pr2 --seed minimal; attached, not committed)

Crew Members DM, 200-row thread: 50 hydrated rows, 14 mounted, top spacer 3456 px Side panel on the shared virtualized scroller Spec-builder embed over a 300-row slot: one 200-row page, Load earlier messages at the top
Members DM: 14 of 50 rows mounted Side chat panel on the virtualized transcript Spec-builder embed with the load-earlier bar

Capture output: dm: mounted 14 (indices 36–49 of 50), spacers [3456, 0], scrollHeight 4826 / clientHeight 763; embed: 17 rows mounted of the 200-row page; after one press the top spacer is 7808 px (400 rows loaded).

What was tested

Static gates only, locally: npx tsc -b and npx eslint on every changed file — both clean. Tests are written for CI:

  • test/VirtualTranscript.test.tsx — short list mounts every row in order; a 40-row list mounts a bounded, contiguous window with two spacers; initialPlacement: 'top'; the shell contract (.chat-container, overflow axes, host padding, no h-16 band); the earlier bar only while hasMore and its press reaches the host; above/below slot order; isRowHiddenvisibility: hidden; the handle; the external scrollerRef + onAtBottomChange.
  • test/ChatMessageList.transcriptMount.test.tsx — fragment mode unchanged; transcript mode owns the scroller, indexes rows, hides the pinned row by identity, still reports onDisplayItems.
  • test/ChatEmbed.boundedPoll.test.tsx?limit=200 and never the bare slot URL; bar only while has_more; 200 → 400 → 500 per press, then the bar stops at the ceiling; a new slot starts at 200 on its first read; a rejected widen keeps the settled page and Retry re-reads that limit; startAtBottom vs top-anchored mount wiring.
  • test/chatCoreTranscript.contract.test.ts — source guards: every host mounts transcript={{, none imports useChatScrollFollow or keeps a transcript scroll div; per-host cache prefixes; the embed's bounded poll; VirtualTranscript composes the page virtualizer + shell + rowKeys; the page's re-export; ChatMessageList's two modes.
  • Existing: ChatEmbed.test.tsx URL assertion gains ?limit=200. ChatPane.scrollChrome.test.tsx's jump-pill case now flushes the virtualizer's frame-deferred pin and dispatches the resulting scroll event (the virtualizer applies its pin on the next animation frame so the tail rows it just mounted have real heights, and reads the position back through the scroll event — as the page's pill already works); the assertions themselves are unchanged. No other test was edited; ChatPage.scrollShell.recipe.test.tsx and virtualizerHeightOwner.test.ts are untouched.
  • Round 3 (CI + lanes on 9ef15fd6b6): GPT F1 — a failed FIRST read rendered the empty-session copy; the embed now shows ErrorNotice + Retry (chat-embed-load-error), retry re-reads the page. GPT F2 — the history bar's ask-the-agent hand-off navigates to the main chat and would discard the embed's unsaved draft; EarlierMessagesBar gains handOff (default on, the page's contract) and the embed turns it off. ChatPane.pinnedPrompt.test.tsx's layout gains a physically contiguous next-prompt row: with requiresMountedHandoff a gap under the fold is unmounted spacer and the hook correctly drops the banner, so the synthetic 260px gap the old layout used was itself the failure. My two new boundedPoll cases awaited findBy inside act (a deadlock) and read data before React Query had settled it — fixed.
  • Round 2 (CI + lanes on 1905fc4576): GPT F1 (a rejected widen blanked the transcript) fixed as above; VirtualTranscriptHandle shrinks to scrollToBottom (its only caller today; the page's navigation methods join in P5-f); useChatScrollFollow deleted with its last consumers (and the source guard on it in FollowController.test.ts); the three PNGs left the branch for --attach; three test defects of mine fixed (the initialPlacement: 'top' case now uses the top-anchored embed's own wiring, followOutput: false; the ChatEmbed mock of ChatMessageList renders the transcript.aboveRows slot the empty state moved into; the new-slot case renders inside act).
  • Two blind adversarial review rounds before opening (logic; React lifecycle + happy-dom compatibility): the top-anchored embed's yank-on-prepend, the cross-slot placeholder bleed, the pane's dropped paneSlot.running, the over-broad streamingIndex, and failed tracking the ambient poll were all found there and fixed before the first push.

UX CONCERNS

  • Side panel row spacing. SideChat's rows previously sat in a space-y-2 wrapper; the virtualized rows have no inter-row gap of their own (same as ChatPane and the page). Visible in screenshot 2's empty state only; a populated side thread reads slightly tighter than before.
  • Embed history is a wider re-read, not a cursor page. Each press re-fetches limit + 200 rows (the same stopgap shape as perf(chat): keep the pane hydrate and warm bounded while a slot runs #10026); a before-cursor pager is P5-f. At the 500 ceiling the bar stops even if the server has more — the handler clamps wider asks, and the embed then shows no cue that older rows exist (UX lane's concern; a static "can't be shown here" line needs a new string across 12 locales, so it rides with the P5-f pager rather than landing as an untranslated placeholder here).
  • Evidence gaps (UX lane): the side-panel shot shows the empty state — populating it needs a live side turn, which the seeded pod cannot run; the embed bar's failed state is pinned by ChatEmbed.boundedPoll.test.tsx rather than screenshotted.
  • No off-screen pre-measurement for hosts. ChatPage's MeasureFarm is not mounted here, so the scrollbar of a pane/embed uses estimated heights for rows it has not seen yet (settles as the reader scrolls). Recorded as P5-f.
  • memo(ChatMessageList) no longer short-circuits for hosts (they pass a fresh transcript object each render); the expensive memos inside are keyed on their own deps and are unaffected.

Follow-ups (recorded in the RFC §4.2 as P5-f)

ChatPage mounts VirtualTranscript (deleting its inline wiring; TranscriptScrollShell moves into chat-core); ChatPane's load-earlier as a before-cursor pager; MeasureFarm for hosts; ChatEmbed incremental (since-cursor) polling instead of a per-poll page re-read.

@CrysisDeu
CrysisDeu requested a review from a team September 11, 2026 16:19
@CrysisDeu CrysisDeu added enhancement New feature or request area: dashboard Dashboard UI and its backend handlers labels Sep 11, 2026
@CrysisDeu
CrysisDeu requested a review from a team as a code owner September 11, 2026 16:19
@CrysisDeu CrysisDeu added enhancement New feature or request area: dashboard Dashboard UI and its backend handlers labels Sep 11, 2026
@github-actions github-actions Bot added the readiness: checking Automated validation is still running label Sep 11, 2026
@github-actions

github-actions Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5) — ✅ PASS

Design-level review of 5de855daedf3e1c61a7c578e1e78688f7099093c — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

A disclosed, RFC-staged extraction: real freeze fixed at the render layer, SDK surface stays backward-compatible, every deferred trade-off is a scheduled P5-f row.

[DESIGN-REVIEWED] 5de855d

@github-actions

github-actions Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

UX Review (Fable 5) — 🟡 CONCERNS

UX-level review of 5de855daedf3e1c61a7c578e1e78688f7099093c — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All evidence reviewed: the blind read ran, the three screenshots match the diff's three hosts, and no recordings were supplied (none are required — the diff adds no in-place transform of a persistent element; row prepending and empty/error/list states are async lifecycle). The one primary new control, the embed's "Load earlier messages" bar, was read correctly and confidently by the blind reader. What keeps this from PASS is the PR's own admitted evidence gaps and the silent 500-row ceiling.

UX-Verdict: CONCERNS

Virtualization is invisible when it works — but three new states ship unscreenshotted, and the history bar dead-ends silently at the 500-row ceiling.

Watch

  • Ceiling dead-end: canWiden goes false at EMBED_PAGE_LIMIT_MAX while the server still has rows → the "Load earlier messages" bar vanishes → the user reads a truncated thread as the whole thread. Low frequency (500+-row embeds), misleading-completeness impact, persists until the P5-f pager. The i18n deferral is stated; a human should explicitly accept shipping the silent stop.
  • Side-chat rows lost their space-y-2 gap (PR admits "a populated side thread reads slightly tighter"); every populated side chat hits it, minor friction, every render — and it was never rendered for review (see gaps).

Evidence gaps

  • Embed first-load failure (chat-embed-load-error ErrorNotice + Retry) appears in no screenshot; a shot of the embed with the slot read failing would close it.
  • EarlierMessagesBar's loading and failed/Retry states in the embed appear in no screenshot.
  • Populated Side Chat after the spacing change — shot-02 shows only the empty state; a shot with a few side-turn rows would close it.

[UX-REVIEWED] 5de855d

@github-actions

github-actions Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 5de855daedf3e1c61a7c578e1e78688f7099093c — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

All verification complete. Every premise the change rests on checks out in the repo: the backend slot handler defaults limit to 200 and clamps at 500 (src/kiro_crew/dashboard/chat_handlers.py:1997) exactly as EMBED_PAGE_LIMIT/EMBED_PAGE_LIMIT_MAX assume; useVirtualChat already supports every option VirtualTranscript passes (nothing widened); usePinnedPrompt.requiresMountedHandoff pre-exists (page controller uses it); the deleted useChatScrollFollow has zero remaining consumers (grep: only the contract test asserting its absence), so its test and the FollowController.test.ts source-pin on it die with their subject, declared. All three non-page hosts are converted — no unfixed siblings; ChatPage is the declared P5-f follow-up. The change reuses the existing virtualizer/shell/rowKeys and deletes the second spelling of follow semantics rather than adding one.

First-Principles-Verdict: PASS

The author ran only static gates locally ("Tests are written for CI") — confirm the CI test lanes are green before merge.

What this change ships

Inventory (10 items) — 9 justified

Intent: Stop long chat threads from freezing the tab in the DM pane, side panel, and app embed by rendering only the visible window and bounding the embed's per-second history fetch — a FIX (#10005, PR 2 of 2), delivered as the RFC's planned P5-e extraction.

  1. DM / split panes render only the visible rows of a long thread — justified
  2. Side panel renders only the visible rows — justified
  3. App embed (and spec-builder column) renders only the visible rows — justified
  4. Embed's poll fetches the newest 200 rows, never the whole slot — justified
  5. Embed gains a "Load earlier messages" bar, widening to the server's 500-row ceiling — justified
  6. Embed's failed first read shows an error with Retry, not "session ready" — justified
  7. Embed's history-failure bar drops the ask-the-agent hand-off (unsaved draft would be discarded) — justified
  8. A pane observing a turn driven elsewhere now follows the bottom — justified
  9. A failed widen keeps the settled rows on screen with a retry — justified
  10. Dev capture script + shared DM-open harness helper — rides along (declared; dev tooling, no shipped surface)

Deleted-pin audit: useChatScrollFollow.test.tsx and the FollowController.test.ts consumer-pin are removed WITH their subject, whose last 3 consumers this PR migrates (counted: 0 remain). Consumer counts: transcript prop — 3 hosts; headerSpacer opt-out — 1 consumer, boolean minimal form; handOff — 1, boolean; backend limit/has_more — pre-existing, not added here. Symptom/cause: the since-cursor incremental poll is named and deferred to P5-f in the RFC, satisfying the level declaration.

[FIRST-PRINCIPLES-REVIEWED] 5de855d

@github-actions

github-actions Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review — ✅ no blocking findings (all downgraded on adjudication)

GPT 5.6 flagged blocking issues on 5de855daedf3e1c61a7c578e1e78688f7099093c; Opus 4.8 adjudication downgraded every one of them to advisory. Adjudication downgraded all 1 blocking finding(s) to advisory: the remedy each one requires is disproportionate to the harm it prevents. Read them as advice, not as merge conditions.

This comment is updated in place on each push.

Review details

BLOCKING -- website/src/app-sdk/ChatEmbed.tsx:145 -- Background poll failures are silently ignored
const widenFailed = ... / const loadFailed = isError && shown == null
Successful load -> polling refetch fails with cached data retained -> neither predicate renders an error, leaving a silently stale transcript.
Anchor: errors-use-error-notice
Fix: Render ambient isError through ErrorNotice while retaining cached rows.

FINDING -- website/src/app-sdk/ChatEmbed.tsx:209 -- Initial history triggers listRef.current?.scrollToBottom('smooth'), overriding top placement and opening non-startAtBottom embeds at the tail -> Fix: seed the hash from the first settled page without scrolling, then scroll only on later tail changes.

[GPT-REVIEWED] 5de855d
[BLOCK-MERGE-DOWNGRADED] 5de855d

Adjudication (Opus 4.8) — is blocking on each finding proportionate?

I've confirmed the mechanism, the rule, and the recovery path. Let me record my ruling.

F1 — The behavior is real: after a successful load, a failed background refetch leaves isError=true with slotData==null but settled retained, so shown=settled (ChatEmbed.tsx:137,148) and limit==EMBED_PAGE_LIMIT (ChatEmbed.tsx:111) — making both widenFailed (needs limit > EMBED_PAGE_LIMIT, ChatEmbed.tsx:145) and loadFailed (needs shown==null, ChatEmbed.tsx:148) false. The ambient poll error is not surfaced.

The anchor errors-use-error-notice is blocking: true, but the rule (website/AUTOSDE.yaml:526–605) governs how surfaced errors render (hand-written div vs ErrorNotice, and the askAgent decision) and mandates in-page surfacing only for a failure that "leaves the user in a failed state (a load that did not happen, a save that did not persist)." This code hand-writes nothing and drops no askAgent decision — it uses ErrorNotice correctly for both loadFailed and widenFailed. A background refresh blip with a full, valid cached transcript retained and polling still live is not a "failed state," so the finding does not match the rule's blocking triggers; the anchor is over-reach, and I weigh on the merits.

  • Harm rung: LOW — silent but the user holds valid cached rows; refetchInterval keeps polling (ChatEmbed.tsx:124–127) and the next successful poll repopulates settledRef (ChatEmbed.tsx:135) and clears isError. Self-correcting, recoverable, no data loss.

  • Conditions confirmed: initial success sets settledRef (ChatEmbed.tsx:135); refetch error with slotData==null (ChatEmbed.tsx:113); not widened (ChatEmbed.tsx:111); both predicates false (ChatEmbed.tsx:145,148).

  • Real fix cost: surfacing every ambient isError flashes an error banner over working content on each transient blip — a UX regression the author deliberately documented avoiding (ChatEmbed.tsx:143–144) — plus new state to debounce/coordinate against the existing widen/load paths. Cost clearly exceeds the self-correcting LOW harm.

    [ADJUDICATION] 5de855d total=1 uphold=0 downgrade=1
    DOWNGRADE F1 website/src/app-sdk/ChatEmbed.tsx:145 reason=disproportionate-remedy
    [GPT-ADJUDICATED] 5de855d

False positive or not applicable? A repository writer can comment:
/ai-review override gpt 5de855daedf3e1c61a7c578e1e78688f7099093c: <one-sentence reason>

@github-actions

github-actions Bot commented Sep 11, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review — ✅ no blocking findings

Reviewed 5de855daedf3e1c61a7c578e1e78688f7099093c — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] 5de855d

Verdict parsed from the review's SHA-scoped output markers for commit 5de855daedf3e1c61a7c578e1e78688f7099093c.

False positive or not applicable? A repository writer can comment:
/ai-review override fable 5de855daedf3e1c61a7c578e1e78688f7099093c: <one-sentence reason>

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Sep 11, 2026
@CrysisDeu
CrysisDeu force-pushed the feat/chat-core-p5e-transcript branch from 1905fc4 to 9ef15fd Compare September 11, 2026 16:41
@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Sep 11, 2026
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

Round 2 (9ef15fd6b6):

  • GPT F1 (blocking) — fixed as suggested: a rejected widen no longer blanks the transcript. The embed keeps the last SETTLED page for the slot on screen (shown = slotData ?? settled), the bar reports the failure, and its Retry re-reads the limit that failed rather than widening again. New test in ChatEmbed.boundedPoll.test.tsx (page stays at count 1, bar failed, Retry hits ?limit=400 and not 500).
  • Design lane — the three PNGs left the branch; the body's images are now gh pr edit --attach uploads per chore: upload PR evidence with gh --attach instead of committing it #10010. (The tuning-constant sharing with ChatPage is noted for P5-f; touching the page's inline wiring here is what P5-f is for.)
  • First-principles laneVirtualTranscriptHandle shrunk to { scrollToBottom } (its only caller); app-sdk/useChatScrollFollow.ts + its test deleted (0 consumers after this PR; the FollowController.test.ts source guard on it removed with it).
  • UX lane — the 500-row ceiling cue and the two evidence gaps are recorded under UX CONCERNS: the cue needs a new string across 12 locales, so it rides with the P5-f cursor pager; a populated side panel needs a live side turn the seeded pod cannot run; the bar's failed state is pinned by test.
  • CI (3 shards) — all three failures were mine: the initialPlacement: 'top' test now uses the top-anchored embed's own wiring (followOutput: false); ChatEmbed.test.tsx's mock of ChatMessageList renders the transcript.aboveRows slot the empty state moved into; the new-slot case renders inside act.

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Sep 11, 2026
@CrysisDeu
CrysisDeu force-pushed the feat/chat-core-p5e-transcript branch from 9ef15fd to ccc29f9 Compare September 11, 2026 17:02
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

Round 3 (ccc29f9564):

  • GPT F1 — fixed as suggested: a failed FIRST read no longer renders the empty-session copy. With no page at all (shown == null) and isError, the embed renders ErrorNotice (chat-embed-load-error) with a Retry that re-reads the page; hand-off stays off for the same reason as F2. Test added.
  • GPT F2 — fixed as suggested: EarlierMessagesBar gains handOff (default true, the page's contract, so ChatPage is unchanged); VirtualTranscript threads it through earlier.handOff; ChatEmbed passes false because its composer draft is unsaved local state the hand-off's navigation would discard. Test pins the wiring.
  • CIChatPane.pinnedPrompt (3): the pane now wears requiresMountedHandoff like the page, so the test's synthetic 260px gap under the fold read as unmounted spacer and the hook correctly dropped the banner; the layout now puts the next prompt where a mounted row actually sits (top 52 ≥ ROW_PAD_Y + card, so no push; ≤ hand-off 54.75, so it reaches the line). boundedPoll (2): my tests awaited findBy inside act and read data-count before React Query settled — fixed with a flush helper; no product change.

@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Sep 11, 2026
@CrysisDeu
CrysisDeu force-pushed the feat/chat-core-p5e-transcript branch from ccc29f9 to 38a14ce Compare September 11, 2026 17:39
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

Round 4 (38a14ce11f) — two shard reds on ccc29f9564, both in this PR's own tests; no product code changed:

  • Frontend Tests (3) ChatPane.pinnedPromptclicking the banner glides backscrollTop ended at 1800 (the bottom). The fixture's "reader scrolled up" was numerically a scroll down (happy-dom's mount-time bottom is 0, the hand-set position 200), so the virtualizer's follow controller kept follow armed and the first row reprice carried the reader back to the end. The test now rests the scroller at the real bottom (1800) first and then wheels up to 200 — the genuine scroll-up resolveUserScrollStick releases on — before clicking the banner. Test-only; the controller behaved as designed.
  • Frontend Tests (4) ChatEmbed.boundedPollkeeps the settled page when a widen fails — asserted data-count after a fixed two-microtask flush, which is not enough for React Query to settle the first page on every shard. Now waits for the earlier-history bar (it mounts only once the page has settled) and for data-failed via waitFor, then asserts the count. Test-only.

Static gates: npx tsc -b exit 0, eslint clean on both files.

@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

Round 5 (d19dec5182) — ChatPane.pinnedPromptclicking the banner glides back still landed at 1800 on 38a14ce11f; round 4's scroll-up fix was necessary but not the cause. Root cause read from useVirtualChat's slot-entry effect: it force-pins to the bottom synchronously and again one frame later (re-arming follow). In a browser that frame lands within 16 ms of hydrate; under the test's rAF queue it was still pending when the hand-set 2000×200 geometry went in, so the deferred forcePin wrote 1800 over the reader's scroll-up and re-armed follow. The test now flushes the queued frames right after mount (the pin lands on the mount-time bottom of 0, as it does live), then lays out and scrolls up. Test-only; the virtualizer behaved as designed.

Static gates: npx tsc -b exit 0, eslint clean.

@CrysisDeu
CrysisDeu force-pushed the feat/chat-core-p5e-transcript branch from 38a14ce to d19dec5 Compare September 11, 2026 17:54
@github-actions github-actions Bot removed the readiness: checking Automated validation is still running label Sep 11, 2026
@github-actions github-actions Bot added the readiness: action required A blocking check or review needs attention label Sep 11, 2026
…face (#10005)

Extract ChatPage's private virtualized transcript into a reusable unit under
website/src/chat-core/transcript/ and mount it from ChatMessageList, so the
split/DM pane, the side panel and the app-SDK embed (and the spec-builder
column through it) render only their viewport window and gain the
earlier-history bar.

- chat-core/transcript/VirtualTranscript: useVirtualChat over grouped
  DisplayItems + the shared TranscriptScrollShell + EarlierMessagesBar, with an
  imperative handle (scrollToBottom / scrollToIndex / mountIndex / getFollow /
  scroller) and slots for host content above and below the rows.
- chat-core/transcript/rowKeys: the page's row-identity builders
  (virtualKeyFor, uniqueRowKeys, stableAnchorIdFor, anchorAltIdFor, ...) moved
  here; ChatPageMessageContent re-exports them so the page, the store and the
  tests keep their import path. useStableMessageKey hoists the page's
  clientTs → ts → minted-id rule.
- ChatMessageList: new `transcript` prop (TranscriptMount) makes the list own
  its scroller through VirtualTranscript; without it the list stays the bare
  fragment the SDK shipped. forwardRef exposes the handle.
- ChatPane, SideChat, ChatEmbed: drop their own overflow-y-auto div and
  useChatScrollFollow; empty states, hydrate errors, the earlier row and the
  footer move into the aboveRows / belowRows slots. ChatPane's pinned prompt
  now waits for mounted rows at the hand-off line, as the page does.
- ChatEmbed: the poll is bounded (?limit=200, the handler's page) and the bar
  widens it by a page per press up to the handler's 500 ceiling;
  keepPreviousData keeps the rows on screen while a wider page loads. A new
  slot starts back at one page.
- TranscriptScrollShell: `headerSpacer` opt-out for hosts without the page's
  overlay header (default unchanged; recipe tokens intact).
- RFC: §4 P5-e note, §4.2 P5-e / P5-f rows, §2.1 embed transport note.

ChatPage keeps its inline wiring (P5-f switches it). Incremental (since-cursor)
polling for the embed and a before-cursor pager for the pane are recorded
follow-ups.
@CrysisDeu

Copy link
Copy Markdown
Collaborator Author

Round 6 (5de855daed) — d19dec5182 was green everywhere except Coverage Gate: src/chat-core/transcript/useStableMessageKey.ts sat at 54.5% (6/11) below the 80% per-file floor — the minted-id branch (a message with neither clientTs nor ts) had no direct test. Added useStableMessageKey.test.ts: timestamp precedence, one id per message object remembered across calls and re-renders, and keys unaffected by a prepend. Tests only; no product code changed.

Static gates: npx tsc -b exit 0, eslint clean.

@CrysisDeu
CrysisDeu force-pushed the feat/chat-core-p5e-transcript branch from d19dec5 to 5de855d Compare September 11, 2026 19:08
@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: passed Eligible automated validation passed for the current revision and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Sep 11, 2026

@bolichen97 bolichen97 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maintainer review (agent-assisted, owner delegated), pinned to 5de855d. Combined review passes cover the full diff and changed tests. VirtualTranscript preserves the ChatPage virtualizer recipe, row visibility predicate, tail-role streaming index and running follow semantics. Content moves into aboveRows/belowRows unchanged; per-host scopes and stable side-message keys preserve identity, and the pane uses requiresMountedHandoff. No remaining imports of the removed scroll hook. Accepted accessibility follow-up: multiple embedded host scrollers now expose polite live regions in split views. Source review in isolated worktrees and current-head CI success; no local suites run this wave.

@bolichen97
bolichen97 merged commit ddffd47 into main Sep 12, 2026
71 checks passed
@bolichen97
bolichen97 deleted the feat/chat-core-p5e-transcript branch September 12, 2026 04:19
@github-actions github-actions Bot removed the readiness: passed Eligible automated validation passed for the current revision label Sep 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: dashboard Dashboard UI and its backend handlers enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants