A detector in my build looks for repeated code shapes. It caught a ten-line window shared between two checks that I had written deliberately, days apart, believing them to be different.
FAIL_SCAFFOLD dup=1 window=10 lines faces=verdict, term
They were not different. They were the same shape with different words in it.
Why a text diff would have missed it
The detector does not compare text. It erases string literals and normalises accessor calls to a single form before comparing:
// as written, in two files, months apart
const kind = value(row, "verdict_kind");
const unit = value(row, "verdict_unit");
const kind = value(row, "term_kind");
const unit = value(row, "term_unit");
// what the detector compares
const _ = value(_, "");
const _ = value(_, "");
Every identifier that made these look like two jobs is exactly the part that gets erased. What remains is the shape, and the shape was identical for ten consecutive lines.
That is the whole value of the thing. Duplication that survives a rename is invisible to grep and to review, because the reviewer reads the words.
Two ways to make a detector stop complaining
| response | what it changes | what it costs |
|---|---|---|
| add an exception for these two files | the rule | every future duplication in them is now unreported |
| raise the window from 10 to 11 lines | the rule | every 10-line duplication anywhere is now unreported |
| remove the duplication | the code | the work of the refactor, once |
The first two are one keystroke and they are how a detector dies. Not in one dramatic decision, but as a list of exceptions each of which was reasonable at the time.
The rule here is a copy ban that applies inside our own tree. It had just caught our own newest control, which is precisely the case where a rule is most tempting to bend and most worth keeping.
The fix, and the control that made it safe
The four repeated cells are now declared once and read through one map, so the per-cell lines do not exist to be duplicated:
before: two files, four near-identical cell blocks each
after: one declaration, one reader
OK_SCAFFOLD faces=8/8 dup=0
scaffold tests 67/67
dup=0 says the duplication is gone. It does not say the behaviour survived, and a deduplication refactor is exactly the kind of change that quietly alters an output while looking tidy.
So the load-bearing check is the other one: every verdict line the build emits is byte-identical to before the refactor.
OK_ALL controls=24 all prints byte-identical to the pre-refactor run
That pair is what makes the change safe to keep. dup=0 alone would be satisfied by a refactor that broke something; byte-identical output alone would be satisfied by doing nothing. Together they say the code changed and the behaviour did not.
What I would take from this
When a rule catches you, the first question is whether the rule is wrong, and the honest answer is usually no. I wrote the detector, I wrote the thing it caught, and my first instinct was still to look for the exception.
Normalise before comparing, if you want to find duplication that matters. The duplication people actually ship is not copy-paste; it is the same structure written twice with local names. A comparison that keeps the names cannot see it.
A dedup refactor needs a behaviour-preservation control, not a duplication count. The count tells you the smell is gone. Only identical output tells you the meaning is.
Top comments (0)