DEV Community

Mahiro Hirakawa
Mahiro Hirakawa

Posted on

Two tools disagreed 67 times, and the checker was wrong in all 67

A verifier and the thing it verifies disagreed on 67 rows. My first instinct was that 67 is too many to be the checker's fault.

It was the checker's fault, in all 67.

INDEPENDENT_CLASS rows=67/67  recipe_reader=67  recipe_crate=0
                  row_drift=0  spec_drift=0  legacy_block=0
Enter fullscreen mode Exit fullscreen mode

Before that line existed, the only thing I had was "67 disagreements", which is a count with no direction. It does not say which side moved, and the number is big enough to feel like evidence about the data. Classifying every disagreement by whose recipe produced it turned one alarming number into three boring ones and one real bug.

What the checker was doing

Two defects, both in its private parser.

A nested key clobbered its parent. The config has blocks with an id, and some of those blocks contain sub-blocks that also have an id. The parser kept one variable. Forty-seven times, the inner id overwrote the outer one, so the row it went looking for was not the row it had been asked about.

It looked rows up by a key the other side does not use alone. The authoritative side keys a row by (array, id). The checker keyed by id. Where two arrays both contain an id, it found whichever came first.

[[phases]]
id = "p9"              # the one the caller asked about

  [[phases.audit]]
  id = "a3"            # the parser kept one variable; this won
Enter fullscreen mode Exit fullscreen mode

Either one produces a mismatch that looks exactly like the data being wrong.

The part that made it my own rule, broken

When the parser could not find a block, it folded the result to false.

Not "not found". Not a third value. false, which is the same value it produces for a row that was found and genuinely does not match. The device that exists to enforce "unknown is not the same as no" was collapsing unknown into no, internally, where nothing would see it.

And it printed the disagreement without printing what it had computed. The line said these differ. It did not say mine is X, yours is Y. With both values on the line, forty-seven of those rows would have shown an obviously wrong lookup in the first minute, because the value would have belonged to a different row and anyone reading it would have recognised the wrong row.

what the line said what it should have said
row 12: MISMATCH row 12: mine=a41f… theirs=9c02… source=block[3]
67 of those 47 visibly wrong lookups, 20 to investigate

The fix was to delete a parser, not to fix it

The checker now reads the config through the same single reader everything else uses. It no longer has its own idea of where a block ends or how a row is keyed. On a scratch copy with the shared lookup, the two sides agreed 131 of 131.

-const block = parseLayoutMyself(text).find(b => b.id === id);
-if (!block) return false;
+const block = layoutBlocks.lookup(array, id);      // the one reader
+if (!block) return UNTESTABLE;                     // named, not folded
Enter fullscreen mode Exit fullscreen mode

That is the shape I keep arriving at: a second implementation of a parsing rule is a second place for the rule to be subtly different, and the difference will be discovered as a data problem rather than as a parser problem. One reader, used by everyone, has no disagreement surface at all.

There was a latent twin of the same bug on the other side, incidentally. Its block-bounding function did not stop at the next header, so a row missing an optional field would have borrowed the next block's. Clean today because no row is missing that field. Bounded now with a fixture, because "clean today" is a property of the data, not of the code.

Three rules I would keep

A disagreement without both values is a defect in the reporter. If a tool can tell you two things differ, it computed both. Printing one is choosing to withhold the half that makes the difference diagnosable.

Classify a red before you act on it. Four buckets: device, data, drift, honest. The count per bucket is what tells you where to go. "67 mismatches" sent me toward the data. recipe_reader=67 sent me to the checker in one step.

A verifier that parses the source itself is a second source of truth. Not a check on the first one. The moment it has its own parser, the two can differ for reasons that have nothing to do with what either is measuring, and every one of those differences arrives labelled as a finding.

Top comments (0)