Skip to content

Commit 9510ab5

Browse files
ssddOnTopautofix-ci[bot]tusharmath
authored
fix(context): prevent duplicate reasoning block (#2782)
Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Tushar Mathur <tusharmath@gmail.com>
1 parent fd60dc7 commit 9510ab5

File tree

1 file changed

+65
-12
lines changed

1 file changed

+65
-12
lines changed

crates/forge_domain/src/context.rs

Lines changed: 65 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -567,21 +567,19 @@ impl Context {
567567
tool_records: Vec<(ToolCallFull, ToolResult)>,
568568
phase: Option<MessagePhase>,
569569
) -> Self {
570-
// Convert flat reasoning string to reasoning_details if present
571-
let merged_reasoning_details = if let Some(reasoning_text) = reasoning {
572-
let reasoning_entry = ReasoningFull {
570+
// Convert flat reasoning string to reasoning_details only when no structured
571+
// reasoning_details are present. When reasoning_details already exists it
572+
// already contains the text (with its cryptographic signature), so adding
573+
// another entry from the raw `reasoning` string would produce a duplicate
574+
// thinking block with a null signature, which Anthropic rejects.
575+
let merged_reasoning_details = match (reasoning, reasoning_details) {
576+
(_, Some(details)) => Some(details),
577+
(Some(reasoning_text), None) => Some(vec![ReasoningFull {
573578
text: Some(reasoning_text),
574579
type_of: Some("reasoning.text".to_string()),
575580
..Default::default()
576-
};
577-
if let Some(mut existing_details) = reasoning_details {
578-
existing_details.push(reasoning_entry);
579-
Some(existing_details)
580-
} else {
581-
Some(vec![reasoning_entry])
582-
}
583-
} else {
584-
reasoning_details
581+
}]),
582+
(None, None) => None,
585583
};
586584

587585
// Adding tool calls
@@ -1641,4 +1639,59 @@ mod tests {
16411639

16421640
assert_eq!(actual, expected);
16431641
}
1642+
1643+
/// Regression test: when both `reasoning` (raw text) and
1644+
/// `reasoning_details` (structured, with a cryptographic signature) are
1645+
/// present, `append_message` must NOT create a duplicate thinking block
1646+
/// with a null signature.
1647+
///
1648+
/// The Anthropic API rejects messages where any thinking block carries a
1649+
/// null or missing signature, so the stored `reasoning_details` must
1650+
/// contain exactly the structured entries that were passed in — no
1651+
/// extras.
1652+
#[test]
1653+
fn test_append_message_does_not_duplicate_reasoning_when_details_present() {
1654+
// Fixture: a structured reasoning detail with a valid signature, as would
1655+
// arrive after aggregating an Anthropic streaming response.
1656+
let fixture_details = vec![ReasoningFull {
1657+
text: Some("Let me think about this.".to_string()),
1658+
signature: Some("EpwFvalidSignatureABC123".to_string()),
1659+
type_of: Some("reasoning.text".to_string()),
1660+
format: Some("anthropic-claude-v1".to_string()),
1661+
index: Some(0),
1662+
..Default::default()
1663+
}];
1664+
1665+
// Both reasoning (raw string) and reasoning_details (structured) are provided,
1666+
// mirroring what orch.rs passes after collecting a streamed Anthropic response.
1667+
let fixture = Context::default().add_message(ContextMessage::user("Hello", None));
1668+
let actual = fixture.append_message(
1669+
"Answer",
1670+
None,
1671+
Some("Let me think about this.".to_string()), // raw reasoning string
1672+
Some(fixture_details.clone()), // structured reasoning_details
1673+
Usage::default(),
1674+
vec![],
1675+
None,
1676+
);
1677+
1678+
// Extract the stored reasoning_details from the assistant message.
1679+
let stored = actual
1680+
.messages
1681+
.iter()
1682+
.find_map(|entry| {
1683+
if let ContextMessage::Text(msg) = &**entry
1684+
&& msg.role == Role::Assistant
1685+
{
1686+
return msg.reasoning_details.as_ref();
1687+
}
1688+
None
1689+
})
1690+
.expect("Assistant message should have reasoning_details");
1691+
1692+
// Expected: exactly the one structured entry that was passed in.
1693+
// No duplicate null-signature entry should have been appended.
1694+
let expected = fixture_details;
1695+
assert_eq!(stored, &expected);
1696+
}
16441697
}

0 commit comments

Comments
 (0)