Speed up PR review thread loading - #144
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 47a0dd6f5d
ℹ️ 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".
| threadId, | ||
| body: parsed.output.text, | ||
| }); | ||
| invalidatePullRequestReviewSurfaceThreadCache({ |
There was a problem hiding this comment.
Invalidate review-thread cache before fallible refresh
When replyToPullRequestReviewThread is used, the helper posts the GitHub reply and then performs a separate fetchPullRequestReviewThread; if that follow-up read times out or is rate-limited after the reply succeeds, execution never reaches this new invalidation. In that scenario a warm /prs/review-threads cache can keep returning the pre-reply thread list for the 15s TTL (the resolve/unresolve helpers have the same pattern), so invalidate in a finally or immediately after the GitHub mutation is known to have run.
Useful? React with 👍 / 👎.
Summary
Why
The production review surface spent only about 11 ms rendering threads after the response, while the uncached GitHub GraphQL request took roughly 0.9–1.1 seconds and transferred 381 KB. The remaining bottleneck was the request path rather than React, TanStack Query, or Pierre.
Measured result
Real registered target:
Kilo-Org/kilocode#12204, 1,019 files and 47 review threads.The initial uncached GitHub read remains about 655 ms, the tree median remains 642 ms, and both stay explicit follow-ups.
Cache safety
The cache is process-local only, capped at 16 entries, expires after 15 seconds, and uses a token fingerprint in its key. Successful review submissions, replies, and resolve/unresolve actions invalidate the target. A mutation racing an in-flight read prevents the stale result from being cached. In-flight calls are not shared, so one caller's cancellation cannot cancel another caller's work.
Validation
npm run verify— lint, layers, database checks, types, 560 unit tests, 33 git tests, 129 integration tests, dashboard/docs builds, package validation, pack smoke, formattingnpm run checkafter cancellation-semantics hardening — 561 unit testsnpm run bench:pr-reviewagainst the immutable PR revision