Skip to content

make it faster#150

Merged
derekmisler merged 1 commit intodocker:mainfrom
derekmisler:make-it-faster
Apr 24, 2026
Merged

make it faster#150
derekmisler merged 1 commit intodocker:mainfrom
derekmisler:make-it-faster

Conversation

@derekmisler
Copy link
Copy Markdown
Contributor

@derekmisler derekmisler commented Apr 24, 2026

Summary

This PR optimizes the PR review workflow for speed and reliability by improving artifact download authentication, refining agent delegation logic, and reducing false positives in code analysis. Key improvements include using a GitHub App token for cross-run artifact downloads, enhanced merge-base detection with fallbacks, and restructuring the verifier to use inline code context instead of file access to stay within token budgets.

Testing locally a few times, review time is down from ~15 minutes to under 4 minutes, so that's nice.

Changes

Workflow & Authentication (.github/workflows/review-pr.yml):

  • Removed unused actions: read permission from workflow and job-level declarations
  • Added setup-credentials step to establish GitHub App token for artifact downloads

Agent Configuration (review-pr/agents/pr-review.yaml):

  • Removed unused sonnet-thinking model (high token cost, not used)
  • Root
    • Removed read_multiple_files from root agent toolset; clarified delegation patterns and error handling
  • Verifier
    • Eliminated file-reading capability to reduce token consumption and prevent context exhaustion
    • Verifier now receives ~30 lines of inline code context (from grep -n) provided by root agent for each finding
    • Simplified verification logic: relies on provided snippets + project context rather than disk access
    • Reduced false-positive rate by requiring proof (specific code citations) for dismissals
    • Clarified DISMISS rules: security findings now require snippet-level evidence, not external assumptions
    • Updated verdict guidance: LIKELY when context is insufficient, CONFIRMED/DISMISSED when verifiable

Decision Logic:

  • Root agent now skips verifier delegation when drafter returns zero findings (efficiency)
  • Verifier must produce one verdict per finding (no empty arrays)
  • Reduced guardrail from 20 to 10 source file reads (root agent only)

Evals

✓ File-based diff reading - agent reads pr.diff from disk (run 1) ($0.091728)
  ✓ relevance 5/5
✓ Large diff chunking - agent delegates pre-split chunks (run 1) ($0.118509)
  ✓ relevance 6/6
✓ Non-Marlin analytics library — should not trigger Marlin guide (run 1) ($0.107864)
  ✓ relevance 7/7
✓ Batched verifier with security findings - single delegation call (run 3) ($0.131752)
  ✓ relevance 7/7
✓ Batched verifier with security findings - single delegation call (run 2) ($0.135489)
  ✓ relevance 7/7
✓ Batched verifier with security findings - single delegation call (run 1) ($0.119877)
  ✓ relevance 7/7
✓ Marlin SDK PageView fired in React render body with wrong timestamp format (run 1) ($0.152787)
  ✓ relevance 5/5
✓ Marlin SDK WebClick with action and elementTag from user input — string_kind and action discriminator violations (run 1) ($0.140483)
  ✓ relevance 5/5
✓ Marlin SDK web event from Node server with raw integer enum and non-UUID account_id (run 1) ($0.167256)
  ✓ relevance 6/6
✓ Marlin SDK Go producer setting ingestor-owned fields, identity mismatch, and UGC truncation (run 1) ($0.181298)
  ✓ relevance 6/6
✓ OAuth redirect URI validation - missing allowlist check (run 1) ($0.106116)
  ✓ relevance 6/6
✓ OAuth redirect URI validation - missing allowlist check (run 2) ($0.119736)
  ✓ relevance 6/6
✓ OAuth redirect URI validation - missing allowlist check (run 3) ($0.123971)
  ✓ relevance 6/6
✓ CLI hooks for Gordon hints - clean PR (run 2) ($0.137423)
  ✓ relevance 6/6
✓ CLI hooks for Gordon hints - clean PR (run 1) ($0.148914)
  ✓ relevance 6/6
✓ CLI hooks for Gordon hints - clean PR (run 3) ($0.160739)
  ✓ relevance 6/6
✓ Marlin SDK AppInvoke analytics with PII risk — err.Error() and command args (run 1) ($0.536097)
  ✓ relevance 9/9

Closes: https://github.com/docker/gordon/issues/439

@derekmisler derekmisler self-assigned this Apr 24, 2026
@derekmisler derekmisler marked this pull request as ready for review April 24, 2026 15:23
@derekmisler derekmisler requested a review from a team as a code owner April 24, 2026 15:23
Copy link
Copy Markdown
Contributor

@docker-agent docker-agent Bot left a comment

Choose a reason for hiding this comment

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

Assessment: 🟡 NEEDS ATTENTION

Comment thread review-pr/agents/pr-review.yaml Outdated
@derekmisler derekmisler force-pushed the make-it-faster branch 5 times, most recently from 7ecf1bc to 8820398 Compare April 24, 2026 17:22
Signed-off-by: Derek Misler <derek.misler@docker.com>
@derekmisler derekmisler enabled auto-merge (squash) April 24, 2026 18:12
@derekmisler derekmisler merged commit 18b50f6 into docker:main Apr 24, 2026
11 checks passed
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.

2 participants