fix(algorithm): close ISC criteria only on a structured worker verdict - #1645
Open
elhoim wants to merge 1 commit into
Open
fix(algorithm): close ISC criteria only on a structured worker verdict#1645elhoim wants to merge 1 commit into
elhoim wants to merge 1 commit into
Conversation
The parallel loop runner decided whether a worker passed its criterion with
a bare substring test on stdout:
stdout.includes(`RESULT: ${cId} PASS`) || stdout.includes(`${cId} PASS`)
The second clause subsumes the first, so the `RESULT:` anchor protected
nothing. Both of these describe a failure and both satisfied it:
RESULT: ISC-3 FAIL: I could not make ISC-3 PASS despite two attempts
Verified whether ISC-3 PASSES: it does not.
The worker's exit code was ignored for pass detection, so a crashed or
killed worker could still have its criterion checked off. Enough falsely
checked criteria drive the ISA to COMPLETE with no re-verification, which
inverts the doctrine that a claim closes only on evidence.
Replace the substring test with a structured, line-anchored contract in a
new IscResult module that both the worker prompt and the parser read from,
so the two ends cannot drift:
ISC_RESULT v1 id=<id> verdict=PASS
ISC_RESULT v1 id=<id> verdict=FAIL reason=<one line>
ISC_REGRESSION v1 id=<id> verdict=PASS|FAIL
A criterion passes only when the worker exited 0, a well-formed PASS line
names it, no FAIL or regression-FAIL line names it, and no sentinel line in
the output failed to parse. Absent, malformed, and contradictory signals all
fail closed. A regression FAIL from any worker vetoes that criterion for the
whole iteration. The prompt states the contract as `verdict=PASS|FAIL`, so
no valid PASS line exists in the prompt for a worker to echo.
Also drop the "fallback detection" comment that had no code beneath it, and
report per-agent verdicts from the parse rather than from the ISA checkbox
state, which could show PASS for a box checked in an earlier iteration.
Tests: bun test test/tools/IscResult.test.ts
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.
The problem
algorithm.tsdecided whether a parallel worker passed its criterion with a bare substring test:The second clause subsumes the first, so the
RESULT:anchor protected nothing. Both of these strings satisfied it while describing a failure:RESULT: ISC-3 FAIL: I could not make ISC-3 PASS despite two attemptsVerified whether ISC-3 PASSES: it does not.The worker's exit code was ignored for pass detection, used only for a display label. The criterion checkbox was then flipped to
[x], and oncepostCriteria.passing >= postCriteria.totalthe ISA status was set to COMPLETE with no re-verification.So a failing worker whose prose happened to mention its own criterion id near the word PASS got that criterion checked off, and enough of those drove the artifact to COMPLETE. For a system whose entire premise is that a claim closes only on evidence, that inverts the doctrine: it reports verification it never performed.
The fix
A structured verdict line, not prose matching. Workers now emit a sentinel-prefixed line:
The contract lives in one place (
IscResult.ts) and is rendered into the worker prompt from the same constants the parser uses, so the two ends cannot drift.Fail closed on anything less than a clean verdict. The judge returns
PASS,FAIL,CRASHED,AMBIGUOUSorNO_SIGNAL, and onlyPASScloses a criterion. Contradictory output — a PASS and a FAIL line for the same id — isAMBIGUOUS, not a pass. A non-zero exit isCRASHEDregardless of what stdout contains, so exit code is a necessary condition for passing.Regression checks parsed with the same rigour, and a regression FAIL from any worker vetoes that criterion for the whole iteration, since one worker's fix can break what another claims to have passed.
Operator-visible distinctions.
NO_SIGNALandAMBIGUOUSare displayed as themselves rather than collapsed into a generic failure, because "the worker said it failed" and "the worker said nothing parseable" call for different responses. The deadfallback detectioncomment that had no code beneath it is gone.Tests
test/tools/IscResult.test.ts, 19 cases:Separately verified with an adversarial probe written to break the parser rather than confirm it. All 19 cases behave correctly:
NO_SIGNALAMBIGUOUSCRASHEDNO_SIGNALid=ISC-30 verdict=PASSdoes not closeISC-3→NO_SIGNALAMBIGUOUSNO_SIGNALNote for review
This changes the worker contract, so a worker still emitting the old
RESULT: <id> PASSshape now yieldsNO_SIGNALand its criterion stays open. That is the intended direction — fail closed rather than open — but it is a behaviour change worth being deliberate about, since an in-flight run started before the upgrade will not close criteria until its workers use the new line.