fix(compact): keep rewritten current-turn messages after compaction - #854
Open
mkoushni wants to merge 6 commits into
Open
fix(compact): keep rewritten current-turn messages after compaction#854mkoushni wants to merge 6 commits into
mkoushni wants to merge 6 commits into
Conversation
…raxis-proxy#661) File resolve and doc extract rewrite the messages tail in place and leave state.input as the original client payload. Rebuild that tail from messages instead of cloning input so resolved file_data and extracted text survive. Signed-off-by: mkoushni <mkoushni@redhat.com>
…-proxy#661) Assert the compact example pipeline keeps resolved file_data and extracted input_text on the current turn, and that the serialized proxy body never restores file_url or file_id. Signed-off-by: mkoushni <mkoushni@redhat.com>
clippy::unused_async fails make lint because the helper never awaits. Signed-off-by: mkoushni <mkoushni@redhat.com>
praxis-bot
reviewed
Sep 1, 2026
praxis-bot
left a comment
praxis-bot
left a comment
Collaborator
There was a problem hiding this comment.
PR Review
Summary: The fix correctly preserves rewritten current-turn messages during compaction by splitting the tail of each message list rather than rebuilding from the immutable state.input. Logic is sound and test coverage is thorough.
Overall: Solid fix. Two convention cleanups needed.
| Severity | Count |
|---|---|
| Critical | 0 |
| Large | 0 |
| Medium | 2 |
Function names and assertion messages already carry the intent; CONTRIBUTING forbids documenting tests with /// or body comments. Signed-off-by: mkoushni <mkoushni@redhat.com>
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
2 participants
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.
Summary
state.input, which file resolve and document extract never update. After afile_urlfetch, that restored the original URL on the outbound body and bypassed resolver SSRF checks. The same path restoredinput_fileafter extraction.replace_messagesnow keeps the already-rewritten tail ofmessages/persisted_messages(laststate.input.len()items) instead of cloningstate.input.file_resolveanddoc_extractbefore compact so that composition is covered end-to-end.Fixes #661
Test plan
cargo test -p praxis-ai-apis --lib compactcargo test -p praxis-tests-integration --test suite compactcargo clippy -p praxis-ai-apis --lib --tests -- -D warningsmake lint