Repository navigation
🛟 refactor: Move Resumable Transport Failure Handling Into the Chat Core - #17120
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. |
9815753 to
2f56b6d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f56b6db95
ℹ️ 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".
| }; | ||
|
|
||
| /** The status route's answer for a conversation's generation. */ | ||
| export interface StreamStatusResponse { |
There was a problem hiding this comment.
Move the stream-status contract back to data-provider
StreamStatusResponse is an API response contract consumed by the client data-provider query layer, but this change moves its ownership into @librechat/chat, reversing the intended dependency and requiring API consumers to depend on the chat package. Keep the response type—and ideally the associated stream URL definition—in packages/data-provider, then consume or inject it from chat so the shared transport contract has one owner.
AGENTS.md reference: AGENTS.md:L203-L205
Useful? React with 👍 / 👎.
2f56b6d to
2ea07cc
Compare
Pull Request
Summary
When a resumable stream's connection fails,
useResumableSSEdecides what the failure means: a 409 hands the attachment to a newer generation, a 404 settles the run from persisted history unless a status read shows it still live, an error the server wrote into the stream becomes the error card, and a dropped connection climbs a backoff ladder until a status read at the retry ceiling either keeps waiting on the run or settles it. That 630-line handler was one closure in the hook, writing to the query client, Recoil and Jotai directly.This moves it into
packages/chat/src/core/resumable/:failure.tsdispatches and owns the backoff and served-error paths,missing.tsthe 404 recovery,ceiling.tsthe retry-ceiling settle. The host interface gains the few operations they need by intent (the conversation snapshot read and its cache writes, the balance refresh, the pane's conversation update, the detached-run end, the error card and the message list), which the hook supplies from what it already reads. No behavior changes. Stacked on #17119.Type of change
Testing
Tested environments/configuration:
Not applicable: the handling is moved, not changed; the hook specs exercise every failure path through the host the hook builds.
Automated tests:
packages/chat/src/core/__tests__/failure.spec.tsagainst a fake host and scheduler: a failure after the final is ignored, a dropped connection backs off and re-attaches, a served error writes the error card and run end once the status proves the run over, a 404 settles from persisted history or keeps a still-live generation attached, and the retry ceiling either waits on a live run or settles a terminal one.npx jest src/coreinpackages/chat(162 pass), and inclientthe hooks/SSE and hooks/Chat specs plus data-provider/SSE (1148 pass, existing specs unchanged).npx tsc --noEmitinpackages/chatandclient, eslint, prettier and sort-imports on the changed files: clean.Screenshots / recordings
No user-facing change.
Risk / compatibility
None.
Checklist