Orchestrata Ultimate › Plans › briefs › qwen › Q2b-D-report

Q2b-D Report — scripts/code-review.py ruff-missing verdict restored under ADR-0089

Source: plans/briefs/qwen/Q2b-D-report.md at 8a2cb42ad · last changed 2026-10-12 · first committed 2026-10-12 · 175 lines.

Prose pending.

Plan text

Q2b-D Report — scripts/code-review.py ruff-missing verdict restored under ADR-0089

DESIGN_WAIVER: ADR-0089 (Accepted). Q2b converted scripts/code-review.py's ruff spawns to <interpreter> -m ruff (the same conversion Phase B applied to harness/evaluators/lint.py); this task finishes the conversion by restoring the "ruff never ran" verdict, which the conversion had silently broken.

The defect

• Q2b changed RuffChecker (then ~line 172) and RuffFormatChecker (~line 281) to spawn [sys.executable, "-m", "ruff", ...].
• With ruff absent, that child no longer raises FileNotFoundError. It exits non-zero with No module named ruff on stderr.
• RuffChecker runs ruff check --exit-zero and only parsed stdout, so it recorded nothing: the review "passed" without linting, and since main() returns 1 if error_count > 0 else 0, a review that never linted exited 0.
• Q2b's own forced run never ran tests/scripts/test_code_review_tool.py because the affected-test oracle cannot map scripts/code-review.py to it (D32 — confirmed again this task: scripts/affected_tests.py reports "untested": ["scripts/code-review.py"]). The module is therefore named explicitly in every run here.

The red (defect, worktree at cc4069fe9, both files at HEAD)

bash .hostlock.sh env -u PYTHONPATH -u PYTEST_ADDOPTS .venv/bin/python -m pytest -q -p no:cacheprovider tests/scripts/test_code_review_tool.py → [hostlock] 05:27:28 rc=1, run rc=1:

FAILED tests/scripts/test_code_review_tool.py::test_ruff_checker_file_not_found_when_ruff_missing_from_path
FAILED tests/scripts/test_code_review_tool.py::test_missing_ruff_fails_the_run
FAILED tests/scripts/test_code_review_tool.py::test_ruff_format_checker_file_not_found
3 failed, 76 passed, 1 warning in 9.54s

Quoted assertion:

E   AssertionError: ruff was missing and the run still had no ERROR — a review that never linted would exit 0

The fix (scripts/code-review.py)

Seam mirrored from lint.LintEvaluator.evaluate: a keyword parameter interpreter: str | None = None, defaulting to sys.executable. The CLI wiring (lines 1960–1962, RuffFormatChecker(report) / RuffChecker(report, fix=args.fix)) is unchanged, so the CLI default stays sys.executable.

Branches, each reporting the pre-Q2b RUFF_NOT_FOUND code at its pre-Q2b severity (ERROR — git show origin/master:scripts/code-review.py lines 207–214, 312–319):

• BaseChecker._report_ruff_not_found(message) — new shared helper, line 163; emits RUFF_NOT_FOUND / Severity.ERROR / category "ruff".
• RuffChecker.__init__ gains interpreter: str | None = None, line 186; self.interpreter = interpreter if interpreter is not None else sys.executable (line 193); the spawn at line 199 uses self.interpreter.
• Missing module: line 216, if result.returncode != 0 and "No module named ruff" in result.stderr → message ruff is not installed for {interpreter} (python -m ruff: No module named ruff) — checked BEFORE the stdout-parsing branch.
• Other non-zero with no parseable output: line 232 (elif result.returncode != 0) → new code RUFF_CHECK_ERROR, message ruff check failed (exit code N), Severity.ERROR. No existing code fit: RUFF_FORMAT_ERROR is format-specific; RUFF_CHECK_ERROR mirrors it for the check path, exactly as lint.py reports ruff check failed (exit code N) for the SIGKILL/no-output shape.
• Missing interpreter binary: except FileNotFoundError (line 252) → message Python interpreter not found: {interpreter}.
• RuffFormatChecker.__init__ gains the same parameter, line 319; spawn at line 328; the No module named ruff check at line 333 precedes the existing RUFF_FORMAT_ERROR stderr branch; a further elif result.returncode != 0 (line 353) reuses RUFF_FORMAT_ERROR with Ruff format failed (exit code N) so a silent non-zero format child is never silence either; FileNotFoundError at line 375 reports Python interpreter not found: {interpreter}.

Test changes (tests/scripts/test_code_review_tool.py)

Old setup → new setup (setup only; no assertion weakened):

• All three PATH-mutation tests: os.environ.update({"PATH": ""}) in try/finally → removed entirely (mutating os.environ in a test is forbidden in this repo).
• New shared precondition helper _base_interpreter_without_ruff() (line 282): the REAL ruff-less base interpreter sys._base_executable (with the line-scoped # pyright: ignore[reportAttributeAccessIssue], the repo's established form), proven ruff-less by a real [base, "-m", "ruff", "--version"] probe BEFORE the checker is asked — if ruff is ever installed for the base interpreter, the tests fail AT the precondition, naming why.
• test_ruff_checker_file_not_found_when_ruff_missing_from_path (line 311): cr.RuffChecker(report) → cr.RuffChecker(report, interpreter=base); assertion unchanged (any(i.code == "RUFF_NOT_FOUND" ...)).
• test_missing_ruff_fails_the_run (line 396): same setup swap; the ERROR assertion and its message ("ruff was missing and the run still had no ERROR — a review that never linted would exit 0") are kept verbatim.
• test_ruff_format_checker_file_not_found (line 485): cr.RuffFormatChecker(report) → cr.RuffFormatChecker(report, interpreter=base); assertion unchanged.
• NEW test_ruff_checker_interpreter_not_found (line 322): interpreter=str(tmp_path / "no-such-python") — a path that does not exist is a real input, not a double — asserts RUFF_NOT_FOUND with "Python interpreter not found" in the message.
• NEW test_ruff_checker_nonzero_exit_with_no_output_is_reported (line 336): covers the new RUFF_CHECK_ERROR branch with interpreter="/bin/false" — a real executable (coreutils), not a test-written stand-in and not pretending to be ruff; it is the subject precisely because it always exits non-zero with no output, which is exactly the SIGKILL/OOM shape lint.py's test drives by killing a real ruff child. Asserts an ERROR with RUFF_CHECK_ERROR and "exit code" in the message.
• TOOL_DID_NOT_RUN gains "RUFF_CHECK_ERROR", so the source-walking severity gate (test_tool_did_not_run_is_error_severity) pins the new code to ERROR too.
• Module docstring: the "PATH manipulation" standards line now documents the interpreter= seam instead (no PATH mutation remains in the file).

The red of the new tests against HEAD's code-review.py

Rule 11: fixed implementation saved to .q2bd/code-review.fixed.py, HEAD's version restored with git show HEAD:scripts/code-review.py, new tests kept. Run:

FAILED tests/scripts/test_code_review_tool.py::test_ruff_checker_file_not_found_when_ruff_missing_from_path
FAILED tests/scripts/test_code_review_tool.py::test_ruff_checker_interpreter_not_found
FAILED tests/scripts/test_code_review_tool.py::test_ruff_checker_nonzero_exit_with_no_output_is_reported
FAILED tests/scripts/test_code_review_tool.py::test_tool_did_not_run_is_error_severity[RUFF_CHECK_ERROR]
FAILED tests/scripts/test_code_review_tool.py::test_missing_ruff_fails_the_run
FAILED tests/scripts/test_code_review_tool.py::test_ruff_format_checker_file_not_found
6 failed, 76 passed, 1 warning in 12.41s

(hostlock rc=1 at 05:32:04). Fixed implementation then copied back; git diff --stat shows neither target file at HEAD. Quick module check after restore: 82 passed, 1 warning in 11.09s, [hostlock] 05:32:45 rc=0.

The forced run

env -u PYTHONPATH -u PYTEST_ADDOPTS bash .run_exclusive.sh --force --json tests/scripts/test_code_review_tool.py tests/harness/test_evaluator_lint.py tests/scripts/test_review_tools_cli.py tests/infrastructure

Affected-set note: scripts/affected_tests.py --json --files scripts/code-review.py tests/scripts/test_code_review_tool.py → "tests": ["tests/infrastructure", "tests/scripts/test_code_review_tool.py"], "escalate": false, "untested": ["scripts/code-review.py"] (D32 as predicted; the explicit module stays).

Result (stdout parsed from the first {, after the [freshness] lines):

{
  "test_totals": {"passed": 109, "failed": 0, "errors": 0, "skipped": 0},
  "reused": {"modules": 0, "tests": 0}
}

stderr tail: [run_exclusive] 05:45:35 run_tests.py rc=0, run rc=0. Green: 0 failed, 0 errors, 0 skipped, reused 0, rc 0.

Lint / gates

• ruff check scripts/code-review.py tests/scripts/test_code_review_tool.py → rc 0 ("All checks passed!")
• black --check both files → rc 0 ("2 files would be left unchanged.")
• pyright scripts/code-review.py tests/scripts/test_code_review_tool.py → rc 0 ("0 errors, 0 warnings, 0 informations")
• First gate.py run → INVALID, solely "fact base does not describe the working tree" for the two changed files; remediated per the gate's own instruction (uv run ast-sync then scripts/build_runtime_callgraph.py reaugment, rc 0, 8696 edges applied).
• gate.py run (re-run) → rc 0: gate: PASS — 6 rule clause(s), no violations (the ADR-0089 gate itself stays PASS).
• Staged: git add scripts/code-review.py tests/scripts/test_code_review_tool.py plans/briefs/qwen/Q2b-D-report.md
• gate.py review-staged → rc 0: commit review: PASS — 1 review(s), no blocking findings
• git diff --cached --stat:

 plans/briefs/qwen/Q2b-D-report.md      | 147 +++++++++++++++++++++++++++++++++
 scripts/code-review.py                 | 103 +++++++++++++++++------
 tests/scripts/test_code_review_tool.py | 113 ++++++++++++++++++-------
 3 files changed, 311 insertions(+), 52 deletions(-)

PENDING marker removed — gates below are final.

Related

• Plans
• Orchestrata Ultimate — project index
• Workflow
• Tasks