Skip to content

ClawSweeper's own review writes expire the review content cache, so unchanged heads are re-reviewed in a loop #967

Description

@masatohoshino

ClawSweeper's own review writes expire the review content cache, so unchanged heads are re-reviewed in a loop

What happens

reviewContentCacheHit is supposed to skip a full re-review when an item's content
has not changed. For pull requests it effectively never gets the chance: every
completed review invalidates its own cache entry, so the same unchanged head is
reviewed again and again.

The durable comment records prior cycles as
- reviewed <ts> sha <sha> :: <verdict> :: <asks>, so the rate is public:

repo=openclaw/clawsweeper
for n in $(gh pr list --repo "$repo" --state all --limit 400 --json number --jq '.[].number'); do
  gh api "repos/$repo/issues/$n/comments" --paginate \
    --jq '.[] | select(.user.login=="clawsweeper[bot]") | .body' 2>/dev/null |
    grep '^- reviewed ' | sed "s|^|$n |"
done | python3 -c '
import sys, collections, datetime as dt, statistics
by=collections.defaultdict(list)
for line in sys.stdin:
    head, _, rest = line.rstrip("\n").partition(" :: ")
    verdict = rest.split(" :: ")[0]
    f = head.split()
    if len(f) < 6: continue
    by[f[0]].append((f[3], f[5], verdict))
gaps=[]; pairs=0; flips=0
for pr, rows in by.items():
    rows.sort()
    for (t1,s1,v1),(t2,s2,v2) in zip(rows, rows[1:]):
        if s1 != s2: continue            # head changed -> a re-review is expected
        pairs += 1
        if v1 != v2: flips += 1
        a=dt.datetime.fromisoformat(t1.replace("Z","+00:00"))
        b=dt.datetime.fromisoformat(t2.replace("Z","+00:00"))
        gaps.append((b-a).total_seconds()/60)
print(f"consecutive re-reviews of an unchanged head: {pairs}")
print(f"median gap: {statistics.median(gaps):.0f} min")
print(f"under 90 min: {sum(1 for g in gaps if g<90)}")
print(f"recorded verdict changed: {flips} ({100*flips/pairs:.0f}%)")
'
openclaw/clawsweeper openclaw/openclaw
consecutive re-reviews of an unchanged head 109 145
median gap between them 42 min 10 min
under 90 min 79 134
pairs where the recorded verdict changed 16 (15%) 22 (15%)

reviewCadenceMs never returns less than HOURLY_REVIEW_MS, so most of those are
below the intended floor. The counts come only from the history block, which
records prior cycles, so they understate the total; they also drift upward as new
reviews land. The verdict changes go both ways, and #835 oscillates on one head:
needs maintainer reviewfound issuesneeds changesfound issues.

Why

itemContentDigest includes the PR's check runs verbatim:

checks: isPull ? (context.pullChecks ?? null) : null,

pullChecksContext fills that from commits/{headSha}/check-runs?per_page=100
with no author or app filter, so it includes ClawSweeper's own runs. That closes a
loop:

  1. a review completes and writes its labels and durable comment
  2. clawsweeper-dispatch.yml (job dispatch) and github-activity.yml (job
    notify) both trigger on labeled / unlabeled / issue_comment
  3. they add check runs to the PR head
  4. pullChecks changes, so itemContentDigest changes
  5. reviewContentCacheHit misses
  6. the unchanged head is re-reviewed — back to step 1

On #948's head 144236be, 26 of 38 check runs are those two jobs: 13 notify and
13 dispatch. After compactCheckRun reduces each run to
{name,status,conclusion,app}, 13 of them are byte-identical to each other, so
they carry no check state the first one did not.

The same bot is already excluded from every sibling digest input — only checks
were missed:

digest input ClawSweeper's own writes test
timeline excluded via CLAWSWEEPER_BOT_AUTHORS content digest is stable across bot-only context churn
labels excluded via isIgnorableSourceRevisionLabel content digest ignores advisory-label timeline churn
PR review comments excluded via isClawSweeperComment content digest ignores ClawSweeper's own PR review comments
checks not excluded only busts when bounded PR check state changes

Reproduction

Driving the exported itemContentDigestForTest and reviewContentCacheHit
against the check-run list actually observed on that head:

=== A. Does ClawSweeper's own reaction churn change the digest? ===

  OK    one more bot reaction run changes the digest               1092d87e96d999aa -> fd84f307765acbe6
  OK    the added entry is a duplicate of an existing one          13 identical {notify,completed,success} entries already present

=== B. The same bot is filtered out of every sibling digest input ===

  OK    bot timeline entry does NOT change the digest              filtered by CLAWSWEEPER_BOT_AUTHORS
  OK    bot advisory label churn does NOT change the digest        filtered by isIgnorableSourceRevisionLabel
  OK    bot PR review comment does NOT change the digest           filtered by isClawSweeperComment
  OK    bot check run DOES change the digest                       pullChecks has no bot filter  <-- the asymmetry

=== E. Effect on the production cache predicate ===

  OK    cache MISSES after only bot reaction churn -> full re-review reviewContentCacheHit = false

The harness is in the linked PR's description; it needs no credentials, only
pnpm run build.

Cost

Two things, and I am more confident about the first than the second.

Every wasted cycle is a full Codex review: a leased runner, a checkout, and a
model call, for an item whose content did not change. At the observed rate that is
the majority of same-head cycles.

The verdict instability is the part contributors see. A PR can be re-rated with
nothing changed on its side. I have measured the correlation (15% of same-head
re-reviews change the recorded verdict) but not established how much of it this
loop causes rather than ordinary model variance — the loop explains why the extra
cycles happen, not why two runs over identical input disagree.

Note the same raw pullChecks also feeds the semantic and structural digests
(src/review-semantic-cache.ts:785, src/clawsweeper.ts:9343). Those are
consulted as separate, earlier skips, so duplicate runs churn them too. Fixing
only the content digest still prevents the expensive outcome — a full Codex review
of unchanged content — but does not remove the hydration and revalidation work.

Possible fix

De-duplicating the compacted check runs inside itemContentDigest is enough:
once a run is reduced to {name,status,conclusion,app}, a repeat of that tuple
adds nothing, while a run that newly fails differs in conclusion and keeps its
own entry. That leaves pullChecksContext alone, so what the reviewer is shown
through reviewContextLedger does not change — only the cache key does.

I have that up as a PR with the harness and tests, but I would rather you pick the
shape. Two alternatives I considered and did not take:

  • de-duplicate in pullChecksContext — also shrinks the noise in the review
    prompt, but changes reviewer input, not just the cache key
  • filter by bot identityapp.slug is github-actions for both the
    dispatch/notify runs and genuine CI, so this would need a workflow-name
    allowlist that drifts as workflows are renamed

This is the same shape as pullBaseDriftDigestParts in #589 ("Excluding raw age
avoids daily cache churn"), which normalizes a different digest input for the same
reason. That PR and this one both add a helper next to reviewTimelineDigestParts,
so whichever lands second will need a trivial merge.

Not part of this report

  • Truncation. checkRunsTruncated trips above 100 runs and would make
    completePullChecksContext false. These runs accumulate toward that, but nothing
    has crossed it: across the 60 most recent PRs on this repo the largest head
    carried 37 check runs (38 counting fix(ci): build the main bundle in the jobs that publish the action ledger #948). Flagging it as a direction, not a live
    failure.
  • Sub-15-minute re-reviews. 30 same-head pairs on openclaw/clawsweeper are
    under 15 minutes and some are under 5, which is faster than this loop alone
    explains. That looks like a separate lane or lease issue and I have not
    separated it out.
  • Whether two reviews over identical input should agree is a different question
    from how often they are asked to.

Metadata

Metadata

Assignees

No one assigned

    Labels

    P2Normal priority bug or improvement with limited blast radius.clawsweeper:linked-pr-openClawSweeper found an open linked pull request for this issue.clawsweeper:no-new-fix-prClawSweeper does not recommend queueing a new automated fix PR for this issue.clawsweeper:source-reproClawSweeper found a high-confidence source-level issue reproduction.impact:otherThis issue has meaningful maintainer-visible impact outside the owned taxonomy.issue-rating: 🦞 diamond lobsterVery strong issue quality with high-confidence source-level or clear reproduction.

    Type

    No type

    Fields

    Priority

    None yet

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions