fix(devin): replay the assistant's reasoning instead of dropping it - #4426
Conversation
The adapter asserted that Cognition has no reasoning-replay field and dropped every assistant thinking block, so a reasoning model restarted its chain on each turn of a tool loop and paid for it again. The field exists. Two independent clients of the same service write it on the assistant prompt: #11 thinking, #12 signature, #18 signature_type. The response side was half-missing too. delta_thinking (#9) was decoded but delta_signature (#10) was not, so even once the prompt could carry a signature there was never one to carry: the replay would always have been unsigned. #10 now surfaces as a thinking_signature event. Reasoning still stays out of the replayed assistant TEXT, which is what the original comment was right about — folding chain-of-thought into the visible content makes the model treat it as something it said to the user. It rides its own field instead. A turn that produced only reasoning is now replayed rather than skipped, since dropping it is what forces the next turn to re-derive the same chain.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe Devin adapter now preserves assistant reasoning and signatures. It maps reasoning into chat history, encodes it in prompt fields 11, 12, and 18, decodes response field 10, and forwards signatures for later replay. ChangesDevin reasoning replay
Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix Sequence Diagram(s)sequenceDiagram
participant AssistantMessage
participant mapOneMessage
participant ChatHistoryItem
participant encodeChatMessagePrompt
participant decodeChatFrame
participant runTurn
AssistantMessage->>mapOneMessage: provide thinking blocks and signature
mapOneMessage->>ChatHistoryItem: store reasoning separately from content
encodeChatMessagePrompt->>ChatHistoryItem: encode fields 11, 12, and 18
decodeChatFrame->>runTurn: deliver reasoning_signature from field 10
runTurn->>ChatHistoryItem: emit thinking_signature for later replay
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 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 |
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
bug
|
⏳ DRAFT
What to do
Automatic draft conversion failed (token cannot change draft status). Please convert this pull request to a draft manually. The required |
리뷰 · 우선순위 71 / 80설명 지금 이 PR은 그 단정이 틀렸다는 쪽에 선다. 같은 서비스를 쓰는 다른 클라이언트 두 곳이 어시스턴트 프롬프트에 thinking(필드 11), signature(필드 12), signature_type(필드 18)을 쓴다고 보고, 그 경로를 연다. 응답 쪽도 반쪽이었다. 범위는 Devin 어댑터와 cloud-direct 인코더/디코더, 그리고 다만 위생 검사( 라인 문제
위생 본문/커밋 - 프로토버프 필드를 메인테이너의 판단이 필요한 지점
너의 추천 위생 본문에서 필드 번호를 이 댓글은 grok-bot이 작성했습니다 |
![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: d2f3dff4af
ℹ️ 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 thinking = blocks.map(b => b.thinking).filter(Boolean).join("\n"); | ||
| // Only one signature can ride the prompt, so take the last block that has | ||
| // one: that is the block the turn actually ended on. | ||
| const signature = blocks.filter(b => b.signature).at(-1)?.signature; |
There was a problem hiding this comment.
When history contains unsigned reasoning or reasoning produced by another provider, OcxThinkingContent.signature is not necessarily a Cognition signature: src/responses/parser.ts can populate it with JSON.stringify(reasoning), while Anthropic histories carry their own opaque signatures. Forwarding every truthy value as Devin field #12 therefore pairs the text with an invalid attestation, so a same-provider unsigned continuation or provider-switched conversation can have its replay ignored or rejected. Preserve Devin provenance in the internal event/history contract and emit #12 only when that provenance matches the current Devin destination.
AGENTS.md reference: src/AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
| function assistantThinking( | ||
| message: OcxAssistantMessage, | ||
| ): { thinking?: string; signature?: string } { |
There was a problem hiding this comment.
This changes Devin adapter and transport replay semantics without updating any of the structure documents mapped to src/adapters/ in structure/INDEX.md. The scoped repository rule requires every document listed for a changed source area to be updated in the same change, so the relevant ownership documentation must be synchronized before landing.
AGENTS.md reference: src/AGENTS.md:L11-L11
Useful? React with 👍 / 👎.
| const thinking = blocks.map(b => b.thinking).filter(Boolean).join("\n"); | ||
| // Only one signature can ride the prompt, so take the last block that has | ||
| // one: that is the block the turn actually ended on. | ||
| const signature = blocks.filter(b => b.signature).at(-1)?.signature; |
There was a problem hiding this comment.
When one assistant message contains multiple thinking blocks, this concatenates every block's text but attaches only the last available signature. The parser deliberately preserves multiple individually signed reasoning items in one assistant turn, and each opaque signature attests only its original block, so the resulting #11/#12 pair is invalid and Cognition cannot reliably replay it. Select a single matching block—typically the final signed block—or otherwise preserve block boundaries instead of combining signed payloads.
AGENTS.md reference: src/AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d2f3dff4af
ℹ️ 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".
| */ | ||
| function assistantThinking( | ||
| message: OcxAssistantMessage, | ||
| ): { thinking?: string; signature?: string } { |
There was a problem hiding this comment.
For Cognition models that require field #18 to identify the signature scheme, every signed replay produced through this adapter still omits it: assistantThinking can return only thinking and signature, so ChatHistoryItem.signature_type remains undefined and the encoder never emits the newly documented #18 field. This leaves the #11/#12 replay in a shape that differs from both verified clients and can cause the signature to be ignored or rejected, defeating the tool-loop reasoning replay; preserve or infer the verified Devin signature type alongside the signature and extend the wire test to assert #18.
AGENTS.md reference: src/AGENTS.md:L19-L19
Useful? React with 👍 / 👎.
1 participant
Summary
The adapter asserted that Cognition has no reasoning-replay field and dropped every assistant thinking block, so a reasoning model restarted its chain on each turn of a tool loop — and paid for it again.
The field exists. Two independent clients of the same service write it on the assistant prompt: #11 thinking, #12 signature, #18 signature_type. This change carries the assistant's own reasoning there.
The response side was half-missing too.
delta_thinking(#9) was decoded butdelta_signature(#10) was not, so even once the prompt could carry a signature there would never be one to carry and the replay would always have been unsigned. #10 now surfaces as athinking_signatureevent, which closes the round trip.Reasoning still stays out of the replayed assistant text, which is what the original comment was right about: folding chain-of-thought into the visible content makes the model treat it as something it said to the user. It rides its own field instead. A turn that produced only reasoning is now replayed rather than skipped, since dropping it is exactly what forces the next turn to re-derive the same chain.
Devin-only. No shared or cross-provider code is touched.
Verification
bun test tests/providers/devin-hardening.test.ts tests/providers/devin-adapter.test.ts— 44 pass, 0 fail. New rows assert the mapped history keeps thinking and its signature while the visible text stays clean, that a reasoning-only turn survives, that the encoded prompt really carries Bug: 注入 opencodex 后 Codex App 左侧 Project 线程列表消失 #11 and Add configurable context window caps #12, and that [codex] fix passthrough stream idle disconnects #10 decodes.bun x tsc --noEmit— clean.bun run test: NOT RUN locally by request; remote CI on this head is the evidence.Field numbers were read from two independent read-only reference clones in scratch (
can1357/oh-my-piandjc01rho/CLIProxyAPIPlus), which agree. Neither is vendored.Checklist
Summary by CodeRabbit
Bug Fixes
Tests