Repository navigation
ci(review): require semantic trace completeness #347
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
e0efe07
ci(review): require semantic trace completeness
Abiorh001 0996b56
docs(review): include schema test in chunk scope
Abiorh001 637d314
fix(review): enforce complete semantic evidence
Abiorh001 a814da7
fix(review): validate semantic agent contracts
Abiorh001 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
100 changes: 100 additions & 0 deletions
100
...4-review-evidence-integrity/chunks/WS-CI-004-05-semantic-review-completeness.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| # Chunk Contract: WS-CI-004-05 — Semantic Review Completeness | ||
|
|
||
| ## Merge state | ||
|
|
||
| - Outcome on merge: `complete` | ||
|
|
||
| ## Goal | ||
|
|
||
| Require reviewers to decompose material requirements into independently | ||
| checkable behavior atoms and trace each atom through its owner, implementation, | ||
| proof, and execution custody before issuing a passing verdict. | ||
|
|
||
| ## Why this chunk exists | ||
|
|
||
| Exact-head review, impact-cone inspection, and adversarial probes prevent stale | ||
| or shallow receipts, but PR #346 demonstrated that a reviewer can still accept | ||
| a polished criterion-to-test table while one material behavior has no named | ||
| proof. Review must establish semantic completeness, not merely the presence of | ||
| a matrix. | ||
|
|
||
| ## Risk class | ||
|
|
||
| L1 — reviewer protocol, skills, agents, evidence schema, and evaluation harness. | ||
|
|
||
| ## Allowed files | ||
|
|
||
| ```text | ||
| .agents/skills/reviewer-evidence-protocol/SKILL.md | ||
| .agents/skills/{architecture-review,security-review,qa-review,test-delta-review,docs-review,product-ops-review,ci-integrity-review,reuse-dedup-review,senior-engineer-review}/SKILL.md | ||
| .codex/agents/*-reviewer.toml | ||
| .agent-loop/templates/INTERNAL_REVIEW_RECEIPT.schema.json | ||
| .agent-loop/initiatives/WS-CI-004-review-evidence-integrity/{CHUNK_MAP.md,STATUS.md,REVIEWER_MATRIX.md} | ||
| .agent-loop/initiatives/WS-CI-004-review-evidence-integrity/chunks/WS-CI-004-05-semantic-review-completeness.md | ||
| .agent-loop/initiatives/WS-CI-004-review-evidence-integrity/evaluations/{CASES.json,EXPECTATIONS.json} | ||
| .agent-loop/initiatives/WS-CI-004-review-evidence-integrity/reviews/WS-CI-004-05-external-review-response.md | ||
| .agent-loop/CURRENT_STATE.md | ||
| scripts/{reviewer_contracts.py,test_reviewer_contracts.py,test_review_target.py} | ||
| ``` | ||
|
|
||
| ## Not allowed | ||
|
|
||
| ```text | ||
| backend product/runtime behavior or migrations | ||
| hosted or committed exact-head receipt custody | ||
| new merge authority, automatic merge, or universal reviewer fanout | ||
| coverage, CI, branch-protection, or existing test weakening | ||
| ``` | ||
|
|
||
| ## Acceptance criteria | ||
|
|
||
| - [x] Every material criterion is decomposed into atomic observable behaviors, | ||
| including actors, resources/tenants, lifecycle states, failures, and | ||
| forbidden side effects where relevant. | ||
| - [x] Every behavior atom maps to its owner, implementation source, named proof, | ||
| and execution custody; missing or narrative-only mappings block PASS. | ||
| - [x] Every final review states a residual escape hypothesis and how the reviewer | ||
| attempted to falsify it. | ||
| - [x] Architecture, security, QA, test-delta, docs, product/ops, CI, reuse, and | ||
| senior-review skills and agents contain specialty-specific completeness | ||
| probes instead of generic boilerplate alone. | ||
| - [x] The canonical receipt schema rejects missing trace rows or residual escape | ||
| analysis, requires all trace rows to be verified for a passing verdict, | ||
| and advances the breaking receipt contract to schema version 2. | ||
| - [x] Realistic multi-file evaluation fixtures reproduce the semantic omission | ||
| observed during PR #346 review without leaking the expected answer. | ||
| - [x] Session evidence remains advisory and GitHub remains the sole merge authority. | ||
|
|
||
| ## Completion evidence | ||
|
|
||
| | Behavior | Owner | Implementation source | Named proof | Execution custody | Result | | ||
| |---|---|---|---|---|---| | ||
| | Atomize material criteria | Shared reviewer protocol | `reviewer-evidence-protocol/SKILL.md` and nine specialty agents/skills | `test_all_agent_skill_contracts_compose_with_protocol` plus blind `qa-compound-trace` replay | local deterministic tests and exact-head QA review | verified | | ||
| | Trace every atom through owner, implementation, proof and custody | Shared protocol and receipt schema | protocol steps 6-7 and receipt `traceability` | receipt schema tests and reviewer-contract mutation tests | local deterministic tests | verified | | ||
| | Require residual escape falsification | Shared protocol and receipt schema | protocol step 8 and receipt `residual_escape` | `test_final_verdict_requires_verified_trace_and_closed_escape` | local deterministic tests | verified | | ||
| | Preserve specialty-specific depth | Nine specialty reviewer owners | matching skills and `.codex/agents/*-reviewer.toml` | `scripts/reviewer_contracts.py` and exact-head reviewer fanout | local validation and advisory review sessions | verified | | ||
| | Reproduce the PR #346 omission | Evaluation harness | `CASES.json` / `EXPECTATIONS.json` | blind `qa-compound-trace` classification plus fixture validation | isolated QA review and local tests | verified | | ||
| | Keep evidence advisory | GitHub authority boundary | receipt `custody`, schema closed fields, contract non-goals | authority-field adversarial schema probe and security review | local deterministic tests and advisory review session | verified | | ||
|
|
||
| ## Verification commands | ||
|
|
||
| ```bash | ||
| python3 -m unittest -v scripts.test_reviewer_contracts scripts.test_review_target | ||
| python3 scripts/reviewer_contracts.py | ||
| python3 scripts/check_chunk_state_sync.py --base-ref origin/main | ||
| python3 scripts/check_active_state_projections.py | ||
| python3 scripts/check_markdown_links.py | ||
| python3 scripts/check_stale_workstream_wording.py | ||
| python3 scripts/check_stale_review_contracts.py | ||
| git diff --check | ||
|
coderabbitai[bot] marked this conversation as resolved.
|
||
| ``` | ||
|
|
||
| ## Required reviewers | ||
|
|
||
| Architecture, security, QA, test-delta, CI integrity, senior engineering, | ||
| reuse/dedup, and documentation. | ||
|
|
||
| ## Human review focus | ||
|
|
||
| Confirm the change forces semantic proof without turning reviewer prose into a | ||
| second permission system or adding unnecessary process ceremony. | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
43 changes: 43 additions & 0 deletions
43
...-004-review-evidence-integrity/reviews/WS-CI-004-05-external-review-response.md
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| # WS-CI-004-05 External Review Response | ||
|
|
||
| ## Comments addressed | ||
|
|
||
| - `CR-347-002` Major: acceptance criteria were implemented but remained | ||
| unchecked. Valid. Every criterion is now checked and mapped through owner, | ||
| implementation source, named proof, execution custody, and result. | ||
| - `CR-347-003` Minor: the contract did not list the stale-wording checks or | ||
| record reviewer-session closure. Valid. Verification now names both stale | ||
| checks, the docs reviewer requires link/stale evidence, and all requested | ||
| internal reviewer sessions were closed before this response. | ||
| - `CR-347-004` Major: specialty-skill adoption checks did not enforce the full | ||
| shared semantic contract. Valid. Stable requirement IDs now cover | ||
| atomization, owner, implementation source, named proof, execution custody, | ||
| result, residual escape, and fail-closed PASS behavior for both skills and | ||
| custom agent TOMLs, with independent mutation proof. | ||
| - `CR-347-005` Minor: both receipt test suites needed unavailable and multi-row | ||
| PASS rejection. Valid. Both suites now exercise those boundaries while | ||
| preserving PROVISIONAL acceptance for unavailable proof. | ||
|
|
||
| ## Comments not applied | ||
|
|
||
| - `CR-347-001` Major requested that durable state use `in review` until merge. | ||
| Not valid under the repository's atomic chunk-state contract. `AGENTS.md` | ||
| requires the same PR to land its declared final outcome and prohibits | ||
| `in review`, `pending review`, or `ready for review` as state that can reach | ||
| `main`. GitHub open PRs remain the transient-work view; the branch projection | ||
| describes the outcome that this PR will atomically land. | ||
|
|
||
| ## Human decisions needed | ||
|
|
||
| None beyond normal approval and merge ownership. | ||
|
|
||
| ## Commands rerun | ||
|
|
||
| Reviewer contract/schema tests, skill validation, reviewer contract validation, | ||
| chunk-state sync, active-state projections, Markdown links, stale wording, | ||
| stale review contracts, and diff checks. | ||
|
|
||
| ## Remaining risks | ||
|
|
||
| Natural-language semantic completeness still requires reviewer judgment beyond | ||
| the structural guarantees of JSON Schema and deterministic contract checks. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.