Uh oh!
There was an error while loading. Please reload this page.
fix(runtime): map failed terminal without error content to runtime_error - #63
Merged
Merged
Conversation
Fresh-eye review (Codex P1 / Claude P3) found that PR #62 only fixed the AgentRunStore ledger failureClass, not the InvocationResult.failure.class that benchmark ResultRecord reads. A complete(stopReason=error) with no preceding error event still surfaced as errorClass='failed' in ResultRecord — indistinguishable from other failures. Root cause: failureFromTerminalEvent returned class=status (i.e. 'failed') for any failed terminal, ignoring whether error content carried a precise reason. The error-content branch (which extracts content.reason) was unreachable for terminal events because the terminal check ran first. Fix: in failureFromTerminalEvent, when status='failed': - with error content reason/code → use that (e.g. 'tool_failed') - without error content → 'runtime_error' (not 'failed') This makes the invocation path consistent with the run ledger, so ResultRecord.errorClass reads 'runtime_error' for the bare-error scenario. Tests: - runtime: updated locked test (failed+error content w/o reason → runtime_error), added reason-code test, added bare-failed-terminal test - headless: added end-to-end test verifying ResultRecord.errorClass='runtime_error' for a backend that emits only complete(stopReason=error)
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 freeto 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.
Summary
Fresh-eye review of PR #62 (Codex P1 / Claude P3) found that the
runtime_errorclassification only reached the AgentRunStore ledger, not the InvocationResult.failure.class that benchmarkResultRecordreads. Acomplete(stopReason=error)with no preceding error event still surfaced aserrorClass='failed'inResultRecord— indistinguishable from other failures.Root cause:
failureFromTerminalEventreturnedclass=status(i.e.'failed') for any failed terminal, ignoring error content. The error-content branch (which extractscontent.reason) was unreachable for terminal events because the terminal check ran first.Fix: in
failureFromTerminalEvent, whenstatus='failed':'tool_failed')'runtime_error'(not'failed')This makes the invocation path consistent with the run ledger, so
ResultRecord.errorClassreads'runtime_error'for the bare-error scenario.Test plan
@maka/runtime588/588 (updated locked test, +2 new: reason-code test, bare-failed-terminal test)@maka/headless93/93 (+1 end-to-end:ResultRecord.errorClass='runtime_error'for backend emitting onlycomplete(stopReason=error))npm run typecheckclean