Skip to content

fix(agent): preserve verify-on-stop attempted final answer - #55361

Closed
DECK6 wants to merge 1 commit into
NousResearch:mainfrom
DECK6:fix/verify-on-stop-preserve-final-answer
Closed

DECK6 wants to merge 1 commit into
NousResearch:mainfrom
DECK6:fix/verify-on-stop-preserve-final-answer

Conversation

@DECK6

@DECK6 DECK6 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix verify-on-stop follow-up handling so the user-facing final response preserves the model's attempted final answer instead of replacing it with only the verification follow-up.

When verify-on-stop triggers, the attempted final answer is kept in conversation history for role alternation, but it was not surfaced if the follow-up produced a verification receipt/blocker. This change stores the pending attempted answer and merges it into the eventual final response unless the follow-up already includes it.

Changes

  • Add _merge_verify_on_stop_final_response() helper.
  • Preserve the first verify-on-stop attempted final answer while the internal verification nudge runs.
  • Merge the attempted final answer with the verification follow-up before appending the final assistant message.
  • Add a regression test exercising the real AIAgent.run_conversation() state machine with a stubbed provider.

Verification

  • scripts/run_tests.sh tests/agent/test_verification_stop.py tests/agent/test_verification_evidence.py -q
    • 65 passed
  • ruff check agent/conversation_loop.py tests/agent/test_verification_stop.py
    • All checks passed
  • git diff --check
    • passed
  • Independent reviewer
    • Approved

Notes

The static added-line scan flags api_key="test-key" in the new test fixture, but this is a fake test value and not a credential.

@alt-glitch alt-glitch added type/bug Something isn't working comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state P3 Low — cosmetic, nice to have labels Jun 30, 2026
@tonydwb

tonydwb commented Jun 30, 2026

Copy link
Copy Markdown

Code Review Summary

Verdict: Approved

Preserves the attempted final answer when verify-on-stop triggers a verification follow-up. The merge function correctly handles four cases: neither present, only one present, attempted embedded in verification, or both present (concatenated with separator).

✅ Looks Good

  • Clean merge logic with good edge case handling
  • Good test coverage: full conversation loop with two model responses
  • Verifies both final_response and messages contain both answers
  • Maintains role alternation invariant

Reviewed by Hermes Agent

@tonydwb tonydwb left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review Summary

Verdict: LGTM

Clean 2-file fix preserving verify-on-stop attempted final answer. When verification stops the agent, the attempted final answer should not be lost.

Looks Good

  • Well-targeted fix for answer preservation
  • Good merge logic (deduplication, fallback to verification)
  • Proper handling of empty/None cases

Reviewed by Hermes Agent

@teknium1

Copy link
Copy Markdown
Collaborator

Thanks for isolating the lost-answer case and adding an end-to-end loop regression.

Problems

  • Current main deliberately requires a genuine later verified report to supersede the pending premature report: tests/run_agent/test_verification_continuation_budget.py:126-145 asserts exactly "verified final report". The proposed helper concatenates every non-embedded pair, so it would change that established behavior as well as cover the receipt/blocker case.
  • The surrounding state machine has since moved to _pending_verification_response (agent/conversation_loop.py:5207-5212) plus provenance-aware fallback handling (agent/turn_finalizer.py:64-82). The new independent pending variable does not integrate with those guards.

Suggested changes

  • Distinguish a verification receipt/blocker from a substantive replacement answer, retaining the existing supersession behavior for the latter; test both cases.
  • Adapt the change to the current pending-response/finalizer flow rather than adding parallel state.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Jul 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P3 Low — cosmetic, nice to have sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-message-delivery Sweeper risk: may drop, duplicate, misroute, or suppress messages sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants