|
| 1 | +# Discovery: WS-CI-005 Semantic Proof Quality |
| 2 | + |
| 3 | +## Current behavior |
| 4 | + |
| 5 | +WS-CI-004 already supplies exact-target inspection, structured receipts, |
| 6 | +specialty routing, atomic traceability, residual-escape analysis, and isolated |
| 7 | +reviewer evaluations. `scripts/reviewer_contracts.py` checks that every reviewer |
| 8 | +agent and skill contains the shared semantic requirements. Its evaluation model |
| 9 | +uses positive, negative, stale-replay, output-contract, and handoff cases. |
| 10 | + |
| 11 | +The remaining gap is semantic discrimination. The validator checks that a |
| 12 | +traceability row has an owner, implementation source, named proof, custody, and |
| 13 | +result. It does not determine whether the named proof can observe the behavior. |
| 14 | + |
| 15 | +## Relevant files and symbols |
| 16 | + |
| 17 | +| Path | Current responsibility | Gap | |
| 18 | +|---|---|---| |
| 19 | +| `.agents/skills/reviewer-evidence-protocol/SKILL.md` | Universal exact-head and semantic review process | No proof-strength or test-of-the-test rule | |
| 20 | +| `.agents/skills/{architecture,security,qa,test-delta,ci-integrity,reuse-dedup,senior-engineer}-review/SKILL.md` | Specialty review depth | No shared database/isolation/strict-fake failure patterns | |
| 21 | +| `.codex/agents/*-reviewer.toml` | Custom reviewer execution contracts | Can repeat named proof without demonstrating discrimination | |
| 22 | +| `.agent-loop/templates/INTERNAL_REVIEW_RECEIPT.schema.json` | Structured advisory receipt | Trace rows do not declare proof strength or adversarial mutation outcome | |
| 23 | +| `scripts/reviewer_contracts.py` | Reviewer contract/evaluation validator | Validates fields and tokens, not proof compatibility | |
| 24 | +| `scripts/test_reviewer_contracts.py` | Mutation and output regression tests | Does not replay non-discriminating proof classes | |
| 25 | +| `WS-CI-004/evaluations/{CASES,EXPECTATIONS}.json` | Blind reviewer evaluation inputs | Covers broad specialties, not the PR #349 escapes | |
| 26 | +| `.agents/skills/evidence-gate/SKILL.md` | Deterministic pre-review evidence | Does not classify infrastructure custody | |
| 27 | +| `.agents/skills/pr-trust-bundle/SKILL.md` | Human-facing evidence summary | Can summarize a named but semantically weak test | |
| 28 | + |
| 29 | +## Failure replay |
| 30 | + |
| 31 | +### PR #338 and PR #346 |
| 32 | + |
| 33 | +These exposed path continuity, atomic state vocabulary, public-owner boundaries, |
| 34 | +completed-history immutability, ledger parity, and compound-criterion gaps. |
| 35 | +WS-CI-004 now covers exact-head and traceability mechanics for those classes. |
| 36 | + |
| 37 | +### PR #349 |
| 38 | + |
| 39 | +The following escaped after initial internal passes: |
| 40 | + |
| 41 | +1. `PROJECT_POINTS` quantity `"1.0"` passed a duplicated runtime validator while |
| 42 | + canonical schema and database rules rejected it. |
| 43 | +2. A COMPENSATION owner fact was trusted after checking only its project, not |
| 44 | + binding identity and instrument type. |
| 45 | +3. Malformed immutable input leaked `AttributeError` before domain concealment. |
| 46 | +4. A mocked repository exception was presented as transaction rollback proof. |
| 47 | +5. Fake authorization methods raised labels for wrong-session/copy/replay cases |
| 48 | + without constructing those conditions. |
| 49 | +6. PostgreSQL `<>` comparisons over nullable operands allowed trigger guards to |
| 50 | + evaluate to unknown and skip rejection. |
| 51 | +7. Independent foreign keys allowed project, policy, and version facts that |
| 52 | + existed individually but did not share composite ownership. |
| 53 | +8. A cross-project read test used a mock returning `None`, duplicating the |
| 54 | + missing-record case rather than proving repository isolation. |
| 55 | +9. PostgreSQL regressions initially failed during shared fixture setup because |
| 56 | + they recreated a globally unique service identity, so the intended |
| 57 | + integrity assertion was never reached. |
| 58 | +10. A required-version regression initially used an invalid non-UUID value the |
| 59 | + old code already rejected instead of `None`, the previously accepted bad |
| 60 | + selector. |
| 61 | + |
| 62 | +These are proof-quality failures: the named proof existed but could not |
| 63 | +distinguish the defect. |
| 64 | + |
| 65 | +## Existing tests and gaps |
| 66 | + |
| 67 | +Existing reviewer-contract tests prove protocol adoption and receipt shape. |
| 68 | +They do not currently prove: |
| 69 | + |
| 70 | +- proof custody is compatible with the claim; |
| 71 | +- a reviewer mutates or contradicts a claimed invariant; |
| 72 | +- strict fakes validate identity, state, and call order; |
| 73 | +- real tenant-isolation proof persists a foreign resource; |
| 74 | +- database review covers NULL semantics, composite ownership, direct SQL, and |
| 75 | + rollback durability; |
| 76 | +- reuse review compares schema, runtime, and database representations of one |
| 77 | + canonical rule; |
| 78 | +- escaped findings become permanent blind evaluation cases. |
| 79 | +- fixture setup reaches the intended assertion rather than merely failing; |
| 80 | +- a regression input distinguishes corrected behavior from the pre-fix code. |
| 81 | + |
| 82 | +## Dependencies and conventions to preserve |
| 83 | + |
| 84 | +- Extend `scripts/reviewer_contracts.py`; do not create a parallel validator. |
| 85 | +- Extend the WS-CI-004 reviewer registry and evaluation harness; do not fork the |
| 86 | + nine-reviewer map. |
| 87 | +- Keep shared rules in one protocol/reference and specialty deltas in their |
| 88 | + existing skill and agent files. |
| 89 | +- Keep receipts advisory and out of tree; GitHub remains durable authority. |
| 90 | +- Keep external review responses separate from internal receipts. |
| 91 | + |
| 92 | +## Risks discovered |
| 93 | + |
| 94 | +| Risk | Consequence | Planned control | |
| 95 | +|---|---|---| |
| 96 | +| Named proof without observability | False PASS | Closed proof-strength and compatibility rules | |
| 97 | +| Permissive fake | Simulated security/isolation evidence | Strict-fake obligations and blind fixtures | |
| 98 | +| ORM-only review | SQL integrity bypass | Database integrity reference and direct-SQL probes | |
| 99 | +| Missing tenant record | Isolation test duplicates not-found | Real foreign-resource proof requirement | |
| 100 | +| Duplicated business rule | Schema/runtime drift | Canonical-rule reuse comparison | |
| 101 | +| More reviewer prose | Token/maintenance growth | One concise shared reference, validated adoption tokens | |
| 102 | + |
| 103 | +## Unknowns to measure during implementation |
| 104 | + |
| 105 | +- Whether proof-strength metadata belongs directly in the receipt schema or in |
| 106 | + a referenced traceability sub-schema. |
| 107 | +- The smallest blind-fixture set that covers each escape without leaking the |
| 108 | + answer or making evaluation slow. |
| 109 | +- Which proof-strength mismatches can be checked deterministically and which |
| 110 | + remain reviewer judgments. |
0 commit comments