Declare local_phash's skip reasons at source (authorised protected-core exception)

[WilfordGrimley]

Stacked on #567. Base is feat/skip-reason-declaration-convention, not master — see "Why this is stacked" below. Review/merge after #567.

The gap

local_phash.find_best_match returned two skip reasons as bare inline string literals: "no-hashable-candidates" and "no-clear-winner". #567's roster tether derives the roster by scanning for module-level *_SKIP_REASON = "<literal>" declarations, and it cannot enumerate literals it cannot see — so a NEW literal added inside find_best_match would reach CardScanLog.skip_reason (~2.7M rows, no choices list, no FK) with no lint failure anywhere.

#567 could not close that, because MPCAutofill/cardpicker/local_phash.py is PROTECTED CORE (docs/upstreaming/license-provenance.md §2). It mirrored the constants in the consuming module and documented the residual gap instead.

Owner ruling 2026-07-29: exception granted, with the added requirement that it "hold up to audit".

What this does

1. Declares at source. PHASH_NO_HASHABLE_CANDIDATES_SKIP_REASON / PHASH_NO_CLEAR_WINNER_SKIP_REASON are declared in local_phash.py, exported in __all__, and returned by name.
2. Removes the mirror. The two declarations in local_identify_printing_tags.py are gone; that module imports the one constant it uses. One declaration per value, nothing that can drift. Verified nothing else mirrored them.
3. Tether covers it automatically — _declared_skip_reasons() globs cardpicker/*.py, so it picks the new site up with no linter change. Both values were already documented; the doc entries that named the mirror's location are corrected to the real one.
4. Records the exception where the policy lives — new §2.1 Authorised exceptions — the log in license-provenance.md, with the date, the change, the reasoning, the authorisation, the proof of zero effect, and an explicit scope statement: it permits declaring skip-reason constants in this one file. It is not a licence to edit protected core, not a licence for the other listed files, and not a standing "lint-driven refactors are exempt" rule. The next change of any shape needs its own ruling and its own entry. local_phash.py stays on the list in §2 and in PROTECTED_CORE_FILES.
5. Guards the hole shut. The tether is structurally blind to the two regressions that would undo this, so test_skip_reason_roster.py gains two tests: one fails if any other cardpicker module re-declares either value (mirror returns), one fails if find_best_match returns a bare literal again. Both mutation-checked.

Verification

Byte-identical, proved not asserted. Three independent static proofs:

• A — AST equality. Taking the after-source, deleting the two new declarations and their __all__ entries, and inlining every load of those names back to its string literal produces a parse tree identical to the before-source. (Docstrings stripped from both: the docstring text deliberately changed to name the constants; it is prose, not executable data.)
• B — return-value enumeration. The sequence of strings find_best_match can return, resolved statically through the module's constant table, is unchanged: ['no-hashable-candidates', 'no-clear-winner', 'no-clear-winner', ''] before and after.
• C — one declaration. Each value: declared in the consumer before, absent from the consumer after, declared in local_phash with the same string.

Proof A also holds against origin/master directly — local_phash.py is byte-identical on master and on #567's branch, so the pre-change baseline is the same file either way.

The constant NAMES were kept identical too, not just the values. That is why test_skip_reason_roster.py's hand-written EXPECTED_SKIP_REASONS pin and the doc's Constant column needed no edit at all — nothing about the roster moved except the declaration site.

Roster tether — passes, and proved it can still fail. With the doc entry for each value stripped in turn, the tether errors and names the new site:

::error file=docs/reference/skip-reasons.md::skip-reason roster drift: reason
`no-hashable-candidates` is declared in code (MPCAutofill/cardpicker/local_phash.py:93
(PHASH_NO_HASHABLE_CANDIDATES_SKIP_REASON)) but has no entry in docs/reference/skip-reasons.md...

::error file=docs/reference/skip-reasons.md::skip-reason roster drift: reason
`no-clear-winner` is declared in code (MPCAutofill/cardpicker/local_phash.py:94
(PHASH_NO_CLEAR_WINNER_SKIP_REASON)) but has no entry in docs/reference/skip-reasons.md...

Restored → clean. Full docs_lint.py: docs-lint: clean.

check_protected_core_license.py: protected-core-license: clean (9 files checked).

Tests: cardpicker/tests/ — 2984 passed, 9 skipped. test_catalog_stats.py is excluded from that count: 5 of its tests fail with ValueError: Missing staticfiles manifest entry for 'cardpicker/favicon.ico', identically on the unmodified baseline (verified by stashing) — a local collectstatic gap, not this change.

Also clean: ruff, isort, black, mypy (all pre-commit hooks passed on commit).

Item 5 — same defect elsewhere in protected core? Reported only, not fixed

Each of these would need its own ruling; that is the point of the policy.

File	Roster-relevant literals the tether cannot see
vote_consensus.py	None. Its one identity literal is bound to DEDUCTIVE_BACKFILL_ANONYMOUS_ID.
printing_consensus.py	None.
tag_consensus.py	None.
artist_consensus.py	None.
federation-hash-tool/hash_my_cards.py	None.
local_fallback.py	Three. run_fallback_for_card returns FallbackOutcome(skip_reason=...) with bare literals "no-evidence" (L687), "eliminated" (L690), "ambiguous" (L694).

On local_fallback.py: none of the three reaches CardScanLog today. Its only non-test caller is local_residual_classify.recover_frame_mismatch_printing_via_fallback_refetch, which reads outcome.printing_pk and discards skip_reason; the module's own write branch was retired on 2026-07-29 and the local-fallback-v1 rows in production are historical. So this is a latent gap, not a live one — but it is the same shape as the one this PR closes, and it would become live the moment anything persists that outcome. Recommend a separate ruling rather than folding it in here. (Its FALLBACK_ANONYMOUS_ID is properly declared; the calculator-identity side of protected core is clean across all six files.)

Incidental, out of scope, not protected core: local_residual_classify.py:135 filters CardScanLog.objects.filter(skip_reason="frame-mismatch") on a bare literal — a read-side coupling to a value declared elsewhere, not a roster-tether gap.

Why this is stacked on #567 rather than based on master

The instruction said to base on master if #567 had not landed. It has not (open as of writing), and basing on master would have made the work unverifiable and items 2–3 vacuous: the mirror to remove, the tether to satisfy, and the doc entries to correct all originate in #567 and none exist on master. Basing there would have produced a PR that declares constants with nothing to check them against. Stacking keeps the deliverable reviewable as one coherent change; the diff against #567 is exactly the five files listed. If #567 is reworked, this rebases onto it.

🤖 Generated with Claude Code

https://claude.ai/code/session_013NhYmT1PxCcyemA16dFDxN

[WilfordGrimley]

Rebased onto the rebased #567

Rebased from the old #567 head (724c513f) onto the new one (3df13228). No conflicts. Base deliberately left as feat/skip-reason-declaration-convention — the mirror this PR removes and the tether it satisfies both originate in #567, so the stacking is kept, not retargeted to master.

Checked that #567's rebase edits and this PR's edits to the same two files still cohere:

• docs/reference/skip-reasons.md — Adopt a *_SKIP_REASON declaration convention; tether the roster to a doc #567's retired multi-faced-v1 row survived intact alongside this PR's protected-core section.
• test_skip_reason_roster.py — this PR is purely additive here (no removed lines), so Adopt a *_SKIP_REASON declaration convention; tether the roster to a doc #567's roster-map edit is untouched.

Verification

• Byte-identical proof run against Adopt a *_SKIP_REASON declaration convention; tether the roster to a doc #567's head as the base: local_phash.py and local_identify_printing_tags.py are IDENTICAL after inlining. Declaring the protected-core reasons at source changed no string value.
• cardpicker/tests/: 3150 passed, 11 skipped, 0 failed (with Linearise the cardpicker migration graph: two 0096 leaf nodes block every deploy #576 layered — the suite cannot build a test database on unmodified master while the two-0096 conflict stands).
• ruff, isort, black, mypy, prettier, docs-lint all clean.

[WilfordGrimley]

Closing as fully subsumed by #584.

#584 carried both commits and merged alone, so this branch's content reached master through #584's squash. Verified against origin/master (d74b84e), comparing this branch's head 25a24f5 line-by-line:

• local_phash.py — byte-identical between this branch and master (same blob).
• local_identify_printing_tags.py, tests/test_skip_reason_roster.py, docs/upstreaming/license-provenance.md — every line this branch adds over its merge base is present on master verbatim; master carries further changes on top (the 2026-07-29 printings_count correction, the LOCAL_FALLBACK_* roster entries, the local_fallback.py exception log entry).
• docs/reference/skip-reasons.md — this branch's version is a strict subset: 322 lines here vs 397 on master, all substance present. The only three added lines that do not match verbatim are the same statements re-wrapped and re-worded by Declare local_fallback's skip reasons at source (second authorised protected-core exception) #584 (master lines 150 and 338-339).

Merging would at best be a no-op and at worst reintroduce the smaller skip-reasons.md. GitHub already reports this branch as CONFLICTING for that reason. Branch left in place.