aws-samples / aws-samples/sample-autonomous-cloud-coding-agents

perf(ci): remaining build-performance levers (shard, coverage-gate, runner, path-filter) — follow-ups to #357

Open
#363 8 comments 0 reactions 1 assignee Claimed by @scottschreckengaust View on GitHub
approved
Dominant language
TypeScript
Stars
143
Forks
46
Avg merge
3d 9h
Merged PRs (30d)
20

Description

## Summary

Umbrella tracking issue for the **remaining CI build-performance levers** identified during the profiling that produced #357. Item #1 of the original 5 recommendations is **done** ([PR #359](https://github.com/aws-samples/sample-autonomous-cloud-coding-agents/pull/359) — skip the redundant jest type-check via `isolatedModules`): build step **710s → 346s (−51%)**, `//cdk:test` **649s → 298s**. This issue tracks the other four.

Each is its own measurable experiment; this issue holds the sequence, rationale, and implementer notes. Spin out a child issue per item when work starts (governance per [ADR-003](./docs/decisions/ADR-003-contribution-governance.md)).

## Recommendations — status & remaining work

| # | Recommendation | Status | Effect |
|---|---|---|---|
| **1** | **Skip the redundant jest type-check** (transpile-only; was originally framed as "swap to `@swc/jest`") | ✅ **Done — PR #359** | build step **710s → 346s** (−51%); `//cdk:test` 649s → 298s |
| **2** | **Shard the CDK suite across a job matrix** (`jest --shard=N/M`) | ⬜ Open | ~298s → **~75–100s** wall (4-way); stacks *on top of* #1 |
| **3** | **Gate `collectCoverage` to `merge_group` only** (skip on PR push) | ⬜ Open | trims instrumentation on the high-frequency PR event |
| **4** | **Bump the default runner** (4-core → 8/16-core; workflow already supports the label path) | ⬜ Open | direct win for jest workers + parallel synth |
| **5** | **Path-filtered builds** (`dorny/paths-filter`) so docs/CLI/agent-only PRs skip `//cdk:test` | ⬜ Open | biggest win for the long tail of non-CDK PRs |

## Suggested sequencing

1. **#2 (shard)** — attacks the now-dominant 298s long pole directly; ~298s → ~75–100s. Biggest remaining bang.
2. **#5 (path filters)** — orthogonal; removes the whole tax from docs/CLI/agent-only PRs.
3. **#4 (bigger runner)** — cheap experiment, immediate, but recurring $ cost; good to A/B against #2.
4. **#3 (PR-only coverage)** — smallest win; fold in opportunistically.

## Insights for implementers

- **The cost shape shifted after #1.** `//cdk:test` was ~91% of the build step; at 298s it's now ~86% of a 346s step — still the long pole, but half the absolute size. The mise parallel DAG **cannot overlap it** (everything else finishes in the first ~90s), so the next dollar of speedup must come from **parallelizing the suite itself (#2)** or **not running it when irrelevant (#5)** — not from the DAG.

- **#2 has a merge-queue wrinkle.** `build` is a **required** status check and must report on the `merge_group` event (see #327 / the comment block atop `build.yml`). If you split CDK tests into a matrix, the *required* check must **aggregate** shard results — either keep one `build` job that runs shards internally, or add a "tests passed" gate job that `needs:` all shards and is the thing marked required. Do **not** mark individual shard jobs required, or the queue can deadlock/flake. Shard count should divide cleanly into the ~113 suites; 4-way is a sane start. Watch for per-shard fixed overhead (checkout, install, synth-cache restore) eroding the win — cache aggressively and measure wall-clock, not sum-of-shards.

- **#3 must preserve the enforcement gate.** Coverage thresholds are the merge gate (`coverageThreshold` in `cdk/package.json` / `cli/package.json` + `agent` pytest `fail_under`, kept in sync via `contracts/coverage-thresholds.json` and `check:coverage-thresholds-sync`). If you skip `collectCoverage` on `pull_request`, you MUST still enforce thresholds on `merge_group` so nothing merges under-covered. Net effect is mostly on the high-frequency PR-push event, so quantify it there.

- **#4 is the cheapest experiment but recurring cost.** `build.yml` already resolves the runner via `vars.DEFAULT_RUNNER_LABEL` and PR labels (`self-hosted`, `ubuntu-latest-4-cores`). A/B is a one-line `vars` change. Jest workers scale with cores (`maxWorkers` defaults to cores−1) and synth is CPU-bound, so more cores helps both — but it's a per-run $ cost, so weigh against #2 (which uses concurrency you're already paying for).

- **#5 has the same required-check constraint as #2.** Because `build` must report on `merge_group`, you can't simply *skip the job* for docs-only PRs (the required check would never report → queue deadlock). Instead keep the job and make the **expensive steps** conditional (e.g. `dorny/paths-filter` gates `//cdk:test`), emitting success when CDK paths are untouched. Always `log()`/annotate what was skipped so a skipped suite doesn't read as "covered."

## Definition of done (this umbrella)

- [ ] Child issue opened per remaining item (#2–#5) as work begins, each with its own acceptance criteria + before/after CI timing.
- [ ] Each child PR posts a real **4-core CI** before/after (the apples-to-apples gate, per #359's method).
- [ ] `docs/design/CI_BUILD_PERFORMANCE.md` kept current as items land (see linked docs PR).

---
🤖 Generated with [Claude Code](https://claude.com/claude-code)

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.