Skip to content

fix: O(1) positional lookup in the web streaming reducers - #99

Merged
saucam merged 1 commit into
mainfrom
perf/web-reducer-index
Jul 3, 2026
Merged

fix: O(1) positional lookup in the web streaming reducers#99
saucam merged 1 commit into
mainfrom
perf/web-reducer-index

Conversation

@saucam

@saucam saucam commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

Problem (#90)

The web streaming reducers scanned the message array with buf.findIndex(...) from index 0 on every message and every delta. The #73/#75 fix added an O(1) idsBySession existence Set, but only for the existence check — the positional lookup stayed linear, and it runs over store-proxied entries inside produce(), which is far more expensive per element than a plain array walk. Deltas almost always target the tail message, so a 5000-message session paid the full O(N) scan per streaming chunk: O(N²) over the session, compounding the markdown re-parse cost (#87, fixed separately in #98).

Fix

The per-session existence Set becomes a messageId → index Map (indexBySession):

  • applyMessage uses it for the upsert position and records the index on append — valid forever, since the buffer is append-only.
  • applyDelta replaces both the hasMessage pre-check and the findIndex with one map lookup.
  • replaceScrollback already computes exactly this map during its dedupe pass (posById) — it's now installed wholesale instead of being collapsed into a Set.
  • hasMessage / clearSessionMessages / the test-reset hook read the same structure; semantics unchanged.

Micro-bench

Same script (public reducer API), run against main and this branch — 2000 tail deltas on a 5000-message session:

OLD  findIndex through store proxies: 91 ms total, 46 µs/delta
NEW  map lookup:                       1 ms total,  1 µs/delta   (~46×)

The old per-delta cost also grew linearly with transcript length; the new one is constant.

Tests

All 17 existing reducer tests pass unchanged (upsert semantics, delta patching, dedupe-on-replay, epoch/version bumps, session clearing). Two new cases pin the positional index specifically: deltas land on the correct non-tail targets in a 2000-message buffer, and the index survives re-broadcast upserts plus replaceScrollback rebuilds with duplicate messageIds.

tsc -b ✓ · eslint identical warning set to main ✓ · vitest run 154/154 ✓

Note: web coverage is not uploaded to codecov, so codecov/patch is unaffected by this PR.

Fixes #90

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Improved message updates so edits and deltas apply to the correct message, even in long conversation histories.
    • Fixed issues where repeated or rebroadcast messages could cause later updates to land on the wrong entry.
    • Reduced the chance of stale message changes being applied after older messages are removed or replaced.
  • Performance

    • Made message lookups and updates faster, especially in large sessions.

applyMessage and applyDelta scanned the session buffer with findIndex
from index 0 on every event. The #73/#75 fix made the EXISTENCE check
O(1), but the positional lookup still walked all N store-proxied entries
inside produce() — O(N) per streaming delta, O(N²) over a session, so
long transcripts got progressively laggier.

The per-session existence Set is now a messageId → index Map, kept in
sync on insert (buffer is append-only) and rebuilt wholesale by
replaceScrollback from the dedupe pass it already runs. Both reducers
use it for the positional lookup.

2000 tail deltas on a 5000-message session: 46 µs/delta → 1 µs/delta
(and the old cost kept growing with transcript length).

Fixes #90

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 3, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a3b4027c-1c16-40b1-90b0-91d2eb22e111

📥 Commits

Reviewing files that changed from the base of the PR and between f7487bc and 2a86a7e.

📒 Files selected for processing (2)
  • web/src/state/messages.test.ts
  • web/src/state/messages.ts

📝 Walkthrough

Walkthrough

Replaces the per-session existence Set (idsBySession) with an O(1) positional index (indexBySession: messageId → array index) in the message store. applyMessage, applyDelta, replaceScrollback, hasMessage, clearSessionMessages, and the test reset helper are updated to use the new index; new tests validate positional lookup correctness.

Changes

Positional message index optimization

Layer / File(s) Summary
Positional index data structure
web/src/state/messages.ts
Introduces indexBySession (sessionId → messageId → array index) replacing idsBySession.
applyMessage/applyDelta positional lookups
web/src/state/messages.ts
applyMessage maintains the index on insert/overwrite; applyDelta uses it for O(1) lookup instead of hasMessage + findIndex.
Index rebuild and cleanup
web/src/state/messages.ts
replaceScrollback rebuilds the index from deduped positions; hasMessage checks the positional map; clearSessionMessages and the test reset helper clean up the new index and dependent version counters.
Positional indexing tests
web/src/state/messages.test.ts
New tests validate correct positional delta targeting in large buffers and after upsert/replaceScrollback rebuilds with duplicate messageIds.

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

Possibly related PRs

  • saucam/codeoid#73: Prior PR introduced the existence-based idsBySession Set in the same file/functions that this PR replaces with a positional index.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: switching streaming reducers to O(1) positional lookup.
Linked Issues check ✅ Passed The PR implements the requested messageId→index map and uses it in applyMessage and applyDelta, matching #90.
Out of Scope Changes check ✅ Passed The changes stay focused on the lookup optimization and related tests, with no obvious unrelated code paths added.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch perf/web-reducer-index

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration.


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

@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.06%. Comparing base (f7487bc) to head (2a86a7e).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #99   +/-   ##
=======================================
  Coverage   73.06%   73.06%           
=======================================
  Files          65       65           
  Lines       11032    11032           
=======================================
  Hits         8060     8060           
  Misses       2972     2972           
Flag Coverage Δ
daemon 73.06% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@saucam
saucam merged commit f187ef7 into main Jul 3, 2026
5 checks passed
saucam added a commit that referenced this pull request Jul 6, 2026
…log (#117)

The 0.2.0 entry only covered the protocol/packages train (#100-#116) and
missed ten PRs that also ship in this release: the untrusted-content
sanitization and cross-tenant memory fixes (#91, #93 — now under a
proper Security heading), the performance run (#94-#99), and the model
catalog work (#78, #79).

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

perf: web streaming reducers do O(N) findIndex per event → O(N²) over a session

1 participant