Knowledge Wiki Orchestrata Ultimate

Orchestrata Ultimate › Gates › review-staged

Gate: review-staged
Purpose Run the code review and the test review on exactly the staged Python files and refuse the commit on a blocking finding — or on a review that produced no sound verdict
Where it runs scripts/hooks/pre-commit (after the secrets gate and the ruff auto-fix); ~/bin/precommit-check runs the same command on a temporary index (see commit-precheck)
Trigger git commit with at least one staged .py file (added, copied or modified)
Exact command PRECOMMIT_WALLS_FILE=<tmp> .venv/bin/python scripts/gate.py review-staged [--repo-root R] [--python P] [--timeout S]
Blocks Exit 1 = blocking findings, exit 2 = invalid (a review did not run, did not finish, or left a report that does not add up). The hook blocks on any non-zero exit
Ruling / ADR RUL-008 (review before every commit; its Enforcement names scripts/hooks/pre-commit and code-review.py --min-severity); ADR-0101 (stage walls measured); ADR-0116 (staged source files enter T21); ADR-0025 (no cuts)

Why it exists

Measured 2026-10-11, the previous shell version of the review "passed every commit", three ways (commit 8e0df6ff3 and the header of commit_review.pl):

Each was "the reviewer produced nothing" read as "the reviewer found nothing". The fix moved the verdict out of bash: the driver (src/orchestrata_ultimate/gates/commit_review.py) writes what each reviewer reported as Prolog facts, and src/orchestrata_ultimate/prolog/commit_review.pl derives the verdict, where "every way a report can fail to be sound is a named review_defect/3".

How it works

Diagram projects-orchestrata-ultimate-gates-review-staged-1

Scope (review_required/2, reviewer_tree/2, reviewer_covers/2).

Reviewer Tree Files it covers
code_review (scripts/code-review.py) src/ every staged .py
test_review (scripts/test-review.py) tests/, end2end/, scripts/ test-named files (test_*.py, *_test.py), plus every staged scripts/*.py (T21 routing, ADR-0116)

A conftest.py under tests/ is not test-named, so no test review is required for it: test-review prints "No test files found" for it, and requiring the review would block every conftest commit as invalid(no_report) (a false block measured 2026-10-11).

Policy (blocking_severity/2). Code review blocks at error; test review blocks at error and warning; info never blocks. Both reviewers run at --min-severity INFO, so the rule file, not a command-line flag, decides what blocks. When reports/t21-worklist-baseline.json exists, the test review gets --worklist-baseline, which downgrades the named-debt exact-key T21 WARNINGs to INFO (run_reviewer).

Soundness (review_defect/3). A review is invalid(Why) when it was required and:

Defect Meaning
not_run no exit recorded
exit(N) an exit code that is not a verdict (only 0 and 1 are)
no_report a verdict exit but no report that parses
not_analyzed the report says it analyzed 0 files
count_mismatch(Stated, Parsed) the stated issue total differs from the findings parsed
unknown_severity(S) a severity outside error / warning / info
exit_disagrees(Exit) exit 1 with no ERROR, or exit 0 with an ERROR

commit_defect/1 adds run-level defects: no staged_count, a staged_count that differs from the listed files (a truncated fact file), and an unrequested_review (driver and rules disagree on scope). invalid dominates fail.

A reviewer past its timeout (default 600 s, DEFAULT_REVIEW_TIMEOUT_S) records exit 124, which is not a verdict exit, so the commit is invalid.

What the code review checks

scripts/code-review.py runs these checkers on each file (its main(), read through the AST toolkit), plus two repo-wide gates:

Checker Checks Severities seen in the source
RuffChecker ruff findings F and E codes ERROR, W WARNING, I and UP INFO, others WARNING; RUFF_NOT_FOUND when ruff is not on PATH
MyPyChecker mypy on the file, 30 s timeout MYPY_TIMEOUT is an ERROR
StyleChecker formatting, docstrings, type hints, naming, imperative accumulators, complexity, test isolation, code patterns (limits: line 88, function 40 lines, class 200 lines, 5 args, 15 locals, 4 returns, 12 branches) per rule
ChecklistChecker class responsibility (method count), method self access, abstract classes, test files per rule
ConnectionStringChecker hardcoded IPs, URIs and localhost:<port>, env-var defaults HARDCODED_IP, HARDCODED_URI, HARDCODED_LOCALHOST are ERROR
TraceabilityChecker @traces on classes and methods (enums, pydantic models, protocols, dataclasses and similar are exempt) WARNING, so it does not block a commit
check_banned_dependencies banned packages in pyproject.toml (from .claude/banned-libraries.txt) ERROR
check_prolog_boundary every Python Prolog query targets an existing predicate/arity ERROR

What the test review checks (T1–T21)

scripts/test-review.py's TestFileChecker.RULES table (read through the AST toolkit, 2026-10-23). The rule ids run T1–T21; T20 is emitted but not in the table, and T11 is in the table but no check emits it. T1–T20 apply to test-named files; T21 applies to every .py passed on the command line. Severity is ERROR unless the column says otherwise.

Rule Text in RULES Severity
T1 No mocking — no unittest.mock, MagicMock, AsyncMock, Mock, patch ERROR
T2 No stubs or fakes — no FakeX, StubX, DummyX classes ERROR
T3 No try/except/pass blocks ERROR
T4 Function-scope fixtures only — module scope solely for annotated IMMUTABLE precomputed results (# T4-immutable-module-scope:), class/session never ERROR (a session-scoped connection fixture may opt out with a reasoned # gate-ok: <why> above the decorator)
T5 No os.environ mutations ERROR
T6 No hardcoded ports ERROR
T7 No pytest.mark.skip or skipif ERROR
T8 No monkeypatch usage ERROR
T9 No global mutable state at module level ERROR
T10 Use test_id fixture for DB/graph (Memgraph) ops WARNING ("File WRITES to Redis/Memgraph without namespacing")
T11 Use isolation fixture for file/DB resources listed only; not emitted
T12 No bare os.environ['X'] reads — use os.environ.get() WARNING
T13 No relative imports ERROR
T14 Async tests need pytest.mark.asyncio or asyncio_mode ERROR
T15 No pytest-xdist (-n/--numprocesses/--dist) — it deadlocks this suite; use the threadpool runner ERROR
T16 No banned-library imports — see .claude/banned-libraries.txt ERROR
T17 A uuid-scoped Memgraph chunk_label must drop its schema in teardown ERROR
T18 Verify through the store/repo's own session() — a bare .driver.session() reads the cluster default database ERROR
T19 DDL a test CREATEs, the test must DROP ERROR
T20 (emitted, not in RULES) @pytest.mark.anyio is forbidden — asyncio_mode is configured, so pytest-asyncio owns async tests ERROR
T21 Env key drift/fabrication — a literal similar to but not equal to a declared *_ENV constant ERROR for a drifted key, WARNING for an exact literal retyped outside its declaring module

Because test review blocks at WARNING, T10, T12 and the T21 exact-literal WARNING all block a commit. T21 also flags pre-existing literals in any test file you touch (improvement log D4).

Rulings behind the rules: T1/T8 RUL-002; T15 RUL-001 and RUL-005; T21 was minted after the RUL-038 cap was found stamping fabricated env keys (25f06c3f6, 2026-10-20; routing 0d329660d, 0ffb2d1ec).

Typical failures and how to fix them

What you see Cause Fix
Commit blocked — the review found blocking issues exit 1; findings are listed above the line Fix each finding; never weaken a test or add a waiver to pass
Commit blocked — a review produced no verdict (exit 2) a review_defect, for example a reviewer crash, a timeout, or a count mismatch Read the reason; run the reviewer by hand on the file (.venv/bin/python scripts/test-review.py --min-severity INFO --stdout <file>)
MYPY_TIMEOUT ERROR mypy took over 30 s on a cold cache (D3) Warm the cache first: .venv/bin/python -m mypy --no-error-summary <changed files>
RUFF_NOT_FOUND ruff not on PATH The hook and run_reviewer put the venv's bin first; run from a provisioned checkout
no project interpreter at .venv/bin/python checkout has no venv (RUL-021) scripts/worktrees.py provision <path>
T21 on a line you did not write T21 covers pre-existing literals in touched files Import the declared constant instead of retyping the key

To meet these before git commit, run ~/bin/precommit-check <repo> <files>.

Measured walls

reports/precommit-walls.ndjson (ADR-0101 rule 2) records the stage on each commit. Of 244 attempts between 2026-10-14 and 2026-10-23 that reached the review stage, the median review_staged_s was 0.18 s and the maximum 123.3 s; 14 attempts were blocked. One test file took 3.4 s to 18.4 s to review in the three newest rows.

Wiring