Skip to content

Commit 2012a60

Browse files
feat: catch up the review base from the nearest saved ancestor
With no saved base for the merge base and no usable baseline committed there, a review analyzed the merge base from scratch, even when a saved analysis of a commit a few steps below it was sitting in the artifact store. find-ancestor-base.sh lists the repository's artifacts once, keeps the base analyses for this configuration that a run on the repository's own code produced (the same provenance rule as fetch-state.sh), and walks the merge base's first-parent history up to 100 commits, deepening the shallow checkout. The first hit seeds an incremental catch-up to the merge base, which is then published under the merge base's own name. Order: exact artifact, committed at the merge base, nearest ancestor, full. base_source=ancestor reports it, with base_from_sha and catchup_commits. A compatible ancestor that exists only beyond the bound gives base_reason=too_far_behind; a saved analysis of the merge base under another configuration gives incompatible. Sync uses the same lookup when the branch has no usable committed baseline, so the first sync after the setup pull request merges catches up from the base that pull request's review saved. FORCE_FULL is now lowercased with tr, which also runs on the bash 3.2 macOS ships. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 7a091f5 commit 2012a60

5 files changed

Lines changed: 397 additions & 15 deletions

File tree

‎action.yml‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -184,7 +184,7 @@ outputs:
184184
description: 'Which state the review head analysis grew from: pr-chain or base.'
185185
value: ${{ steps.review_analyze.outputs.seed_source }}
186186
base_source:
187-
description: 'How the review base was obtained: saved, committed, or computed.'
187+
description: 'How the review base was obtained: saved, ancestor, committed, or computed.'
188188
value: ${{ steps.review_analyze.outputs.base_source }}
189189
merge_base_sha:
190190
description: 'Merge base used as the review comparison baseline.'
@@ -451,6 +451,13 @@ runs:
451451
CHECKOUT_DIR: ${{ github.workspace }}/.codeboarding-target
452452
STAGE_DIR: ${{ runner.temp }}/cb-state/${{ github.action }}/out
453453
FORCE_FULL: ${{ inputs.force_full }}
454+
CFG_HASH: ${{ steps.state.outputs.cfg_hash }}
455+
# Lets a branch without a usable committed baseline catch up from a saved analysis.
456+
ANCESTOR_LOOKUP: ${{ github.server_url == 'https://github.com' && steps.state.outputs.cfg_hash != '' }}
457+
GIT_TOKEN: ${{ inputs.github_token }}
458+
REPOSITORY: ${{ github.repository }}
459+
GH_HOST: ${{ github.server_url }}
460+
GITHUB_SERVER_URL: ${{ github.server_url }}
454461
DEPTH_CAP: ${{ inputs.depth_cap }}
455462
MODEL: ${{ inputs.model }}
456463
AGENT_MODEL_INPUT: ${{ inputs.agent_model }}
@@ -539,6 +546,8 @@ runs:
539546
BASE_FETCH_SECONDS: ${{ steps.fetch_base.outputs.seconds }}
540547
WARMSTART_DIR: ${{ runner.temp }}/cb-state/${{ github.action }}/warmstart
541548
RENEW_BASE: ${{ steps.fetch_base.outputs.renew }}
549+
# Lets a branch without a usable committed baseline catch up from a saved analysis.
550+
ANCESTOR_LOOKUP: ${{ github.server_url == 'https://github.com' && steps.state.outputs.cfg_hash != '' }}
542551
# For rewriting the progress comment while a base is built from scratch.
543552
PROGRESS_HEADER: ${{ steps.guard.outputs.comment_id }}
544553
BASE_REF: ${{ steps.guard.outputs.base_ref }}

‎docs/COMMIT_STRATEGY.md‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -72,8 +72,8 @@ never fires.
7272
| `base_sha` | string | the base branch tip when the event fired — *not* what was compared against |
7373
| `kind` | string | always `review`, so a reader can tell this artifact from a base or warm-start bundle |
7474
| `analysed_files_changed` | string | analysed files whose content hash differs between base and head; `unknown` when the analyses cannot say |
75-
| `base_source` | string | how this run obtained the base graph: `saved` (the artifact for the merge base), `committed` (`.codeboarding/` committed at the merge base, caught up), or `computed` (full analysis in this run) |
76-
| `base_reason` | string | only with `computed`: `no_baseline` (nothing to seed from) or `incompatible` (a candidate existed but its depth cap differed, or the engine demanded a full run); empty otherwise |
75+
| `base_source` | string | how this run obtained the base graph: `saved` (the artifact for the merge base), `committed` (`.codeboarding/` committed at the merge base, caught up), `ancestor` (the artifact of an older base-branch commit, caught up), or `computed` (full analysis in this run) |
76+
| `base_reason` | string | only with `computed`: `no_baseline` (nothing to seed from), `incompatible` (a candidate existed but its depth cap or configuration differed, or the engine demanded a full run) or `too_far_behind` (the nearest saved ancestor is beyond the catch-up bound); empty otherwise |
7777
| `base_from_sha` | string | the commit whose saved analysis seeded the base; the merge base for `saved`, empty for `computed` or when it lies beyond the fetched history |
7878
| `catchup_commits` | string | first-parent commits from `base_from_sha` to the merge base that change anything outside `.codeboarding/`; `0` when exact, empty when unknown |
7979
| `base_seconds` | string | wall time spent obtaining the base, the artifact lookup included |
@@ -109,14 +109,24 @@ them:
109109
|---|---|
110110
| the published `codeboarding-base-<cfg>-<merge_base>` artifact with a compatible depth cap | none |
111111
| no usable artifact — check out the merge base, seed from a compatible baseline committed there, catch up | one incremental, full if Core requires it |
112-
| no compatible committed baseline either | full analysis directly, at the configured `depth_cap` |
112+
| no compatible committed baseline either: the nearest `codeboarding-base-<cfg>-<sha>` artifact among the merge base's last 100 first-parent ancestors | one incremental from that commit to the merge base |
113+
| none within 100 commits either | full analysis directly, at the configured `depth_cap` |
113114

114115
A trusted run that computed the base publishes it, so the next pull request
115-
forking from that commit gets the first row. The rows are `base_source` `saved`,
116-
`committed` and `computed` in the review metadata, and the review comment says
116+
forking from that commit gets the first row; that includes a base caught up from
117+
an ancestor. The rows are `base_source` `saved`, `committed`, `ancestor` and
118+
`computed` in the review metadata, and the review comment says
117119
which one ran, with measured times. While a base is computed, the progress
118120
comment says so in two steps, with the elapsed time and the reason.
119121

122+
The ancestor lookup lists the repository's artifacts once (up to 1,000, newest
123+
first), keeps those named for this configuration and produced by a run on the
124+
repository's own code, and walks the merge base's first-parent history, deepening
125+
the shallow checkout to 101 commits. Sync uses the same lookup when the branch has
126+
no usable committed baseline, so the first sync after the setup pull request
127+
merges catches up from the base that pull request's review saved, instead of
128+
analyzing from scratch.
129+
120130
The configuration hash includes `depth_cap`. The workflow input controls depth
121131
for both fresh and fallback analyses; stored legacy depth values never override it.
122132

‎scripts/action/analyze.sh‎

Lines changed: 60 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -124,14 +124,24 @@ analyze_sync() {
124124
rm -rf "$work"
125125
seed_state "$CHECKOUT_DIR" "$state"
126126

127-
if [ "${FORCE_FULL,,}" = true ] || [ "$(depth_cap_from "$state/analysis.json")" != "$DEPTH_CAP" ]; then
128-
full "$CHECKOUT_DIR" "$state" "$DEPTH_CAP"
129-
else
130-
incremental "$CHECKOUT_DIR" "$state"
131-
if [ "$REQUIRES_FULL" = true ]; then
132-
full "$CHECKOUT_DIR" "$state" "$DEPTH_CAP"
127+
REQUIRES_FULL=true
128+
if [ "$(printf '%s' "${FORCE_FULL:-false}" | tr '[:upper:]' '[:lower:]')" != true ]; then
129+
if [ "$(depth_cap_from "$state/analysis.json")" = "$DEPTH_CAP" ]; then
130+
incremental "$CHECKOUT_DIR" "$state"
131+
fi
132+
# A branch with no usable committed baseline, such as the first sync after the
133+
# setup pull request merged, catches up from that pull request's saved base.
134+
if [ "$REQUIRES_FULL" = true ] &&
135+
seed_from_ancestor "${REPOSITORY:-}" "$(git -C "$CHECKOUT_DIR" rev-parse HEAD 2>/dev/null || true)" "$state" true; then
136+
incremental "$CHECKOUT_DIR" "$state"
137+
[ "$REQUIRES_FULL" = true ] ||
138+
echo "::notice::Caught up from the saved analysis of $ANCESTOR_SHA instead of analyzing from scratch."
133139
fi
134140
fi
141+
unset GIT_TOKEN
142+
if [ "$REQUIRES_FULL" = true ]; then
143+
full "$CHECKOUT_DIR" "$state" "$DEPTH_CAP"
144+
fi
135145
# Sync already computes the graph every review of this branch compares against,
136146
# so publish it instead of making the first pull request recompute it.
137147
stage "$state" base
@@ -140,8 +150,9 @@ analyze_sync() {
140150
}
141151

142152
# How far below the merge base this run looks for the commit a saved analysis
143-
# describes. Past it, a catch-up count is reported as unknown.
144-
CATCHUP_BOUND=100
153+
# describes, and for a saved ancestor to catch up from. Past it, a catch-up count
154+
# is unknown and an ancestor counts as too far behind. It is also the fetch depth.
155+
CATCHUP_BOUND="${CATCHUP_BOUND:-100}"
145156

146157
# A depth above 1 also deepens a commit the shallow checkout already holds.
147158
fetch_commit() {
@@ -187,6 +198,35 @@ catchup_count() {
187198
echo "$count"
188199
}
189200

201+
# Replaces $3 with the saved analysis of the nearest first-parent ancestor of $2
202+
# (or of $2 itself when $4 is true). Sets ANCESTOR_SHA, or when there is none to
203+
# use, ANCESTOR_REASON: too_far_behind or incompatible.
204+
ANCESTOR_SHA="" ANCESTOR_REASON=""
205+
seed_from_ancestor() {
206+
local repository="$1" tip="$2" state="$3" include_tip="$4" found dest="$RUNNER_TEMP/codeboarding-ancestor"
207+
ANCESTOR_SHA="" ANCESTOR_REASON=""
208+
[ "${ANCESTOR_LOOKUP:-false}" = true ] && [ -n "${CFG_HASH:-}" ] && [ -n "${REPOSITORY:-}" ] && [ -n "$tip" ] || return 1
209+
fetch_commit "$repository" "$tip" "$(( CATCHUP_BOUND + 1 ))" || true
210+
found="$(GH_TOKEN="${GIT_TOKEN:-}" GH_ENTERPRISE_TOKEN="${GIT_TOKEN:-}" TIP_SHA="$tip" DEST="$dest" \
211+
INCLUDE_TIP="$include_tip" CATCHUP_BOUND="$CATCHUP_BOUND" \
212+
"$ACTION_PATH/scripts/action/find-ancestor-base.sh" || true)"
213+
ANCESTOR_SHA="$(awk -F= '$1 == "ancestor_sha" {print $2; exit}' <<< "$found")"
214+
if [ -z "$ANCESTOR_SHA" ]; then
215+
if grep -qx 'other_cfg_at_tip=true' <<< "$found"; then
216+
ANCESTOR_REASON=incompatible
217+
elif grep -qx 'too_far_behind=true' <<< "$found"; then
218+
ANCESTOR_REASON=too_far_behind
219+
fi
220+
return 1
221+
fi
222+
if [ "$(depth_cap_from "$dest/analysis.json")" != "$DEPTH_CAP" ]; then
223+
ANCESTOR_SHA="" ANCESTOR_REASON=incompatible
224+
return 1
225+
fi
226+
rm -rf "$state"
227+
cp -a "$dest" "$state"
228+
}
229+
190230
# Rewrites the sticky progress comment while the base is built from scratch. A
191231
# fork's read-only token makes every call fail, which costs nothing.
192232
PROGRESS_PID=""
@@ -262,9 +302,20 @@ analyze_review() {
262302
elif [ -f "$base_state/analysis.json" ]; then
263303
base_reason=incompatible
264304
fi
305+
# Nothing at the merge base to grow from: catch up from the nearest saved
306+
# ancestor, and publish the result under the merge base's own name below.
307+
if [ "$REQUIRES_FULL" = true ] && seed_from_ancestor "$REVIEW_BASE_REPO" "$REVIEW_BASE_SHA" "$base_state" false; then
308+
incremental "$base_checkout" "$base_state"
309+
if [ "$REQUIRES_FULL" = true ]; then
310+
base_reason=incompatible
311+
else
312+
base_source=ancestor base_from_sha="$ANCESTOR_SHA"
313+
catchup_commits="$(catchup_count "$ANCESTOR_SHA" "$REVIEW_BASE_SHA")"
314+
fi
315+
fi
265316
if [ "$REQUIRES_FULL" = true ]; then
266317
base_source=computed
267-
base_reason="${base_reason:-no_baseline}"
318+
base_reason="${base_reason:-${ANCESTOR_REASON:-no_baseline}}"
268319
trap progress_stop EXIT
269320
progress_start "$base_started"
270321
full "$base_checkout" "$base_state" "$DEPTH_CAP"
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
#!/usr/bin/env bash
2+
# Finds the nearest first-parent ancestor of TIP_SHA with a saved base analysis
3+
# under this configuration and downloads it into DEST, so a run without an exact
4+
# base can catch up from it instead of analyzing from scratch. Best effort.
5+
#
6+
# Prints key=value lines on stdout and everything else on stderr:
7+
# ancestor_sha the commit whose analysis is now in DEST, or empty
8+
# too_far_behind true when a compatible ancestor exists only beyond the bound
9+
# other_cfg_at_tip true when TIP_SHA has a saved analysis under another configuration
10+
#
11+
# Needs the tip's first-parent history in CHECKOUT_DIR, at least CATCHUP_BOUND deep.
12+
set -euo pipefail
13+
: "${REPOSITORY:?}" "${CFG_HASH:?}" "${TIP_SHA:?}" "${CHECKOUT_DIR:?}" "${DEST:?}"
14+
BOUND="${CATCHUP_BOUND:-100}"
15+
rm -rf "$DEST"
16+
17+
api() { gh api -H 'Accept: application/vnd.github+json' "$@"; }
18+
export GH_HOST="${GH_HOST:-github.com}"
19+
GH_HOST="${GH_HOST#*://}"
20+
21+
ancestor="" too_far=false other_cfg=false
22+
report() {
23+
printf 'ancestor_sha=%s\ntoo_far_behind=%s\nother_cfg_at_tip=%s\n' "$ancestor" "$too_far" "$other_cfg"
24+
}
25+
trap report EXIT
26+
27+
# One listing of every artifact, newest first. The name filter on this endpoint is
28+
# exact, so a prefix needs the whole list. The provenance rule is fetch-state.sh's:
29+
# only artifacts from a run on this repository's own code, since a fork's workflow
30+
# can upload under any name and its bytes would reach a pickle loader.
31+
trusted='.artifacts[]?
32+
| select(.expired == false)
33+
| select(.workflow_run != null)
34+
| select(.workflow_run.head_repository_id == .workflow_run.repository_id)
35+
| select(.name | startswith("codeboarding-base-"))
36+
| .name'
37+
names="" page=1
38+
while [ "$page" -le "${MAX_PAGES:-10}" ]; do
39+
if ! listing="$(api "repos/$REPOSITORY/actions/artifacts?per_page=100&page=$page" 2>/dev/null)"; then
40+
echo "::warning::Could not list artifacts in $REPOSITORY; not looking for an older saved analysis." >&2
41+
exit 0
42+
fi
43+
names="$names$(jq -r "$trusted" <<< "$listing" 2>/dev/null || true)"$'\n'
44+
returned="$(jq -r '.artifacts | length' <<< "$listing" 2>/dev/null || echo 0)"
45+
[ "${returned:-0}" -eq 100 ] || break
46+
page=$(( page + 1 ))
47+
done
48+
49+
prefix="codeboarding-base-$CFG_HASH-"
50+
saved="$(grep "^$prefix" <<< "$names" | cut -c$(( ${#prefix} + 1 ))- || true)"
51+
if grep "^codeboarding-base-.*-$TIP_SHA\$" <<< "$names" | grep -vq "^$prefix"; then
52+
other_cfg=true
53+
fi
54+
[ -n "$saved" ] || exit 0
55+
56+
# Distance 0 is the tip itself, which a review already looked up by its exact
57+
# name; sync has no such lookup and passes INCLUDE_TIP=true.
58+
distance=0
59+
for commit in $(git -C "$CHECKOUT_DIR" rev-list --first-parent --max-count=$(( BOUND + 1 )) "$TIP_SHA" 2>/dev/null); do
60+
if { [ "$distance" -gt 0 ] || [ "${INCLUDE_TIP:-false}" = true ]; } && grep -qx "$commit" <<< "$saved"; then
61+
ancestor="$commit"
62+
break
63+
fi
64+
distance=$(( distance + 1 ))
65+
done
66+
67+
if [ -z "$ancestor" ]; then
68+
# Saved analyses of other branches are no reason to call this one far behind,
69+
# so ask whether the newest one is an ancestor at all.
70+
newest="$(head -n 1 <<< "$saved")"
71+
if [ "$(api "repos/$REPOSITORY/compare/$newest...$TIP_SHA" --jq .status 2>/dev/null || true)" = ahead ]; then
72+
too_far=true
73+
fi
74+
exit 0
75+
fi
76+
77+
# Downloaded by its exact name, through the same checks as every other lookup.
78+
if ! GITHUB_OUTPUT="" ARTIFACT_NAME="$prefix$ancestor" DEST="$DEST" \
79+
"$(dirname "$0")/fetch-state.sh" >&2 || [ ! -f "$DEST/analysis.json" ]; then
80+
ancestor=""
81+
fi

0 commit comments

Comments
 (0)