Skip to content

Explain selections: whole-suite rules, a why line, fuller console output - #8

Merged
baronunread merged 5 commits into
mainfrom
selection-explained
Sep 27, 2026
Merged

baronunread merged 5 commits into
mainfrom
selection-explained

Conversation

@baronunread

@baronunread baronunread commented Sep 26, 2026 •

Copy link
Copy Markdown
Owner

Acts on the feedback from leanest's first real run (rdyrct #263: 37/37 selected, with no way to tell why from the log).

  • Whole-suite rules, decided before the judge. A change to runner config (playwright/vitest/vite config, package.json, lockfiles, .github/workflows/) runs everything. A Markdown-only change skips everything. No judge call, so matrix shards can't disagree on these.
  • Why line, in the console and the PR comment, as a plain sentence: All 37 run because the runner setup changed (.github/workflows/test.yml). or 2 touch the change directly, the judge wasn't sure enough to skip 9 and it thinks 1 is affected.
  • Console: each RUN line carries its reason, and the RUN and Changed lists say ... and N more past 20.
  • inspect during a judge outage selects every test and gives the reason, instead of showing nothing selected.

Checked against a local clone of rdyrct:

  • PR #263's diff: 37/37 selected: All 37 run because the runner setup changed (.github/workflows/test.yml)..
  • A README-only diff: 0/37 selected: Skipping all 37 tests: only Markdown changed..
  • inspect with an unknown provider: ⚠ Judge unavailable (…), all 37 tests would run.

Not in this PR: keeping shards consistent when the judge does decide (select once, then shards consume the list). That changes how the Action is wired up, so it'll be a separate change.

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: d934e470-df82-4667-8383-daaf1ffb0cca


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.

❤️ Share

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

@github-actions

github-actions Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

leanest: 8 of 8 playwright test files selected

All 8 run because the runner setup changed (.github/workflows/ci.yml).

8 test files
Test Decision Reason
fixtures/action/price.pw.ts RUN runner setup changed (.github/workflows/ci.yml)
src/cli.test.ts RUN runner setup changed (.github/workflows/ci.yml)
src/import-graph.test.ts RUN runner setup changed (.github/workflows/ci.yml)
src/leanest.test.ts RUN runner setup changed (.github/workflows/ci.yml)
src/route-heuristic.test.ts RUN runner setup changed (.github/workflows/ci.yml)
src/runner.test.ts RUN runner setup changed (.github/workflows/ci.yml)
src/selection-policy.test.ts RUN runner setup changed (.github/workflows/ci.yml)
src/test-discovery.test.ts RUN runner setup changed (.github/workflows/ci.yml)

baronunread and others added 5 commits September 27, 2026 23:56
Feedback from leanest's first real run (rdyrct #263: 37/37 selected, with
no way to tell why from the log).

- Whole-suite rules, decided before the judge: a change to runner config
  (playwright/vitest/vite config, package.json, lockfiles, CI workflows)
  runs everything; a Markdown-only change skips everything. No judge call,
  so matrix shards can't disagree on these.
- "Why they run: N by rule, N judge unsure (c < 0.5), N judged affected."
  in the console and the PR comment.
- Console: each RUN line carries its reason, and the RUN and Changed lists
  say "... and N more" past 20.
- inspect during a judge outage selects every test and says why, instead
  of showing nothing selected.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
"37 by rule" read like a log line. Whole-suite rules now say their
reason once ("All 37 run because the runner setup changed (…)",
"Skipping all 37 tests: only Markdown changed.") instead of on every
test, and mixed runs get a sentence: "2 touch the change directly, the
judge wasn't sure enough to skip 9 and it thinks 1 is affected."

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Every row repeats the reason the sentence above already gives, so the
37 rows go into a collapsed section like the skipped ones.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The Markdown-only comment showed "0 of 37" and a collapsed table with
no sentence; it now reads "Nothing runs because only Markdown changed."

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
From a cold review of this PR:
- "Only Markdown changed" skipped every test, overriding the rules that
  force a test to run: a test importing the changed .md file was
  skipped. Those tests now run with their own reason; the rest skip.
- inspect never applied the whole-suite rules, so it could disagree with
  select and the real run. It now does, and says so.
- Under --shadow/--full the comment said "Nothing runs because…" and
  then "The full suite runs anyway". The sentence is left out there.

Adds an end-to-end test on a real git repo, which fails without the fix.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@baronunread
baronunread changed the base branch from action-sticky-comment to main September 27, 2026 21:57
@baronunread
baronunread merged commit 79774b5 into main Sep 27, 2026
3 checks passed
@baronunread
baronunread deleted the selection-explained branch September 27, 2026 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant