fix(flows): route Copilot reasoning to insights + persist the copilot chat (B4, B11)#4650
fix(flows): route Copilot reasoning to insights + persist the copilot chat (B4, B11)#4650graycyrus wants to merge 3 commits into
Conversation
… chat (B4, B11)
B4 — the builder's between-tool narration ('Let me check…','Now let me build…') rendered
as chat BUBBLES instead of the collapsed 'Agentic task insights'. onInterim persisted each
interim narration as a permanent agent message → a bubble. Now those messages are tagged
extraMetadata.isInterim, and the copilot renders a filtered displayMessages (user + non-
interim only), so only the terminal proposal/clarifying-question shows as a bubble; the
narration streams into the ToolTimelineBlock as before. Main chat unchanged (additive tag).
tinyhumansai#4630's clarifying-question turn (addMessageLocal, no isInterim) survives the filter.
B11 — the copilot chat was lost on reload. Three layers fixed: workflowCopilotThreads now
persists the per-flow thread id to localStorage (was a module-level Map); and the hook, on
mount with a seedThreadId, dispatches loadThreadMessages + fetchAndHydrateTurnState/History
(mirroring Conversations.tsx) to rehydrate the transcript from the core (messages were always
durable there, just never loaded back). isInterim survives the persist→rehydrate round-trip,
so B4's filter behaves identically on live + reloaded messages.
typecheck/eslint/format clean; 89 tests (displayMessages filter, rehydration dispatch,
localStorage round-trip+reload sim, isInterim bubble suppression) + 150 collateral pass.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 14 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 00a7c68199
ℹ️ 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".
| useEffect(() => { | ||
| if (!seedThreadId) return; | ||
| log('rehydrate: loading persisted messages + turn state/history thread=%s', seedThreadId); | ||
| void dispatch(loadThreadMessages(seedThreadId)); |
There was a problem hiding this comment.
seedThreadId is not just an initial persisted seed here: WorkflowCopilotPanel reports every newly created threadId back to FlowCanvasPage, which then passes it back as seedThreadId. On the first send from a fresh copilot, this effect can dispatch loadThreadMessages for the just-created thread while addMessageLocal or the streamed assistant append is still in flight; the loadThreadMessages.fulfilled reducer replaces messagesByThreadId[threadId], so a stale empty response can erase the just-added turn from the visible transcript. Please only hydrate the original stored seed, or skip hydration when this hook created the id / messages already exist.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2f93637 — added a createdThreadIdRef that tracks thread ids this hook creates itself via send(). The rehydrate effect now skips loadThreadMessages/fetchAndHydrateTurnState/fetchAndHydrateTurnHistory when seedThreadId matches a locally-created thread, so the echo-back through onThreadIdChange → FlowCanvasPage → seedThreadId no longer races the in-flight turn. A genuinely pre-existing seed (e.g. resumed from workflowCopilotThreads.ts on mount) still rehydrates as before. Added two regression tests covering both cases.
| } | ||
|
|
||
| function storageKey(flowId: string | null): string { | ||
| return `${STORAGE_PREFIX}${copilotThreadKey(flowId)}`; |
There was a problem hiding this comment.
These localStorage keys are global (copilot-thread:draft, copilot-thread:<flowId>), unlike the rest of user-scoped persisted app state. After an auth-to-auth flip/restart or a clear-data flow that removes only ${userId}: keys, the next user opening an unsaved draft reads the previous user's draft thread id and loadThreadMessages/addMessageLocal target a stale thread instead of creating a fresh builder thread. Include the active user id in this key or clear all copilot-thread: entries during identity resets.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2f93637 — storageKey() now namespaces with getActiveUserId() (from userScopedStorage.ts), matching the ${userId}: convention clearAllAppData.ts/userScopedStorage.ts already use for every other per-user localStorage blob (#900/#983). Falls back to the unscoped key when no user is active yet (pre-login), same as the rest of the app. Added tests for the scoping, the pre-login fallback, and cross-user isolation.
…d (addresses @chatgpt-codex-connector on app/src/hooks/useWorkflowBuilderChat.ts:184) WorkflowCopilotPanel reports every threadId change back up via onThreadIdChange, and FlowCanvasPage re-passes it straight back in as seedThreadId on the next render — so seedThreadId also changes right after send() creates a fresh thread, not just when a genuinely pre-existing persisted thread is (re)selected. The rehydrate effect was refiring in that case, racing the in-flight turn: loadThreadMessages.fulfilled replaces messagesByThreadId[threadId] wholesale, so a fetch resolving before the backend reflects the just-appended turn could wipe it back out of the transcript. Track thread ids this hook creates itself (createdThreadIdRef) and skip rehydration for them; a truly pre-existing seed still rehydrates as before.
…ser (addresses @chatgpt-codex-connector on app/src/components/flows/workflowCopilotThreads.ts:27) copilot-thread:<flow> keys were global, unlike every other per-user localStorage blob in the app (userScopedStorage.ts / clearAllAppData.ts both use the ${userId}: prefix convention from tinyhumansai#900/tinyhumansai#983). After an identity flip, or a "clear my data" on one account that only purges that account's ${userId}:* keys, the next user opening the same flow/draft would read the previous user's builder thread id and rehydrate into their stale thread instead of starting fresh. Namespace the key with getActiveUserId() (synchronous, primed at boot) when a user is active; fall back to the unscoped key pre-login, same as the rest of the app.
The Copilot chat thread id was only cached in a module-level Map, and
useWorkflowBuilderChat never reloaded a thread's messages/turn state on
mount — so a reload (or a fresh app launch) always started the copilot
transcript from empty even though the thread itself is durable
server-side (threadApi persists every turn).
- workflowCopilotThreads.ts: back the flow -> thread-id cache with
localStorage (namespaced `${userId}:copilot-thread:<flow>`, matching
the existing per-user localStorage convention) instead of an in-memory
Map, wrapped in try/catch so private-mode/quota errors degrade to a
no-op rather than throwing.
- useWorkflowBuilderChat: on mount, when `seedThreadId` names a thread
this hook did not just create itself via `send()` (tracked via
`createdThreadIdRef`), dispatch `loadThreadMessages` +
`fetchAndHydrateTurnState` + `fetchAndHydrateTurnHistory` to rehydrate
the full transcript + turn state from the core. The `createdThreadIdRef`
guard avoids racing an in-flight turn when a caller echoes a
just-created thread id back in as `seedThreadId`.
Depends on B4 (#<B4 PR>) — rehydration reloads the FULL transcript
(including interim narration bubbles), so without B4's `displayMessages`
filter, those would render as bubbles again after a reload.
Split from tinyhumansai#4650.
1 participant
Two Copilot-UX fixes (share the same files → one PR).
B4 — reasoning renders as chat bubbles instead of "Agentic task insights"
The builder narrates between tool calls ("Let me check…", "Now let me build…", "The mock doesn't match…"), and
ChatRuntimeProvider'sonInterimpersisted each narration as a permanent agent message → a chat bubble. Only the terminal turn should bubble; the rest is thinking.ChatRuntimeProvider: tag interimaddInferenceResponsemessages withextraMetadata.isInterim(additive — main chat unchanged).useWorkflowBuilderChat: newdisplayMessages=messagesminus interim agent messages (fullmessageskept intact for B11).WorkflowCopilotPanel: rendersdisplayMessagesfor bubbles;ToolTimelineBlock+ streaming preview unchanged.addMessageLocal, noisInterim) survives the filter — verified.B11 — copilot chat not persistent (lost on reload)
Three layers: the per-flow thread id lived in a module-level
Map(lost on reload); messages aren't in the redux-persist whitelist; the hook never re-fetched them. Messages are durable in the core — just never loaded back.workflowCopilotThreads: persist the thread id to localStorage (copilot-thread:<flowId>, try/catch).useWorkflowBuilderChat: on mount with aseedThreadId, dispatchloadThreadMessages+fetchAndHydrateTurnState/History(mirrorsConversations.tsx) to rehydrate.isInterimsurvives the persist→rehydrate round-trip, so B4's filter is consistent on reload.Verification
typecheck/eslint/formatclean · 89 tests (filter, rehydration, localStorage round-trip + reload sim, isInterim suppression) + 150 collateral pass · no new i18n.