fix(devtools): let the corpus run write the testmon graph again - #4696
Conversation
The complete-corpus tier ran with `-p no:pytest-testmon` and no testmon flags, so it wrote no fingerprints and left the checkout datafile exactly as it found it. Combined with the provision step discarding an unusable seed at run start, a 58-minute corpus run ended with an empty .cache/testmon directory and every later affected run forced to reseed. The `all` tier now loads testmon and passes `--testmon-noselect`: it executes the whole collection and records what it traced. `descriptor` still opts out; its collection is a contract slice, not a corpus. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DNGJ3awfNrsLaMdHgQZvid
|
Important
This repository does not receive automatic reviews because it has fewer than 10 stars. ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Team Run ID: Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e9d823ca85
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| testmon = selection != "descriptor" | ||
| select_flag = "--testmon-noselect" if selection == "all" else "--testmon-forceselect" |
There was a problem hiding this comment.
When devtools verify --all runs at this head, it collects tests/unit/devtools/test_pytest_invocation.py::test_the_default_tier_selects_and_the_all_tier_drops_testmon; _command("all") now includes both --testmon and pytest-testmon, while that test explicitly asserts that both are absent. Consequently every complete-corpus verification finishes red after running the full suite unless the superseded contract test is updated alongside this behavior change.
AGENTS.md reference: AGENTS.md:L192-L193
Useful? React with 👍 / 👎.
1 participant
Summary
devtools verify --alltraces again, so a complete corpus run leaves a usable testmon datafile instead of an empty directory.Problem
Today's corpus run in
/realm/project/polylogue(18:47-19:45, 20,600 tests, 58 minutes) left.cache/testmon/present and empty._pytest_stepscomputedtestmon = selection not in {"all", "descriptor"}, so thealltier ran with-p no:pytest-testmonand no--testmonflags at all. A pytest session without the testmon plugin writes no fingerprints, so the run left the datafile exactly as it found it — and the provision step had already discarded the unusable seed at run start, leaving nothing behind. Every subsequentdevtools verifythen had to reseed by running everything.This contradicts the documented contract in
CLAUDE.md: "--allruns every test and still updates fingerprints."The tracing was removed by #4650 on the premise that "a complete-corpus run already executes every collected test, so loading testmon adds dependency tracing state without changing the selected corpus". True of that run's own selection, false of the graph it was the only thing producing. #4650's other half — a two-worker ceiling, the actual memory remedy — is no longer present (
CORPUS_MAX_WORKERS = 8), so only the removal survived.Mechanism found
Established by experiment, not inference. Running the real verifier argv against
tests/unit/devtools(739 tests) with testmon forced on:-n 2 --testmon-forceselect:SELECT count(*) FROM test_execution= 739, 1 environment row (polylogue).-n 8 --testmon-noselect: 739 rows, 1 environment row — with 12 tests failing in the session.So xdist width,
--testmon-noselect, and aFAILEDexit path all record correctly. Under xdist the controller saves incrementally frompytest_runtest_logreporton each teardown report, not once at session end.sync_db_fs_tests(retain=...)only inserts; it never prunes rows for uncollected tests, so a partial run cannot truncate the graph either.The absence had one cause: the plugin was not loaded.
Solution
_pytest_stepstraces for every tier whose collection is a corpus.allpasses--testmon-noselect(execute the whole collection, record what it traced);affectedkeeps--testmon-forceselect.descriptorstill opts out — it collects a contract slice, so its fingerprints would describe a collection no later run has.Verification
devtools test tests/unit/devtools/test_verify.py -k "traces_and_deselects or records_a_usable_testmon_graph" -n 0:2 passed.testmon = selection != "descriptor"to the shipped expression:2 failed—assert 'pytest-testmon' in command, and the executing test's session reports2 passedwhileassert datafile.exists()is False. That is this defect reproduced in miniature.devtools test tests/unit/devtools/test_verify.py tests/unit/devtools/test_run_tests.py -n 2:8 failed, 65 passed. The same 8 fail onmasterwith the change stashed (8 failed, 63 passed) — seven are thebuild_pytest_cmdworker-cap tests reacting to the agent cgroup this lane runs in, one is a pre-existing descriptor red. The two added tests are the entire delta.devtools verify --quick: all 13 gates ok, includingtestmon-selection.Residual risk
An earlier lane datafile holding only 53 rows is not explained by this change and was not reproduced here; the experiments above rule out xdist,
--testmon-noselect, and failing exits as its cause, leaving an interrupted session as the likeliest remainder. The corpus was not re-run end to end in this lane — that thealltier now records is proven at 739 tests, not at 20,600. #4650's memory observation stands on its own: tracing at eight workers has a cost, and the two-worker ceiling it added is already gone frommasterindependently of this change.🤖 Generated with Claude Code
https://claude.ai/code/session_01DNGJ3awfNrsLaMdHgQZvid