Skip to content

ci(perf): extend scoped test selection to non-engine, non-config backend PRs - #7709

Merged
JSONbored merged 1 commit into
mainfrom
ci-scope-backend-changed-tests
Jul 21, 2026
Merged

ci(perf): extend scoped test selection to non-engine, non-config backend PRs#7709
JSONbored merged 1 commit into
mainfrom
ci-scope-backend-changed-tests

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • validate-tests' scoped vitest --changed fast path previously only applied to PRs confined to miner/mcp/discoveryIndex. Any PR touching src/** -- the majority of real contributor work -- always ran the full ~2,900+-test suite across all 3 shards, unconditionally, regardless of diff size.
  • Verified this extension empirically before shipping it, given this repo already has a real precedent for a scoping mistake (chore(miner): migrate the CLI bin entry-points to TypeScript #7318-chore(miner): migrate batch 4.6 foundational lib modules to TypeScript (#7314) #7321): replayed 5 real historical single/few-file src/**-only commits spanning orb, review, api, db, and selfhost, with the target commit actually checked out (caught and fixed a methodology bug mid-analysis where an earlier pass left the worktree on main, producing a bogus 66%-selection result) and --changed run against its real parent.
    • Zero false negatives across all 5 samples -- every direct dependent test file was correctly selected every time.
    • Selection size scaled sensibly with how foundational the changed file was: a leaf module selected ~0.1% of the suite; the shared DB repositories layer selected ~31%. Always less than the unscoped 100%, never more.
    • The worst-case stress test (src/server.ts, the main entrypoint) correctly selected zero tests -- the diff sat entirely inside a /* v8 ignore */-marked block with no real test dependents, so zero was the right answer, not a miss.
  • Added a new backendConfig path-filter: the subset of the existing backend filter that this verification did NOT cover and that --changed's import-graph tracing structurally can't reach anyway -- migrations, package.json/tsconfig*.json/vitest*.config.ts, and workflow/tooling files have no import-graph edge a test file could ever have to them. A PR touching any of those still gets the full unscoped suite even if it also touches src/**.
  • engine and rees stay excluded exactly as before -- engine's dist/ package-boundary issue is a separately-documented, previously-reproduced real miss, unrelated to this verification.
  • Confirmed live that combining --changed with an explicit positional filename narrows (intersects) the selection rather than unioning it -- so test/unit/mcp-output-schemas.test.ts (the one deliberately wide-fan-out file) is not force-included via that composition; it relies on --changed's own graph tracing, which this verification confirmed is reliable for direct same-tree imports (its exposure is via src/mcp/server.ts, a plain in-repo import chain, not a package boundary like engine's).

Safety net (unchanged)

  • Every push to main still runs the full unscoped suite unconditionally -- a scoped miss in an untested corner is caught within minutes of merge, not never.
  • Kill switch unchanged: the SCOPED_TEST_SELECTION_ENABLED repo variable set to "false" forces the full suite for every PR without a revert PR.

Test plan

  • python3 -c "import yaml; yaml.safe_load(...)" -- ci.yml parses as valid YAML.
  • git diff --stat -- single-file change, all edits confined to the changes and validate-tests jobs' scoping logic and its filter inputs.
  • Verification methodology and raw results described above were run against real historical commits on this branch's base, not synthetic examples.

…end PRs

validate-tests' scoped `vitest --changed` fast path previously only applied to
PRs confined to miner/mcp/discoveryIndex -- any PR touching src/** always ran
the full ~2,900+-test suite across all shards, regardless of diff size, even
though most real contributor PRs touch src/**.

Verified before extending it, not by assumption: replayed 5 real historical
single/few-file src/**-only commits (spanning orb, review, api, db, selfhost)
with the target commit actually checked out and --changed run against its
parent. Zero false negatives across all 5 -- every direct dependent test file
was correctly selected. Selection size scaled with how foundational the file
was (a leaf module: ~0.1% of the suite; the shared DB repositories layer:
~31%), always less than the unscoped 100%.

Added a new `backendConfig` path-filter capturing the subset of `backend`
that verification didn't cover and --changed structurally can't trace anyway:
migrations, package.json/tsconfig/vitest.config, and workflow/tooling files
have no import-graph edge a test file could have to them. A PR touching any
of those still gets the full unscoped suite even if it also touches src/**.
engine and rees stay excluded as before -- engine's dist/ package-boundary
issue is a separately-documented, previously-reproduced miss, unrelated to
this verification.

Every push to main still runs the full unscoped suite unconditionally, so a
scoped miss (if one exists in an untested corner) is caught within minutes of
merge, not never. Kill switch unchanged: SCOPED_TEST_SELECTION_ENABLED=false
repo variable forces the full suite for every PR.
@JSONbored JSONbored self-assigned this Jul 21, 2026
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.56%. Comparing base (3a9450b) to head (0078fe3).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7709   +/-   ##
=======================================
  Coverage   88.56%   88.56%           
=======================================
  Files         725      725           
  Lines       76266    76266           
  Branches    22705    22705           
=======================================
  Hits        67544    67544           
  Misses       7680     7680           
  Partials     1042     1042           
Flag Coverage Δ
rees 88.56% <ø> (ø)
shard-1 52.84% <ø> (ø)
shard-2 51.95% <ø> (ø)
shard-3 48.47% <ø> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

@JSONbored
JSONbored merged commit 0fd0c9b into main Jul 21, 2026
14 checks passed
@JSONbored
JSONbored deleted the ci-scope-backend-changed-tests branch July 21, 2026 10:26
JSONbored added a commit that referenced this pull request Jul 21, 2026
…rigin/main (#7728)

The "Test with coverage" step's scoped-selection branch (#7709) re-fetched
origin/main with `git fetch --depth=1 origin main`, reasoning it was a cheap,
idempotent guarantee on top of this job's own fetch-depth: 0 checkout. It
wasn't idempotent: a --depth=1 fetch of a ref that already has full history
locally re-shallows that ref specifically, cutting its reachable history at
the single fetched commit. vitest's --changed=origin/main needs `git
merge-base <head>...origin/main` to resolve, and once origin/main was
re-shallowed with no history shared with a PR branch forked further back,
merge-base returned nothing -- vitest silently fell back to "no changed
files" and every shard printed "No test files found, exiting with code 0",
failing the "Verify coverage report exists" step on every scoped PR,
contributor and maintainer alike, for any PR whose branch point wasn't the
single most recent commit on main.

Reproduced directly: a fresh, deliberately shallow clone confirms merge-base
fails after `git fetch --depth=1 origin main` and succeeds once that ref has
real history -- confirmed both via a full unshallow and via this fix's
replacement fetch (same fetch, --depth dropped), which stays genuinely
idempotent on top of the checkout step's already-full history instead of
re-shallowing it.
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