amirbena / amirbena/code-review-skill
Consolidate shared-policy wiring-suite tests in tests/policy/review/
- Dominant language
- Python
- Stars
- 7
- Forks
- 2
- Avg merge
- 27m
- Merged PRs (30d)
- 188
Description
## Type
Refactor
## Area
Packaging / Portability
## Priority
P2 — Medium
Contributor-owned: bounded to `tests/policy/review/`, no policy content or semantics touched.
## Problem
`tests/policy/review/{test_change_risk_signals_docs,test_large_pr_partitioning_docs,test_review_stopping_criteria_docs}.py` and `repository_intelligence/test_repository_expansion_docs.py` (95 tests across 4 files) each repeat the same ~9-test wiring-verification block for a different shared policy: shared README policy-map row, packaged in the one shared manifest, both Skills load and list the policy, runbooks have a dedicated step, local/GitHub templates render it, architecture/comparison/feature-index mention it, changelog records the added policy. Separately, `specialist_depth/test_dependency_supply_chain_deepening.py` and `specialist_depth/test_api_contract_compatibility.py` (33 tests) share a near-identical 8-test tail (recognition signal is never itself the finding, reuses existing fail-closed discipline, no new severity/score, design record named not linked, forwarding/wiring into GitHub reasoning, runbooks, and parallel-review). 128 tests total across 6 files.
## Goal
Consolidate the generic policy → manifest → Skills → runbooks → rendering/docs/architecture/changelog wiring verification into shared, parametrized infrastructure representing the same wiring contract across the 6 files, while every policy's own semantic/content assertions remain local and are never merged across policies just because they share a wiring pattern.
## Scope
- Extract the ~9-test wiring block shared by the 4 primary files into parametrized infrastructure over `(policy_name, policy_path, manifest_entry, runbook_step_marker, ...)`.
- Confirm on inspection that the 8-test tail shared between the dependency-supply-chain and API-contract-compatibility files is genuinely the same abstraction before extracting it into its own small parametrized pair; if inspection finds a real difference, keep them separate and say so.
- Keep every policy's own semantic assertions (deterministic threshold values, canonical invariant prose, domain-specific content) fully separate — never merged just because two policies share a wiring pattern.
## Non-Goals
- Does not change any shared policy's content, semantics, or thresholds.
- Does not merge unrelated policies' wiring tests just because both happen to touch the same consumer file (a Skill, the manifest, a runbook).
## Acceptance Criteria
- [ ] Baseline test count for the 6 scoped files recorded (128).
- [ ] After-change test count recorded and reported.
- [ ] Every wiring check and every semantic check remains individually verifiable; no semantic assertion silently dropped.
- [ ] A failing case identifies both the policy and the specific wiring point that broke.
- [ ] Scoped tests and the full suite (`python3 -m unittest discover -s tests -t .`) pass.
## Dependencies
Depends on: none
Blocks: none
Parent: #395
Independent of the other four children (disjoint files, no shared-helper collision).
## Validation
- Before/after: `grep -rc "def test_" tests/policy/review/test_change_risk_signals_docs.py tests/policy/review/test_large_pr_partitioning_docs.py tests/policy/review/test_review_stopping_criteria_docs.py tests/policy/review/repository_intelligence/test_repository_expansion_docs.py tests/policy/review/specialist_depth/test_dependency_supply_chain_deepening.py tests/policy/review/specialist_depth/test_api_contract_compatibility.py`.
- `python3 -m unittest discover -s tests -t .`
Contributor guide
Assessment
This issue has not been assessed yet.