-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix(devin): replay the assistant's reasoning instead of dropping it #4426
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -160,15 +160,44 @@ function assistantToolCalls(message: OcxAssistantMessage): Array<{ id: string; n | |
|
|
||
| function assistantText(message: OcxAssistantMessage): string { | ||
| return message.content | ||
| // Thinking stays out of the replayed content. Cognition has no reasoning | ||
| // replay field, and folding chain-of-thought into assistant text sends it | ||
| // back as visible prior output - which the model then treats as something | ||
| // it said to the user. | ||
| // Thinking stays out of the replayed TEXT: folding chain-of-thought into | ||
| // assistant text sends it back as visible prior output, which the model | ||
| // then treats as something it said to the user. It is replayed in its own | ||
| // field instead — see assistantThinking below. | ||
| .map((part) => (part.type === "text" ? part.text : "")) | ||
| .filter(Boolean) | ||
| .join("\n"); | ||
| } | ||
|
|
||
| /** | ||
| * The assistant turn's own reasoning, for replay in ChatMessagePrompt #11. | ||
| * | ||
| * This adapter previously asserted that Cognition has no reasoning-replay | ||
| * field and dropped the thinking outright, so a reasoning model restarted its | ||
| * chain on every turn of a tool loop. The field exists: two independent | ||
| * clients of the same service write #11 thinking with #12 signature and #18 | ||
| * signature_type on the assistant prompt. | ||
| * | ||
| * The signature attests the thinking it was produced with, so a block without | ||
| * one contributes its text and nothing else rather than borrowing a neighbour's. | ||
| */ | ||
| function assistantThinking( | ||
| message: OcxAssistantMessage, | ||
| ): { thinking?: string; signature?: string } { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For Cognition models that require field #18 to identify the signature scheme, every signed replay produced through this adapter still omits it: AGENTS.md reference: src/AGENTS.md:L19-L19 Useful? React with 👍 / 👎. |
||
| const blocks = message.content.filter( | ||
| (part): part is Extract<typeof part, { type: "thinking" }> => part.type === "thinking", | ||
| ); | ||
| if (blocks.length === 0) return {}; | ||
| 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When history contains unsigned reasoning or reasoning produced by another provider, AGENTS.md reference: src/AGENTS.md:L19-L19 Useful? React with 👍 / 👎.
Comment on lines
+191
to
+194
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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 👍 / 👎. |
||
| return { | ||
| ...(thinking ? { thinking } : {}), | ||
| ...(signature ? { signature } : {}), | ||
| }; | ||
| } | ||
|
|
||
| export function mapOcxMessagesToDevin(parsed: OcxParsedRequest): ChatHistoryItem[] { | ||
| const items: ChatHistoryItem[] = []; | ||
| // Cognition is not an OpenAI host, and this adapter does advertise a real | ||
|
|
@@ -203,11 +232,15 @@ function mapOneMessage(message: OcxMessage): ChatHistoryItem | undefined { | |
| if (message.role === "assistant") { | ||
| const toolCalls = assistantToolCalls(message); | ||
| const text = assistantText(message); | ||
| if (!text && toolCalls.length === 0) return undefined; | ||
| const reasoning = assistantThinking(message); | ||
| // A turn that produced only reasoning is still worth replaying: dropping it | ||
| // is what makes the next turn re-derive the same chain. | ||
| if (!text && toolCalls.length === 0 && !reasoning.thinking) return undefined; | ||
| return { | ||
| role: "assistant", | ||
| content: text || "", | ||
| ...(toolCalls.length > 0 ? { tool_calls: toolCalls } : {}), | ||
| ...reasoning, | ||
| }; | ||
| } | ||
| if (message.role === "toolResult") { | ||
|
|
@@ -335,6 +368,12 @@ export function createDevinAdapter( | |
| if (event.text) emit({ type: "thinking_delta", thinking: event.text }); | ||
| continue; | ||
| } | ||
| if (event.kind === "reasoning_signature") { | ||
| // Carried back out so the next turn can replay it in the prompt's | ||
| // signature field; an unsigned replay is what the service ignores. | ||
| emit({ type: "thinking_signature", signature: event.signature }); | ||
| continue; | ||
| } | ||
| if (event.kind === "tool_call_start") { | ||
| closeOpenTool(); | ||
| openToolId = event.id; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This changes Devin adapter and transport replay semantics without updating any of the structure documents mapped to
src/adapters/instructure/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 👍 / 👎.