Three places in our product said the same thing about a report:
One report contains at most one signal per family. A family either fires once, with all its findings collected into a single
evidenceobject, or it does not appear at all.
The public field guide says it. docs/ARCHITECTURE.md restates it. And engine/message.ts relies on it, mapping a family id to a single explanation on the strength of it.
Nothing enforced it. Three families had never obeyed it, on our own checked-in fixtures:
office-doctype.docx office-structure-anomalies [medium] [low] [info]
office-macro-template.docx office-active-content [high] [medium] [medium]
office-injection-markers.docx office-hidden-content [medium] [low]
Three consumers had been built on a promise that nothing was keeping. This is what it cost, what the fix looked like, and the one violation we decided to publish instead of repair.
Three things went wrong at once, and none was visible from inside the family
The score inflated. scoreSignals decays the second and later signals of a given severity, on the assumption that they are different families corroborating each other. That is a good assumption and it is the reason the score is not just a sum. A family emitting three findings therefore collected three independent contributions for one finding. office-macro-template.docx scored 100 where the same evidence now scores 70: high 70 plus medium 25 plus medium 25 is 120, clamped to 100. Two more fixtures moved with it, 100 to 95 and 35 to 30.
Worth noting what did not move: no fixture changes risk band. The fold always keeps the strongest finding, and the band turns on the presence of a severity rather than on how many findings carried it, so high stays high. That is the difference between a scoring bug and a detection bug, and it is why nobody noticed for as long as they did.
One number meant two things. summary.signalCount is what the report headline prints. It counted findings on Office files and families everywhere else. A single API field with two definitions, decided by which engine ran.
The UI repeated itself. The report card keys a signal's brief text and its "What this signal means" link by family. So three signals of one family rendered three identical cards with three identical links.
None of those three failures is visible from inside the family that caused them. They live in the scorer, in the summary, and in the renderer, which is the general shape of an invariant violation: the family that breaks it is never the code that pays.
The fold, which is not a summary
The fix is a shared helper, src/engine/fold.ts, generalised from code one PDF family had used since engine 1.14.0.
- The strongest finding keeps its title and its whole paragraph, so a reader gets the full explanation of the thing that mattered most.
- Every other finding appends one sentence through
notes, a mechanism two families already used. - No evidence is dropped. Every check's evidence object is merged whole.
-
evidence.checksnames which checks fired, so a consumer can branch without reading prose.
That last point is the one I would carry to another codebase. A fold that summarises is a fold that loses data, and the loss shows up months later in a support thread. A fold that merges and then names what it merged costs one array and turns "did the doctype check fire?" into a lookup instead of a string match against a title that copy review is free to rewrite.
Making the merge lossless took two evidence-key renames: two different checks had both been writing parts, so merging them would have silently dropped one. If you are folding evidence objects, the collisions are the work.
One family did not move onto the helper, deliberately. embedded-image-anomalies keeps its own inline copy, because the strings in its evidence.checks are evidence key names rather than check names. That is a shipped shape in a public API, and changing it would have been a breaking change with no defect behind it. Consistency inside the engine is worth less than a stable contract outside it.
The duplicate that no amount of folding could prevent
Folding inside a family fixes a family that emits three. It does nothing about a registry that appends a fourth.
Ours did: a coverage-disclosure family was in the registry's loop and was appended again by the loop's failure path, for any family that threw. So it ran twice on exactly the documents that had gone wrong.
It left the loop. It now runs last, once, and is handed the list of families that threw. Which is what the comment beside it always claimed it did.
The always-on coverage disclosure also repeats in full when it becomes a note rather than being shortened, and that was a choice: an abbreviated statement of what was not looked at is not a disclosure.
The exception we published instead of fixing
One family still emits two signals, knowingly.
redaction-exposure reports a filled box drawn over text (high, conclusive) and a picture drawn over text (info) as separate findings. Merging them was the earlier behaviour and it was a measured bug: one document announced four exposed text runs at high severity when three of them were a letterhead. The two findings also carry the same evidence keys at different scopes, so the fold above would silently drop one set of numbers.
Resolving it properly means either nesting that family's evidence or changing the published sentence, and both are decisions about what the product promises rather than refactors. So the exception is now stated in all three of the places the invariant is stated: on the public page, in the architecture doc, and in the test that enforces it, each carrying the argument for why.
The alternative was leaving a sentence on a public page that the engine disagrees with, which is the failure mode this whole exercise exists to fix. An exception you state is documentation. An exception you leave implicit is a lie with a timestamp on it.
The test, and the two things it does that a normal test would not
test/engine/one-signal-per-family.test.ts:
- It runs all three engines over every fixture in the repository, enumerated from the directory rather than from a list. The defect was in the Office engine, and the test is not a case in the Office suite, because the property is about all three and the one that broke it was the one nobody was looking at. Enumerating the directory is the other half of that: a fixture added tomorrow is checked tomorrow.
- It collects every violation before asserting, rather than stopping at the first. A test that fails on the first violation makes you fix and re-run once per instance, which is how you end up believing you are done.
Each engine also carries a floor ("fewer than 60 PDF fixtures means the sweep stopped seeing them"), because a glob that silently matches nothing is a green test.
It found a fourth violating fixture the review had not, which is how the redaction-exposure exception came to light in the first place. It also pins that summary.signalCount counts families rather than findings, on all three engines, because the invariant and the count are the same promise stated twice.
The other half: a published number needs a derivation, not a reviewer
Two counting bugs turned up next to this one, and they rhyme.
A coverage test had been counting Office families with a regex over the source of a private array. It read the right number on the day it was written. It would have read 7 the moment the disclosure family left that array, which is precisely what this refactor did. It imports an exported constant now. A published number that a refactor can quietly change is the failure this repository keeps rediscovering.
And a family count was baked into a PNG. The social card for the flagship guide said "All ten signal families" while the guide's own heading said eighteen, and that card is the og:image on ten pages. The sweep that checks every count on the site could not see it twice over: the string lives in a build script outside the sweep's roots, and what ships is an image, which no text lint can read. So a test now reads the card generator's source and derives the expected number from the engine's own registry.
The rule under all three: if you publish a number or an invariant, you owe it a derivation. Not a reviewer, not a style note, not a comment. A reviewer does not count families across ninety-six pages, and a comment cannot fail CI.
The invariant, the eighteen families it applies to and the one stated exception are in the fraud signals field guide. If you want to see the report shape the invariant is about, the free checker takes a PDF and gives back the same JSON the API returns, with the evidence under every finding. No account, and nothing is stored.
If you have shipped a guard that turned out to be enforced at the wrong granularity, I would like to read about it. That is the class of bug I now assume I have more of.
Top comments (0)