test(integration): refine() round-trip integration tests — closes #106#124
Merged
Conversation
…State persistence Closes #106.
When re_derive_brief returns an identical brief the background task skipped the pipeline but rebuilt noop_state from new_state (which already had plan=None and findings=None), silently losing both fields. Fix: capture original_plan and original_findings in refine() before clearing them, thread them through _run_refinement's signature, and restore them explicitly in the noop branch. Closes #106.
CodeNinjaSarthak
added a commit
that referenced
this pull request
May 5, 2026
#124) * test: integration test for full refine() round-trip with real SessionState persistence Closes #106. * fix: restore plan and findings on no-op refinement path When re_derive_brief returns an identical brief the background task skipped the pipeline but rebuilt noop_state from new_state (which already had plan=None and findings=None), silently losing both fields. Fix: capture original_plan and original_findings in refine() before clearing them, thread them through _run_refinement's signature, and restore them explicitly in the noop branch. Closes #106.
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
1 participant
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
tests/integration/test_refine_roundtrip.pywith 6 integration tests covering the fullSessionManager.refine()lifecycle againstFakeAsyncRedis(no mocks, real state persistence)re_derive_briefreturns an identical brief the background task was restoringstatus="complete"but leavingplan=Noneandfindings=Nonebecausenew_statealready had them cleared; noworiginal_planandoriginal_findingsare captured before clearing and explicitly restored in the noop branchTests added
test_refine_full_pipeline_runs_and_persists_final_statecomplete, plan and cost records persisttest_refine_noop_skips_pipeline_and_restores_completetest_refine_lock_is_released_after_completiontest_refine_rejects_at_iteration_capValueErrorraised wheniteration >= MAX_REFINEMENTStest_refine_rejects_duplicate_of_last_history_answerValueErrorraised for duplicate inputtest_refine_raises_409_when_lock_already_heldRefinementInProgressErrorraised when lock is pre-heldProduction fix (
manager.py)refine()now capturesoriginal_plan/original_findingsbefore nulling them and passes both to_run_refinement. The noop branch restores them so a no-op refinement leaves the session data fully intact.Closes #106.
Test plan
uv run pytest tests/ --no-cov— 375 passed, 3 skipped