Skip to content

fix(agent): run pre_verify on every candidate final answer - #68142

Open
trac3r00 wants to merge 1 commit into
NousResearch:mainfrom
trac3r00:fix/pre-verify-all-final-answers
Open

fix(agent): run pre_verify on every candidate final answer#68142
trac3r00 wants to merge 1 commit into
NousResearch:mainfrom
trac3r00:fix/pre-verify-all-final-answers

Conversation

@trac3r00

@trac3r00 trac3r00 commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Summary

  • invoke registered pre_verify hooks for every candidate final answer, not only turns with file edits
  • keep changed_paths as an empty/non-empty payload so code-specific hooks can still scope themselves
  • add regression coverage for non-edit browser-style turns and continuation-budget fallback
  • update hook docs and CLI sample payload

Why

pre_verify is the host's bounded same-turn stop gate, but its file-edit precondition made non-coding recovery policies impossible. A browser/search/communication plugin could detect a premature "cannot do this / you do X" answer, yet its callback was never invoked. This change removes only the invocation precondition; code-specific policies can continue checking coding or changed_paths.

Verification

  • uv run --with pytest --with python-dotenv pytest -q tests/run_agent/test_verification_continuation_budget.py tests/hermes_cli/test_plugins.py tests/hermes_cli/test_hooks_cli.py (136 passed)
  • python3 -m py_compile agent/conversation_loop.py hermes_cli/plugins.py hermes_cli/hooks.py
  • git diff --check

Companion Bob policy PR: https://github.com/trac3r00/bob/pull/411

@trac3r00
trac3r00 force-pushed the fix/pre-verify-all-final-answers branch from 9dd0a90 to 4d5f76c Compare July 20, 2026 16:59
@alt-glitch alt-glitch added type/feature New feature or request comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint comp/cli CLI entry point, hermes_cli/, setup wizard comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have labels Jul 20, 2026
@trac3r00

Copy link
Copy Markdown
Contributor Author

[bob] Status check: PR is mergeable (no conflicts) but blocked because GitHub Actions CI is awaiting maintainer approval to run on this fork-submitted PR (ACTION_REQUIRED on the checkSuite). Local verification is already documented above (136 tests passed via targeted pytest run, py_compile clean, git diff --check clean). No code changes needed on our end -- this just needs a maintainer to approve running the workflow. Following up per our contribution-tracking process; will keep watching for review.

@GottZ GottZ 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.

This was generated by AI during triage.

Summary

Three PRs are associated with this complex: #62657 and #62676 address suppression of candidate responses during verification continuations, while #68142 addresses the separate invocation gap that prevents pre_verify hooks from seeing non-edit candidate answers. #62676 expands #62657 from persistence-only handling to UI emission and replay/finalization semantics; #68142 removes the changed-files precondition while preserving changed_paths for hook-level scoping.

Related pull requests

  • #62657 [closed] related — (+46/-11) — superseded by #62676: Removes the synthetic flag from the candidate assistant response so it survives persistence, but does not emit that response to the UI and therefore only partially fixes the reported suppression.
  • #62676 [closed] related — (+445/-63) — broader closed reference implementation: Extends #62657 with forced UI emission, durable candidate rows, ephemeral user continuations, replay replacement, and exact-once finalization. Despite the keep_open review on #62676, reopening it is not required for #68142: that review concerns continuation protocol and persisted/resume semantics, whereas #68142's diff only removes the changed_paths invocation precondition and relies on the existing continuation machinery.
  • #68142 related — (+56/-21) — merge: Removes the _edited guard so registered pre_verify hooks inspect every candidate final answer, including browser/search/communication turns with no file changes, while retaining empty changed_paths for policy scoping and adding focused tests and documentation.

Duplicates

#62657 and #62676 target the same response-suppression defect; #62676 is the functional superset because it covers both persistence and UI delivery plus replay/finalization behavior. #68142 is not a duplicate.

Suggested consolidation

Merge #68142 because its focused diff closes the non-edit pre_verify invocation gap without duplicating the earlier response-delivery work. Leave #62657 closed as superseded by #62676 and leave #62676 closed as a related broader reference; neither should be reopened or merged as part of #68142, and the keep_open review on #62676 does not apply to #68142's distinct changed_paths gating change.

Cross-PR triage: Reviewed 3 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 46 kB of PR diffs, 6 kB of issue/PR text, 3 kB of discussion (4 comments), 1 verify verdict. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

@teknium1

Copy link
Copy Markdown
Contributor

Thanks for the focused fix. Current main still has the reported invocation gap: agent/conversation_loop.py:6799 requires _edited before invoking a registered pre_verify hook, while the helper already safely forwards an empty changed_paths list (hermes_cli/plugins.py:2298-2333).

Problems

  • website/docs/user-guide/features/hooks.md:396 remains stale after this documentation update: its quick-reference row still says pre_verify fires only when code was edited, contradicting the proposed behavior and the updated detailed section.

Suggested changes

  • Update that quick-reference row to say the hook fires for every candidate final answer, after the code-specific verify-on-stop gate.

This is an automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users labels Jul 30, 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 comp/cli CLI entry point, hermes_cli/, setup wizard comp/plugins Plugin system and bundled plugins P3 Low — cosmetic, nice to have sweeper:blast-contained Sweeper blast radius: contained — one narrow path / opt-in / few users sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants