fix(deepseek): canonical item-id repair for UUID message/reasoning ids (#938) - #1027
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8403fe695e
ℹ️ 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".
| // #938: WS turns reframe this JSON into events in the bridge — normalize | ||
| // ids here so both bounded-JSON paths agree (raw record already happened). | ||
| const outboundJson = options.inboundTransport === "websocket" | ||
| && providerModelResponsesUpstreamStreaming(route.providerName, route.provider, route.modelId) === false |
There was a problem hiding this comment.
In DeepSeek’s bounded-JSON path, route.provider has already been mutated by resolveWireProtocolOverride to openai-responses, while providerModelResponsesUpstreamStreaming() only returns DeepSeek’s false hint when providerMatchesRegistryTransport() still sees the registry adapter (openai-chat). This new WS normalization therefore skips the built-in DeepSeek case, so the bridge reframes raw JSON with UUID item ids and Codex can still remain stuck on the uncommitted turn; keep the pre-override streaming hint from request normalization or make this lookup tolerate the wire override.
AGENTS.md reference: src/AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
| // Registry-only repair policy (#938): fill only when the runtime provider has | ||
| // no explicit policy, and deep-clone so saved/user values never alias the | ||
| // registry constant. | ||
| if (prov.responsesItemIdRepair === undefined && entry.responsesItemIdRepair) { |
There was a problem hiding this comment.
This backfill also runs through enrichProviderFromCatalog() before save in src/cli/init.ts:153 and src/server/management/provider-routes.ts:131, so adding DeepSeek through those workflows persists the current registry-only responsesItemIdRepair into config.json. From then on routedProviderConfig() treats it as an explicit user policy and will not inherit future registry fixes/default changes, contrary to the intended route-time fill behavior; keep this repair policy out of the save-time enrichment path or strip it before saving.
AGENTS.md reference: src/AGENTS.md:L18-L18
Useful? React with 👍 / 👎.
a3dd247 to
ec5e666
Compare
8403fe6 to
68c165e
Compare
#938) DeepSeek's Responses route emits bare UUID item ids that leave Codex stuck on an uncommitted turn. - ResponsesItemIdRepairConfig.repairInvalidIds: existing message/ reasoning ids without the canonical msg_/rs_ prefix mint stable type-scoped canonical ids (function_call ids and call_id untouched) - built-in DeepSeek gets the registry-only policy { repairInvalidIds, repairMissingTerminalIds } filled by derive (explicit config wins) - the bounded-JSON paths (#875) apply the same client normalization on both HTTP SSE synthesis and WS reframes; raw continuation snapshots keep upstream ids Supersedes PR #940 with a narrow reimplementation (its id-prefix idea; its response-id aliasing, envelope synthesis, and relay surgery are deliberately excluded, recorded in the 060 doc).
…e route path (#938) routedProviderConfig never enriched providers, so saved configs and fresh provider adds never received the registry repair policy — the e2e test masked this by enriching manually. Fill-only-when-absent with cloned arrays, matching the file's established backfill pattern; activation test now uses the ordinary un-enriched seed.
68c165e to
a26d6a7
Compare
ec5e666 to
710492c
Compare
Summary
Stack 05 of the bug-stack campaign (
devlog/_plan/260805_bug_stack_campaign/060), stacked on #1026. Fixes #938.DeepSeek's Responses route emits bare UUID item ids, which leave Codex clients stuck on Thinking — the lifecycle never correlates into a committed turn.
ResponsesItemIdRepairConfig.repairInvalidIds: existing message/reasoning ids without the canonicalmsg_/rs_prefix mint stable, type-scoped canonical ids keyed by output_index. function_call ids andcall_idpairing are never rewritten;response.idis untouched (no continuation aliasing).{ repairInvalidIds, repairMissingTerminalIds }, filled fill-only-when-absent on the live route (routedProviderConfig) with cloned arrays — explicit config always wins; the policy never seeds into saved config.repairResponsesJsonItemIds). Raw continuation snapshots keep upstream ids, so synthetic ids never flow back upstream.Tests
tests/deepseek-responses-item-id-repair.test.ts: derive/route fill with explicit-override preservation, stable canonical ids across every lifecycle occurrence, function_call identity byte-identical, canonical ids pass through unchanged, whole-JSON repair, and the live un-enriched-seed path (no UUIDs in the synthesized SSE).bun run typecheck0 errors;bun run privacy:scanpass.bun run teston Linux (ssh lidge): 8288 pass / 0 fail.Provider docs
Five locale files document
repairInvalidIdsand the DeepSeek default.