refactor(chat): move ChatPage session identity into a controller hook

[bolichen97]

Stack: bottom, on main. #8978 and #9079 are merged. Remaining merge order: this → #9078. #9078 targets this branch; read every diff relative to its own base.

Problem / Motivation

website/src/pages/ChatPage.tsx is a 10k-line page. About 550 lines of it decide which session the page is showing: the lazy history seed, the per-mode localStorage slot memory, session tabs, the ?sid deep-link and URL-sync effects, the mobile drawer's pop bookkeeping, the new-slot mutation and its retry, slot auto-create, and handleResumeSession. They sit in one contiguous run in the middle of the component with no name.

Why it matters

The whole-page split in #7255 grew past the point where CI's Opus review lane can finish one pass and collided with main's chat-core transport work. It lands as slices that each fit one review pass. This slice owns session identity — the one region main has not touched since #7255 was cut (zero drift), so it is the safest hook to move first.

What changed (motivation → approach → change)

The moved code reads page inputs (slots, drafts refs, router state) and produces a small set of values the rest of the page uses. That is the shape of a custom hook.

ChatPage.tsx lines 4313–4839 and 4922–4940 move into pages/chat/useChatPageSessionController.ts. The page calls useChatPageSessionController({...}) at the exact spot the first moved line used to be and destructures the eighteen values it still reads. Because the call sits where the code sat, every effect keeps its registration order and every ref is created in the same render phase as before.

Three edits to the moved text, none of them behaviour. location.key / location.pathname / location.hash become hook arguments (locationKey, locationPathname, locationHash) so the hook does not call useLocation a second time. The dependency arrays name the refs the hook now receives as arguments (activeSlotRef, newSessionRef, tokenConsumingRef, the draft refs); they are referentially stable, so no effect re-fires. Seven comments drop issue-number references per AGENTS.md (docs/system-specs/common/code-style.md bans ticket ids in code comments); the moved file now carries none. Three page-side dependency arrays likewise add setHighlightTs, initialMsgRef/initialMidRef and drawerPopRef, which react-hooks/exhaustive-deps can no longer see as stable once they arrive through a hook return.

Tests

• ChatPageMoreCoverage.test.tsx: one added window-event characterization — a foreground kirocrew-tool-call for a playwright-cli command opens the Browser panel. It is here because of this slice, not beside it: the moved session code was 99% covered, so taking it out of ChatPage.tsx lowers the page's own per-file rate to 80.06% against the 80% floor check_per_file_coverage.py enforces (the page is not baselined). Covering a listener the page keeps lifts it to 80.45%, clear of shard noise.
• ChatPage.sessionChipOffline.test.tsx: the "one session-entry path" source contract now reads the selectSessionTab declaration from the controller (exactly once) and the onSessionOpen={selectSessionTab} wiring from the page, and asserts the page no longer declares it.
• Every ChatPage* spec plus the session, drawer, URL-history and source-contract specs pass unchanged (67 files / 602 tests targeted; full suite result in the first comment).
• tsc -b, eslint (0 warnings), jscpd (0 clones), lint:phantom-classes, check_feature_map.py, check_brand_name.py, docs-lint.sh all exit 0.

Manual verification

N/A — the moved effects are covered by the ChatPage.sid, ChatPage.sessionTabs, ChatPage.drawerBackClose, ChatPage.resumeSurfaceGate and ChatPage.newSessionModel characterizations, which pass unchanged.

Screenshots / video

Why no screenshot: hook extraction; nothing renders differently.

Related Issues

Part of the split of #7255. no linked issue: the parent PR has no tracking issue.

Checklist

• At most two commits (one is the norm), with a Conventional Commits title (feat|fix|docs|refactor|perf|test|chore|ci|build|revert: ...)
• Existing tests pass and new tests added for new functionality
• Self-review completed; code follows project style guidelines
• Documentation updated (if applicable)
• No secrets, credentials, or internal references in the diff

[bolichen97]

Local gate run for 2ce011a09 (on d7fb9c55e):

• Full website vitest suite with coverage: 1885 files, 29703 passed, 1 expected fail, 2 skipped, exit 0.
• Frontend line-rate 92.56% (floor 90). scripts/check_per_file_coverage.py (floor 80, frontend baseline): exit 0 — src/pages/ChatPage.tsx 80.45%, src/pages/chat/useChatPageSessionController.ts 98.96% (de-duplicated line counts, the gate's own measure).
• tsc -b 0 errors outside src/stories/ (environment-only storybook types, identical on a clean main checkout); eslint 0 warnings; jscpd 0 clones; lint:phantom-classes, check_feature_map.py (FEATURE_MAP_BASE_REF=origin/main), check_brand_name.py, docs-lint.sh exit 0.

[github-actions]

UX Review (Fable 5) — ✅ PASS

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

This is a pure logic refactor — ~550 lines of session-identity state management moved verbatim from ChatPage.tsx into a useChatPageSessionController hook, plus test and feature-map updates. No JSX is added, removed, or changed; no user-facing string, control, or state is touched. Under lens 12, a diff that adds or changes no user-visible control has no evidence gaps and needs no screenshot, so the absent blind read does not count against it.

UX-Verdict: PASS

Pure code move — no user-visible control, string, or state changes; nothing for a first-time user to encounter differently.

[UX-REVIEWED] 0b11264

[github-actions]

Design Review (Fable 5) — ✅ PASS

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

Design-Verdict: PASS

A mechanical, order-preserving extraction following the established controller-hook precedent, with source-contract and characterization tests pinning the moved seam — sound slice design.

[DESIGN-REVIEWED] 0b11264

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed 0b112649a242c230731dfddea8c65d0fa68e1cd4 — this comment is updated in place on each push.

Review details

I've verified the extraction is clean:

• Removed imports (setActiveSlot, fetchHistory, resumeFromHistory, toSlug, focusComposer, shouldReplaceSessionUrl, popMaySwitchSession, useSessionTabs) have no remaining references in ChatPage.tsx; only switchSlot, which was retained, still appears.
• All 14 hook-internal refs/vars are gone from ChatPage.tsx — nothing dangles.
• All 17 returned values are consumed at their call sites.
• Effect declaration order inside the hook matches the former inline block, the hook is invoked at the same position, and every dependency-array delta is either a stable ref or a primitive prop (locationKey/Pathname/Hash) carrying the identical value, so no effect's firing condition changed.

The candidate list correctly found nothing, and my own falsification pass produced no grounded defect meeting (a)/(b)/(c).

No findings.

[OPUS-REVIEWED] 0b11264

Verdict parsed from the review's SHA-scoped output markers for commit 0b112649a242c230731dfddea8c65d0fa68e1cd4.

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

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of 0b112649a242c230731dfddea8c65d0fa68e1cd4 and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] 0b11264

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

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of 0b112649a242c230731dfddea8c65d0fa68e1cd4 — 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 claims verified: the code-style ban on ticket ids is real (docs/system-specs/common/code-style.md:67), the coverage gate exists (scripts/check_per_file_coverage.py), the moved symbols are fully gone from the page, the listener the new test covers stays in the page, and every one of the 17 values the hook returns is consumed downstream in ChatPage.tsx. Final review:

First-Principles-Verdict: PASS

A pure code move with declared mechanical adaptations; every rider is either mandated by a documented invariant or derived from a measured CI constraint.

What this change ships

Intent: make the 10k-line ChatPage reviewable by CI's one-pass review lane by slicing out its session-identity block — a FIX to a process constraint, executed as a move.

1. Session identity (deep-link ?sid, tabs, slot memory, auto-create, resume) now lives in a named hook file — justified; follows the existing split pattern (useChatPageTranscriptController.tsx)
2. Nothing a user can do changed; effects keep order and registration — justified (move, not behavior)
3. location.key/pathname/hash become hook arguments — justified mechanical adaptation
4. Dependency arrays name the now-hook-provided stable refs — justified lint adaptation, no re-fires
5. Seven comments lose issue-number ids — mandated by docs/system-specs/common/code-style.md:67
6. Feature-map Sessions row lists the new file — mandated same-commit doc update (AGENTS.md)
7. New browser-panel listener test — declared rider, derived from the 80% per-file floor (scripts/check_per_file_coverage.py; 0.06pp margin without it)
8. Source-contract test reads controller + page — adaptation of the move

Counts run: hook consumers = 1 (ChatPage.tsx — an internal split, not a generalized surface); all 17 returned values re-consumed in the page (grep per name, each ≥1 use past the destructure); moved refs (historySeededRef|pushedSessionEntryKeys|repairPoppedSid) = 0 occurrences left in the page; the remaining 9.5k lines are counted, declared siblings deferred to the #9079/#9078 stack.

[FIRST-PRINCIPLES-REVIEWED] 0b11264

[github-actions]

GPT 5.6 Review — ⚠️ review incomplete

GPT 5.6 did not produce a complete verdict for aa4986f8f5a0f0298dd940fc1078617178071837; inspect the workflow logs and re-run it.

This comment is updated in place on each push.

See the GPT 5.6 Review job logs; this commit has no completed GPT verdict.

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

[bolichen97]

Drop the 4 remaining ticket ids from useChatPageSessionController.ts comments (L95, L127, L385, L447) — fixed in 77fb03c96.

grepped #[0-9]+ in useChatPageSessionController.ts: #765 (L95), #4477 (L127), #8209 (L385), #8207 (L447), all banned by code-style.md L67. The rule was invoked, then applied to 3 of 7.

Correct, and the same edit class the slice already performs. All four are gone; the file now has zero ticket ids and the description says seven, not three. The two that carried meaning beyond the number (#8209: "has already needed a follow-up fix once"; #8207: the drawer pop arriving a commit late) keep their sentence without the id. Slices 3 and 4 were re-stacked on this head.

[bolichen97]

Defer the kirocrew-tool-call test to its own test: PR — rebutted: the test exists because of this slice, and the PR body now says so.

The kirocrew-tool-call characterization in ChatPageMoreCoverage.test.tsx covers a window-event listener with no relation to session identity — an unrelated item riding along.

The relation is the per-file coverage gate, not the feature. check_per_file_coverage.py holds every non-baselined file to an 80% floor, and src/pages/ChatPage.tsx is not baselined. The moved session code was 99% covered (190/192 lines in the new file), so removing it from the page lowers the page's own rate: measured on the full suite with coverage, 80.06% without this test and 80.45% with it. At 80.06% one shard's noise can red the gate on a PR that changed no behaviour; the test covers a listener the page keeps and that nothing exercised before. Landing it in a separate test: PR first would mean the gate, not the review, decides this slice's order in the stack. The PR body's Tests section now states this reason instead of leaving the test to look like a passenger.

[bolichen97]

#8978 merged (4aa07f928, squash), so GitHub retargeted this PR to main and the stack collapsed one level. Rebased onto main at 31cd87f29 — no conflicts, still one commit — head is now 580bf4f7f. On the new head: tsc -b 0 errors outside src/stories/, eslint 0 warnings, every ChatPage* spec plus the session-chip contract green (65 files / 587 tests), check_feature_map.py exit 0. #9078 and #9079 were re-stacked on this head.

The GPT "review incomplete" and Opus "could not read this PR's comments" results on the previous head were API-read failures during the re-run wave my description edit triggered, not findings; this push gives them a fresh head.

[bolichen97]

@iamwhatever thanks for taking #8978 and approving the stack. Your approval here was dismissed automatically (dismiss_stale_reviews_on_push) by the rebase that #8978's squash-merge forced: this PR's head was built on the pre-squash slice-1 commit, so once GitHub retargeted it to main it read as conflicting until rebased. The rebase was clean (still one commit, 580bf4f7f); the only other change since your review is none on this PR — the First Principles subtractions landed on #9078 and #9079, which were re-stacked on top.

Every review lane has re-stamped 580bf4f7f (Design, First Principles, GPT 5.6, Opus 4.8, UX all PASS); readiness is finishing. When it flips to passed, this one is ready to re-approve and merge, and I will collapse #9078 → main next.

[bolichen97]

Stack reordered: #9079 now goes first

Coverage Gate on 580bf4f7f failed the per-file floor: src/pages/ChatPage.tsx 79.7% (2127/2668) against 80%, not baselined. Nothing in this PR is under-tested — the session code it moves out was 99% covered, so taking it out of the page lowers the page's ratio. The uncovered lines that dominate the page are the pinned-prompt glide / nav-poll block (scrollToPinnedPrompt, ~70 lines), which happy-dom cannot reach from a ChatPage mount — and that is exactly the block #9079 extracts and covers directly (98.8% in its own module).

So the order of the stack, not its content, was the problem. Computed from this run's own coverage-frontend artifact:

page state	ChatPage.tsx
main (≈ this head + session region back)	≈ 81.0%
minus pre-virtualizer transcript state (#9079 first)	82.5%
then minus session (#9072)	81.2%
then minus resources (#9078)	80.85%
— versus the old order: main minus session first	79.7% ✗

Every step of the new order clears the floor. The three commits are unchanged in content — the stack top is byte-identical to the previously verified one except for two adjacent import/feature-map lines — and each was re-verified on its new base (tsc -b, eslint, every ChatPage* spec green). New heads: #9079 e1acd70fc (base main), #9072 08ce0854e (base #9079), #9078 f8e5c9a61 (base #9072). The full suite with coverage is running on the stack top now; result to follow.

[bolichen97]

Review-ready on 08ce0854e (base refactor/chat-page-transcript-early-controller): PR Readiness success, readiness: passed, 0 failing check-runs (the five cancelled runs are the superseded wave from the base retarget), and all five review lanes stamped this head with no blocking findings — Design PASS, UX PASS, First Principles PASS, GPT 5.6 and Opus 4.8 clean. Both earlier First Principles CONCERNS are dispositioned above.

Merge order is #9079 → this → #9078; once #9079 lands, this is retargeted to main and rebased, and the approval needs refreshing then.

[bolichen97]

#9079 merged (5e5679b29), so the stack collapsed again: rebased onto main at 018c9ecf3 — clean, still one commit — head 0b112649a. Re-verified on the new base: tsc -b 0 errors outside src/stories/, eslint 0 warnings, every ChatPage* spec green (67 files / 604 tests), check_feature_map.py exit 0. #9078 was re-stacked on this head (f39308104). This is now the next PR to approve and merge.