🐛 fix false-positive error/empty-handoff detection in VerificationController - #3567
Open
yuehuiqi wants to merge 2 commits into
Open
🐛 fix false-positive error/empty-handoff detection in VerificationController#3567yuehuiqi wants to merge 2 commits into
yuehuiqi wants to merge 2 commits into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the SDK’s VerificationController to reduce false-positive “tool error” detection in JSON observations, avoid misclassifying substantive handoffs as empty, and optionally require real execution evidence (tool observations) for execution-demanding tasks before accepting a final answer.
Changes:
- Hardened error-signal detection by adding word boundaries and a JSON-aware
"error": ...heuristic. - Improved empty-handoff detection by allowing structured “substantive” markers (e.g., citations/evidence fields) to override limitation phrases.
- Added an execution-evidence requirement for certain task intents, plus new tests covering the above behaviors.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
sdk/nexent/core/agents/verification.py |
Tightens error/empty-handoff heuristics and adds optional “tool observation required” gating for some final answers. |
test/sdk/core/agents/test_core_agent.py |
Adds regression tests for JSON false-positives, substantive handoffs, and the new execution-evidence gate. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
371
to
+372
| assert result.passed is True | ||
| assert "previous_errors_acknowledged" not in result.failed_criteria | ||
|
|
Comment on lines
70
to
+72
| _ERROR_RE = re.compile( | ||
| r"(traceback|exception|error:|failed|timeout|unauthorized|permission denied)", | ||
| r"(traceback|\bexception\b|\berror\s*:|\bfailed\b|\btimeout\b|\bunauthorized\b|permission denied)", | ||
| re.IGNORECASE, |
Comment on lines
+401
to
+405
| policy = self._build_final_verification_policy(task, memory_summary) | ||
| if policy["tool_result_required"] and not self._has_observed_result(memory_summary): | ||
| missing_result = VerificationResult( | ||
| passed=False, | ||
| severity="blocking", |
…assertion, drop tool_result_required - _ERROR_RE: traceback lacked a word boundary, so it could still false-positive on JSON keys like traceback_info even with a successful value. Added \b. - Restored `assert "previous_errors_acknowledged" not in result.failed_criteria` in test_verification_feedback_does_not_count_as_tool_error -- it was moved to a different test instead of being kept in both, so this test no longer actually verified its own stated purpose. - Dropped _TOOL_RESULT_DEMAND_RE / tool_result_required / _has_observed_result entirely. Copilot correctly pointed out that _build_verification_memory_summary only keeps the last 8 steps (core_agent.py:259), so a run with >8 steps could have its real tool observation fall outside that window and get falsely blocked here. Fixing that properly needs CoreAgent to track "any real observation this run" independently of the (intentionally truncated) LLM prompt digest, which is out of scope for this focused bug-fix PR. Happy to revisit as a follow-up if maintainers want the feature.
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.
Fixes #3564.
What
_ERROR_REpreviously matched bare substrings like "failed"/"error" anywhere in the observation text, including inside JSON field names (failedFileNum,planner_error), regardless of actual value._looks_empty_handoffclassified structured handoffs with real evidence as "empty" just because they also contained a limitation phrase.How
_ERROR_RE: added word boundaries._JSON_ERROR_RE+_contains_error_signal(): only counts"error": ...as real when the value isn't null/none/false/0/empty string._looks_empty_handoff: added asubstantive_markersallow-list (evidence_used,citations,task_id, ...)._TOOL_RESULT_DEMAND_RE+_has_observed_result()for the execution-evidence check described above (feel free to push back on this part specifically if it's too opinionated -- happy to split it out).Tests
Added to
test/sdk/core/agents/test_core_agent.py:All existing tests pass (105 passed).