Repository navigation
🤖 feat: mutation gate (make mutate-check, Quality / Mutation) on unclebob/mutator - #7148
Conversation
…ob/mutator Runs mutator's mutation sites in the changed functions against the Bun unit tests that import the file (colocated first, nearest next, at most 10), each test file in its own process, in private copies of the tracked files. Warn-only. CI reuses the Test / Unit LCOV to skip uncovered sites and stops at 300 mutants or 15 minutes.
…RLF-safe offsets, PASS with not-run)
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. |
ThomasK33
changed the title
🟢 e2e stories (Haiku): 8 passed21 agent steps · 7 replayed from cache · 24 model calls · 196.0k tokens (74% cached) All 8 tests in 8 files
e2e 0.20.0 · 2m 35s · web · run artifacts 🟢 e2e Electron stories (Haiku): 3 passed3 agent steps · 3 model calls · 8.0k tokens (32% cached) All 3 tests in 3 files
e2e 0.20.0 · 21.1s · electron · run artifacts 🟢 e2e agent repros (Haiku): no tests selectedSpend: $0.00 of the $1.00 cap (proxy ledger) e2e 0.20.0 · 6.4s · 2 targets (web, phone) · run artifacts |
![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: 93f248ff2a
ℹ️ 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".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1011048724
ℹ️ 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".
… baseline deadline) Review round 2 on #7148: - The import graph parses each test with tree-sitter, so a type-only import, a comment or a string fixture is no importer (1,523 of 7,842 edges on the repo were phantoms). - A site that a no-coverage test (test-unit-ci.sh --list-no-coverage) can reach is not skipped as uncovered. - The mutant cap applies before baselines, and a baseline gets only the time left before the run's deadline. - node_modules is a directory link, and a timeout kills bun on Windows.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b575414ad2
ℹ️ 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".
…ummary) Review round 3 on #7148: - mutate-check refuses Windows up front with a tool error: run_test kills process groups and the workers run bash helpers. This replaces the partial Windows fallbacks from round 2. - The CI summary lists each error row with its path, line and reason.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 991dfe6641
ℹ️ 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".
…workers Review round 4 on #7148: the None slot of a retired worker let an idle thread mark the pending sites as errors faster than the healthy workers ran them. Now None enters the queue only when the last worker retires.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 72be907090
ℹ️ 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".
…ery imports Review round 5 on #7148: @shared/, xum/ and mux/ (tsconfig paths, all mapped to src/) now resolve like @/, and a `?real=1` suffix names the same file.
|
The maintainer approved one more review round past the cap (Option A). The next push fixes the open #7147 finding (a function rewritten line for line keeps its DRY base form) and merges it up the stack. A finding in this round stops the loop. |
|
The maintainer approved one manual review request past the cap. A finding stops the loop. |
|
@codex review |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3f76266d3
ℹ️ 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".
|
Review loop stopped. The approved manual round on e3f7626 found one new P2: with full CI coverage, a site whose anchor line has zero hits is skipped as Generated with |
# Conflicts: # docs/AGENTS.md # src/node/services/agentSkills/builtInSkillContent.generated.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3273eb5692
ℹ️ 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".
…oder#7147) ## Summary Adds a DRY gate next to the CRAP gate. `make dry-check` (and a new warn-only `Quality / DRY` CI job) reports near-duplicate functions that a change **creates**. It compares each changed function with every production function in `src/`, using structural fingerprints from Uncle Bob's [dryer](https://github.com/unclebob/dryer). A pair that already matched on main stays quiet. Stack 1 of 3. coder#7148 adds the mutation gate, and coder#7149 adds accept markers and an opt-in block mode. ## Background Uncle Bob's "gauntlet" runs three tools: crapper (complexity × coverage), dryer (duplication) and mutator (mutation testing). Xum already runs Crapper (`scripts/crap/`). This PR adds dryer, and coder#7148 adds mutator. ## Implementation - `scripts/dry/dryer_report.py` (uv script, pins dryer `66ff6d2`, plus the Crapper and grammar pins that CRAP uses). Upstream `dryer` compares every function with every other one, and that run did not finish in 15 minutes on `src/` (about 14,000 functions). This script compares only the changed functions with all the others. Extraction takes about 19 s of CPU, plus about 0.13 s per changed function. - **New-pair rule:** a pair counts only when the diff made it. The script finds each changed function's base version through git's `-U0` hunks, not by name. A function made only of added lines has no base version. Matching by name hid a new copy that shared a name with an old match, for example two `render` methods. Regression test: `a new copy that shares a name with an old match is a finding`. - `scripts/crap/changes.ts`: the diff code moves out of `crap/check.ts` so both gates agree on what "changed" means. CRAP behavior does not change: its 61 script tests pass unchanged. - `Makefile`: `dry-check`, `test-dry-scripts` (in `static-check-full`), and a shared `tree-sitter-grammars` download step with retries. - `.github/workflows/pr.yml`: `Quality / DRY`. It needs only `changes`, so it starts at once. Every step is soft. `Required` does not list it. - `.xum/skills/gauntlet/SKILL.md` and one AGENTS.md line: how to read and repair findings. ## Validation On the real repo: - I copied `centsBucket` as `feeBucket` in `workspaceGoalService.ts`. `make dry-check` reported 2 warnings at score 1.00, with exit 1. - I edited only `centsBucket`. The check passed, because that pair is old. I confirmed that the new-pair test and the same-name test fail on the old rule and pass on the new one. A 200-function sample at the 0.82 threshold found that 7.5% of functions have a near-twin. ## Risks Low. The gate only warns and lives in `scripts/`. The CRAP refactor is the one shared path, and its tests cover it. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$12.44`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=12.44 -->
…oder#7149) ## Summary Adds accept markers and an opt-in block mode to the DRY and mutation gates. - `// dry-duplicate-ok: <reason>` in a changed function, or in the comment lines right above it, accepts a duplicate pair. - `// mutation-equivalent: <reason>` on a mutant's line, or right above it, accepts a surviving mutant as equivalent. - `DRY_MODE=block` and `MUTATION_MODE=block` (repo variables, default warn) make unaccepted findings fail the job. Stack 3 of 3, on coder#7148. ## Background Some findings are correct but on purpose. Two parsers can share a shape and still need to change apart, and a mutant like `m[2] ?? ""` to `m[2] || ""` is equivalent when `m[2]` is never `""`. Without a way to say so next to the code, neither gate can block. ## Implementation - `scripts/crap/markers.py`: one shared parser. A marker needs a reason. A DRY marker counts only on a changed side, because a marker on an old function would excuse every future copy of it. A mutation marker affects only survivors: a killed mutant stays killed. - **DRY block mode:** `Quality / DRY` gets a Verdict step and joins `Required`, filtered by `DRY_MODE`, the same way `crap` is filtered by `CRAP_MODE`. The job starts at once and takes a few minutes, so it adds no time to `Required`. - **Mutation block mode:** fails `Quality / Mutation` but stays out of `Required`. The job starts after Test / Unit, which is the last job that `Required` waits for (about 9.5 min). Listing it would add its whole run, up to 15 minutes, to every PR. To gate merges on it later, add it to `Required`'s `needs` and accept that wait. - A tool error, a timeout, a baseline failure, an `error` row or a mutant that did not run never blocks. ## Validation - I marked the real equivalent mutant at `budgetParser.ts:42`. `make mutate-check` reported `PASS, 7 killed, 1 equivalent`, with the reason. - End-to-end tests cover each case: a DRY marker accepts both pairs, a marker on the old side excuses nothing, and a mutation marker turns a survivor into `equivalent`. - I ran the `Required` jq filter by hand: `dry` drops out in warn mode and counts in block mode. ## Risks None until a maintainer sets a mode variable to `block`. Rollback is setting it back to `warn`. --- _Generated with `xum` • Model: `anthropic:claude-opus-5-5` • Thinking: `high` • Cost: `$12.44`_ <!-- mux-attribution: model=anthropic:claude-opus-5-5 thinking=high costs=12.44 -->
1 participant
Summary
Adds a mutation gate.
make mutate-check(and a warn-onlyQuality / MutationCI job) makes small changes ("mutants") in the functions a PR changed, such as===to!==,>to>=or??to||. It then lists the mutants that no unit test catches. The mutation sites come from Uncle Bob's mutator. Xum runs the mutants itself.Stack 2 of 3, on #7147.
Background
A mutant that survives means no test checks that behavior. The first run found a real gap:
parseGoalTurnCapInputhad no direct test, and all 5 of its mutants survived. #7151 adds that test.Implementation
mutatorcommand fails on this repo with ENAMETOOLONG, because its import scan reads a string in a source file as a path. It also follows only relative imports, so with Xum's@/imports a mutant can run against the original file.scripts/mutate/mutate_report.pyuses mutator'sdiscover_raw/apply_sitewith UTF-8 byte offsets and runs them in its own workers.node_moduleslinked andsrc/version.tsgenerated. The real checkout is never touched.bun testprocess. Several Xum test files in one process break each other on shared temp config. A failing file or a timeout (10× the file's baseline, at least 2 s) kills the mutant. The runner kills the whole process group.errorrow. If the crash left the worker's copy mutated, the runner retires that worker.not-run.Requireddoes not list the job.Validation
On the real repo:
parseGoalTurnCapInput. Exactly its 5 mutants survived, with exit 1.BASELINE FAILED, with exit 3.agentMessageEnvelope.ts, which 10 unit tests import. Its 4 mutants were killed in 54 s at a host load average of about 200.The end-to-end tests in throwaway repos run real
bunprocesses, including an@/import kill. I confirmed that the crash tests and the survivor test fail when the code under test is broken.Risks
Low for merges, because the job only warns and
Requireddoes not wait for it. The job adds runner minutes on every PR that changessrc/or CI config.Generated with
xum• Model:anthropic:claude-opus-5-5• Thinking:high• Cost:$12.44