fix(ui): bound, cache and coalesce the scoped folder-count walk that keeps emails ui spinning

[andrei-hasna]

Closes the remaining half of the emails ui idle-CPU defect, tracked as todos 90e98ccc.
Found by adversarial review of #198 (Cato) and deliberately not folded in there: pre-existing,
different code path, and it would have made an already substantial diff unreviewable.

Revised after two NO_GO reviews. Cato (correctness-and-safety) refuted the original bound;
Seneca (evidence-quality) refuted the original narrative using this PR's own numbers. Both are
fixed below and the corrections are called out inline rather than quietly reworded, because in
both cases the first version was confidently wrong in a checkable way.

What is wrong

mailboxCounts → scanScopeRows follows the cursor chain with no bound that can fire, no cache
and no coalescing, and follows it twice for an address (the {to}/{from} union). Its only
bound is seen.size > MAX_SCAN_ROWS, which counts rows matched — so a serve that ignores
?to=/?from= matches little per page and the walk effectively never terminates early.

Reachability is proven, not inferred: sourceForSelection sets source.address from the selected
inbox, so selfHostedScopeOf returns a scope and mailboxCounts takes the scanScopeRows branch.
Once any single inbox is selected rather than "All inboxes", every idle 30s tick runs that double
walk.

Two more, both named in the same review

The state bug. setAddress committed selectedAddressId before persistSetting, and
setSetting throws in self-hosted mode. The scoped state landed while the action aborted — the
reload never ran and the user was pinned to one inbox by an action that had visibly failed. This
reproduced live: driving the picker on unmodified main throws out of selectActive
(select-dialog.tsx:29 → dialogs.tsx:144) and the app falls through to the message reader.

A guard that excluded nothing. The refresh interval read !state.busyPull; busyPull was
initialised false and never set true anywhere. It now guards on loading, which reload()
maintains. This is a correctness fix, not a performance one — see the isolated measurement
below, where it contributes nothing.

The fix

scanScopeRows has a second caller: the destructive clear(), whose own comment states the
complete walk is "preflighted before the first destructive request" — it deletes exactly what the
walk returns. A budget or TTL pushed down into the shared helper would make clear() delete a
partial or stale subset while reporting a plausible count. So the counting walk is its own method:

1. BOUND — on rows SCANNED, per filter set, against the same MAX_SCAN_ROWS.

   Corrected after Cato's P1. The first version capped requests at 200, arguing
   200 × PAGE_LIMIT == MAX_SCAN_ROWS so nothing working would break. That is false whenever
   pages are not full. Two stores that resolve exactly on current main and threw under it —
   independently reproduced by me before accepting the finding:

   PROBE DEEP(300 pages x 2 rows):  inbox=600    requests=600
   PROBE 60k(120 pages x 500 rows): inbox=60000  requests=240
   

   Both sit far below MAX_SCAN_ROWS, so a request cap is strictly tighter than the row cap it
   claimed to mirror. It is now bounded on rows scanned, per filter set — a shared bound would
   have thrown on the 60k store, since today's bound counts deduped matches and the union reads
   the same store twice. A request cap of 10,000 remains only as a runaway guard for a serve
   returning near-empty pages with fresh cursors. does NOT break scoped stores that complete today is now a test.

2. CACHE — SCOPED_COUNT_TTL_MS = 60_000, above the 30s refresh or it buys nothing. Keyed per
   scope; the store-wide key that is correct for the label tally would serve one inbox's counts
   for another.
3. COALESCE — one in-flight promise per scope behind a generation fence, cleared by
   invalidate(). Honest scope: this contributes nothing to the measurement below (see P1-2). It
   is there for the case where a walk outlives the 30s tick, which this benchmark's 8.75s walk does
   not reach.

It also never retains the rows — counting needs a tally and the ids already seen, not every
message object.

Measurements

setsid script -qec "… ui" /dev/null; CPU from /proc/<pid>/stat utime+stime deltas; target found
by walking pty descendants and reading /proc/<pid>/exe, never a cmdline grep; idle sleep as
negative control in the same run. 50,000-message store, single 181s window for both metrics.

Corrected after Seneca's P1-1. The first version compared main+setAddress against a branch
that also carried the busyPull→loading change — two variables, one attribution. BEFORE now
carries BOTH TUI changes, so the counts fix is the only difference.

BEFORE  control(sleep) delta_ticks=0 window=10s        load 11.89
        w1 44.9  w2 42.2  w3 46.2  w4 42.6  w5 40.8  w6 42.1     <- flat
        /v1/messages over 181s idle = 1312  (7.25 req/s)
        MEAN cpu over 181s = 42.9%

AFTER   control(sleep) delta_ticks=0 window=10s        load 11.35
        w1 14.2  w2 6.7  w3 36.1  w4 13.6  w5 6.3  w6 37.6       <- periodic
        /v1/messages over 181s idle = 512   (2.83 req/s)
        MEAN cpu over 181s = 19.0%

What this delta actually is, stated because the first version got it wrong.

Corrected after Seneca's P1-2, which refuted the original narrative using these very numbers.
The first version inherited #198's stacking story. This PR's own trace refutes it: the BEFORE
windows are flat (40.8–46.2%) and were labelled flat; the CLI line below bounds one walk at 8.75s
wall, and an 8.75s walk cannot stack on a 30s tick; and 1312 req / 181s is ≈ 6 ticks × 200,
i.e. exactly one walk per tick in steady state. The measured delta is the 60s-vs-30s TTL cache
alone — predicted 2.0x, observed 2.26x on CPU and 2.56x on requests. Stacking was real for
#198's ~340-request label walk; it is not what is happening here.

The isolated run also settles the guard: BEFORE carried the loading guard and still measured
42.9%, versus 41.2% without it. The guard changes nothing here, which is what you would expect
once you accept that nothing was overlapping. It stays as a correctness fix.

Real CLI, same code path (inbox mailboxes --address …, one cold call):

BEFORE  200 requests   8.75s wall   10.18s user   maxrss 486672 kB   (n=1)
AFTER   200 requests   6.54s wall    8.44s user   maxrss 236952 kB   (n=1)
counts: byte-identical

What these numbers do NOT establish

• The bench serve does not implement ?to=/?from= filtering, and that inflates the magnitude.
  It is internally fair — same serve both sides — but against a serve that honours those filters
  an ordinary inbox's pre-fix walk is one or two requests and the absolute saving collapses toward
  zero. The win is real for the filter-ignoring case and for very large scopes; it is not a
  fleet-wide 2x.
• The benchmark was structurally blind to Cato's P1. 50,000 rows / PAGE_LIMIT 500 = 100 pages
  × 2 filter sets = exactly 200 requests, one page under the original > 200 guard. It could not
  have revealed that bound's defect. Flagged by Seneca; recorded because the coincidence is the
  point, not the escape.
• Axes not varied: store size, page fullness, scope selectivity, serve filtering, and
  inbox-switching frequency — the cache is per-scope, so a user switching inboxes faster than 60s
  gets no hits at all.
• A 65s window is too short to rate a 60s TTL. An earlier attempt produced a "10x" that was pure
  window alignment. Not quoted anywhere.
• Memory figures are n=1 maxrss.

Freshness, corrected

The first version said "counts stay exact". True about sampling — they are never a sample,
unlike the label tally — and misleading about freshness. invalidate() covers only this
client's writes, so mail arriving from outside is now invisible to the sidebar counts for up to
60s, where before it appeared within one 30s tick. That is the trade this cache buys, and it
is the honest cost line.

Regression tests — twelve, every one load-bearing

Asserting request counts and result content, never timing. Three fail on unmodified main with
the defect's own numbers: 240 requests for three concurrent calls (expected ≤ 90); 160 for a
repeat inside TTL (expected 80); and the bound test resolving instead of failing closed.

The nine guards were each proven to fire by mutation:

mutation	caught by	result
the naive copy: budget + TTL pushed into shared scanScopeRows	clear() preflight test	1 fail
cache key drops the domain dimension	two-different-domain-scopes test	1 fail
removing both defensive copies	caller-poisons-the-cache test	1 fail
cache never expires and writes never drop it	TTL expiry, write-drop, mid-walk fence	3 fail
no generation fence	mid-walk fence	1 fail
tightening the row bound	does NOT break stores that complete today	1 fail
budget of 1 (anti-vacuity)	exactness guard + 7 others	8 fail

The domain-key and defensive-copy tests exist because Cato demonstrated wrong implementations that
passed all nine of the original tests. My first attempt at the domain test still did not catch it —
it compared an address scope against a domain scope, which produce different keys even when broken;
the collision is between two domain-only scopes, which both have no address. Fixed and re-verified.

tsc --noEmit: rc=0, stdout and stderr both 0 bytes. Data-source file: 124 pass, 0 fail.

Not changed, deliberately

The sort === "oldest" branch is already bounded by MAX_FILTER_WALK_REQUESTS and is a full-chain
walk by construction; state.sort is not persisted. setSetting remains unguarded at four other
dialogs.tsx call sites — Cato's P3, the same class, but those are settings actions where the error
is the correct outcome, unlike selecting an inbox. A server-side scoped counts aggregate would remove
the residual walk entirely; /v1/messages/counts accepts ?domain= but has no recipient filter, and
pushing the domain case down is unsafe while an older serve would silently ignore it and return
whole-store counts as scoped ones.

Landed with gh pr merge --squash --body-file, last line the Agent: trailer.

[andrei-hasna]

[REVIEW] GO — #201 @ 76df59c — lens: correctness+security+gates, reviewer unresolved-account002 (1 of 1)

Acceptance scope: emails-201-76df59c-scoped-counts-v1 — bound/cache/coalesce self-hosted scoped folder counts without weakening exact destructive clear(), keep TUI address selection usable, and prevent periodic reload overlap.

What I read:

• git log --oneline origin/main..HEAD — exit 0; one commit, exact head 76df59c84f4cf8f98c622f6267117e2c057eb163.
• git diff origin/main...HEAD --stat — exit 0; 5 files, 432 insertions, 17 deletions.
• Full three-dot diff for all changed files: package.json, src/cli/tui-solid/component/sidebar.tsx, src/cli/tui-solid/context/emails-state.tsx, src/lib/self-hosted-mail-data-source.ts, and src/lib/self-hosted-mail-data-source.test.ts.
• Surrounding callers and invariants: TUI reload / sidebar metadata / timer actions, scope normalization and server filter sets, cursor pagination, cache invalidation and every write caller, mailbox counts/status, and the exact uncached clear() preflight.

Correctness/security result:

• Blocking P0/P1 findings: none.
• The new count walk is request-bounded across the address union, keyed per normalized scope, coalesced only within a generation, copy-on-return, and kept separate from the exact destructive clear() walk. Scope values still enter requests through URLSearchParams; bearer handling and authorization boundaries are unchanged.

Declared commands:

• bun install — exit 0. Setup only; no test pass/fail count applies.
• bun run test — exit 0: 4298 pass, 156 skip, 0 fail, 20,949 assertions across 4,454 tests / 288 files.
• The repository declares no typecheck script, so no typecheck gate was invented or run.

Non-blocking follow-up:

• P2: invalidate() advances the new cache generation before PATCH/DELETE/POST completes. A count walk that begins after that invalidation but before the write commits can install pre-write sidebar counts for up to the 60-second TTL. This is reachable under a concurrent refresh, but it neither changes mail nor survives the TTL, so it does not meet the blocking bar; a later change can invalidate again after successful writes or add a write-in-flight fence.

Verdict: GO. The exact-head declared gate is green and no reachable in-scope P0/P1 defect remains.

[andrei-hasna]

[REVIEW] NO_GO — #201 @ 76df59c — lens: correctness-and-safety, reviewer Cato (1 of 2)

One blocking finding (P1). The concurrency design is sound and I could not break it; what fails is the budget's stated equivalence to the old bound, which I refuted by running the PR's own fixture against main. The remedy is ~3 lines and also fixes P2-1.

Staleness check — refs/pull/201/merge^1 == origin/main == fda89b531e22ca060f8c087434fc3420e42b9f68, so the tree I reviewed is the tree that would land.

Gates — bunx tsc --noEmit -p tsconfig.json → rc=0, stdout and stderr both 0 bytes. bun test src/lib/self-hosted-mail-data-source.test.ts:

 121 pass
 0 fail
 1812 expect() calls
Ran 121 tests across 1 file. [1.92s]

Both worktrees left clean (git status --short --untracked-files=all → 0 bytes; git diff --stat HEAD → 0 bytes).

---

P1 — src/lib/self-hosted-mail-data-source.ts:277-280: "a store that works now keeps working" is FALSE, measured on main

The comment justifying the constant:

// 200 x PAGE_LIMIT is 100_000 rows: the same worst case
// as MAX_SCAN_ROWS, which is what bounds this path today, so a store that works
// now keeps working — the change is that the bound now counts rows SCANNED
// rather than rows matched, which is what makes it reachable at all.

The second clause is correct and the conclusion does not follow from it. 200 × PAGE_LIMIT == MAX_SCAN_ROWS only when the serve honours ?to=/?from= and returns a full 500 rows per page. Where either does not hold, scanned ≫ matched and the new ceiling collapses below the old one.

I built the counterexample by running this PR's own scopedDeepServe fixture — which ignores ?to=/?from= — against the pre-fix implementation in the baseline worktree at fda89b5:

PROBE pre-fix DEEP(300 pages x 2 rows): inbox=600 requests=600
PROBE pre-fix 60k(120 pages x 500 rows): inbox=60000 requests=240

 3 pass
 0 fail
Ran 3 tests across 1 file. [6.48s]

The third of those three is a negative control asserting the pre-fix code does still throw past MAX_SCAN_ROWS (210 pages × 500 = 105,000 matched rows → rejects.toThrow(/safety limit/i)), so the probe can fail and its two passes are observations rather than a rigged instrument.

Read those two lines against the new budget:

store	matched rows	requests	pre-fix (seen.size > 100_000)	this PR (requests > 200)
300 pages × 2 rows	600	600	resolves, inbox=600	throws
120 pages × 500 rows	60,000	240	resolves, inbox=60000	throws

The first row is literally the store in the PR's own first test (DEEP_PAGES = 300). That test asserts the new code throws; the same store returns exact counts today.

Why this class is reachable rather than theoretical, as evidence and not assertion:

• The PR's own error text names it as an expected condition — "Either this server ignored the GET /v1/messages ?to=/?from= recipient filters" — and the design comment at :263 depends on that class existing ("a serve that ignores ?to=/?from= matches little per page and never terminates early at all"). The PR cannot both justify the change by that class existing and claim nothing that works today breaks.
• The client has no server-version negotiation and no capability probe: grep -rn "MIN_SERVER_VERSION\|minServerVersion\|server_version\|X-Emails-Version\|capabilit" over self-hosted-mail-data-source.ts and self-hosted-wire.ts returned rc=1, 0 lines. It cannot detect a filter-ignoring serve.
• It explicitly supports older serves (LEGACY_MAX_OFFSET, the legacy-offset resume path at :137/:408/:416).
• A second, independent path to the same place: the shipped serve's to/from filters are LIKE '%…%' substring matches over to_addrs::text / from_addr (src/server/self-hosted/store.ts:2322-2329), while scopeMatch (:845-851) requires an exact address or an endsWith('@'+domain). The server over-fetches and the client under-selects by construction, so scanned exceeds matched on any store with overlapping address or domain text.

Impact when it fires: mailboxCounts is awaited inside the sidebar Promise.all (src/cli/tui-solid/context/emails-state.tsx:270-273), so the rejection takes counts, labels and the addresses picker list down with it into the catch at :276 — those four are set in one setState at :275. listMailboxStatus (self-hosted-mail-data-source.ts:1490-1491) routes through the same call, so the scoped CLI status path fails identically. Deployments in this class get a permanent error where they currently get correct counts.

Suggested remedy (not a fix I applied): the walk already iterates every row, so bound it on rows actually scanned rather than requests — scanned += page.length; if (scanned > MAX_SCAN_ROWS) throw …. That makes the new bound genuinely identical to the old one on every server, removes this entire class, and turns P2-1's fabricated figure into a real measurement. Keep a generous request cap alongside it so a serve emitting an endless chain of empty pages still terminates; the two caps answer different questions.

---

P2-1 — src/lib/self-hosted-mail-data-source.ts:425-433: the exhaustion error reports a fabricated row count and points at the wrong cause

scopedCountWalkExhausted(requests * PAGE_LIMIT) multiplies the request count by the requested page size — an upper bound presented as a measurement. Verbatim output on a store I built to hold exactly 600 rows:

PROBE real store size = 600 rows (300 pages x 2). Error text:
self-hosted emails: scoped folder counts scanned 100500 rows over 200 requests without completing. Either this server ignored the GET /v1/messages ?to=/?from= recipient filters, or this one address holds more than 100000 messages — upgrade the emails-serve deployment, or scope the read to a domain instead of an address.

600 actual rows reported as scanned 100500 rows, off by 167×. The message offers two causes and the number it prints actively corroborates the wrong one: an operator reading "100500 rows … holds more than 100000 messages" concludes the mailbox is enormous, not that their serve ignores the filters. It also prints the constant 200 while the walk had issued 201 requests when it threw. Counting real rows fixes the number and the P1 together.

P2-2 — src/lib/self-hosted-mail-data-source.test.ts:2852-3117: two wrong implementations pass all nine new tests

I wrote them and ran them. Both mutations applied at once:

• W11 — scopedCountsKey drops the domain dimension (return \a=${scope.address ?? ""}`;`).
• W3 — remove all three defensive copies, handing out the cached MailboxCounts object by reference.

 121 pass
 0 fail
 1812 expect() calls
Ran 121 tests across 1 file. [2.04s]

Byte-identical to the unmutated run. Both are genuinely exploitable — a probe I wrote fails on the mutant and passes on the shipped code, which is the two-sided gate:

mutant : PROBE domain-key: alpha.inbox=3 beta.inbox=3 (expect 3 and 7)     rc=1
mutant : PROBE alias: second.inbox=999999 (expect 3)
shipped: PROBE domain-key: alpha.inbox=3 beta.inbox=7 (expect 3 and 7)     rc=0
shipped: PROBE alias: second.inbox=3 (expect 3)

The shipped code is correct on both properties — the tests would not notice if it stopped being. That matters because test 4's own comment claims to guard exactly this ("it would serve one inbox's folder counts for another") and only exercises address-vs-address; there is no two-domain case anywhere, and the key's comment at :437 asserts "distinct scopes cannot collide on one key" — the claim the missing case would pin. The caller-mutation hazard the copy at :1473-1475 exists for is likewise unasserted. Two tests, ~15 lines.

P3 — non-blocking observations

1. setSetting is still unguarded at four live call sites. The PR correctly root-causes the class at emails-state.tsx:516-522 (self-hosted setSetting throws for every key — data.remote.ts:813-816) and then patches one of five call sites. dialogs.tsx:754, 793, 811, 816 still call emails.actions.setSetting(...) from onPress handlers via the unguarded action at emails-state.tsx:626-630. Pre-existing and out of scope for blocking, but the seam is the right place to fix it once rather than a try/catch per site.
2. The swallow at emails-state.tsx:526-528 is silent rather than reported. In local mode setSetting can fail on a real config write (saveConfig), and the bare catch {} discards it. setState("settings", …) sits inside the try after persistSetting, so no false "saved" state is displayed — the loss is only the diagnostic. setState("lastError", …) in that catch would keep the selection and tell the user.
3. The loading guard cannot wedge — but it does not cover the expensive walk. loading is set before the try at :285 and cleared in finally at :317, and every await beneath it is bounded by AbortSignal.timeout(this.timeoutMs) (self-hosted-mail-data-source.ts:996-998, default 30s), so it cannot stay true forever on the self-hosted path — I did not audit the local SQLite path for unbounded awaits. Note though that scheduleSidebarMeta is fired via setTimeout(…, 0) and is not awaited by reload, so the counts walk runs entirely outside loading; what actually stops those stacking is the new coalescing, not this guard. busyPull has no remaining consumer — repo-wide grep -rn "busyPull" returns exactly one hit, the comment at emails-state.tsx:649.
4. clear() is genuinely unaffected — confirmed from code, not from the assertion. clear() (:1939-1959) calls scanScopeRows, which shares no state with scopedCounts: separate cache Map, separate generation counter, separate budget, and scanScopeRows reads no cache on the scoped path. The only coupling runs the other way — each deleteMessage inside clear() calls invalidate(), which fences an in-flight counts walk. That is correct. Test 9 covers the warmed-cache case but not the in-flight one; nothing in the code can make that case differ. Separately and pre-existing: for an unscoped clear, scanScopeRows delegates to the 15s-TTL scanAll(), so the "complete cursor walk is preflighted" comment at :1941-1942 is already only true of the scoped path.
5. Cross-process writes are invisible to the fence, so counts lag new mail by up to 60s. invalidate() only fires for writes this instance performs; mail arriving at the serve, or a write from another process, does not bump the generation. With SCOPED_COUNT_TTL_MS = 60_000 against a 30s refresh, at least every other refresh serves a cached tally, while the message list (listFilteredMailboxPage) is uncached and live — so the sidebar count and the list beside it can visibly disagree for up to a minute. Same trade already shipped for LABEL_TALLY_TTL_MS in fix(ui): bound the label-summary scan that made emails ui spin at ~92% CPU #198, so consistent rather than novel; worth one line in the PR body.
6. The cache is defeated under interactive reading. openMessage fires void ds.setRead(...) (emails-state.tsx:353-354) → invalidate() → generation bump. A user opening messages faster than a walk completes fences every walk in turn, and the next tick cannot join the previous generation, so walks stack again. The PR's target is idle CPU and there are no writes when idle, so the headline claim holds; this only bounds how much the cache buys in use.
7. scopedCountsKey collision is unreachable in practice but the comment overstates it. :436-437 says the separator "cannot occur in either" field. Neither address nor domain is validated in selfHostedScopeOf (:819-843) beyond trim().toLowerCase(), so {address: "x d=y"} and {address: "x", domain: "y d="} both key to a=x d=y d=. Malformed input with no real path — noted only so the comment is not read as a proof. Separately, a scope carrying both fields keys differently from the address-only scope it behaves identically to (scopeMatch and scopeServerFilterSets both ignore domain when address is set), which costs a redundant walk, never a wrong number.

---

Answers to the review questions, in one line each

1. Wrong number? Not from the concurrency design. Generation fencing, coalescing, per-scope keying, union dedupe and copy-on-return are each correct as written and I could not produce a wrong tally from them. The reachable staleness is item P3-5 (cross-process writes, ≤60s) and the reachable failure is P1.
2. clear()? Genuinely unaffected — P3-4, argued from the code paths, not from the PR's assertion.
3. Budget? The claim is false and the counterexample is the PR's own fixture — P1.
4. Wedge / swallow / busyPull? Cannot wedge; swallow loses a local-mode diagnostic (P3-2); busyPull has no orphan consumer.
5. Vacuous tests? Yes — two wrong implementations pass all nine — P2-2.

What I did not check

The local (SQLite) data source's mailboxCounts and its await-boundedness; any real deployed emails-serve older than the one in this tree (I reasoned about that class from the client's own compatibility code and the PR's own error text, and could not confirm a live instance); and whether any individual one of the nine new tests is wrong as opposed to insufficient — I attacked their completeness, not their claims.

[andrei-hasna]

[REVIEW] NO_GO — #201 @ 76df59c — lens: evidence-quality, reviewer Seneca (2 of 2)

Scope: do the measurements support the claims. I did not review correctness of the caching mechanism itself except where a claim rests on it. The code mechanism is sound and the test suite is genuinely strong — I verified it in both directions and it is better than most on this fleet. Everything blocking below is in the attribution of the measurement and in claims contradicted by the PR's own numbers.

---

P1-1 — The pty comparison is a TWO-variable experiment reported as one

BEFORE is main + the setAddress fix. AFTER is the branch, which is setAddress fix + counts fix + the refresh-guard change (busyPull → loading). The entire 41.2% → 20.0% delta is attributed to the counts fix.

Choosing BEFORE = "main + setAddress only" is defensible and is disclosed — unmodified main cannot reach the scoped state, and that change makes BEFORE worse, not better. That part is fine. The problem is the guard change riding along on the AFTER side only.

This produces a dilemma the PR cannot escape in either direction:

• If the guard change had a real effect, the counts fix is over-credited by an unmeasured amount.
• If it had no effect, then the "stacking" narrative used to justify it (P1-2) is false.

Both cannot hold. Nothing in the evidence separates them. The fix for this is cheap: a third run, or an explicit statement that the guard change is a no-op in this benchmark with the reasoning shown.

P1-2 — The "stacking / climbing idle CPU" narrative is contradicted by this PR's own numbers

The body says "every 30s tick stacked another reload on the last"; the code comment at scopedCounts says "the accumulation behind the climbing idle CPU".

Three of the PR's own measurements refute accumulation:

1. Its own BEFORE trace is flat, and the PR labels it so itself: w1 44.1% w2 41.3% w3 39.7% w4 42.3% w5 41.0% w6 39.9% <- flat, continuous. Flat over 181s is a steady state. Climbing is what accumulation looks like, and it is not there.
2. Its own CLI number bounds the walk duration: BEFORE 200 requests 8.75s wall. A walk that finishes in 8.75s cannot stack on a 30s tick. The useless busyPull guard is a genuine code defect, but in this benchmark it excluded nothing because nothing needed excluding.
3. The request arithmetic is one walk per tick, not N: 1312 requests / 181s ≈ 6 ticks × 200 requests + bounded label walks. That is steady state.

Consequence for the headline: BEFORE is one 200-request walk per 30s tick, AFTER is one per 60s TTL. The delta is exactly the cache ratio — 2.0 expected, 2.06 observed on CPU, 2.56 on requests. The measured improvement is the TTL cache alone. The BOUND and the COALESCE contribute nothing measurable here (see P2-1 for why the bound cannot fire in this bench, and no stacking occurred for coalescing to prevent). The three-part framing is not what was measured.

---

P2-1 — The headline is a property of the bench serve, and the bench store sits exactly on the budget boundary

PAGE_LIMIT = 500; an address scope is two filter sets (scopeServerFilterSets: [{to}, {from}]); the bench store is 50,000 messages against a serve that ignores ?to=/?from=, so each set walks the whole store:

100 pages x 2 filter sets = 200 requests == MAX_SCOPED_COUNT_REQUESTS
guard is `if (requests > MAX_SCOPED_COUNT_REQUESTS)`  -> the 201st throws

The AFTER run was therefore taken at the largest store size that does not trip the new bound, one page below the cliff. A store 500 messages larger produces an error instead of counts — and would have scored better on idle CPU while being broken. The measurement cannot detect the cliff it is standing on, and the PR reads it as headroom ("one exact 200-request scoped walk").

On question 2 — does the filter-ignoring caveat hold for both sides equally: for the ratio, yes; for the magnitude, no. Both sides run the same serve and the same 200-request walk, so the comparison is internally fair and not flattered. But against a serve that honours the filters, the pre-fix walk for an ordinary inbox is already one or two requests, so the absolute saving collapses toward zero and the residual has nothing to remove. The caveat is stated honestly in "Stated honestly" but is not carried into the headline numbers, which read as properties of the fix rather than of this bench.

Corroboration only, not re-litigated: the related claim "200 × PAGE_LIMIT is 100,000 rows: the same worst case as MAX_SCAN_ROWS ... so a store that works now keeps working" is false. Cato established this independently and remediation is in flight. My instrument agrees by a different route: baseline bounds seen.size — matched rows (self-hosted-mail-data-source.ts:1251) — while the branch bounds requests across both filter sets, so the two coincide only when every scanned row matches, i.e. exactly when the budget is not needed. I flag it here only because it is the same constant this measurement was taken against.

P2-2 — Two mutations survive the suite the PR calls uniformly load-bearing

The PR states "a passing test that no wrong implementation can fail is not evidence". I wrote my own wrong implementations. Four were caught; two were not.

=== M5_no_defensive_copy rc=0      9 pass   0 fail
=== M6_budget_off_by_one rc=0      9 pass   0 fail

• M5 removes both defensive copies — return cached.counts and return await walk instead of the spreads. The code comment claims this guard is needed: "Copied on the way out so a caller holding the result cannot mutate the cached entry that later callers will be served." A documented invariant with no covering test.
• M6 flips requests > MAX_SCOPED_COUNT_REQUESTS to >=, silently making the budget 199. Uncaught — and per P2-1 the exact boundary is load-bearing for the benchmark itself, since the bench needs all 200.

(A third, SCOPED_COUNT_TTL_MS 60s → 31s, also survives at 9 pass 0 fail. Not a defect — still above the 30s refresh — but the constant the PR argues for is not pinned either.)

---

P3 — Smaller

• Axes not varied. The bench holds fixed: store size (one 50,000 store, at the cap), page fullness, scope selectivity, serve filtering behaviour, folder distribution, and inbox-switching frequency. Two of these carry the defect class. Scope selectivity decides whether the budget trips at all. Inbox-switching matters more than it looks: the cache is keyed per scope, so a user moving between inboxes faster than the 60s TTL gets zero cache hits and a full walk per switch — the ordinary interactive case, and the one case where the pre-fix and post-fix CPU would converge. Nothing measures it.
• "Counts stay exact" is true for sampling and misleading for freshness. invalidate() drops the cache on writes this client makes; inbound mail arriving from anywhere else is now invisible for up to 60s, where pre-fix it appeared within 30s. A real behaviour change, undisclosed.
• "20% idle CPU remains. That is one exact 200-request scoped walk per TTL." Asserted, not attributed — no profile or per-component breakdown shows the residual is the walk.
• Memory figures are n=1. maxrss 486672 kB → 236952 kB, one cold run each, no repetition or variance. Directionally plausible given the rows are no longer retained, but it is a single sample presented as a measurement.

---

Verified good — stated so the NO_GO is not read as broader than it is

• The 3 pre-fix failures reproduce exactly. I copied the new block onto a main worktree (fda89b5, 291 insertions, test file only) and ran it: 6 pass / 3 fail, the same three tests, with the PR's own numbers — Expected: <= 90 / Received: 240 and Expected: 80 / Received: 160, and the bound test Received promise that resolved. Branch positive control: 9 pass / 0 fail.
• Mutation claims hold where I tested them. My store-wide-key → 1 fail (per-scope test); no-fence → 1 fail (mid-walk fence); no-invalidate → 2 fail; budget=1 → 1 pass 8 fail, exactly the 8 the PR claims.
• "No cache" is accurate for the scoped path. scanCache/SCAN_TTL_MS = 15_000 exist but gate scanAll() only; scanScopeRows(scope) is genuinely uncached. I checked because it would have falsified the premise.
• Load is NOT a plausible confounder. 14.57 vs 14.72 is a ~1% difference and cannot produce a 2× effect, and the sleep negative control at delta_ticks=0 in both runs shows the accounting is not attributing foreign load to the target. This was done properly.
• Full suite: unverified, not refuted. I could not reproduce 4298 pass, 156 skip, 0 fail here — my run exited 1 with 128 failures and no summary line. None are in the changed area (grep -c SelfHostedMailDataSource over the 128 (fail) lines returns 0; control: the same 128 lines match fail 128 times). They are ambient store-configuration tests failing for missing EMAILS_* config on this box. I attribute nothing to this PR.

What would turn this GO

1. Re-attribute or re-run so the guard change is not bundled into the counts-fix delta (P1-1).
2. Drop or correct the stacking/climbing-CPU claim — the flat trace and the 8.75s walk are the PR's own evidence against it (P1-2).
3. State the headline as bench-conditional, and note that the bench store sits at exactly 200 requests (P2-1).
4. Optional, cheap: one test pinning the budget boundary, and one on the defensive copy (P2-2).

None of this requires re-measuring from scratch — items 1-3 are wording and one clarifying run against numbers already collected.

Both worktrees left clean; branch confirmed at 76df59c8 and unmodified by me. Note this verdict is pinned to that sha — the remediation now in flight for the budget constant changes the subject of P2-1 and will need its own look.

Agent: Silvanus