Skip to content

perf(ci): typecheck only the packages a pull request affects - #28820

Open
only21mil wants to merge 1 commit into
pingdotgg:mainfrom
only21mil:fix/audit-m208
Open

only21mil wants to merge 1 commit into
pingdotgg:mainfrom
only21mil:fix/audit-m208

Conversation

@only21mil

Copy link
Copy Markdown
Contributor

Problem

CI's Build job typechecks all 39 workspace packages on every pull request, even when changes has already worked out that only one or two can be affected. Fork pull requests can't read the shared task cache (setup-ci-deps turns it on only for same-repo branches), so each of those typechecks runs cold. 95 of the last 100 PR CI runs came from forks. Five recent fork runs each logged vp run: 0/39 cache hit (0%) for changes that affected 1 or 2 packages. When the desktop build is skipped, that typecheck is the job's critical path (17–19 s in those runs).

Change

  • ci-changes.cjs gets a typecheck_packages output: the affected packages, or empty when every package is affected. This is the same convention test_packages uses. The Start typecheck step turns it into --filter arguments, as the Test job already does. With nothing listed, it typechecks every package (-r).

  • The step now runs check:changed's typecheck command, vp run --cache --parallel --concurrency-limit 4 … typecheck. tsc --noEmit writes nothing another package reads, so dependency ordering only made dependents wait.

  • Pushes to main still pass --full. A failed diff still falls back to every job. A manifest, lockfile, root-file or shared-setup change still selects every package. If changes produces no output, the list is empty, which means a full typecheck.

  • To make sure the selected set is complete, I compared every file vp recorded as a typecheck input against the packages decide() selects for a change to that file. Two reads were missing, and both now go through the existing tables:

    • apps/server typechecks the root vite.config.ts, which imports infra/task-cache/src/remoteCache.ts (EXTRA_OWNERS).
    • Two server tests import packages/client-runtime/src/state/orchestrationV2Projection.ts by relative path (TEST_ONLY_DEPS).

    Because ci-changes.cjs has one affected set, a client-runtime change now also runs the server tests that import it, which main skips today. A remoteCache.ts change now also runs the server's build and tests.

Scope and approval

No issue or discussion. This is CI scheduling maintenance with no product behavior change. The selection logic already exists: #19587 built the affected graph, and check:changed already applies it to local typechecks. This PR points the CI typecheck at the same set and the same command. Main keeps its full run as the backstop, and release workflows are unchanged (release.yml still runs vp run typecheck).

For maintainers to confirm: a PR now typechecks only what ci-changes.cjs says it can affect. Any read the graph misses is caught by main's full run, not by the PR. The same is already true of the Test job and of check:changed.

Verification

  • vp test run scripts/check-changed.test.ts: 9/9 pass. The new case fails on main and covers a leaf, dependents, both added graph rows, shared inputs, diff failure and --full.

  • vp run check:changed 3fd2b855c5c6b9169cdfe3733b34011c1c5ed4b7: format, lint, knip, all 39 typechecks and both test files pass.

  • The recorded-input audit covers 38 cached tasks plus the scripts compiler's files. Its four remaining reported gaps are directory reads and an absent resolution probe.

  • Shell simulations ran 1, 10 and 39 tasks for leaf, provider-grok and lockfile changes; all passed.

  • Cold timings on one 32-core machine, concurrency 4, cache disabled, three interleaved runs:

    Change Tasks Old command New command
    Full graph 39 11.9–12.9 s 8.7–9.0 s
    provider-grok 10 11.9–12.9 s 6.5–7.3 s
    mobile 2 11.9–12.9 s 1.6–1.7 s
    oxlint plugin 1 11.9–12.9 s 0.3 s
  • With a warm local cache, full and leaf runs took about 0.7 s and 0.2 s.

Not checked:

  • A filtered run in real CI: this PR's .github/ changes select the full graph. Historical CI timings use the old command; new-command timings are local.
  • Same-repo CI with the remote cache.

Prior PRs

Generated with Claude Opus 5.5 and GPT-6.1 Terra and GPT-6.1 Sol in T3 Code Nightly

The Build job typechecked all 39 workspace packages on every pull request,
in dependency order, even when ci-changes.cjs had already found a smaller
affected set. Fork pull requests cannot read the shared task cache, so each
of those checks ran cold.

Pass the affected set to the typecheck step, as the Test job already does,
and run it with check:changed's command, which drops dependency ordering
that tsc --noEmit does not need. Pushes to main and changes that reach every
package still typecheck everything. Add two reads the server's typecheck and
tests make without a declared dependency, so the set stays complete.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Nov 1, 2026
@macroscopeapp

macroscopeapp Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at 6103c99

Macroscope's review found this PR approvable — CI typechecks only affected packages and their dependents, aligning with the existing local check command. Full-run fallbacks and full checks on main remain intact, with no product behavior or diagnostic-suppression changes.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 3fd2b85 and 6103c99.

Files selected for processing: 5
  • .github/actions/ci-changes/action.yml
  • .github/scripts/ci-changes.cjs
  • .github/scripts/ci-changes.d.cts
  • .github/workflows/ci.yml
  • scripts/check-changed.test.ts

Limit details: You’ve used all 10 included reviews currently available.

Walkthrough

The CI change detector now returns a package list for typechecking. The workflow passes that list to the build job, which selects package filters when the list is nonempty and runs recursive typechecking when it is empty.

Changes: CI Typecheck Selection

Layer / File(s) Summary
Determine and expose affected packages
.github/scripts/ci-changes.cjs, .github/scripts/ci-changes.d.cts, .github/actions/ci-changes/action.yml, scripts/check-changed.test.ts
The change detector updates package ownership and dependency rules, returns affected package names as typecheck_packages, and tests package selection for changed packages, server-read files, shared inputs, and full runs.
Pass package selection to typechecking
.github/workflows/ci.yml
The changes job exposes typecheck_packages. The build job runs typechecks with a filter for each listed package, or uses -r when the list is empty. It runs the command through vp run --cache --parallel --concurrency-limit 4.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Sequence Diagram(s)

sequenceDiagram
  participant decide as decide
  participant detect as detect step
  participant changes as changes job
  participant build as build job
  participant vp as vp
  detect->>decide: calculate affected package names
  decide-->>detect: return typecheck_packages
  detect-->>changes: expose typecheck_packages
  changes-->>build: pass package list
  build->>vp: run filtered typechecks or -r
Loading

Suggested reviewers: t3dotgg, juliusmarminge

Merge Risk: Minimal · up to 6103c

Affected packages are typechecked on pull requests, with full checks for global changes. No merge-blocking issue was established.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title clearly and concisely describes the main change: limiting CI typechecks to affected packages.
Description check Passed The description includes complete Problem, Change, Scope and approval, and Verification sections. It explains the behavior, scope rationale, tests, measured results, and unchecked scenarios.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Finishing Touches
Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Usage-based review receipt

Note

This review was completed with usage-based billing: files reviewed beyond your plan's included limits are billed at $0.25/file. View usage-based billing.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). vouch:unvouched PR author is not yet trusted in the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant