Repository navigation
Conversation
|
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
|
@greptileai Added a regression for reasoning events before response assembly and refreshed the live replay evidence for the latest commit. Please review again |
|
@greptileai Addressed the mutation review with an immutable grouping implementation. Full lint, 244 tests, and updated live replay evidence pass. Please review again |
|
@Sameerlite Required checks pass, coverage is complete, and Greptile reports 5/5. Could you review and rerun cost-map-guard, which timed out downloading multidict? |
236df8e to
1086b1c
Compare
|
@greptileai Rebased onto main with all 27 required checks passing and refreshed live replay evidence matching the current tip. Please review again |
|
bugbot run Please review the rebased main diff after all required checks and live replay validation passed successfully |
|
@Sameerlite The PR is rebased onto main, mergeable, and green across required CI, coverage, Greptile, Veria, and Bugbot. Ready for review |
1086b1c to
ec81208
Compare
|
@greptileai Rebased onto current main, preserved upstream lifecycle tests, and refreshed 327 regressions plus live Anthropic replay evidence. Please review again |
|
bugbot run Please review the current main diff after 327 regressions, full lint, and refreshed live Anthropic replay validation passed |
ec81208 to
2f19d21
Compare
|
@greptileai Rebased onto main with the upstream CI fix. Local lint, 332 tests, and refreshed live replay pass. Please review again |
|
bugbot run Please review the updated main diff with the upstream CI fix, passing local checks, and refreshed live replay evidence |
|
@Sameerlite All 27 required checks pass after rebasing onto main. Patch coverage is complete, with Greptile 5/5, Veria, and Bugbot clear. Please review |
2f19d21 to
8b61968
Compare
|
@greptileai Resolved an upstream test import conflict. Full lint, 342 tests, and refreshed Opus 5.5 replay evidence pass. Please review the updated commit |
|
bugbot run Please review the updated commit after resolving the upstream import conflict, passing 342 tests, and refreshing live replay evidence |
|
@greptileai Please review 069f5a1. Converted tool indexes and late citations are fixed; 433 regressions, full lint, and refreshed provider checks pass |
|
bugbot run Please review 069f5a1 for converted tool indexes and late annotations. Regression, mutation, lint, and refreshed real-provider checks pass |
|
@greptileai Please review 4292140. Positionless citations now retain segment ownership; completion timing matches the merge base. Regression, lint, and live checks pass |
|
bugbot run Please review 4292140 for positionless citation preservation and replay consistency. All 434 regressions, full lint, and refreshed live checks pass |
PrimeFoldTools
left a comment
There was a problem hiding this comment.
Rechecked current commit 4292140a after the resumed-reasoning changes.
Independent verification: 409 relevant tests passed. Separately, 120 synthetic reasoning/tool/text combinations passed checks covering item identity, output indexes, completion order, message text, and exact reasoning-block ownership.
The original reasoning → tool → reasoning boundary now produces two distinct reasoning done items, each with its own signed block and each matching the corresponding completed output item. JSON persistence, OpenAI SDK parsing, and replay also continue to pass.
I haven’t independently repeated the live-provider calls, but the local regression coverage no longer shows a blocker in the expanded revision.
Approved from my review scope.
4292140 to
acb1e3a
Compare
|
@greptileai Please review acb1e3a after rebasing onto current main. Incomplete events are retained; 441 regressions, full lint, and fresh provider checks pass |
|
bugbot run Please review acb1e3a for replay preservation after the main rebase and annotation consolidation. Regression, lint, and live checks pass |
|
@greptileai Please review cc233e3. Pending thinking survives empty, usage-only, and metadata-only chunks; 444 regressions, 18 mutations, lint, and refreshed live proof pass |
|
bugbot run Please review cc233e3. The reported unsigned-thinking buffer loss is fixed and covered by regressions, mutation checks, and refreshed live validation |
cc233e3 to
f429906
Compare
|
@greptileai Please review f429906 after resolving upstream helper renames. Replay behavior is unchanged; 446 regressions, lint, 18 mutations and refreshed live proof pass |
|
bugbot run Please review f429906 after the upstream rebase. Public helper names are retained; regression, mutation, lint and real API checks pass |
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit f429906. Configure here.
|
Superseded by #45044, which keeps only the reasoning done-event replay fix in 119 lines. Thanks @PrimeFoldTools for the earlier review. |
TLDR
Problem this solves:
How it solves it:
Intentional product change: Resumed thinking and intervening text now produce separate output items, preserving their chronological order. Clients should retain every completed item by its output_index. Text continues streaming immediately; message completion events wait for final citations
User Flow
Before: a client saving reasoning done items cannot retain all signed thinking from a response that resumes reasoning after a tool
After: the same client retains the replay data for every reasoning phase
Relevant issues
Builds on #44127 and the shared replay encoder from #44132. The provider-side duplicate-text fix is retained
Community review before this rebase: PrimeFoldTools approved 4292140a, reporting 409 passing tests and 120 synthetic reasoning/tool/text combinations. Their recheck covers item identity, output indexes, completion order, message text, exact reasoning-block ownership, JSON persistence, SDK parsing, and replay. They did not repeat live-provider calls; the real API evidence below is from my runs
Rebased onto cb138ba to retain the upstream public helper names, hidden-parameter accessors, incomplete terminal events and request-parameter echo. The replay logic is unchanged, including pending thinking across empty, usage-only and metadata-only chunks. Annotation conversion uses the shared public helper. The approval above refers to the pre-rebase commit
The earlier #43010 duplication report was closed after #44127. #43196 was closed by its author in favor of this PR before the scope changed
Pre-Submission checklist
Local validation: 446 related tests and full make lint passed. All 94 iterator cases pass on this branch. The merge base has 50 failing cases and 44 passing controls. Eighteen targeted mutations were detected, with all iterator controls passing before mutation and after restoration
The regression cases cover resumed visible, signature-only and redacted thinking, ordinary tools, successful and failed web search, tool-first responses, combined result/reasoning chunks, upstream exhaustion, empty-message placeholders, unsigned-only blocks, empty/usage/metadata chunks before a signature, message chronology, mixed chunks, late and positionless citations, and converted code-tool indexes. Saved items are JSON-round-tripped, reasoning items are parsed with the OpenAI SDK, and decoded replay data is checked
Screenshots / Proof of Fix
Fresh requests against real Anthropic-backed proxies, using claude-opus-5-5. Each column below is a separate model response; the comparison checks preservation within that response
Before cb138ba, after f429906
The four client-tool follow-up requests used the saved done items and returned HTTP 200 with 7418. Server-tool cases verify same-response resumption and payload/index consistency; they are not separate tool-result round trips
Reproduce the real requests and inspect their captured output
Run the base checkout on port 44100 and this branch on 44101 with the same locked dependencies. Set ANTHROPIC_API_KEY in the environment and use this proxy.yaml
Generate one temporary proxy key and export it to both proxy processes and the curl shell
Save as client-tool-omitted.json
{ "model": "thinking-model", "input": [ { "role": "user", "content": "Find the smallest positive integer n such that n mod 17 = 5, n mod 19 = 7, and n mod 23 = 11. Work out n before calling the add tool with a=n and b=1. Wait for the tool result, then reply with only the total." } ], "tools": [ { "type": "function", "name": "add", "description": "Return the sum of two integers", "parameters": { "type": "object", "properties": { "a": { "type": "integer" }, "b": { "type": "integer" } }, "required": [ "a", "b" ], "additionalProperties": false } } ], "reasoning": { "effort": "max" }, "include": [ "reasoning.encrypted_content" ], "store": false, "stream": true, "thinking": { "type": "adaptive", "display": "omitted" } }Create client-tool-visible.json
Save as server-tool-omitted.json
{ "model": "thinking-model", "input": [ { "role": "user", "content": "Use web search once to find the latest stable CPython release on python.org. First decide which official source to search, then inspect the returned evidence and explain whether that release is newer than Python 3.12. Keep your final answer under 80 words." } ], "tools": [ { "type": "web_search", "search_context_size": "low" } ], "reasoning": { "effort": "high" }, "include": [ "reasoning.encrypted_content" ], "store": false, "stream": true, "thinking": { "type": "adaptive", "display": "omitted" } }Create server-tool-visible.json
Save as inspect-stream.jq. It prints counts and comparisons without exposing signatures
For client-tool cases, define this helper to collect done items in their output order
Before (cb138ba)
Client-tool, visible thinking
{ "reasoning_done_items": 1, "completed_reasoning_items": 1, "done_replay_blocks": 0, "completed_replay_blocks": 1, "done_payload_matches_completed": false, "done_item_indexes_match_completed": false, "message_chronology_preserved": true, "output_item_types": [ "reasoning", "message", "function_call" ], "server_tool_statuses": [] }Client-tool, omitted thinking
{ "reasoning_done_items": 1, "completed_reasoning_items": 1, "done_replay_blocks": 0, "completed_replay_blocks": 1, "done_payload_matches_completed": false, "done_item_indexes_match_completed": false, "message_chronology_preserved": true, "output_item_types": [ "reasoning", "message", "function_call" ], "server_tool_statuses": [] }Server-tool, visible thinking
{ "reasoning_done_items": 1, "completed_reasoning_items": 1, "done_replay_blocks": 0, "completed_replay_blocks": 2, "done_payload_matches_completed": false, "done_item_indexes_match_completed": true, "message_chronology_preserved": false, "output_item_types": [ "reasoning", "message", "web_search_call" ], "server_tool_statuses": [ "completed" ] }Server-tool, omitted thinking
{ "reasoning_done_items": 1, "completed_reasoning_items": 1, "done_replay_blocks": 0, "completed_replay_blocks": 2, "done_payload_matches_completed": false, "done_item_indexes_match_completed": true, "message_chronology_preserved": true, "output_item_types": [ "reasoning", "message", "web_search_call" ], "server_tool_statuses": [ "completed" ] }After (f429906)
Client-tool, visible thinking
{ "reasoning_done_items": 1, "completed_reasoning_items": 1, "done_replay_blocks": 1, "completed_replay_blocks": 1, "done_payload_matches_completed": true, "done_item_indexes_match_completed": true, "message_chronology_preserved": true, "output_item_types": [ "reasoning", "function_call" ], "server_tool_statuses": [] }Client-tool, omitted thinking
{ "reasoning_done_items": 1, "completed_reasoning_items": 1, "done_replay_blocks": 1, "completed_replay_blocks": 1, "done_payload_matches_completed": true, "done_item_indexes_match_completed": true, "message_chronology_preserved": true, "output_item_types": [ "reasoning", "function_call" ], "server_tool_statuses": [] }Server-tool, visible thinking
{ "reasoning_done_items": 2, "completed_reasoning_items": 2, "done_replay_blocks": 2, "completed_replay_blocks": 2, "done_payload_matches_completed": true, "done_item_indexes_match_completed": true, "message_chronology_preserved": true, "output_item_types": [ "reasoning", "message", "web_search_call", "reasoning", "message" ], "server_tool_statuses": [ "completed" ] }Server-tool, omitted thinking
{ "reasoning_done_items": 2, "completed_reasoning_items": 2, "done_replay_blocks": 2, "completed_replay_blocks": 2, "done_payload_matches_completed": true, "done_item_indexes_match_completed": true, "message_chronology_preserved": true, "output_item_types": [ "reasoning", "message", "web_search_call", "reasoning", "message" ], "server_tool_statuses": [ "completed" ] }Type
Bug Fix
Caveats
Medium
This change handles asynchronous reasoning phases. It does not add synchronous reasoning done events
As in the merge base, message done events wait until upstream exhaustion so providers can deliver final citations. Text deltas remain immediate; clients acting only on message done will see those segments finalized later
Final Attestation