🎛️ refactor: Hand Chat Preferences to the Host and the Follow-Up Queue to Jotai - #16604
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. |
![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: 56dd0b90d8
ℹ️ 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.
Update useNewChat.spec.ts to supply saveDrafts through ChatSettingsContext. The suite still changes mockState.saveDrafts through its Recoil mock (including the draft-saving-off cases), but this hook now receives the context's default true, so the if (saveDrafts) predicate at line 362 never exercises its false branch; several tests can therefore pass without testing the behavior named by the scenario.
AGENTS.md reference: AGENTS.md:L49-L52
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 60de4dd: the spec now supplies saveDrafts through useChatSettings, and the drafts-off case shares the drafts-on fixture so it fails if drafts are forced on.
codegraph-librechat
Bot
added
the
🗺️ Chat State Mgmt
56dd0b9 to
a275911
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a275911724
ℹ️ 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".
| /** Whether composer text and attachments are kept as drafts across navigation. */ | ||
| saveDrafts: boolean; |
There was a problem hiding this comment.
When a host supplies saveDrafts independently of the app's Recoil store, this new contract splits the preference between two sources: the migrated draft hooks read this context value, but ChatView.tsx:61 still reads store.saveDrafts and passes it to AskAnswerHostProvider at line 177. Ask-answer draft handling can therefore remain enabled while the surrounding composer has drafts disabled, or vice versa; migrate that remaining reader to the host value before adding the preference to this contract.
AGENTS.md reference: AGENTS.md:L163-L170
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Deferred to berry-13#224: ChatView.tsx belongs to the open #15911, so its read stays on the store until that merges; inside the app both sources hold the same value.
8093dc1 to
3fa0394
Compare
berry-13
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3fa0394a88
ℹ️ 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".
| export const queuedMessagesByConvoId = atomFamily((_conversationId: string) => | ||
| atom<QueuedMessage[]>([]), | ||
| ); |
There was a problem hiding this comment.
These queue, receipt, and run-end atoms are read and written exclusively by chat behavior, but placing them in the app-wide client/src/store directory leaves this newly converted feature-owned state outside its owning feature. Move this module and its imports under the Chat feature boundary so the queue can be isolated or moved without depending on the global store.
AGENTS.md reference: AGENTS.md:L161-L166
Useful? React with 👍 / 👎.
Both preferences are app-global: the settings dialog writes saveDrafts, and the chat route and keyboard shortcuts write isTemporary. Every chat reader (composer, autosave, pasted-text edit, new chat and new convo, file uploads, submissions, the temporary toggle, landing, run code) now takes them from the host ChatSettings context, which the app fills from its store. ChatView still reads saveDrafts from the store while #15911 owns it.
The follow-up queue, its settled receipts and pending enqueue ids, the pane and parked run-end signals, and the interrupt-drain flag are written and read only by chat logic (queue drain, steering, the SSE hook, the queue rail). They now live in store/queue as Jotai atoms with every consumer converted. The run-end FIFO keeps its nullable one-shot API as a writable derived atom.
The queue atoms and their types move from store/queue to hooks/Chat/queue, next to the drain that owns them. store/families re-exports the types so existing imports keep working.
e8fecab to
5df00df
Compare
1 participant
Summary
The chat reads the
saveDraftsandisTemporarypreferences straight from the Recoil store. Both are app-global: the settings dialog writessaveDrafts, and the chat route and keyboard shortcuts writeisTemporary. Under the client state ownership rule inAGENTS.md, a preference the chat only consumes should come from the host. This PR addssaveDrafts,isTemporaryandsetIsTemporaryto the hostChatSettingscontext, fills them from the store inChatSettingsProvider, and moves every chat reader to that context: the composer, autosave, pasted-text edit, new chat and new conversation, upload retention, submissions, the temporary toggle, the landing screen and Run Code. The exception isChatView, which #15911 owns, so it still readssaveDraftsfrom the store (berry-13#224 tracks moving it).The follow-up queue and its run-end bookkeeping are state the chat both writes and reads, so the same rule makes them feature-owned Jotai. That covers the queue, settled receipts, pending enqueue ids, the pane and parked run-end signals, and the interrupt-drain flag. They now live in
hooks/Chat/queue.ts, inside the chat feature, and every consumer is converted.This PR is larger than the usual stack link: 2,771 changed lines, or 1,799 with whitespace ignored. It was opened as the first link of a stack, and the second link, #16615 (the queue conversion), was squash-merged into this branch after its own review. The commits after that merge are review fixes. The PR has been reviewed and closed out as one unit, so it stays as one PR.
Closes berry-13#186.
Type of change
Testing
Tested environments/configuration:
reviewctl verify(desktop light, desktop dark, mobile).Automated tests:
reviewctl precheck: static checks plus related jest, about 6,500 tests.chat-settings-owners.spec.tscovers the temporary turn and draft saving, andqueue-owners.spec.tscovers interrupt and send. The existing composer-queue scenarios cover queueing, draining, reordering and steer-to-queue.resetQueueFamilies()isolates tests that share the default store.atoms.sh(chat C2): 6 to 12 of the 26 feature-owned atoms are on Jotai. Chat C1 is unchanged, because every touched hook still imports Recoil for other atoms.Screenshots / recordings
No user-facing change.
Risk / compatibility
Removing
useRecoilCallbackwrappers re-indents callback bodies, so review with "Hide whitespace" on. Jotai store reads are live, whereas Recoil callback snapshots were frozen. Every converted callback reads before it writes in one synchronous pass, so its decisions are unchanged. Run Code readsisTemporaryfrom a ref at execution time. An existing gap stays out of scope: a follow-up queued in a chat whose run finishes while the user is in another chat doesn't auto-send (berry-13#225, reproduced on canary).Checklist