Skip to content

fix(coverage-report): check_canonicalizer named two codes nothing emits - #10

Merged
lywinged merged 1 commit into
mainfrom
fix/check-canonicalizer-dead-codes
Aug 27, 2026
Merged

lywinged merged 1 commit into
mainfrom
fix/check-canonicalizer-dead-codes

Conversation

@lywinged

Copy link
Copy Markdown
Owner

The four problems this script has been reporting are not defects in the corpus. The script was wrong, in two ways that compounded so that fixing either alone would have changed nothing.

What it reported

[signatures] 53 verified through the naive path, 4 correctly refused (negative vectors)
[signatures] 4 PROBLEM(S):
  14-receipt-issuer-key-unknown.json [receipt]: signing key is not pinned, but no key or signature failure is expected
  23-receipt-issuer-key-case-variant.json [receipt]: ...
  07-gap-disclosure-unknown-key.json [gap_disclosure]: ...
  09-gap-disclosure-key-case-variant.json [gap_disclosure]: ...

All four vectors are correctly written. Each declares failures: [], a warnings entry naming the unknown key, and a *_unverified status.

Two faults, and neither is sufficient on its own

The set named codes nothing emits. deliberately_bad listed issuer_key_untrusted and disclosure_key_untrusted. Neither appears anywhere under src/, tests/, examples/ or schema/:

issuer_key_untrusted         0 files
disclosure_key_untrusted     0 files
issuer_key_unknown           8 files
disclosure_key_unknown       8 files

They were renamed to *_unknown and the script kept the old spelling. For as long as that stood the two negative classes those names identify were invisible, and every vector carrying one read as a defect.

The match read only failures. Both renamed codes are registered warning, not failure, and the match ran against expected["failures"] alone. Correcting the spelling without this would still have matched nothing.

Which severity the verifier assigns is orthogonal to why this check cares. The question here is whether the vector meant the key to be unresolvable, not what the verdict does about it. Membership of deliberately_bad is what keeps the match narrow; reading only failures is what made it wrong.

before after
verified through the naive path 53 53
correctly refused (negatives) 4 8
reported as problems 4 0
exit 1 0

The differential half was never affected and still compares 2004 JSON values byte for byte through both canonicalizers.

The guard, and why it needs a test to mean anything

_reconcile_against_the_registry refuses to run when a code in the set is one the verifier's RULES registry does not carry, and names it. A set entry that nothing emits does not fail loudly by itself: the match simply never fires, which is how this survived a rename.

That guard is inert unless something runs the script, and nothing did. This is the same root cause as the stale figures in #8, in the same directory: coverage-report/scripts/ was referenced by no test and no workflow, so a script run by hand once keeps reporting what it said that day.

tests/test_canonicalizer_check_runs.py runs it and asserts the exit status, with lower bounds on values compared and signatures verified so an empty run cannot pass silently. It falls back to returning the set unchecked when the registry cannot be imported, since the script is also run against trees that predate the registry, and a reconciliation that cannot run is not a reason to refuse the rest.

Load-bearing

probe result
revert one code to its dead spelling script exits 2 naming it; both new tests fail
read failures only, as before script exits 1 with the original four problems

1033 passed, 1 skipped.

Two em dashes and one 106-character line in the script are pre-existing; the diff introduces neither. They belong to the separate sweep of the 142 dash occurrences remaining in files this fork does not share with upstream.


Generated by Claude Code

The script reported four vectors as defects:

    14-receipt-issuer-key-unknown.json [receipt]
    23-receipt-issuer-key-case-variant.json [receipt]
    07-gap-disclosure-unknown-key.json [gap_disclosure]
    09-gap-disclosure-key-case-variant.json [gap_disclosure]

    signing key is not pinned, but no key or signature failure is expected

All four are correctly written. The script was wrong, in two ways that compounded.

Its `deliberately_bad` set listed `issuer_key_untrusted` and
`disclosure_key_untrusted`. Neither name appears anywhere under `src/`, `tests/`,
`examples/` or `schema/`: they were renamed to `issuer_key_unknown` and
`disclosure_key_unknown`, and the script kept the old spelling. For as long as that
stood, the two negative classes those names identify were invisible, and every
vector carrying them read as a defect.

The renamed codes are also registered `warning` rather than `failure`, and the
match ran against `expected["failures"]` alone, so correcting the spelling by
itself would have changed nothing. Which severity the verifier assigns is
orthogonal to why this check cares: the question here is whether the vector meant
the key to be unresolvable, not what the verdict does about it. Membership of
`deliberately_bad` is what keeps the match narrow.

After both: 53 verified through the naive path and 8 correctly refused, where it
was 53 verified, 4 refused and 4 reported. The differential half was never
affected and still compares 2004 values byte for byte.

`_reconcile_against_the_registry` now refuses to run when a code in the set is one
the verifier does not register, naming it. A set entry that nothing emits does not
fail loudly on its own, which is how this survived a rename.

That guard is inert unless something runs the script, and nothing did, which is the
same root cause as the stale figures in REPORT.md. `tests/test_canonicalizer_check_runs.py`
runs it and asserts the exit status, plus lower bounds on the values compared and
the signatures verified so an empty run cannot pass. Reverting one code to its dead
spelling fails both of them.

1033 passed, 1 skipped. Two em dashes and one long line in the script are
pre-existing; nothing added here introduces either.

Signed-off-by: Louielunz <48041247+lywinged@users.noreply.github.com>
@github-actions

github-actions Bot commented Aug 27, 2026 •

Copy link
Copy Markdown

❔ Contributor Check: UNKNOWN

Check Result
Profile UNKNOWN
Credential LOW
Overall UNKNOWN

Automated check by AgenTrust Contributor Check.

@github-actions github-actions Bot added the needs-review:UNKNOWN Contributor check flagged UNKNOWN risk label Aug 27, 2026
@lywinged
lywinged merged commit 3709369 into main Aug 27, 2026
4 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-review:UNKNOWN Contributor check flagged UNKNOWN risk

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant