fix(compaction): preserve provider reasoning replay state - #917
Open
breedx wants to merge 2 commits into
Open
Conversation
Contributor
Author
|
The focused fix remains independently testable, but full CI is red on an existing completion assertion: a quoted Details:
No failing checks or warnings were suppressed. The full CI run also reports deprecation/resource/coroutine warnings beyond the locally reproduced stream warnings; I am not claiming that all of those have independent base controls. No merge-ready or all-green CI claim is made. Once the separate completion correction lands, these unchanged fixes can be requalified on the updated base. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Compaction can discard reasoning items that have no visible text but still carry provider replay state. Preserve those items so later requests retain the state they need, while continuing to remove genuinely empty reasoning parts.
Problem
The history cleanup checks only content and signature. An empty
ThinkingPartwith anidorprovider_detailsis therefore treated as disposable, including when it is the only part of a response. Visible text is not a reliable test for whether provider replay state is present.Change
This is a history-cleanup correction, not a new compaction strategy or a change to prompt/retry custody. It does not depend on the open capability refactors; #829 touches the same module and may need to retain this predicate when rebased.
Validation
Tested on Linux / Python 3.13.13 with the unchanged upstream dependency lock (
pydantic-ai-slim2.35.0, harness 0.23.0), based on1d25d696.python -m pytest -q -o addopts= tests/agents/test_reasoning_replay_state.py tests/agents/test_compaction.py tests/test_compaction_reasoning.py: 39 passed.python -m pytest -q -o addopts= tests/agents tests/test_compaction_reasoning.py: 476 passed, 2 warnings.git diff --checkpass for the changed files.Tests ran with disposable HOME/XDG directories, no inherited credentials, and socket connect/DNS/bind blocked. The two unawaited-coroutine warnings come from
test_fire_stream_event_import_errorandtest_fire_stream_event_exception; both reproduce on the unpatched base. They were not suppressed.Compatibility and limits
No dependency, package-version or public API changes. Tests use actual Pydantic message objects; no live-provider replay or end-to-end provider acceptance claim is made.