[qinghuanandejiangshi]

Python test modules received run="python3 ..." even when importing the file runs no tests or fails without Django settings. Run hints now require a top-level main guard for direct Python execution, or pytest configuration between the test directory and its crawl root. Modules without that evidence retain the unknown-runner disclosure.

Python rows use only their own evidence: an unknown module cannot borrow a sibling pytest/main-guard command, a same-stem shell script, or a script that mentions a matching substring. Non-script files retain the existing same-root fallback. Candidate commands are cached, and the parsers distinguish executable guards and configuration sections from documentation examples.

Closes #229.

Validation on macOS at final head 1c6ebbf4:

• The added ownership regressions fail on the previous behavior. Dev and G1 sanitizer runhintcheck.sh pass, covering test-gate JSON, affected XML, situ text, positive runner controls, unknown runners, root boundaries, and hostile paths.
• Full suite after merging main f57a9df3: 635 gates, 632 passed, 3 conditional skips, 0 failures, tree_writes=0 (1949.9 seconds). Both GOLD_BIN and RIPWIRE_HEADBIN point to a clean binary built from exact HEAD 1c6ebbf4.
• The skips are optional old-reference comparisons (editchecknotecheck, argvdiffcheck) and g1freshcheck, which looks specifically for an in-tree asan/ripwire. The sanitizer build here used a separate build directory.
• G1 sanitizer runner checks and a full-repository --no-cache scan pass; scan stderr is empty and XML is valid. LeakSanitizer is unavailable on this Apple arm64 runtime, so detect_leaks=0 was used; this is not a leak-check claim.
• Four dev scans using --no-cache are byte-identical across all three comparisons, and their XML is valid. The full suite's cppqual/format gates pass.
• The final merge reconciles the readWholeFile count to 22 (main's 19 plus the three Python evidence readers). The functional correction, regressions, and acknowledgements are unchanged from c2dc4472.
• Only the changed TestRunnerIndex churn acknowledgement was re-healed with --quality-ack --scope=src/testmap.h --ack-only=gating. The explicit whole-PR quality comparison f57a9df3..1c6ebbf4 reports 0 gating findings, 1 acknowledged finding, and 1 non-gating new-symbol complexity finding (hasPytestSection: 16, threshold 15).

Known follow-ups: additional pytest configuration formats remain unknown, and pytest's -p pre-parser needs separate leading-dash path handling. Those optional review items are not included in this correction.

[coderabbitai]

Note

Reviews paused

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 9d7b2212-668a-4650-8ec0-e3e36c391989

📥 Commits

Reviewing files that changed from the base of the PR and between 2d2b10f and 6995356.

📒 Files selected for processing (3)

• src/pythonrunner.h
• src/testmap.h
• test/runhint_python.py

🚧 Files skipped from review as they are similar to previous changes (3)

• src/testmap.h
• test/runhint_python.py
• src/pythonrunner.h

Included review availability: Your plan provides up to 8 included reviews per hour; 7 remain after this review.

---

📝 Walkthrough

Walkthrough

The change adds evidence-based Python runner detection, root-scoped matching, command caching, safe path handling, and end-to-end tests for pytest, main guards, fallback runners, and isolated project roots.

Changes

Python runner detection

Layer / File(s)	Summary
Python evidence parsing src/pythonrunner.h, test/cppqualcheck.sh	Adds parsing for Python main guards and pytest configuration. The qualification gate updates the expected whole-file read count.
Root-scoped runner selection src/testmap.h	Uses main-guard and pytest-project evidence, restricts candidates to matching roots, skips candidates without commands, caches generated commands, and places -- before unsafe paths.
Runner hint validation test/runhint_python.py, test/runhintcheck.sh, .ripwire_quality_acks	Adds end-to-end coverage for Python runner hints, root isolation, configuration parsing, unsafe paths, and shell-gate integration. Updates quality acknowledgments.

Priority: ➖ Normal

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant TestRunnerIndex
  participant PythonEvidence
  participant ProjectRoot
  participant RunnerHintTests
  TestRunnerIndex->>PythonEvidence: inspect source and pytest evidence
  PythonEvidence->>ProjectRoot: read configuration within crawl root
  ProjectRoot-->>PythonEvidence: return matching project evidence
  PythonEvidence-->>TestRunnerIndex: return validated runner
  TestRunnerIndex-->>RunnerHintTests: emit runner hint
  RunnerHintTests->>TestRunnerIndex: verify roots, commands, and unsafe paths

Loading

Merge Risk: ⚪ Minimal · up to 69953

The runner-hint paths have no identified merge-blocking issue.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 18 functions across 5 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	The PR satisfies the coding requirements in [#229]. src/pythonrunner.h recognizes only top-level main guards and valid pytest configuration. src/testmap.h emits pytest <path> only when configura…
Out of Scope Changes check	✅ Passed	The changes remain within [#229]. The parser helpers implement runner evidence detection. The command cache, crawl-root checks, quoting, option termination, and fallback handling prevent unsafe or mis…
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the main change: preventing Python modules from being incorrectly offered as runnable test scripts.

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands.}

[joyful-ii-V-I]

Thanks @qinghuanandejiangshi. This makes run= trustworthy on Python repos: an agent in a Django/pytest tree now gets pytest <path> or an honest run_unknown instead of a python3 <module> that dies with ImproperlyConfigured. The negative fixtures are exactly the cases that matter: guards inside docstrings, comments and nested defs; [tool.pytest.ini_options] inside a TOML string; [tool:pytest] on a setup.cfg continuation line; config above the crawl root; and multi-root isolation. Giving runhintcheck arm (6)'s dash runner a real main guard, so that arm stays live, was a careful catch.

Must fix before merge

1. A .py row with no evidence now falls through to the stem/mention fallback in derive() (src/testmap.h:612-617), which it never reached before. The stem pass then picks a different file with the same basename. Take a/pytest.ini, a/tests/test_api.py and b/tests/test_api.py: the row for b/tests/test_api.py gets run="pytest a/tests/test_api.py", which passes without running the changed file. The mention pass is a substring match, so a Django app/tests.py also matches any script that says runtests.py. It also sends every evidence-less Python row through a scan of all runner texts, which grows as rows × bytes on big unittest/Django repos. Suggested fix: for a self-runnable kind, return spell( fileId ); even when empty, as before this PR (or at least skip same-extension candidates in the stem pass). Please also add a runhint_python.py arm: same basename in two directories, only one with evidence, and the other row must read run_unknown.

Optional, fine as a follow-up

• pytest's -p plugin pre-parse ignores -- (checked on pytest 9.1.1), so a file named -pfoo_test.py gives "Error importing plugin" instead of running the test. Spelling a dash-leading path as ./-… fixes that for bash, python3 and pytest; otherwise narrow the comment at testmap.h:737.
• pytest also reads .pytest.ini, pytest.toml/.pytest.toml, tox.ini [pytest] and [tool.pytest] (pytest ≥ 9). For now those projects honestly read run_unknown.
• If the fix moves TestRunnerIndex's measured size or churn, re-heal .ripwire_quality_acks with the binary (--quality-ack), not by hand.

CodeRabbit comments you can safely ignore

• Docstring Coverage (66.67% < 80%): a generic CodeRabbit threshold, not a project rule. The /// contracts you added are enough.

Queue: this PR is 3rd in landing order — #231 → #237 → #236 → #235 + #233 (reconciled together) → #224 → #44 (Windows, being refactored).

[joyful-ii-V-I]

CI note for @qinghuanandejiangshi: 69953560 is 31/31 green. The one remaining item is the must-fix in the review above: a .py row with no evidence falls through to another file's runner, e.g. the b/tests/test_api.py row borrowing pytest a/tests/test_api.py. Once that's pushed with its arm, this can merge. #237 landed earlier today. Thank you!

[joyful-ii-V-I]

@coderabbitai pause

[coderabbitai]

✅ Action performed

Reviews paused.

[qinghuanandejiangshi]

Fixed the must-fix in c2dc447: self-runnable kinds now return their own spell(fileId) result even when empty. The new regressions cover sibling pytest/main-guard files, a same-stem shell runner, and substring-only script mentions; the unknown Python row stays unknown while the positive controls still have commands.

Also merged current main and reconciled the readWholeFile count separately in 87ba372, then refreshed the churn acknowledgement using the binary. Dev/ASan runhint gates, a cold ASan repository scan, and three deterministic comparisons passed. The full 634-gate run plus permission-only reruns finished with 631 passing and 3 conditional skips; details and the sanitizer limitation are in the updated PR description. The optional pytest follow-ups remain separate.

[qinghuanandejiangshi]

The required Python runner ownership correction is in c2dc4472. Main advanced during validation, so I also merged f57a9df3 in 1c6ebbf4 and resolved the cppqual reader-count conflict (19 upstream + 3 Python readers = 22). The functional patch, regression cases, and acknowledgements are unchanged.

Rebuilt and reran the full suite on 1c6ebbf4: 635 gates, 632 passed, 3 conditional skips, 0 failures, tree_writes=0. The skips are the two optional old-reference comparisons and the in-tree sanitizer freshness check; the separate G1 sanitizer build, runner regressions, and cold full-repository scan pass. Four cold dev scans are byte-identical and valid XML. Apple arm64 LeakSanitizer remains unavailable (detect_leaks=0).

The whole-PR quality comparison against f57a9df3 has no gating findings; the remaining non-gating new-symbol complexity finding is disclosed in the updated PR description. The latest fetched main (316dbf26) also merges without conflicts.

[joyful-ii-V-I]

This PR is part of integration train 1b, #277, and will show as merged when that train lands.