fix(agent): name the main-loop stop trigger in item failure reasons - #855
Open
Fanzzzd wants to merge 1 commit into
Open
fix(agent): name the main-loop stop trigger in item failure reasons#855Fanzzzd wants to merge 1 commit into
Fanzzzd wants to merge 1 commit into
Conversation
StopEmptyRounds and StopCompression previously collapsed into the same "main task stopped before completing" string as StopNone, so a failed item's manifest could not say whether the model spun without usable tool results or the context outgrew its compression threshold. In --format json runs the progress lines that name the trigger are discarded, which made the manifest reason the only diagnostic that leaves a CI runner — and it said the same thing for every stop. Keep the unknown failure class (the taxonomy has no fitting category) but give each stop its own reason, mirroring the StopMaxRounds precedent. Fixes alibaba#842
Contributor
|
✅ OpenCodeReview: Review complete: 0 finding(s) across 1 selected item(s). |
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.
Fixes #842.
When a per-file review stops without completing,
classifyMainLoopStopcollapsedStopEmptyRounds,StopCompressionandStopNoneinto one opaque reason,"main task stopped before completing". The two named stops call for opposite responses (model/tool-loop trouble vs. context budget), and in--format jsonruns the[ocr]progress lines that would say which one fired are discarded bystdout.Quiet()— so on an ephemeral CI runner the manifest reason is the only stop diagnostic that survives, and it said the same thing for every exit. In practice that meant a recurring per-file failure could not be triaged from uploaded artifacts at all (how #842 was found).Change — exactly the two-line shape proposed in the issue, keeping the
FailureUnknownclass untouched:StopEmptyRounds→"stopped after repeated rounds without a usable tool result"StopCompression→"stopped because context compression exceeded its threshold"StopNone(non-completed) keeps the existing catch-all string, andStopMaxRoundsis unchanged.Test —
TestClassifyMainLoopStop(beside the existingTestClassifyItemErrorit mirrors) pins the full stop→(class, reason) mapping and additionally asserts no two stops share a reason string, so a future regression back into a collapsed message fails the table even if the strings are edited.No behavior change beyond the reason text: classes, exit codes and manifest shape are untouched. Verified with
go vetand the fullinternal/agenttest suite.