Skip to content

Measure real PR review performance - #137

Merged
pandemicsyn merged 1 commit into
mainfrom
agent/real-pr-performance-trace
Jul 17, 2026
Merged

pandemicsyn merged 1 commit into
mainfrom
agent/real-pr-performance-trace

Conversation

@pandemicsyn

Copy link
Copy Markdown
Owner

Summary

  • add a repeatable npm run bench:pr-review command for immutable revisions in registered repositories
  • measure backend cold/warm/local/fallback behavior and production-browser tree, thread, and first-patch readiness
  • reconcile the frontend audit and PR performance plan with phases 1–5 that already shipped
  • retain the original findings and document candidate fixes without changing production behavior

Real target

Measured open Kilo-Org/kilocode#12204 at a fixed head/base revision:

  • 1,019 files, 45,053 additions, 23,744 deletions
  • 47 review threads; two unresolved paths
  • both target objects were absent before the first local request

Results

  • cold local metadata: 4,978 ms, 295 kB, zero truncated files; misses the 3 second target
  • warm backend: 311 ms file list and 643 ms sequential first patch
  • production browser medians: 869 ms tree, 1,971 ms first patch, 2,183 ms threads
  • production rendering remained healthy: 876 ms LCP, 0.0012 CLS, zero long tasks
  • every browser sample started two review-thread requests as optimistic PR state settled to authoritative detail
  • four concurrent local patches took 1,952 ms because each patch path recomputes full 1,019-file metadata
  • GitHub fallback: 3,894 ms cold / 24 ms cached, 765 kB metadata, one truncated file

Discussion gate

This PR intentionally does not implement the measured production fixes. The plan now proposes, in order:

  1. stabilize review-thread query identity while preserving explicit invalidation
  2. reuse or single-flight immutable local metadata across patch reads
  3. remeasure before delaying neighbor/unresolved prefetch or changing the cold-fetch target

Verification

  • npm run bench:pr-review -- ... --samples 3
  • npm run build:dashboard
  • npm run check (84 files, 524 tests)
  • npm run format:check
  • standalone Oxlint and syntax checks for the new benchmark
  • direct Chrome DevTools production trace
  • git diff --check

@pandemicsyn
pandemicsyn merged commit a3127f6 into main Jul 17, 2026
5 checks passed
@pandemicsyn
pandemicsyn deleted the agent/real-pr-performance-trace branch July 17, 2026 12:58

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c814f1ca48

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +262 to +264
!state.threadsVisibleMs &&
body.includes(`/${expectedThreadCount}`) &&
body.includes('threads')

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Require loaded thread state before timing zero-thread PRs

If this benchmark is run against a PR with no review threads, the review header already renders 0/0 threads before the review-threads request resolves, so body.includes('/0') && body.includes('threads') can mark threadsVisibleMs at initial paint instead of endpoint/UI readiness. That makes zero-thread PR samples under-report the thread path the harness claims to measure; wait for a completed thread response or a loaded-state marker before accepting the 0 case.

Useful? React with 👍 / 👎.

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.

1 participant