Skip to content

Harden review instructions against self-supplied evidence - #18

Merged
tylerwillis merged 5 commits into
mainfrom
panopticon/self-supplied-evidence
Aug 2, 2026
Merged

Harden review instructions against self-supplied evidence#18
tylerwillis merged 5 commits into
mainfrom
panopticon/self-supplied-evidence

Conversation

@tylerwillis

Copy link
Copy Markdown
Collaborator

Summary

  • require test-quality reviewers to prove a concrete production failure is reachable independently of test setup
  • require conditional producer, shape, sentinel, and runtime-environment provenance with file:line evidence
  • keep verdict-summary scope bounded to cited evidence in standard, direct-judgment, and audit instructions

Specification

Published task artifact: panopticon://tasks/e28cde1041a24c99b3b26a5b72183247/artifacts/self-supplied-evidence-spec.md

Focused verification

  • npm test -- --run tests/self-supplied-evidence.test.ts (12 passed)
  • npx rfc2119 check (green before implementation; full TESTING phase follows)

@tylerwillis

Copy link
Copy Markdown
Collaborator Author

Fable 5 adversarial review — round 1

MUST-FIX 1: tests/self-supplied-evidence.test.ts:44-49,186-193 looks up standard instructions by requirement-ID prefix. Multiple evidence blocks can produce multiple review IDs for one requirement, and .audit.md also matches, so the “every target” test can repeatedly inspect one arbitrary file. Read ${target.reviewId}.md exactly.

MUST-FIX 2: eval/calibration/013-pass-control-negative-positive.md claims production provenance while its added synthetic field assertions do not exist in the cited production test.

SHOULD-FIX 3: The blanket weakening-language regexes scan the whole generated task body, risking keyword-theater false positives outside the provenance guidance.

SHOULD-FIX 4: Conditional requirements 2.2–2.4 and 3.2 read like generation-time boundary inference, while implementation correctly asks the reviewer to conditionally apply always-present guidance.

SHOULD-FIX 5: The REQ 6.1 test asserts the basename of a temp directory that it created itself; remove this self-supplied assertion.

NOTES: Magic lower-bound counts are loose; the 7.1 test intentionally couples to committed verdict state; hardcoded calibration file:line pointers can rot; text requirements remain instruction change detectors whose behavioral effect ultimately depends on reviewer compliance.

The reviewer also found the issue #17 scope boundary, literal-lint rejection, and calibration 014 internally consistent.

@tylerwillis

Copy link
Copy Markdown
Collaborator Author

Sol 5.6 adversarial review — round 1

MUST-FIX: eval/calibration/013-pass-control-negative-positive.md fabricates and overclaims its cited production evidence. It says the case comes from tests/verdict-validation.test.ts:47, but the added kind, note, owner, and stamp assertions do not exist there. The production test validates verdict-record fields and exercises only a bare record, invalid verdict, and genuine CLI-written record. The fixture and its passing verdict therefore rely on self-supplied evidence—the precise class this PR is intended to prevent.

No other actionable findings identified.

@tylerwillis

Copy link
Copy Markdown
Collaborator Author

Fable 5 adversarial review — round 2

MUST-FIX: none. The accepted fixes hold.

SHOULD-FIX 1: tests/hook.test.ts:107 executes the exact installed npx rfc2119@<version> command from a temp cwd, so resolution may use the published package rather than this branch's local build.

SHOULD-FIX 2: tests/self-supplied-evidence.test.ts:283 retains a redundant negative keyword regex in expectBoundedGuidance; complete standard/direct/audit body comparisons already catch any change.

SHOULD-FIX 3: Calibration 012/013 production pointers use hardcoded line numbers that can silently drift; add stable test-title anchors or mechanical validation.

NOTES: The session-context length cap is a tripwire; the lint test intentionally proves only the deferred syntax-lint boundary; non-vacuity floors and committed-verdict coupling fail loudly; narrowing calibration 013 away from filename loudness is intentional.

Overall conclusion: the spec/test story is honest, direct judgments and audits are covered, and issue #17 cleanly owns mechanical set enumeration.

@tylerwillis

Copy link
Copy Markdown
Collaborator Author

Sol 5.6 adversarial review — round 2

No actionable findings.

The prior gaps are genuinely closed:

  • Calibration 013 cites real production-backed test provenance.
  • Review tasks are enumerated and matched by exact review ID.
  • Full test-quality, direct-judgment, and audit task invariants are asserted.
  • Conditional spec wording matches generated instructions.

The reviewer independently verified npx rfc2119 lint, npx rfc2119 check, and 15 targeted tests.

@tylerwillis

Copy link
Copy Markdown
Collaborator Author

Final dual-review triage

Accepted and fixed

  • Fable R1 MUST-1 — prefix lookup could skip review targets: accepted. Standard instructions are now read by exact target.reviewId; complete target sets are mechanically enumerated.
  • Fable R1 MUST-2 / Sol R1 MUST — calibration 013 fabricated production provenance: accepted. Case 013 now accurately condenses the real missing-verdict, invalid-verdict, and CLI-writer controls at tests/verdict-validation.test.ts:47-75. Fresh corpus judgments pass.
  • Fable R1 SHOULD-3 — blanket keyword blacklists: accepted. The tests now normalize only dynamic IDs and compare complete approved test-quality task bodies, eliminating semantic keyword guessing.
  • Fable R1 SHOULD-4 — conditional requirements sounded generation-time: accepted. Requirements 2.2–2.4 and 3.2 now explicitly require the reviewer to apply always-generated guidance whenever the boundary exists.
  • Fable R1 SHOULD-5 — self-created temp basename assertion: accepted and removed.
  • Fresh 2119 findings during the fix: accepted. Complete normalized task invariants now cover every direct-judgment target and every generated audit, preventing contradictory guidance outside a locally checked block.

All accepted fixes were followed by the full suite (17 files, 168 tests), npx rfc2119 check (0 violations/uncovered/failing/stale), and npm run check under pipefail. A second Fable/Sol round ran against final head a3d5b7f; neither reported a must-fix, and Sol reported no actionable findings.

Rejected or deferred

  • Fable R1 notes — loose lower bounds / live verdict coupling: rejected as defects. The floors are explicit non-vacuity guards, while exact target/audit set equality performs the completeness check; using committed verdicts is intentional real-artifact provenance and fails loudly on drift.
  • Fable R1 structural wording-test limitation: accepted as a limitation, not a defect. These requirements intentionally govern generated instruction text; the tests prove the real dispatch pipeline emits the approved text, while reviewer compliance remains judgmental by design.
  • Fable R2 SHOULD-1 — installed hook command may resolve the published package: deferred. REQ-004.1.7's integration test intentionally executes the exact version-pinned command users receive; replacing it with the local CLI would stop proving that installed command. Making this hermetic without weakening that evidence needs separate package-resolution design.
  • Fable R2 SHOULD-2 — residual recording-guidance negative regex is redundant: rejected as non-actionable. It is scoped to one fixed recording section and complete normalized task comparisons are authoritative; redundancy has no correctness impact in this PR.
  • Fable R2 SHOULD-3 / R1 note — calibration file:line pointers can drift: deferred. The citations are currently accurate, but mechanically validating their anchors is useful follow-up tooling.
  • Fable R2 notes — context length cap, lint tripwire, fixture floors, narrowed case 013: no change. They are intentional guardrails or explicitly documented scope choices.

Suggested placeholder issues

These are recommendations for the operator to endorse, reject, or edit at the PR approval gate.

Hermetic execution of version-pinned installed hooks. Preserve the end-to-end property that tests execute the exact npx rfc2119@<version> hook … command emitted to users, while ensuring resolution cannot silently fetch a published package instead of the artifact under test. This was deferred because substituting the local CLI would weaken the current production-command evidence; an implementer should provide a controlled local package/tarball or registry fixture and assert the resolved package identity.

Mechanically validate calibration provenance anchors. Calibration 012 and 013 currently cite accurate file:line ranges, but ordinary edits can move those lines without changing the prose. A follow-up should bind each provenance citation to a stable test-title or machine-readable anchor and fail loudly when the cited source disappears or no longer contains the claimed controls; empty or unmatched enumeration must itself fail.

@tylerwillis
tylerwillis marked this pull request as ready for review August 2, 2026 23:25
@tylerwillis
tylerwillis merged commit ec1a56b into main Aug 2, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant