Skip to content

fix(web): move useAttentionTracking to ChatLayout (LUM-1736)#31349

Merged
ashleeradka merged 7 commits into
mainfrom
claude/lum-1736-attention-tracking-to-chatlayout
May 20, 2026
Merged

fix(web): move useAttentionTracking to ChatLayout (LUM-1736)#31349
ashleeradka merged 7 commits into
mainfrom
claude/lum-1736-attention-tracking-to-chatlayout

Conversation

@ashleeradka
Copy link
Copy Markdown
Contributor

Summary

Phase 1 of the chat-page data lifecycle redesign (LUM-1734). Moves useAttentionTracking from ChatPage to ChatLayout so the sidebar's processing/attention indicators stay live on every chat-layout route — not only /assistant.

Tracking: LUM-1736. Parent: LUM-1734.

Why this wasn't just "move the hook call"

The hook shared two page-level refs with useSendMessage:

  • processingSnapshotsRef: Map<string, string | undefined> — graduation logic compares current latestAssistantMessageAt against the snapshot taken when a key was added to processingKeys
  • conversationsRef: Conversation[] — used inside the 10s polling interval to read latest conversations without stale closures

A clean move requires fixing both ref dependencies first; otherwise we'd just relocate the violation.

Approach

1. Fold processingSnapshots into the conversation-list store

useConversationListStore (apps/web/src/domains/conversations/conversation-list-store.ts):

  • Added processingSnapshots: Map<string, string | undefined> to state
  • addProcessingKey(key)addProcessingKey(key, snapshot?) — records snapshot atomically with the set add
  • removeProcessingKey, removeMultipleProcessingKeys, graduateProcessingKey, transferProcessingKey, reset — all clear/transfer the matching snapshot in the same set() call

Snapshots and processing keys are now a single conceptual entity owned by the store. No more parallel ref to keep in sync.

2. Drop conversationsRef

Replaced in-callback reads (conversationsRef.current.find(...)) with useConversationListStore.getState().conversations.find(...). The getState() pattern is the idiomatic "always latest" read for async work — see the existing usage in attention-tracking's polling interval.

3. Rewrite useAttentionTracking

Now zero-prop. Reads assistantId + assistantStateKind from useAssistantContext() and everything else from useConversationListStore.use.* selectors. Inside the 10s polling tick, uses useConversationListStore.getState() to read the latest values (effects captured at scheduling time would be stale 10s later).

4. Mount in ChatLayout

Single call: useAttentionTracking(). No new state — reads from the same stores that already live there.

5. Drop the now-orphaned ref plumbing

useEventStream, useStreamEventHandler, useConversationLoader, ChatPage, ChatRouteContent all carried processingSnapshotsRef props purely to forward it to the hook that no longer takes it. removeProcessingKey now atomically clears the snapshot, so the explicit .current.delete(...) calls in the stream-event handlers became dead code.

Bugs fixed

None directly — this is the foundational move. Bugs #1#5 in LUM-1734 are addressed in Phases 2–5.

Architectural deletes

  • processingSnapshotsRef page-level ref in ChatPage
  • conversationsRef page-level ref in ChatPage (the last consumer was useAttentionTracking)
  • processingSnapshotsRef prop in 4 files
  • processingKeys / attentionKeys / activeConversation props on useConversationLoader (only consumed by the moved hook)

Test plan

  • bunx tsc --noEmit — clean
  • bun run lint — clean (pre-existing unrelated warning)
  • bun run test:ci — 84/84 test files pass
  • Manual: send a message on /assistant, then navigate directly to /assistant/home. Verify the processing indicator on that conversation in the sidebar graduates (or clears) without revisiting /assistant.
  • Manual: same for /assistant/library, /assistant/contacts, /assistant/identity.
  • Manual: in Network tab, verify exactly one listConversationKeysWithPendingInteractions call per 10s window (once per layout mount, not per route).
  • Manual: send a message on a draft conversation; verify the snapshot transfers correctly when the draft key resolves to the server ID.

https://claude.ai/code/session_013dBXRbLF218UhdLq7FEAvv


Generated by Claude Code

claude added 5 commits May 20, 2026 19:42
The sidebar conversation list is rendered by ChatLayout and visible on
every /assistant/* route — chat, home, library, contacts, identity. But
the Zustand store backing it was hydrated only by useConversationLoader,
which is mounted only inside ChatPage. Direct navigation to any non-chat
route mounted ChatLayout without ChatPage, so the init effect never
fired and the sidebar stayed empty until the user navigated into
/assistant.

Extract the list fetch into a new useConversationListInit hook mounted
in ChatLayout. The new hook uses TanStack Query (getChatContext) and
syncs results into the existing Zustand store, so consumers keep their
subscription model. useConversationLoader's init effect now reads from
the same query's cache for active-key resolution — no duplicate fetch.

The deeper architectural issue — server-derived conversation list
living in Zustand instead of TanStack Query — is tracked separately in
LUM-1731.

https://claude.ai/code/session_013dBXRbLF218UhdLq7FEAvv
…on-list-hydration-fix

# Conflicts:
#	apps/web/src/domains/chat/chat-layout.tsx
Two findings flagged independently by Codex (P2) and Devin (BUG_0001 +
ANALYSIS_0001), both fixed properly:

1. Init effect was short-circuiting on cached data via `getQueryData`,
   so refreshEpoch / reachabilityReadyEpoch re-runs (pull-to-refresh,
   pod recovery) returned stale results instead of fetching fresh data
   as documented at lines 381–384. Replaced with `fetchQuery({
   staleTime: 0 })` — always forces a fresh request, and TanStack
   Query's same-key dedup means concurrent fetches on initial mount
   (between this hook and `useConversationListInit` in ChatLayout)
   still collapse to one network call. Updates flow back through the
   shared cache so the sidebar refreshes alongside.

2. `setActiveKey` was firing one render cycle before
   `setConversations` (which had moved to `useConversationListInit`'s
   effect). Consumers like ChatPage's `activeConversation` lookup
   briefly saw an active key with empty conversations. Restored
   atomic writes by setting conversations alongside activeKey in the
   init effect — idempotent with `useConversationListInit`'s write
   but guarantees ordering for the chat path.

https://claude.ai/code/session_013dBXRbLF218UhdLq7FEAvv
…on-list-hydration-fix

# Conflicts:
#	apps/web/src/domains/chat/chat-layout.tsx
Phase 1 of LUM-1734's chat-page data lifecycle redesign.

Same root cause as LUM-1732 (sidebar conversation list not hydrating
on non-chat routes): per-conversation processing/attention indicators
were live only while the user was on /assistant, because
useAttentionTracking was mounted in ChatPage. Now mounted in
ChatLayout so the 10s polling loop + graduation logic cover home /
library / contacts / identity too.

The move required first untangling two shared refs between
useAttentionTracking (the reader) and useSendMessage (the writer):

- processingSnapshotsRef → fold into useConversationListStore as
  state. addProcessingKey now takes an optional snapshot; remove* and
  transfer* actions clear/transfer the matching snapshot atomically.
  No more parallel ref to keep in sync.
- conversationsRef → drop in favor of
  useConversationListStore.getState().conversations inside async
  callbacks. The ref was a workaround for stale closures; the store's
  getState() is the idiomatic always-latest read.

useAttentionTracking is now zero-prop — reads everything from
useConversationListStore selectors + useAssistantContext.

Also removes the now-orphaned processingSnapshotsRef plumbing from
useEventStream, useStreamEventHandler, useConversationLoader, and
ChatPage. removeProcessingKey now atomically clears the snapshot, so
the explicit .delete() calls in those hooks became dead code.

https://claude.ai/code/session_013dBXRbLF218UhdLq7FEAvv
@linear
Copy link
Copy Markdown

linear Bot commented May 20, 2026

LUM-1736

…tion-tracking-to-chatlayout

# Conflicts:
#	apps/web/src/domains/chat/chat-layout.tsx
#	apps/web/src/domains/chat/hooks/use-conversation-loader.ts
@ashleeradka ashleeradka marked this pull request as ready for review May 20, 2026 21:27
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6b7e8bb414

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +67 to +68
const { assistantId, assistantState } = useAssistantContext();
const assistantStateKind = assistantState.kind;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P0 Badge Avoid consuming outlet context in layout-mounted hook

useAttentionTracking now runs from ChatLayout, but it immediately calls useAssistantContext() and destructures assistantId/assistantState. useAssistantContext() is a useOutletContext wrapper intended for ChatLayout children; when called in the layout itself there is no parent outlet context providing this shape, so this resolves to null/undefined and the destructure throws at runtime. That makes /assistant routes crash as soon as ChatLayout mounts.

Useful? React with 👍 / 👎.

Codex P0 flag: useAttentionTracking was calling useAssistantContext()
(a useOutletContext wrapper) from inside ChatLayout. ChatLayout is the
ROUTE THAT PROVIDES that outlet context — there's no parent providing
it. The destructure would throw at runtime, crashing every chat-layout
route mount.

Fix: hoist the lifecycle values up. ChatLayout already has
`lifecycle.assistantId` and `lifecycle.assistantState.kind` from
useAssistantLifecycle. Pass them to useAttentionTracking as explicit
params (same pattern as useConversationListInit).

https://claude.ai/code/session_013dBXRbLF218UhdLq7FEAvv
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 2 potential issues.

View 4 additional findings in Devin Review.

Open in Devin Review

Copy link
Copy Markdown
Contributor

@vex-assistant-bot vex-assistant-bot Bot left a comment

Choose a reason for hiding this comment

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

APPROVE

Value: Sidebar processing/attention indicators now stay live on every chat-layout route (home, library, contacts, identity) — not only /assistant — eliminating the state loss users experience when navigating away mid-stream.

What this does: Moves useAttentionTracking from ChatPage/useConversationLoader up to ChatLayout, migrates processingSnapshotsRef into useConversationListStore for store-level atomicity, and fixes a stale-closure bug in the 10s polling loop.


Codex P0 — Verified fixed at a039fe55

The P0 was real: useAssistantContext() is useOutletContext<AssistantContextValue>(), and calling it from within ChatLayout reads from the parent outlet (RootLayout), which provides no context — the destructure would have thrown at runtime on every chat-layout route mount. TypeScript missed it because useOutletContext<T>() returns T, not T | undefined.

The fix is exactly right: remove useAssistantContext(), add UseAttentionTrackingParams, and pass lifecycle.assistantId + lifecycle.assistantState.kind from ChatLayout directly. Same pattern as useConversationListInit. CI all-green at the new HEAD.


Rest of the PR

processingSnapshots folded into store — atomic. addProcessingKey(key, snapshot?), removeProcessingKey, removeMultipleProcessingKeys, transferProcessingKey, and graduateProcessingKey all keep snapshots in sync with the processing key set. The transferProcessingKey fix (snapshot migrates with the key) was a latent bug — draft→resolved key transfers would have lost the snapshot.

deleteFromMap helper — clean Zustand shallow-eq optimization. Same-reference return when key is absent lets Zustand bail out of re-renders without allocating a new Map.

Stale-closure fix in the 10s poll — reading useConversationListStore.getState() inside the interval tick is the correct pattern. Values captured at scheduling time would be 10s stale; getState() gives the current snapshot without adding them to the dep array.

Ref thread-out — removing conversationsRef and processingSnapshotsRef from useSendMessage, useEventStream, useStreamEventHandler, and useConversationLoader is a meaningful API simplification. Each site now calls the store directly.

clearProcessingKey empty dep array — correct. removeProcessingKey now handles snapshot cleanup atomically, so there's no ref to close over.


Note: Devin reviewed at b65b2c8e (before the P0 fix). Worth triggering a re-review at HEAD: @devin review this PR.

Vellum Constitution — Trust-seeking: sidebar attention signals are a promise that background conversations are tracked; keeping that signal live across every route (not only /assistant) makes the guarantee complete and auditable.

@ashleeradka ashleeradka merged commit 49f9989 into main May 20, 2026
7 checks passed
@ashleeradka ashleeradka deleted the claude/lum-1736-attention-tracking-to-chatlayout branch May 20, 2026 21:37
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.

2 participants