Repository navigation
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 73e44265ba
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| ptcStore.set(ptcTraceByToolCallId(atomKey), EMPTY_PTC_TRACE); | ||
| } | ||
| knownPtcAtomKeys.current.clear(); |
There was a problem hiding this comment.
Evict reset PTC family members
When a long-lived tab visits conversations containing many PTC invocations, this cleanup replaces each value but never removes the corresponding member from Jotai's atomFamily cache. Because atomFamily retains every parameter in an internal map, clearing knownPtcAtomKeys makes those message/tool keys permanently unreachable by later cleanup, so memory grows across conversation switches despite this boundary being intended to release the atoms. After publishing the empty value to any mounted subscriber, call ptcTraceByToolCallId.remove(atomKey), as the subagent cleanup already does for its dynamic families.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 5d8c3fc: resetPtcAtoms now calls ptcTraceByToolCallId.remove(atomKey) after publishing the empty trace; useStepHandler.spec asserts the eviction.
5d8c3fc to
27eaf27
Compare
27eaf27 to
921fbe9
Compare
Pull Request
Summary
Three pieces of chat run state were still Recoil atoms in the app store:
activeRunFamily(the run a pane displays, which audio autoplay keys "already played" on),messageAttachmentsMap(attachments that arrive mid-stream, with its per-conversation selector) andptcTraceByToolCallId(the live trace of a programmatic tool call's inner calls). The chat package is moving off Recoil one atom at a time, each with every reader and writer.This converts all three to Jotai with all their consumers: the SSE and resumable bindings, the audio hooks and
useClearStates, the attachment preview and message hooks,useChatHelpers, the parts host anduseStepHandler. They now live with the stream code that owns them (hooks/SSE/atoms.ts,hooks/SSE/ptc.ts, and the sandbox-starting atom moved beside them tohooks/SSE/sandbox.ts) instead of~/store, souseSSE,useStepHandleranduseAttachmentHandlerno longer import Recoil or~/storeat all. No behavior changes. Stacked on #17136.Type of change
Testing
Tested environments/configuration:
Not applicable: the atoms hold the same values with the same defaults; only the store changes.
Automated tests:
useResumableSSE.spec.tssetter mock.client: hooks/SSE, hooks/Chat, hooks/Files, hooks/Messages, hooks/Audio, hooks/Config, the audio input specs, components/Chat/Messages/Content, Providers and data-provider/SSE: 170 suites, 3,422 tests pass.npx tsc --noEmitinclient, eslint, prettier and sort-imports on the changed files: clean.Screenshots / recordings
No user-facing change.
Risk / compatibility
None for users. Logout clears
activeRunFamilythrough the Jotai store as it clears the other run atoms.Checklist