Skip to content

perf(prepush): key the pro-test gate on its own inputs, not the whole tree (#6765) - #6875

Open
yzxcj797 wants to merge 2 commits into
koala73:mainfrom
yzxcj797:perf/prepush-per-gate-cache-6765
Open

perf(prepush): key the pro-test gate on its own inputs, not the whole tree (#6765)#6875
yzxcj797 wants to merge 2 commits into
koala73:mainfrom
yzxcj797:perf/prepush-per-gate-cache-6765

Conversation

@yzxcj797

Copy link
Copy Markdown
Contributor

Closes #6765 (framework + the highest-payoff gate; the remaining gates from the issue's starting set can adopt the same two attest modes incrementally).

The structural miss

The green-tree cache keys every gate on one whole-tree hash. In the merge / amend / re-push loop that is a new HEAD^{tree} on literally every push — 10 cache writes, 0 observed hits across the Canada campaign's 66 pushes, median 45.6s, worst 216.5s. A docs-only amend after a red CI run re-pays the full suite, including gates whose inputs did not change.

This PR

A per-gate cache alongside (not replacing) the whole-tree one, starting with the most expensive gate — the pro-test bundle freshness check (54.9s cold / 7.1s warm):

  • Key: one hash of the gate's INPUTS (pro-test/src, lockfile, package.json, vite.config.ts, tsconfig.json, product catalog, generator) and its VERIFIED OUTPUTS (src/config/*.generated.ts, public/pro/), computed straight from HEAD's tree via git ls-tree -r | git hash-object --stdin — blob hashes, so it costs path lookups, not file reads.
  • Why outputs are in the key: an amend that swaps verified-good bundle bytes for stale ones under identical inputs must not ride a hit minted against the good bytes. Cache hit ⟺ inputs and outputs are both byte-identical to a previously-verified-green combination.
  • New attest modes: gate-cache-read <dir> <gate> <hash> / gate-cache-write <dir> <gate> <hash> <attestable>, with the same three-valued exit discipline as the existing primitives (0 hit/written, 3 miss/refused, 2 usage). One file per gate holding a single hash line an operator can inspect and clear.
  • Write refuses a non-attestable worktree — the hash names HEAD bytes; a gate that ran against a drifted worktree proved something else. The hook only reaches for a read while ATTESTABLE=true.

Tests

  • 5 unit tests on the new attest modes: round-trip, cross-hash/cross-gate/absent-file misses, replace-not-accumulate, non-attestable refusal, usage errors.
  • 2 end-to-end hook tests in the real-git fixture: after a green run, a docs-only commit (new tree → whole-tree cache misses) skips the rebuild entirely — zero generator invocations — while a gate-input change re-runs it exactly once.

Verified locally: no outcome changes relative to a clean checkout on either suite (17 pre-existing Windows-path failures identical before and after; green on CI's Linux).

… tree (koala73#6765)

The green-tree cache keys every gate on one whole-tree hash, so a merge,
conflict re-resolve, amend, or regenerated-artifact commit — each a new
HEAD^{tree} — invalidates gates whose inputs did not change. In the Canada
merge campaign that was 10 cache writes and 0 observable hits across 66
pushes, median 45.6s, worst 216.5s.

This adds a per-gate cache alongside the whole-tree one, starting with the
most expensive gate: the pro-test bundle freshness check (54.9s cold, 7.1s
warm). The entry hashes the gate's INPUTS (sources, lockfile, package/vite/
tsconfig, catalog, generator) AND its VERIFIED OUTPUTS (generated config,
tiers, locales, public/pro/) straight from HEAD's tree — blob hashes, so it
costs path lookups rather than file reads. Outputs are part of the key on
purpose: an amend that swaps verified-good bundle bytes for stale ones under
identical inputs must not ride a hit minted against the good bytes.

Reads and writes go through two new prepush-attest.sh modes with the same
three-valued exit discipline as the whole-tree primitives. Writes refuse a
non-attestable worktree (the hash names HEAD bytes; a drifted worktree
proving something else must not stamp it), and the hook only reaches for a
read while ATTESTABLE=true.

On the merge-loop shape the whole-tree cache structurally cannot serve — a
docs-only amend after a green run — the rebuild is now skipped entirely.
@vercel

vercel Bot commented Aug 17, 2026

Copy link
Copy Markdown

@yzxcj797 is attempting to deploy a commit to the World Monitor Team on Vercel.

A member of the Team first needs to authorize it.

@cursor

cursor Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Review — head 42403daf6

No blockers. The red check is a GitHub infrastructure error, not a defect in this PR. One P2 gap in the cache key is worth closing before merge, but it narrows an existing guarantee rather than breaking anything.

The red check is infrastructure

RequestError [HttpError]: No server is currently available to service your request.
Sorry about that. Please try resubmitting your request and contact us if the problem persists.
##[error]Unhandled error: HttpError: No server is currently available …

That is the whole failure — the job never reached any of this PR's code. A re-run should clear it. The red Vercel is the usual fork-PR authorization failure.

Verified locally on an exported head tree

  • bash -n .husky/pre-push and bash -n scripts/prepush-attest.sh both parse — worth checking explicitly given the added if/else/fi nesting around the existing gate body.
  • tests/prepush-attest.test.mjs40/40 pass
  • tests/prepush-hook-gate.test.mjs20/20 pass
  • Non-vacuous under mutation: making gate-cache-read always report a hit (replacing the -f and grep -qxF checks with :) reds 3 attest tests and 2 gate tests. The new cases genuinely pin the read path.

The design holds up

Three decisions are load-bearing and all three are right:

  • Outputs are in the key, not just inputs. src/config/products.generated.ts, product-ids.generated.ts and public/pro are hashed alongside the sources. Without that, an amend swapping verified-good bundle bytes for stale ones under identical inputs would ride a hit minted against the good bytes. The comment says exactly this and the path list backs it up.
  • gate-cache-read takes no attestable term, and the call site gates on ATTESTABLE = true instead. Keeping that decision at one place leaves the primitive with a single rule to test, and gate-cache-write still refuses independently when the worktree is not byte-identical to HEAD.
  • The key is built from git ls-tree -r HEAD, so it is blob hashes and path lookups rather than file reads — which is what makes a per-gate key cheap enough to be worth having.

P2 — the key omits the gate's own definition

The ls-tree path list covers the gate's inputs and outputs, but not the code that is the gate:

pro-test/src  pro-test/package.json  pro-test/package-lock.json
pro-test/vite.config.ts  pro-test/tsconfig.json
convex/config/productCatalog.ts  scripts/generate-product-config.mjs
src/config/products.generated.ts  src/config/product-ids.generated.ts
public/pro

.husky/pre-push and scripts/prepush-attest.sh are absent. So if someone adds a verification step to the pro-test gate, a cache entry minted before that step existed stays valid, and the new step never runs for that tree — the gate reports "already passed" for a standard it was never held to. The whole-tree cache this replaces did not have that hole, because HEAD's tree hash covered the hook too. Adding both paths to the ls-tree list closes it at the cost of invalidating entries whenever the hook changes, which is the correct trade.

P3

If every listed path were absent from HEAD, git ls-tree emits nothing and git hash-object --stdin returns the empty-blob hash e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 — a fixed value that any other all-absent tree would also produce. Reachable only via RUN_ALL=true on a tree with no pro-test/, so it is a curiosity rather than a live risk, but a guard that refuses to cache an empty key would cost one line.

Conflict note

tests/prepush-hook-gate.test.mjs is also edited by #6870 and by #6871 (which carries #6870's commit). This PR is based on a merge base 10 commits behind main and does not include that work — its gate suite is 20 tests where #6871's is 21. Whichever lands second will need a rebase, and #6870's copy of that file currently has a failing test.

Verdict: mergeable once check is re-run. Recommend adding .husky/pre-push and scripts/prepush-attest.sh to the gate key first.

@koala73 koala73 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review (not ready)

Per-gate cache logic looks sound (inputs+verified outputs in the hash; refuse to write when the worktree is not HEAD).

Required variant-smoke-full did not pass on this head (cancelled → gate red). Please re-run the Test workflow until that job completes green. I will not put this on the merge list while a required gate is red.

Vercel fork-auth is not a required gate.

@koala73

koala73 commented Aug 18, 2026

Copy link
Copy Markdown
Owner

@yzxcj797 pls don't forget this one - you are churning a lot of PRs, but I want to close the older ones first

@cursor

cursor Bot commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Status after #6909 (2026-08-18)

Keep this PR. Do not merge it as written. #6909 (1b86e801) did not implement #6765. It rewrote the same pre-push /pro gate this PR wraps, so GitHub now reports CONFLICTING / DIRTY.

What still holds

The whole-tree green cache still keys on one HEAD^{tree} hash, so a merge / amend / docs-only re-push still misses. The surviving gate still triggers on the whole branch delta vs origin/main, not the last commit: once a branch touches pro-test/, later unrelated commits still re-run npm run build. That is the skip this PR is for.

The gate-cache-read / gate-cache-write primitives in scripts/prepush-attest.sh are still the right framework. #6765 stays open even after this lands — this PR is framework + the first gate, not proto / typecheck / markdown lint.

What #6909 made stale

#6909 stopped committing public/pro/ (Vercel builds it via build:full). The hook now only requires that pro-test builds and that the still-committed generated config is fresh.

This head still:

  • wraps the old path_matches '^(pro-test|public/pro)/…' block and hashes public/pro as a verified output — that path is gitignored, so git ls-tree HEAD -- public/pro is empty
  • does not hash pro-test/src/generated/tiers.json or pro-test/src/locales/, which are what the surviving git diff --exit-code actually checks
  • does not know about the earlier /pro build chore(build): build pro-test on Vercel instead of committing public/pro/ (#6898) #6909 added when a changed test imports _lib/pro-built-output (caching only the later gate will not skip that one)

The old “outputs in the key so a stale committed bundle cannot ride a hit” rationale died with #6909. Remaining verified outputs are the generated config files, not public/pro/.

#6853 already shared npm/Vite caches across worktrees, so the leftover cost is smaller than the original 54.9s cold figure. The skip is still worth doing; it has to wrap today’s gate.

Rebase checklist

  1. Rebase onto current main (conflicts in .husky/pre-push and tests/prepush-hook-gate.test.mjs, as chore(build): build pro-test on Vercel instead of committing public/pro/ (#6898) #6909 predicted).
  2. Keep the attest primitives and their unit tests.
  3. Wrap the current “build + generated config freshness” block, not the deleted committed-bundle diff.
  4. Drop public/pro from the key. Add pro-test/src/generated/tiers.json and pro-test/src/locales/.
  5. Worth also adding .husky/pre-push and scripts/prepush-attest.sh to the key (review note on the previous head: a cache entry minted before a new verification step would report “already passed” for a standard it was never held to).

Not merge-ready until that rebase. Not a close.

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.

perf(prepush): the green-tree cache never hits in a merge loop — key expensive gates on their own inputs

2 participants