Three fixtures, three separate calls into the same reducer. Here is the complete
failure_reasons each one returned, unedited:
unreadable arriving args -> ["SANDBOX_EVENT_CARDINALITY_INVALID",
"EXEC_ARGUMENTS_MISMATCH",
"TOOL_RESPONSE_CARDINALITY_INVALID"]
usable args, different -> ["SANDBOX_EVENT_CARDINALITY_INVALID",
"EXEC_ARGUMENTS_MISMATCH",
"TOOL_RESPONSE_CARDINALITY_INVALID"]
our comparison threw -> ["SANDBOX_EVENT_CARDINALITY_INVALID",
"EXEC_ARGUMENTS_MISMATCH",
"TOOL_RESPONSE_CARDINALITY_INVALID"]
These are minimal fixtures with no sandbox event and no tool response, so the first and last
codes fire in all three and are expected. I am printing them anyway. A post about a receipt that
hides which party failed has no business showing you a cleaned-up receipt.
The middle line is the one that matters, and across three genuinely different failures it never
changes.
Fixture one sends arguments the parser rejects.
Fixture two sends a usable call that disagrees with what I froze.
Fixture three sends an object my own canonicalizer rejects, so the comparison never completes.
Constructed inputs, so none of this establishes who caused a failure in production. But one name
covers all three, and that name says argument mismatch even when nothing was compared. A failure
in the checking stage reads as a deviation in the thing being checked.
The reason it reads that way
pm25coder put it in one line, in the comments of the schema-comparison
piece
(permalink to the comment):
EXEC_ARGUMENTS_MISMATCHmisreads as a verdict on the model precisely because its name carries no
subject.
Three different observations arrive under one name: arguments that were rejected, a comparison that
completed and found a difference, and a comparison that never finished.
The label is not wrong that something happened. It is silent about which of the three, and a reader
fills that in.
What the code actually did
One try was wrapping three different jobs:
if (call) {
try {
const actual = parseStrictJson(call.function.arguments); // can the arriving args be read
if (typeof actual?.command === 'string' && Buffer.byteLength(actual.command, 'utf8') > 256) {
failures.add('EXEC_COMMAND_OVERSIZE');
} else if (!canonicalJsonBytes(actual) // does the comparison work
.equals(canonicalJsonBytes(prepared.expectedExecArguments))) {
failures.add('EXEC_ARGUMENTS_MISMATCH'); // do they differ
}
} catch {
failures.add('EXEC_ARGUMENTS_MISMATCH'); // ...everything lands here
}
}
parseStrictJson throws when what arrived is unreadable. canonicalJsonBytes throws when the
comparison itself cannot run. Both fell into the same catch, and the catch named the arguments.
Full file if you want to read around it:
scripts/judgment/live.mjs.
The third catch site was in
scripts/pr2/reducer.mjs.
The fix
Split the parse from the compare, and give each stage its own catch:
let actual;
try {
actual = parseStrictJson(call.function.arguments);
} catch {
failures.add('EXEC_ARGUMENTS_INVALID');
}
if (actual !== undefined) {
try {
if (typeof actual?.command === 'string' && Buffer.byteLength(actual.command, 'utf8') > 256) {
failures.add('EXEC_COMMAND_OVERSIZE');
} else if (!canonicalJsonBytes(actual)
.equals(canonicalJsonBytes(prepared.expectedExecArguments))) {
failures.add('EXEC_ARGUMENTS_MISMATCH');
}
} catch {
failures.add('EXEC_COMPARATOR_ERROR');
}
}
Same three fixtures, same unedited arrays, only the middle line moves:
unreadable arriving args -> ["SANDBOX_EVENT_CARDINALITY_INVALID",
"EXEC_ARGUMENTS_INVALID",
"TOOL_RESPONSE_CARDINALITY_INVALID"]
usable args, different -> ["SANDBOX_EVENT_CARDINALITY_INVALID",
"EXEC_ARGUMENTS_MISMATCH",
"TOOL_RESPONSE_CARDINALITY_INVALID"]
our comparison threw -> ["SANDBOX_EVENT_CARDINALITY_INVALID",
"EXEC_COMPARATOR_ERROR",
"TOOL_RESPONSE_CARDINALITY_INVALID"]
The change spans three source files, plus the tests:
dd1a654.
The part that was not a rename
pm25coder called it "a rename, not a redesign" — his words, verbatim, in
3eanf — and from outside the repo that is exactly what it
looks like. Inside, two things were waiting.
There are two whitelists, not one. Failure reasons are filtered through an ordered array before
they reach the output. In live.mjs, a reason that is not in the array is silently dropped. Register
the new name in the code and not in the array, and that failure stops appearing. Other failures in the same run still show, so the run
does not go green by itself, but the one you just added becomes invisible.
There are two registries for this namespace and they fail differently.
live.mjs holds 26 codes and emits output by filtering against the list — FAILURE_ORDER.filter(...)
at lines 169 and 572, no guard. An unknown reason is silently omitted. scripts/pr2/constants.mjs
holds 20, a strict subset, and its consumer refuses to guess:
if (!FAILURE_ORDER.includes(reason)) throw new TypeError(`unknown failure reason: ${reason}`);
So the narrower list is the strict one, and the list covering more surface is the one that fails
quietly. That is backwards, and it is a worse defect than the naming problem this patch fixes: a
filter that discards unrecognised codes is designed to fail invisibly.
The throwing version is the correct behaviour. The silent filter should be replaced by it, and the
two lists should be one registry. I did not do that here, because bundling a registry refactor into a
naming fix would make both harder to review and would put a behaviour change in a commit that
claims to be about labels. It is on the list as its own change.
And there was a third catch site doing the same collapse in a different module, which I only
found by grepping for every place that name was added rather than trusting the two I knew about.
The tests, including the one that is supposed to pass
Six of them. That they pass is not the interesting part. Run them against the parent commit and five
of the six fail. Run against the patch, all six pass. The one that passes both ways is there on purpose:
One correction to the commit message before you click it. It says three of the six tests fail
on the parent. That line is stale: it was written when the file had four tests, and two more were
added in the amendment. The real number is five, and the ablation below is the thing to trust. I am
not force-pushing a rewrite of a public SHA to tidy a sentence, so the contradiction stays visible
and this paragraph is the correction.
All six are in one file, if you want to run the ablation yourself:
test/exec-comparator-error.test.mjs.
arriving args unusable -> EXEC_ARGUMENTS_INVALID fails on parent
usable args, differ -> EXEC_ARGUMENTS_MISMATCH passes on both <- control
comparison cannot complete -> EXEC_COMPARATOR_ERROR fails on parent
pr2 reducer, unusable args -> EXEC_ARGUMENTS_INVALID fails on parent
prepared transport, digest -> EXEC_COMPARATOR_ERROR fails on parent
pr2 reducer, digest read -> EXEC_COMPARATOR_ERROR fails on parent
The control is doing something specific. Splitting one catch into two created two new boundaries
that the mismatch path now has to survive. If either boundary swallowed a case it should have passed
through, a suite that only asserted the two new names would still be green, because the case it ate
would simply never be asserted. The control fails the moment the original path stops producing the
original name.
The comparator failures are induced, not waited for. The arriving call parses cleanly and my own
expected object carries a BigInt the canonicalizer refuses.
That refusal is deliberate, not fragile. The serializer accepts a closed set — null, boolean, safe
integer, NFC string, array, plain object — and rejects everything else, because its output is
hashed and a canonical form cannot have alternatives. So a BigInt in an expected object is an
invalid internal type, and the test is contrived at the type level.
Be precise about what that buys: it proves the catch fires when the comparison cannot complete on
otherwise-valid input. It does not prove a spontaneous bug in the canonicalizer, and I am not
claiming one. The two digest-read tests induce a
different internal failure using a throwing getter, and each one asserts the intended read was
actually reached and that the error survives the final filter.
Two things I got wrong on the way, both caught by someone else
My first repair ran the collapse backwards. I moved the whole catch to
EXEC_COMPARATOR_ERROR, which meant unreadable arriving args were now reported as my machinery
failing. Same defect, opposite direction.
The reason it survived my own review is worth more than the bug. I wrote the implementation, then
wrote a test asserting what the implementation did. An assertion written against unverified output
cannot fail, because it was derived from the thing it is supposed to check. That workflow
guarantees you codify your own bugs, and it produced a test that defended the error.
Then I claimed one of the three catches was dead code. I had constructed a bad expected object,
watched it get caught by an earlier gate, and concluded nothing could reach that catch. One input
class, generalized to all inputs. It is reachable — a failing read on the manifest digest lands
there, because that read sits inside the try while the one I tested sits outside it.
Neither was caught by me. The backwards repair was caught before commit. The dead-code claim went
into the first commit and came out in an amendment, so it was caught before push.
What this does not do
It does not put the contract in the name. pm25coder's fuller point was that the label should
say what it was compared against, something closer to args_mismatch_under_contract=<id>, so the
name states the authority rather than leaving a reader to infer a subject. That is not built. The
names are separated. They still carry no contract id.
EXEC_ARGUMENTS_INVALID does not identify who produced bad arguments. It marks a boundary.
The arriving arguments were rejected, either by the parser or, on the PR2 path, by argument
validation. Valid JSON can still fail that validation. Who produced them is not established by
that result. The model, the relay, or the transport are all still live possibilities, and the name
stops where the evidence stops.
Naming a producer there would be the same defect with a friendlier label.
That neutrality costs something real, and it is fair to say so. An operator wants to know if the
model is emitting garbage, and unparseable JSON on a structured tool call may well be exactly that.
I have not measured how often it is, so I am not putting a frequency on it.
The code declines to tell them, which is semantically clean and operationally thinner. The fix is not to guess in the name. It is to preserve enough provenance that attribution can be
made afterward: what arguments were observed at the model-event boundary, whether any relay or
transport transformation happened in between, and which contract governed the call.
The contract-id work above would settle the authority half of that. It would not identify who
corrupted an invalid argument stream — a contract id says which expectation was in force, not where
bad bytes came from. I conflated those two in an earlier draft. Neither half is built.
EXEC_COMPARATOR_ERROR names the stage, not the cause. An un-canonicalizable expectation and a
genuine comparator bug are both on my side and are not distinguishable from the outside. I declined
to split them, because inventing a distinction the code cannot detect is the defect I was fixing.
The general shape, if you want to check your own
Find every place your system writes a failure name, and ask what the name is a statement about.
If it names a thing rather than a party — arguments, response, payload, schema — check what else
falls into the same branch. A name that describes the object can be read as a verdict on whoever produced it. Trace each error
from the operation that raised it all the way to the receipt, and check whether the name it arrives
under still tells the truth.
The question that found this one: when this fires, whose fault does a reader assume it is, and is
that always true?
If you want to check the claim rather than take it:
git clone https://github.com/keniel13-ui/self-correcting-integration-maintainer
cd self-correcting-integration-maintainer
git checkout dd1a654
node --test test/exec-comparator-error.test.mjs # 6 pass
git checkout dd1a654~1 -- scripts/judgment/live.mjs \
scripts/pr2/constants.mjs scripts/pr2/reducer.mjs
node --test test/exec-comparator-error.test.mjs # 5 fail, 1 passes
That last run is the one worth doing. The test that keeps passing is the control.
If you have one of these in your own harness, I would genuinely like to see it. Different domains,
same shape.
Top comments (0)