Bind the classifier-equivalence invariant and close the invalid-JSON target arm (deferred #1044 review suggestions)

[The01Geek]

Follow-up to PR #1044 (issue #1006), which re-keyed pin retirement on site identity. Its review graded these 🟡 Suggestion and they were not merge-blocking; captured here rather than dropped. Both are about future drift, not current correctness — the change's own negative control (a literal retired at its own site is still reported) was verified before merge and covers today's behavior.

1. The classifier-equivalence coupled invariant has no binding test

test_resolved_target_token_matches_the_classifier_three_shapes asserts _resolved_target_token / _site_retirement_key against hardcoded literals ("docs/x.md", "/__pin_corpus_runtime__/CI_BUNDLE") rather than against what the classifier actually produces. So the two are only coincidentally in agreement: a change to the classifier's target resolution would leave this test green while retirement keying silently diverged from it.

This is the repo's own coupled-invariant hazard — two sites that must agree, with the test asserting one side's literal instead of binding them. The fix is to derive the expected token from the classifier in the test, so a divergence cannot pass.

Why it matters more here than usual: retirement keying is a security-adjacent gate. A silent divergence between "how a target is classified" and "how a retirement is keyed" reopens exactly the class of mismatch #1006 existed to close — a pin poisoned or spared by the wrong key — and it would present as an unsatisfiable finding with no exit, the original symptom.

2. Fail-closed coverage gap: invalid-JSON resolved_target cell

test_malformed_retirement_manifest_site_fields_fail_closed exercises the source-side JSONDecodeError and the non-string source/target arms, but not an invalid-JSON resolved_target cell — the json.loads(row[target_index]) decode-error arm. Bundle-target retirements are the rows that carry a non-trivial target, so this is the arm most likely to meet a malformed cell in practice.

3. Informational — docstring nit

_repo_relative_or_none's docstring calls itself "the one copy of the abspath/commonpath/relpath computation the file otherwise repeats" while the same sentence points at a genuine second, raising copy (_relative_target_path). Behavior-inert; the accurate phrasing is "the non-raising copy".

Acceptance Criteria

• The classifier-equivalence test derives its expected token from the classifier rather than from hardcoded literals, so a divergence between target classification and retirement keying turns the test RED. Prove it is binding: change the classifier's resolution and confirm the test fails.
• The invalid-JSON resolved_target decode-error arm is exercised and asserted to fail closed.
• Neither change re-adjudicates any individual pin (out of scope, per Pin retirement is keyed by literal while revival authorization is keyed by site #1006) and neither weakens the own-site negative control — a literal retired at its own site must still be reported, with that assertion still discriminating.
• The docstring is corrected.
• lib/test/run.sh reports zero failures and zero skips.

[The01Geek]

Audited before dispatch. PR #1044 merged, so the keying change these gaps sit on is now on main.

The first item is the one that matters, and it is security-adjacent. test_resolved_target_token_matches_the_classifier_three_shapes asserts _resolved_target_token / _site_retirement_key against hardcoded literals rather than against what the classifier actually produces — so the two agree only coincidentally. A change to the classifier's target resolution would leave the test green while retirement keying silently diverged from it, which reopens exactly the mismatch #1006 existed to close: a pin poisoned or spared by the wrong key, presenting as an unsatisfiable gate finding with no exit.

Derive the expected token from the classifier so a divergence cannot pass, and prove it binds: change the classifier's resolution and confirm the test goes RED. A test that still passes under that mutation has not closed the gap.

Do not weaken the own-site negative control. #1044's discriminating property is that a literal retired at its own site is still reported (("own-site", "lib/test/new.sh", 1) against ("different-site", …, 0)). That pair is what distinguishes the fix from a hole; it must still discriminate after your change.

Re-adjudicating individual pins stays out of scope, per #1006.

Do not merge the resulting PR — leave it green and ready.

/prflow:implement 1057

[github-actions]

DevFlow Workpad — Issue #1057

Status: 🎉 Complete
Branch: issue-1057-bind-classifier-equivalence
Run: View run
PR: #1063
Last updated: 2026-08-01 16:11 UTC

Progress

• Setup — branch & workpad
  • 14:58:42 — /devflow:implement run started
  • 14:58:57 — Claude job setup complete; invoking agent
  • 15:01:30 — classification: non-bug — Follow-up test-hardening task from fix(pin-corpus): key retirement on site identity, not literal alone (#1006) #1044 review; describes future-drift risk in tests not a malfunction; unlabelled
  • 15:01:35 — resume-kind: fresh
• Implement
  • code + sweeps
• Review
  • /simplify
  • review-and-fix
  • acceptance-criteria gate
  • 16:03:31 — Phase 3.2 /simplify: reviewed the 44-line test diff (reuse/simplification/efficiency/altitude); already clean. One reuse observation (load_classifier repeats the load_linter/load_extractor loader pattern) consciously skipped to match the established one-loader-per-module convention.
  • 16:07:27 — Phase 3.3 review: prflow:code-reviewer traced the diff and ran both affected tests — no high-confidence findings. Verified AC1 binding is real (classifier divergence turns test RED), AC2 exercises the json.loads(row[target_index]) decode arm and fails closed, AC3 negative control preserved and no re-adjudication, docstring accurate.
  • 16:09:06 — Phase 4.1: prflow:docs subagent ran clean; no documentation changes warranted (test-harness-only change, no documented contract/interface/user-facing surface affected). No changeset (lib/test/ internal).
  • 16:10:20 — 4.2 claim audit (behavioral): body claims the test binds (a classifier divergence turns it RED) and the invalid-JSON arm fails closed — both verified against the code/tests. Pass.
  • 16:10:20 — 4.2 claim audit (verification): Test Plan rows reflect actually-run verification (binding perturbation, subcase raise, negative control green, suite segments 0/0) — pass.
  • 16:10:20 — 4.2 claim audit (artifact-existence): 'Resolves Bind the classifier-equivalence invariant and close the invalid-JSON target arm (deferred #1044 review suggestions) #1057' names a real issue; no other artifact claims. Pass.
  • 16:10:47 — checkpoint 4: observed token UP_TO_DATE — clean, proceeding to the publish decision
• Documentation
• PR marked ready
  • 16:11:07 — /prflow:implement run finished, PR published (gh pr ready): Bind the classifier-equivalence invariant and close the invalid-JSON target arm (#1057) #1063

Plan

• Add a load_classifier() helper to test_pin_corpus_lint.py (loads pin-corpus-classifier.py)
• Rewrite test_resolved_target_token_matches_the_classifier_three_shapes to derive the expected file-target token from classifier._portable_target and the runtime-bundle token from classifier.recover_override_names, binding site-side to classifier-side
• Add an invalid-json-target case (valid-JSON source, invalid-JSON resolved_target cell) to test_malformed_retirement_manifest_site_fields_fail_closed
• Fix _repo_relative_or_none docstring: "The one copy" → "The non-raising copy"
• Run the test module; prove the classifier-equivalence test is binding by perturbing the classifier locally

Acceptance Criteria

• The classifier-equivalence test derives its expected token from the classifier rather than from hardcoded literals, so a divergence between target classification and retirement keying turns the test RED. Prove it is binding: change the classifier's resolution and confirm the test fails.
• The invalid-JSON resolved_target decode-error arm is exercised and asserted to fail closed.
• Neither change re-adjudicates any individual pin (out of scope, per Pin retirement is keyed by literal while revival authorization is keyed by site #1006) and neither weakens the own-site negative control — a literal retired at its own site must still be reported, with that assertion still discriminating.
• The docstring is corrected.
• lib/test/run.sh reports zero failures and zero skips.

Devflow Reflection

Devflow Reflection (click to expand)