fix(server): scrolling back through a long thread no longer re-reads the whole thread on every page by bompus · Pull Request #11 · bompus/t3code

bompus · 2026-10-17T02:04:19Z

Fork review only, never merged: this is the diff intended for pingdotgg/t3code (see pingdotgg#20427).

Scrolling back through a long thread rebuilt the whole thread index on every history page. This keeps the index for the 4 most recently read threads and reuses it while the newest event of the thread and of its fork sources is unchanged.

The revision query moved from ThreadFind.ts to threadRevision.ts. Focused tests: ThreadFind, ProjectionStore, threadHistoryPaging and ThreadSearch, 90 passed.

Summary by cubic

Scrolling back through a long thread no longer rebuilds the whole thread index on every history page, cutting scrollback time from quadratic in thread length (13k items: 72 pages, 2.3–2.7s) to a few cached reads.

Caches the timeline index for the 4 most recently read threads, invalidated when the newest event of the thread or its fork sources changes.
Moves the shared revision query from ThreadFind.ts to the new threadRevision.ts so both callers use the same check.
Adds a test covering cache reuse and invalidation as the thread and its fork source change.

^{Written for commit 5dcc490. Summary will update on new commits.}

coderabbitai · 2026-10-17T02:12:24Z

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 59 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.config.ts
Review profile: CHILL
Plan: Advanced
Run ID: 970bc0ce-993b-441c-8488-5e74ddfcbd2c

📥 Commits

Reviewing files that changed from the base of the PR and between a93b03c and 5dcc490.

📒 Files selected for processing: 4

apps/server/src/orchestration-v2/ProjectionStore.ts
apps/server/src/orchestration-v2/ThreadFind.test.ts
apps/server/src/orchestration-v2/ThreadFind.ts
apps/server/src/orchestration-v2/threadRevision.ts

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.config.ts
Review profile: CHILL
Plan: Advanced
Run ID: a335624f-702e-40c8-b5d3-ae2e94cc1141

📥 Commits

Reviewing files that changed from the base of the PR and between 2328eba and 0f04d16.

📒 Files selected for processing: 1

apps/server/src/orchestration-v2/ProjectionStore.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.

📝 Walkthrough

The change adds a shared reader for thread revisions, including run-fork ancestors. ProjectionStore uses nonzero revisions to cache timeline indexes for history and timeline page reads, with a four-entry limit.

Changes: Thread Timeline Reads

Layer / File(s)	Summary
Shared thread revision reader `apps/server/src/orchestration-v2/threadRevision.ts`, `apps/server/src/orchestration-v2/ThreadFind.ts`	Adds `makeThreadRevisionReader` to read the maximum version-2 thread event sequence across a thread and its run-fork ancestors. `makeThreadFind` uses the shared reader instead of its local implementation.
Revision-keyed timeline index cache `apps/server/src/orchestration-v2/ProjectionStore.ts`, `apps/server/src/orchestration-v2/ThreadFind.test.ts`	Caches timeline indexes by thread when the nonzero revision matches. Cache hits refresh recency, and the cache evicts its least-recently-used entry above four entries. History and timeline page reads use the cache. The test checks that local and parent thread items update the timeline total.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: noojuno

Merge Risk: ⚪ Minimal · up to 0f04d

No actionable change-specific risk remains: ordinary writes to the thread or its fork sources advance the revision used to validate cached indexes.

Security architecture risk: 🔵 Low · up to 0f04d

History caching adds a new freshness requirement. Normal writes preserve the existing read behavior, and no expanded access or privilege was established. Recovery behavior and security coverage remain incompletely verified.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

observed — The new retained state consists of indexes for at most four requested threads, including visibility metadata and inherited run-fork rows. Ordinary item payloads are still fetched from the projection tables when each page is materialized; the cache does not replace those reads with cached full-message payloads.

Trust Boundaries and Controls

observed — Cache lookup is keyed by the requested ThreadId. Inherited rows retain their source thread and item identifiers, and both page materializers use that qualified identity for payload lookup. Revision inputs are schema-encoded and supplied through SQL parameters. These mechanisms preserve the inspected identity flow; they do not establish end-to-end authorization coverage.

Hardening Proposals

proposed — Before supporting projection repair against a live, warmed store, enforce the documented lifecycle requirement by recreating the store or coordinating cache invalidation with rebuild completion. This would make recovery independent of an unenforced fresh-store convention; it is not a verified vulnerability in the current production path.

Pre-merge checks: 3 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Description check		The description explains the problem, the change, and focused verification results. It does not include the required Scope and approval section or a qualifying issue, maintainer approval, or explanati…	Add a Scope and approval section. Link the triaged issue or approved discussion and include the maintainer approval comment. If this focused fix does not require prior approval, explain why it is an obvious bug fix that qualifies for the ex…

Passed checks: 3 passed

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: avoiding repeated full-thread reads while scrolling through history. It also follows the repository's conventional commit format.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Description check

Explanation

The description explains the problem, the change, and focused verification results. It does not include the required Scope and approval section or a qualifying issue, maintainer approval, or explanation for an approval exemption.

Resolution

Add a Scope and approval section. Link the triaged issue or approved discussion and include the maintainer approval comment. If this focused fix does not require prior approval, explain why it is an obvious bug fix that qualifies for the exemption.

✨ Finishing Touches

📝 Generate docstrings

Commit to this branch
Create a new PR

🧪 Generate unit tests (beta)

Commit to this branch
Create a new PR

Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

bompus · 2026-10-17T02:13:18Z

Handled, no push needed (head stays 2328eba, so this review is current).

Description check (no Scope and approval section): this fork PR is for review only. The upstream body will carry that section, linking Scrolling to the top of a synthetic 13,000-item thread takes ~2.4 s of server time: every history page re-reads the whole thread pingdotgg/t3code#20427; that issue has no maintainer verdict yet, so the upstream PR waits on one.
Hardening note (cached index after a projection rebuild): known and already stated as a decision for the reviewer. rebuild appends no events and nothing calls it today; a code comment says a future caller needs a fresh store or must clear the cache.

bompus · 2026-10-17T02:19:21Z

cubic waiver for this pull request: on 2026-10-16 the owner selected the option "Waive cubic for fork PR #11" (reply "9c, 9b"), because cubic is over its account limit ("AI review line limit reached", expected until about 2026-11-11). CodeRabbit finished on the final head 2328eba with no actionable comments.

…the whole thread index on every page Each history or timeline page read every turn item of the thread and rebuilt the visible-item index in JS, so a full scrollback cost time quadratic in thread length (13,000 items: 72 pages, 2.3 to 2.7 s). The index is now kept for the 4 most recently read threads and reused while the newest event of the thread and its fork sources is unchanged, using the same revision check thread find already relies on.

bompus · 2026-10-21T07:32:05Z

@coderabbitai full review

coderabbitai · 2026-10-21T07:32:12Z

⚠️ Action not completed

Review rate limited.

Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 59 minutes.

bompus force-pushed the perf/history-page-index branch from 2328eba to 0f04d16 Compare October 17, 2026 07:19

fix(server): reconcile history index cache with current upstream

5dcc490

Navigation Menu

Repository navigation

fix(server): scrolling back through a long thread no longer re-reads the whole thread on every page - #11

fix(server): scrolling back through a long thread no longer re-reads the whole thread on every page#11
bompus wants to merge 2 commits into
mirror/upstream-mainfrom
perf/history-page-index

bompus commented Oct 17, 2026 •

edited by cubic-dev-ai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 17, 2026 •

edited

Loading

Review limit reached

Uh oh!

bompus commented Oct 17, 2026

Uh oh!

bompus commented Oct 17, 2026

Uh oh!

bompus commented Oct 21, 2026

Uh oh!

coderabbitai Bot commented Oct 21, 2026 •

edited

Loading

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

bompus commented Oct 17, 2026 • edited by cubic-dev-ai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by cubic

Uh oh!

coderabbitai Bot commented Oct 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

bompus commented Oct 17, 2026

Uh oh!

bompus commented Oct 17, 2026

Uh oh!

bompus commented Oct 21, 2026

Uh oh!

coderabbitai Bot commented Oct 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

bompus commented Oct 17, 2026 •

edited by cubic-dev-ai Bot

Loading

coderabbitai Bot commented Oct 17, 2026 •

edited

Loading

coderabbitai Bot commented Oct 21, 2026 •

edited

Loading