fix(openai-chat): Pi developer role and duplicated serialized tool calls (lane F2) - #5674
Conversation
) Pi sends its system prompt as `developer` for reasoning models. The native Chat Completions route forwards caller roles verbatim unless a destination has recorded foldDeveloperRoleToSystem, so upstreams that reject the role (DashScope compatible-mode answers "developer is not one of [...]") failed every request. Users could not work around it: every export rewrites the whole provider block and drops a hand-set compat key. The Pi export now writes compat.supportsDeveloperRole: false next to the session-affinity key, on both the export and the managed-contribution path. omo keeps its byte-identical-to-Pi contract because senpi documents the same key. Prime and Aside keep their compat-free block.
…ble text (#5548) Some Chat gateways send one model-produced call twice: as a bare <tool_call><function=...>...</function></tool_call> block in content and as a structured tool_calls entry, sometimes with the block body prefixed onto the JSON arguments too. Codex then showed the raw call syntax as the answer and could receive malformed arguments. The adapter now holds a possible bare block (outside Markdown fences, quotes and inline code) and removes it only when its function name and freeform body match a structured call in the same response; the argument prefix is repaired only for that exact duplicated shape. Mismatched markup stays byte-exact, held text is drained on every terminal path, and the held bytes use the translator budget. Carried from #5548 and rebuilt on dev's inline <think> splitter: reconciliation sees only answer text; reasoning that arrives behind a held block is queued in place (with a heartbeat) so order is kept and the duplicate is not exposed early; buffered responses replay through the same buffer so both paths share one rule set. The adapter glue lives in serialized-tool-call-content.ts to stay under the file-size ratchet. #5548's unrelated codex/home.ts import-cycle, WSL test, and service test edits are not carried. Co-authored-by: Vadevious <56196048+Vadevious@users.noreply.github.com>
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. |
github-actions
Bot
added
the
intake: hygiene-blocked
|
✅ READY
Hygiene✅ Deterministic PR hygiene checks passed. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Repository: lidge-jun/opencodex/.coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (23)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
removed
the
intake: hygiene-blocked
![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: caf43c9a52
ℹ️ 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 nextBytes = Buffer.byteLength(next); | ||
| const reservation = this.budget.reserveTransient(nextBytes, { kind: "live_transient" }); |
There was a problem hiding this comment.
When a serialized-call header is split before its closing > (for example, "<tool_call><fun" followed by "ction=exec>"), the first fragment is already retained, but replace reserves the entire combined buffer before releasing those old bytes. The translator budget therefore temporarily counts oldBytes + nextBytes; with a 26-byte budget, the 15-byte and 11-byte chunks already reproduce a translation_buffer_limit even though the final retained buffer is exactly 26 bytes. Larger valid streams can consequently return a 502 solely because of transport chunk boundaries. Use the append/growth accounting path when next extends the retained prefix, while retaining atomic replacement for unrelated text.
AGENTS.md reference: src/AGENTS.md:L17-L19
Useful? React with 👍 / 👎.
1 participant
Summary
Lane F2 (openai-chat compatibility) of the 260923 PR-consolidation round. Two commits:
developerfor reasoning models, and the native/v1/chat/completionsroute forwards roles verbatim unless a destination recordsfoldDeveloperRoleToSystem, so upstreams that reject the role (DashScope compatible-mode:developer is not one of [...]) failed every request. Each export rewrites the whole provider block, so a hand-set fix did not survive. The Pi export (and omo, which stays byte-identical to Pi) now writescompat.supportsDeveloperRole: falseon both the export and the managed-contribution path. The key is in Pi's model-config schema (checked in the installed pi-coding-agent 0.83.0) and senpi documents it (docs/models.md). Prime and Aside keep their compat-free block. The Pi guide is updated in all 8 locales.<tool_call><function=…>…</function></tool_call>block in content and as a structuredtool_callsentry, sometimes with the block body also prefixed onto the JSON arguments. Codex showed the raw call syntax as the answer. The adapter now holds a possible bare block and removes it only when the function name and freeform body match a structured call in the same response; the argument prefix is repaired only for that exact shape. Quoted, fenced, inline, indented and mismatched markup stays byte-exact. Rebuilt on dev's inline<think>splitter: reconciliation sees answer text only, reasoning that arrives behind a held block is queued in place with a heartbeat (order kept, no early leak, no stall), and buffered responses replay through the same buffer. The glue lives insrc/adapters/openai-chat/serialized-tool-call-content.tssoopenai-chat.tsstays under its ratchet (816/822). Decision record renumbered ADR-5548 (ADR-0099 is taken). fix(codex-home): avoid WSL config import cycle #5548's unrelatedsrc/codex/home.tsimport-cycle, WSL, and service-test edits are not carried.Closes #5664
Partially carries #5548, which stays open (PARTIAL: the
src/codex/home.tsimport-cycle fix, its WSL test andstructure/codex-home.mdnote, and the Docker/systemd service-test edits were left out as unrelated to this defect; they need their own review before #5548 is closed)Co-authored-by: Vadevious 56196048+Vadevious@users.noreply.github.com
Assessed and excluded
ultrapasses through,openai-responses-passthrough.test.ts:867) and drops provider spellings; the 400 is unreproduced. Needs a destination-policy design.<think>e9643875f0(#5619) added the opt-ininlineThinkTagModelssplitting this issue asks for.e9643875f0widened the placeholder gate);deepseek-v4-prois deliberately retired from the thinking sets; the remaining resumed-history case needs a reproduction.<parameter=input>residue insidecustom_tool_call.inputand the intermittent leak need a reproduction;src/adapters/command-code-tool-text.tsis the model for a model-scoped parser.web_searchVerification
The owner reserves the full local suite for after every lane in this round has landed, so it was not run here. Focused runs on the final tree:
bun test tests/adapters/openai tests/responses tests/config/client-config-export.test.ts tests/clients/prime-client.test.ts tests/clients/omo-client.test.ts tests/test-layout.test.ts tests/test-layout-tooling.test.ts tests/ci-workflows/structure-ssot.test.ts tests/ci-workflows/file-size-ratchet.test.ts— 3718 pass, 0 fail (171 files).bun run typecheck,bun run structure:check,bun run privacy:scan,git diff --check origin/dev...HEAD— pass.tests/adapters/openai/openai-chat-serialized-tool-call-think.test.ts: duplicate removal behind a think block (streamed + buffered), different-input blocks stay byte-exact, same-line prose across a think section, event order across interleaved think sections, a duplicate between two think sections with a late structured call, context after a drain, and heartbeats while reasoning is queued.tests/clients/client-lifecycle-lock.test.tsfails in this worktree only because it lives under the real~/.codexand the test-home cleanup guard refuses to delete there; it touches nothing in this PR. CI covers it.Checklist
Summary by CodeRabbit
systemrole instead ofdeveloper, improving compatibility with upstreams that reject thedeveloperrole.