🎛️ refactor: Supply App-Global Chat Settings From the Host - #16466
Conversation
The chat hooks read the during-run default, steer interrupts, save drafts, temporary chat and the artifacts reset from a ChatSettings context that Root fills from the existing Recoil atoms. liveAppliedSteerIds moves to Jotai with its reader and writer, and manual compaction submits through the useChat facade.
|
Inventory of the Recoil and App-global: stay in Recoil, now host-supplied through
App-global, stay on the store until every reader can move
App-global conversation state: stays in Recoil, read by the shell too
Feature-owned: converted to Jotai here
Feature-owned: blocked, because
Feature-owned: blocked by #16457 (
Feature-owned: deferred, because they share Recoil transactions with
Feature state also cleared or read outside the feature: deferred
Already on Jotai: |
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. |
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. |
|
Lighthouse CI failed. The last 80 log lines contain the measured budgets and assertion failures. |
![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: 4bd832a1e0
ℹ️ 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".
| const { sendMessage, status } = useChat(); | ||
| const isSubmitting = status === 'submitted' || status === 'streaming'; |
There was a problem hiding this comment.
When contextUsage is enabled, TokenUsageIndicator mounts this hook even if compaction itself is disabled. Calling the full useChat() facade here subscribes that control to every message-cache write and makes facade.ts iterate and map the complete conversation on each streamed frame, in addition to the existing useLatestMessage and token-usage projections; long conversations therefore gain another O(history) pass and render on every token. Keep using the lightweight contract's ask/isSubmitting, or expose an action/status-only facade that does not materialize messages.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 03e1ef3: compaction now reads status and submits through useChatActions, the facade's action/status surface. Its snapshot is the status string, so a stream frame re-renders only when the status changes and never maps the conversation. Covered by the new useChatActions tests in facade.spec.tsx (no re-render across frames that keep the status).
| const { | ||
| duringRunDefaultAction: defaultAction, | ||
| setDuringRunDefaultAction: setDefaultAction, | ||
| steerInterruptsByDefault, | ||
| } = useChatSettings(); |
There was a problem hiding this comment.
When a host supplies steerInterruptsByDefault from somewhere other than the app's Recoil store, submitDuringRun now honors this injected value, but DuringRunSendButton.tsx:43 still independently reads store.steerInterruptsByDefault to choose its shortcut labels. If the two values differ, Enter performs an interrupt while the menu advertises ordinary steer, or vice versa; the button can use the already exposed steering.steerInterruptsByDefault instead of retaining a second source of truth.
AGENTS.md reference: AGENTS.md:L120-L124
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 03e1ef3: DuringRunSendButton reads steering.steerInterruptsByDefault instead of the store, so the menu labels and Enter share one source. DuringRunSendButton.test.tsx now drives the preference through the steering controls.
…ault Compaction reads status through useChatActions, which re-renders only when the status changes instead of mapping every stream frame. The during-run send button reads the interrupt default from the steering controls instead of a second store read.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 03e1ef3901
ℹ️ 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".
| const conversationId = useRecoilValue(store.conversationIdByIndex(index)); | ||
| const files = useRecoilValue(store.filesByIndex(index)); | ||
| const saveDrafts = useRecoilValue(store.saveDrafts); | ||
| const { saveDrafts } = useChatSettings(); |
There was a problem hiding this comment.
When a non-Recoil host supplies a saveDrafts value that differs from the Recoil default, this hook uses the injected value but immediately calls useNewConvo(), which still reads store.saveDrafts at useNewConvo.ts:100 and branches on it at lines 426 and 459; composer autosave also retains its independent Recoil read in useAutoSave.ts:57. With injected false and Recoil true, startNewChat skips its saved-draft deletion path while newConversation skips its drafts-disabled deletion path, orphaning discarded uploads and continuing to persist drafts. Pass the host value through all of these consumers instead of splitting the behavior between two stores.
AGENTS.md reference: AGENTS.md:L120-L124
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4e189a1 by narrowing the host surface instead of chasing readers: saveDrafts is also read by ChatView (#15684), which this stack cannot edit, so it stays on the store and useNewChat reads it there again. A preference joins ChatSettings only once every chat reader of it can take it from there (now stated on the type); moving saveDrafts is a follow-up for after #15684.
| const getEphemeralAgent = useGetEphemeralAgent(); | ||
| const agentsMap = useAgentsMapContext(); | ||
| const isTemporary = useRecoilValue(store.isTemporary); | ||
| const { isTemporary } = useChatSettings(); |
There was a problem hiding this comment.
When an alternate host supplies isTemporary: true while the Recoil atom remains at its default false, turn submission is marked temporary here, but useFileHandling.ts:169,387-389 still reads the Recoil atom and omits isTemporary from the upload form. For a new conversation there is no stored conversation from which the server can infer retention, and packages/api/src/files/retention.ts:175-177 consequently returns no expiry when this flag is absent, so attachments from a temporary chat can be retained as ordinary files. Make the upload path consume the same host-supplied setting.
AGENTS.md reference: AGENTS.md:L120-L124
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 4e189a1 the same way: isTemporary is read by ChatForm (#16457), useTemporaryChat and useResumeOnLoad (#13811) as well as the upload path, so it stays on the store and useChatFunctions reads it there again. Uploads and turns share one source; moving it to the host is a follow-up for after those PRs.
saveDrafts and isTemporary are also read by useNewConvo, useAutoSave and the upload path, and by ChatView, ChatForm and the resume hook, so a host value for them would split one preference across two sources. They stay on the store until all of their readers can move together.
1 participant
Pull Request
Summary
The chat hooks in
hooks/Chatandhooks/SSEread app-global preferences (the during-run default action, steer interrupts, and the artifacts panel reset) straight out of Recoil~/store, so the chat cannot run under a host that keeps its settings anywhere else. The AI SDK-shapeduseChatfacade also had no production consumer, only its spec.This adds a small
ChatSettingscontext that the host supplies.Rootfills it from the existing Recoil atoms, so every preference behaves as before, anduseSteering,DuringRunSendButtonanduseIdChangeEffectread it instead of~/store. A preference joins the context only when every chat reader of it can move, so no reader acts on a different value than the host supplies. A render with no host falls back to the stock defaults.liveAppliedSteerIds, which only the steering feature reads and writes, moves to Jotai with its readers (SteerPart) and its writer (useResumableSSE). Manual compaction (useCompactConversation) now submits and reads its status through the facade's newuseChatActions: theuseChatsurface withoutmessages, which re-renders only when the status changes, so the facade has a real call site that does not map the conversation on every stream frame. Giving it one exposed a facade bug: it re-read on every query cache event, synchronously, so a component mounting a messages query mid-render updated the facade's consumer inside that render (a "Maximum update depth" crash on the first send of a new chat). It now listens only to data events and delivers them after the render, anduseCompactConversationleaves thehooks/Chatbarrel so importing~/hooksdoes not pull the facade's message components into a cycle.saveDraftsandisTemporaryfail that test, sinceChatView(#15684),ChatForm(#16457) and the resume hook (#13811) read them too, so they stay on the store. Atoms that files in open PRs read or write stay on Recoil for now: #13811 (useResumeOnLoad,useTemporaryChat,useBookmarkItems), #16457 (ChatForm) and #15684 (ChatView). The queue run-end atoms also stay on Recoil, because they share Recoil transactions withqueuedMessagesByConvoId. The per-atom inventory is in a comment below.Type of change
Testing
Tested environments/configuration: local dev servers, production client build, headless Chromium.
Automated tests: added
routes/__tests__/ChatSettings.spec.tsx. Added a facade regression test for a query mounted mid-render (fails without the fix) anduseChatActionstests (no re-render across frames that keep the status). Updated theuseSteering,useNewChat,useCompactConversation(now through the real facade) andSteerPartspecs.jest --findRelatedTestson the changed hooks: 482 suites, 6327 tests passed. Clienttsc --noEmitand eslint clean.Screenshots / recordings
No user-facing change.