Skip to content

feat(#108): short-circuit review on rebase-only pushes - #111

Closed
fullsend-ai-coder[bot] wants to merge 1 commit into
mainfrom
agent/108-rebase-short-circuit
Closed

feat(#108): short-circuit review on rebase-only pushes#111
fullsend-ai-coder[bot] wants to merge 1 commit into
mainfrom
agent/108-rebase-short-circuit

Conversation

@fullsend-ai-coder

Copy link
Copy Markdown
Contributor

Add step 2a-1 to the pr-review skill to detect rebase-only pushes early and skip sub-agent dispatch. When PRIOR_REVIEW_SHA is set, provenance is app-verified, the compare API returns 0 changed files, and the base branch diff is unchanged, the orchestrator reuses the prior review's findings directly — skipping steps 3-6 and proceeding to step 7 to write the result.

This avoids the token cost of dispatching opus-tier sub-agents for a result identical to the prior review, as observed on PR #90 where review run 4 (triggered by a rebase) produced the same output as review run 3.

Guards fall through to full review when the compare API fails, provenance is untrusted, files changed between the prior SHA and HEAD, or the base branch has diverged.

Note: pre-commit could not run in sandbox (network restriction blocked shellcheck-py install). One pre-existing test failure in post-triage-test.sh is unrelated to this change.


Closes #108

Post-script verification

  • Branch is not main/master (agent/108-rebase-short-circuit)
  • Secret scan passed (gitleaks — 8fb64a25b70e4d364f6d535bb252c1bad521629d..HEAD)
  • Pre-commit hooks passed (authoritative run on runner)
  • Tests ran inside sandbox

Add step 2a-1 to the pr-review skill to detect rebase-only
pushes early and skip sub-agent dispatch. When
PRIOR_REVIEW_SHA is set, provenance is app-verified, the
compare API returns 0 changed files, and the base branch
diff is unchanged, the orchestrator reuses the prior
review's findings directly — skipping steps 3-6 and
proceeding to step 7 to write the result.

This avoids the token cost of dispatching opus-tier
sub-agents for a result identical to the prior review, as
observed on PR #90 where review run 4 (triggered by a
rebase) produced the same output as review run 3.

Guards fall through to full review when the compare API
fails, provenance is untrusted, files changed between
the prior SHA and HEAD, or the base branch has diverged.

Note: pre-commit could not run in sandbox (network
restriction blocked shellcheck-py install). One
pre-existing test failure in post-triage-test.sh is
unrelated to this change.

Closes #108
@fullsend-ai-coder
fullsend-ai-coder Bot requested a review from a team as a code owner July 10, 2026 15:50
@ralphbean

Copy link
Copy Markdown
Member

This sounds dangerous to me. I don't want to introduce it without pre-existing evals.

@ralphbean ralphbean closed this Jul 10, 2026
@fullsend-ai-retro

fullsend-ai-retro Bot commented Jul 10, 2026

Copy link
Copy Markdown

🤖 Finished Retro · ✅ Success · Started 7:07 PM UTC · Completed 7:17 PM UTC
Commit: dbe1379 · View workflow run →

@fullsend-ai-retro

Copy link
Copy Markdown

Retro: PR #111 — short-circuit review on rebase-only pushes (closed without merge)

Timeline

  1. Issue #108 created by the retro agent (from PR fix(ci): persist fullsend version and validate before use #90 retro) proposing to skip review agent sub-agent dispatch on rebase-only pushes. A valid optimization idea backed by real evidence.
  2. Triage (run 29104087079) labeled it ready-to-code within 5 minutes — sufficient verdict, correct understanding of the proposal.
  3. Code agent (run 29104398019) implemented in 12 minutes — single commit, 75 additions to skills/pr-review/SKILL.md adding step 2a-1 with well-structured guard conditions.
  4. PR feat(#108): short-circuit review on rebase-only pushes #111 created at 15:50 UTC. All CI checks passed.
  5. Human reviewer ralphbean closed the PR at 19:04 UTC: "This sounds dangerous to me. I don't want to introduce it without pre-existing evals."
  6. Issue Short-circuit review on rebase-only pushes to avoid redundant sub-agent dispatch #108 remains open with ready-to-code label, making it eligible for re-dispatch.

What went well

  • The code agent produced a clean, well-structured implementation in one pass — correct scope, good guard conditions, clear fallback logic.
  • The PR description was thorough and honest about limitations (pre-commit sandbox issue, pre-existing test failure).
  • The human reviewer correctly identified the fundamental blocker: no review evals exist to validate behavioral changes to the review skill.

What went wrong

Existing issues with new evidence

Proposals filed

@ben-alkov

Copy link
Copy Markdown
Member

This sounds dangerous to me. I don't want to introduce it without pre-existing evals.

Agreed, though it is a feature which we need to have eventually. Time for 🤔.

@ben-alkov

Copy link
Copy Markdown
Member

What about something deterministic on the invocation side? If the "changes" to the PR are ONLY commit SHA changes (no diff changes), fail early and don't launch the review agent...

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.

Short-circuit review on rebase-only pushes to avoid redundant sub-agent dispatch

2 participants