Bind the classifier-equivalence invariant and close the invalid-JSON target arm (#1057) - #1063
Conversation
…-JSON target arm Derive the resolved-target-token expectations in test_resolved_target_token_matches_the_classifier_three_shapes from pin-corpus-classifier.py (_portable_target, recover_override_names) instead of hardcoded literals, so a divergence between the classifier's target resolution and the linter's retirement keying turns the test RED. Add an invalid-JSON resolved_target case to test_malformed_retirement_manifest_site_fields_fail_closed exercising the json.loads(row[target_index]) decode-error arm, asserting it fails closed. Fix the _repo_relative_or_none docstring to describe itself as the non-raising copy of the abspath/commonpath/relpath computation. Closes #1057 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Requesting review. Implements #1057, the deferred suggestions from PR #1044 (issue #1006). This touches a security-adjacent gate, so please weight the failure direction. The substantive item: What matters most:
Adjacent context the reviewer may find useful: #1061 documents that a pin outside the existence census has no reachable disposition at all. Nothing here should widen or narrow that census; if this change touches /prflow:review |
There was a problem hiding this comment.
✅ Devflow Review — APPROVE
Diff profile: engine_self_modifying (touches lib/**) → full checklist. 2 files, ~30 lines; no new types, not config-only, not a detect-all audit.
Reviewed HEAD: def7f4e9759fedac96703c1df90cf017da427bb4
Test evidence: CI lib + python tests: success for this commit; additionally ran the three affected tests locally at HEAD — all pass.
Verification
All checklist claims verified against source:
- Classifier-equivalence binding (
test_resolved_target_token_matches_the_classifier_three_shapes) — expectations now derive frompin-corpus-classifier.py(_portable_target,recover_override_names) instead of hardcoded literals. Confirmed the derivations equal the linter's independent_resolved_target_tokenfor both the in-repo-file shape (docs/x.md) and the runtime-bundle sentinel (/__pin_corpus_runtime__/CI_BUNDLE). The test binds the test-side expectation to the classifier while the linter remains the independent SUT, so classifier target-resolution drift turns it RED — a genuine coupling, not coincidental agreement. - Invalid-JSON
resolved_targetarm — traced the newinvalid-json-targetcase throughcsv.reader(double-encoded cells) +json.loads:source_filedecodes cleanly, thenresolved_target=not-jsonraises atjson.loads(row[target_index])(line 1804), failing closed withInfrastructureError. It exercises the intended target decode arm, not the earlier literal arm. - Docstring —
_repo_relative_or_nonecatchesValueError/returnsNone(non-raising);_relative_target_pathraisesInfrastructureError. The "non-raising copy" phrasing is accurate.
Acceptance criteria (issue #1057) — all met
- Classifier-derived expectations; binding (divergence → RED).
- Invalid-JSON
resolved_targetarm exercised, fails closed. - No pin re-adjudicated; own-site negative control (
test_retirement_is_scoped_to_the_retired_site) untouched and still green. - Docstring corrected.
- Suite green (CI
lib + python tests: success).
Findings
None. Well-scoped test-hardening + docstring follow-up; no correctness, silent-failure, or coverage concerns.
|
Verdict APPROVE (no findings) on Closes #1057: the classifier-equivalence invariant now binds to what the classifier actually produces rather than to hardcoded literals, so a divergence between target classification and retirement keying can no longer pass green — which is the mismatch #1006 closed and this could silently have reopened. |
Summary
test_pin_corpus_lint.pyso it derives its expected resolved-target tokens from the classifier itself, turning a real classifier/linter divergence RED instead of leaving the two coincidentally in agreement.resolved_targetdecode arm, and correct an inaccurate docstring.Changes
Pin-corpus test harness (
lib/test/test_pin_corpus_lint.py):test_resolved_target_token_matches_the_classifier_three_shapesnow derives its expectations frompin-corpus-classifier.py—_portable_targetfor the in-repo file shape andrecover_override_namesfor the runtime-bundle sentinel — rather than hardcoded literals, so a change to the classifier's target resolution turns the test RED. Added aload_classifier()loader mirroring the existingload_extractor(). The deliberate out-of-repo divergence stays a hardcoded assertion (the linter fails toward not-matched there, unlike the classifier).Malformed-manifest coverage (
lib/test/test_pin_corpus_lint.py): added aninvalid-json-targetcase totest_malformed_retirement_manifest_site_fields_fail_closedexercising thejson.loads(row[target_index])decode-error arm (valid-JSON source, invalid-JSONresolved_targetcell) and asserting it fails closed withInfrastructureError— the arm bundle-target rows are most likely to meet in practice.Docstring (
lib/test/pin-corpus-lint.py):_repo_relative_or_nonenow describes itself as the non-raising copy of the abspath/commonpath/relpath computation (the file has a second, raising copy,_relative_target_path).Resolves
Resolves #1057
Test Plan
test_resolved_target_token_matches_the_classifier_three_shapespasses, and perturbing the classifier's resolution makes it fail (binding proven:equal=Falseafter perturbation).test_malformed_retirement_manifest_site_fields_fail_closedpasses, with the newinvalid-json-targetsubcase raisingInfrastructureError.test_retirement_is_scoped_to_the_retired_siteremains green (unchanged).lib/test/run.shreports zero failures and zero skips (verified locally in segments: monolith core 7752/0,test_python_scripts3424/0,test_module_runner120 OK,test_pin_corpus_lint199 OK, all 15 suite modules 0 failed).Visual Changes
N/A
Breaking Changes
None