Skip to content

ci: balance server test shards by splitting the largest files - #12

Closed
ashutoshpw wants to merge 1 commit into
ci/path-gate-desktop-buildfrom
ci/balance-server-shards
Closed

ashutoshpw wants to merge 1 commit into
ci/path-gate-desktop-buildfrom
ci/balance-server-shards

Conversation

@ashutoshpw

Copy link
Copy Markdown
Owner

Problem

Test Server shards are unbalanced because Vitest partitions by sha1(path) contiguous slices, never by duration. From CI-sourced per-file timings (run 35655900272, 338 files), file-time totals were 35.7s / 40.5s / 46.3s and job walls 193 / 226 / 217s — the slowest shard is ~17% above the mean, so it holds up the suite.

Fix

Split the four dominant test files into coherent groups, preserving every test, assertion, and order (structure-preserving moves only):

File CI time New file
src/git/GitManager.test.ts 14,164ms src/git/GitManager.http.test.ts
src/provider/Layers/GrokAdapter.test.ts 8,386ms src/provider/Layers/GrokAdapter.sessions.test.ts
src/server.test.ts 8,206ms src/server.sessions.test.ts
src/serviceLauncher.test.ts 6,150ms src/serviceLauncher.core.test.ts

The three-shard simulation (implementing Vitest's exact algorithm — sha1 of "/" + path-relative-to-apps/server, sorted, contiguous ranges) predicts 39.9 / 43.3 / 39.3s, max/mean 1.06. Four shards were simulated and rejected: they add runner minutes and buy nothing while the Test job (~424s) is the critical path.

Evidence

  • Partition simulation validated file-by-file with 0 mismatches against run 35655900272.
  • Full local suite before/after: 5,281 tests; same 24 pre-existing environment-only Git/worktree failures, no new failures.
  • No snapshots in apps/server; none needed moving.
  • The ci.yml comment now reflects the real resolved count (338 files, was 239).

Diff note: the ~13k inserted lines are moved test bodies, not edits — review by test names, not line counts.

Stack

#9 (ci: batch short jobs) ← #11 (ci: skip the desktop build on documentation-only changes) ← this PR: ci: balance server test shards

Merge in order; after the previous layers merge, this PR is retargeted to main.

Manual Testing Guide

  1. Reproduce the simulation: take the ✓ <file> (N tests) <ms> lines from the Test Server 1/2/3 job logs of run 35655900272, apply the sha1 partition described above, and confirm the three-shard totals land near 39,890 / 43,332 / 39,278 ms with the four split files' halves distributed as simulated.
  2. On the CI run for this PR: gh run view <id> --repo ashutoshpw/t2code --json jobs — confirm Test Server 1/2/3 all pass with a tight spread (baseline 193/226/217s; expect a visibly smaller max-min gap).
  3. Confirm coverage is unchanged: compare the Tests summary counts per shard against run 35655900272.
  4. Confirm apps/server/vite.config.ts still sets fileParallelism: false, and review the diff as moves (test names before/after) rather than rewrites.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
🔒 Security Review ✅ Completed 2026-09-22T06:14:14.301837Z 8e45305 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

Copy link
Copy Markdown

Thread transfer impact

✅ Thread transfer remains within every enforced ceiling.

ℹ️ No successful main baseline artifact is available yet. This run establishes the initial measurement.

Provider Metric Main baseline This PR Impact PR ceiling
Codex Total thread wire — 13.5 KiB — 15.1 KiB ✅
Codex Thread snapshot wire — 7.1 KiB — 7.3 KiB ✅
Codex Live turn WebSocket wire — 6.5 KiB — 7.8 KiB ✅
Codex Live turn WebSocket decoded — 56.3 KiB — 66.4 KiB ✅
Codex Live turn messages — 10 — 21 ✅
Claude Total thread wire — 13.5 KiB — 15.1 KiB ✅
Claude Thread snapshot wire — 7.1 KiB — 7.3 KiB ✅
Claude Live turn WebSocket wire — 6.5 KiB — 7.8 KiB ✅
Claude Live turn WebSocket decoded — 57.1 KiB — 66.4 KiB ✅
Claude Live turn messages — 10 — 21 ✅

Baseline: unavailable · PR result: 8e45305 · Source CI: success

Scenario and decoded snapshot size

10 historical turns, 5 command tools per turn, 878.9 KiB retained MCP result per historical turn, and a 1.05 MiB retained result in the measured turn.

  • Codex decoded thread snapshot: 113.9 KiB
  • Claude decoded thread snapshot: 114.6 KiB

Updated in place by a trusted workflow. PR artifacts are strictly validated and never executed.

@ashutoshpw

Copy link
Copy Markdown
Owner Author

Closing: fork-weight validation shows this layer does not pay.

Fork's own per-file timings (run 35655900272, pre-split, parsed from the three shard logs):

  • shard file-time totals 67,820 / 82,566 / 76,143 ms → max/mean 1.093 — the shards are already balanced within ~9%. The wall-time imbalance (193/226/217s) is runner noise, not file distribution.
  • the split was chosen using upstream (pingdotgg/t3code) durations, which are not representative: e.g. GitManager.test.ts is 14.2s upstream but 22.2s here.

After the split (run 35693398182, 338 files):

  • shard file-time totals 85,644 / 61,181 / 60,539 ms → max/mean 1.239 — worse.
  • GitManager split duplicated per-file setup: 22,197ms → 12,446 + 18,348 = 30,794ms for the same 113 tests, and both halves hash into shard 1, so no work left the heaviest shard.

Net: no balance win, extra setup cost, and the server shards are not the critical path anyway (Test is ~424s). Dropped per the same rule that dropped the filtered-install layer; branch kept for the record. The stale 239 files comment in ci.yml stays as a known trivial nit for the follow-up Test-job work.

@ashutoshpw ashutoshpw closed this Sep 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:XS vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant