Skip to content

Commit 7fdd9be

Browse files
mattmilleraiclaude
andauthored
perf(cursor-review): drop the full-history clones from the review round's critical path (#433)
* perf(cursor-review): drop the full-history clones from the review round's critical path Every round cloned the consumer's whole history (~2 min on a large monorepo) in `diff-size`, every Cursor cell, `judge-direct` and `consolidate`, though only `diff-size` reads any history. - `diff-size`: depth-1 checkout plus the narrowed BASE/HEAD fetch and merge-base trust ladder pr-size.yml already ships, now one shared script (scripts/check-pr-size/fetch-history.sh) both workflows call. The last-reviewed head and its recorded merge base ride along best-effort so the incremental block's ancestry checks see a graph that can vouch for them. The incremental step now also requires the last-reviewed head to be an ancestor of this head, keeping the block off force-pushed rounds now that a by-SHA fetch can still reach the old head. - `preflight`: needs gate + round-cap, not diff-size, so it runs alongside the size check instead of after it. - `review`, `judge-direct`, `consolidate`: fetch-depth 1. - fetch-history-test.sh: local-upstream test, run by test-pr-size.yml. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(cursor-review): address review round on the narrowed fetch - preflight is gated on diff-size's within_cap again, so a catalog or config failure cannot redden an over-cap pass-through run. - walk_is_complete feeds grep from a here-string and treats only exit 1 as "no boundary": under pipefail a SIGPIPE'd printf no longer reads as a complete walk. - Only the BASE/HEAD merge base drives the deepen ladder and the --unshallow fallback; the advisory extras get one bounded --deepen after the merge base is settled, and no longer deepen on every rung. - Extras are validated with a whole-string =~ and a rejected value is never echoed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 30cccb9 commit 7fdd9be

5 files changed

Lines changed: 438 additions & 181 deletions

File tree

‎.github/workflows/cursor-review.yml‎

Lines changed: 71 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1886,10 +1886,16 @@ jobs:
18861886
- name: Checkout PR head
18871887
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
18881888
with:
1889-
# Full history so the BASE...HEAD three-dot diff resolves and the
1890-
# tool's base-ref reads (linguist-generated via `check-attr --source`,
1891-
# deleted-file blob fallbacks) work without a separate fetch.
1892-
fetch-depth: 0
1889+
# The head commit and its tree, nothing else. This used to be
1890+
# a full-history clone — every branch and tag of the consumer, ~2 min per
1891+
# round on a large monorepo, on the review's critical path. The
1892+
# BASE...HEAD three-dot diff, the tool's base-ref reads
1893+
# (linguist-generated via `check-attr --source`, deleted-file blob
1894+
# fallbacks) and the incremental block need only BASE, HEAD, their
1895+
# merge base, and the last-reviewed commits; "Fetch the base commit
1896+
# and the merge-base history" below fetches exactly that, with the
1897+
# same shared script pr-size.yml uses.
1898+
fetch-depth: 1
18931899
persist-credentials: false
18941900
ref: ${{ github.event.pull_request.head.sha }}
18951901

@@ -1950,6 +1956,40 @@ jobs:
19501956
exit 1
19511957
fi
19521958
1959+
- name: Load check-pr-size tool
1960+
# The SAME classifier the PR-size cap uses — the single source of truth
1961+
# for "what is codegen". Comes from THIS repo (public, pinned via
1962+
# workflows_ref), never the PR checkout, so a malicious PR can't rewrite
1963+
# the logic that decides which of its files skip review.
1964+
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
1965+
with:
1966+
repository: Comfy-Org/github-workflows
1967+
ref: ${{ inputs.workflows_ref }}
1968+
path: _pr_size_tool
1969+
persist-credentials: false
1970+
1971+
- name: Fetch the base commit and the merge-base history
1972+
# The other half of the narrowed checkout above (pr-size.yml's BE-8940
1973+
# fetch, from the ONE script both workflows call). Fails this job — fail
1974+
# closed, no review — when BASE/HEAD cannot be fetched or no trustworthy
1975+
# merge base exists, with an error naming which. The last-reviewed head
1976+
# and its recorded merge base ride along best-effort so the incremental
1977+
# block's ancestry checks are answered from a graph that can vouch for
1978+
# them; neither is ever fatal here. The script comes from the pinned
1979+
# checkout of THIS repo, never the PR's.
1980+
env:
1981+
# The job's own read-only token (`permissions: contents: read`),
1982+
# handed to git through GIT_CONFIG_* env for this step only — never
1983+
# written to `.git/config`.
1984+
FETCH_TOKEN: ${{ github.token }}
1985+
BASE_SHA: ${{ github.event.pull_request.base.sha }}
1986+
HEAD_SHA: ${{ github.event.pull_request.head.sha }}
1987+
LAST_REVIEWED_SHA: ${{ needs.ledger.outputs.last_reviewed_sha }}
1988+
LAST_REVIEWED_MERGE_BASE: ${{ needs.ledger.outputs.last_reviewed_merge_base }}
1989+
run: |
1990+
bash _pr_size_tool/scripts/check-pr-size/fetch-history.sh \
1991+
"$LAST_REVIEWED_SHA" "$LAST_REVIEWED_MERGE_BASE"
1992+
19531993
- name: Resolve this round's merge base
19541994
# Recorded in the consolidated review by `post-review`, and read back by the
19551995
# NEXT round to pin the OLD side of its incremental block (BE-15598). Plain
@@ -1960,7 +2000,7 @@ jobs:
19602000
#
19612001
# Best-effort by construction — it must never fail the job. `git merge-base`
19622002
# exits non-zero when the two commits share no history (an unrelated-histories
1963-
# base, a base ref the fetch-depth:0 checkout still cannot see), and there is
2003+
# base), and there is
19642004
# nothing to do about that except record nothing and say so.
19652005
id: merge_base
19662006
env:
@@ -1983,18 +2023,6 @@ jobs:
19832023
fi
19842024
echo "merge_base_sha=${MERGE_BASE}" >> "$GITHUB_OUTPUT"
19852025
1986-
- name: Load check-pr-size tool
1987-
# The SAME classifier the PR-size cap uses — the single source of truth
1988-
# for "what is codegen". Comes from THIS repo (public, pinned via
1989-
# workflows_ref), never the PR checkout, so a malicious PR can't rewrite
1990-
# the logic that decides which of its files skip review.
1991-
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
1992-
with:
1993-
repository: Comfy-Org/github-workflows
1994-
ref: ${{ inputs.workflows_ref }}
1995-
path: _pr_size_tool
1996-
persist-credentials: false
1997-
19982026
- name: Set up Go
19992027
uses: actions/setup-go@b7ad1dad31e06c5925ef5d2fc7ad053ef454303e # v7.0.0
20002028
with:
@@ -2144,7 +2172,7 @@ jobs:
21442172
# The subset of the reviewed diff that changed since the last reviewed
21452173
# round — PRIORITIZATION, not narrowing, the full diff above stays the
21462174
# primary input; this just tells the panel where to spend budget. Built
2147-
# here (this job already holds the fetch-depth:0 checkout) so it can
2175+
# here (this job already holds the fetched history) so it can
21482176
# ride the same shared artifact instead of every consumer re-deriving it.
21492177
# Best-effort: an unresolvable SHA (force-push, base-branch rewrite)
21502178
# simply produces no incremental block.
@@ -2254,8 +2282,15 @@ jobs:
22542282
if [ -z "$LAST_REVIEWED_SHA" ] || [ "$LAST_REVIEWED_SHA" = "$HEAD_SHA" ]; then
22552283
echo "No usable last-reviewed SHA — skipping the incremental diff block."
22562284
echo "incremental_state=none" >> "$GITHUB_OUTPUT"
2257-
elif ! git cat-file -e "${LAST_REVIEWED_SHA}^{commit}" 2>/dev/null; then
2258-
echo "Last-reviewed SHA ${LAST_REVIEWED_SHA} is unreachable (force-push or base-branch rewrite) — skipping the incremental diff block."
2285+
elif ! git cat-file -e "${LAST_REVIEWED_SHA}^{commit}" 2>/dev/null \
2286+
|| ! git merge-base --is-ancestor "$LAST_REVIEWED_SHA" "$HEAD_SHA" 2>/dev/null; then
2287+
# The ancestor test keeps the block's old reach. Under the former
2288+
# full-history clone a force-pushed-away head was simply absent; the
2289+
# narrowed fetch asks for it BY SHA, and GitHub can still serve it, so
2290+
# presence alone would now build a block (and widen approve_scope's
2291+
# `delta`) on exactly the rounds that used to get none. "An ancestor
2292+
# of this head" is what an un-rewritten branch's last round always is.
2293+
echo "Last-reviewed SHA ${LAST_REVIEWED_SHA} is unreachable or no longer in this head's history (force-push or base-branch rewrite) — skipping the incremental diff block."
22592294
elif [ -z "$LAST_REVIEWED_MERGE_BASE" ]; then
22602295
# Fail CLOSED, and do NOT fall back to ${BASE_SHA}: that fallback IS the bug
22612296
# (BE-15598). Every live PR sees this once after rollout — the round it is
@@ -3233,7 +3268,11 @@ jobs:
32333268
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
32343269
with:
32353270
ref: ${{ github.event.pull_request.head.sha }}
3236-
fetch-depth: 0
3271+
# The head tree only, as `review-anthropic-direct` takes it. No step
3272+
# here reads history — the reviewed diff, the incremental diff and the
3273+
# ledger arrive as artifacts — and a full clone of a large consumer
3274+
# cost ~2 min of this round's critical path.
3275+
fetch-depth: 1
32373276
persist-credentials: false
32383277

32393278
- name: Require a pinned workflows_ref
@@ -4696,7 +4735,11 @@ jobs:
46964735
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
46974736
with:
46984737
ref: ${{ github.event.pull_request.head.sha }}
4699-
fetch-depth: 0
4738+
# The head tree only, as `review-anthropic-direct` takes it. No step
4739+
# here reads history — the reviewed diff, the incremental diff and the
4740+
# ledger arrive as artifacts — and a full clone of a large consumer
4741+
# cost ~2 min of this round's critical path.
4742+
fetch-depth: 1
47004743
persist-credentials: false
47014744

47024745
- name: Require a pinned workflows_ref
@@ -5249,7 +5292,7 @@ jobs:
52495292
# payload upload with it and is the one failure mode that can never reach
52505293
# the degraded panel-union fallback. The step trips first and is absorbed,
52515294
# so a hung judge costs the adjudication, not the panel. The slack over
5252-
# the step (10m) covers this job's own overhead — a `fetch-depth: 0`
5295+
# the step (10m) covers this job's own overhead — a PR-head
52535296
# checkout, the CLI install, three artifact downloads and the upload —
52545297
# which a panel cell does not all pay, so the judge's effective MODEL
52555298
# budget is the step cap, not this one.
@@ -5296,7 +5339,11 @@ jobs:
52965339
uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
52975340
with:
52985341
ref: ${{ github.event.pull_request.head.sha }}
5299-
fetch-depth: 0
5342+
# The head tree only, as `review-anthropic-direct` takes it. No step
5343+
# here reads history — the reviewed diff, the incremental diff and the
5344+
# ledger arrive as artifacts — and a full clone of a large consumer
5345+
# cost ~2 min of this round's critical path.
5346+
fetch-depth: 1
53005347
persist-credentials: false
53015348

53025349
- name: Require a pinned workflows_ref

0 commit comments

Comments
 (0)