Clarify the search in-flight ceiling's dual role after live-search coalescing

[lukemelia]

Fixes CS-12916.

What

Comment-only change to SERVER_MAX_IN_FLIGHT_SEARCHES in packages/runtime-common/search-bounds.ts. The default stays 30; no behavior changes.

Why

Two heap-protection changes recently landed on _federated-search:

• the admission gate (SERVER_MAX_IN_FLIGHT_SEARCHES, default 30) — caps concurrent searches, shedding the excess with 429 + Retry-After before the body is parsed so a shed is cheap;
• the live-search cache — coalesces byte-identical concurrent live searches into one compute (join) or a short-TTL body (hit).

Read together, the gate's original rationale ("each in-flight search holds tens of MB, so cap at ~30") invites a wrong conclusion: "identical bursts are now cheap, so raise the ceiling." That's unsafe. Because the gate admits before body-parse, it cannot tell a cheap join/hit from an expensive distinct miss, so 30 is doing two jobs at once:

1. it bounds request concurrency, and
2. it is still the distinct-query heap bound — the worst case is 30 concurrent distinct multi-MB result documents, which is the figure that exhausts a 2 GB heap.

The live-search cache collapses identical bursts to ~one document, but does nothing for distinct concurrent queries. So raising the ceiling to be friendlier to identical bursts would also raise the distinct-query worst case and re-expose the OOM the gate exists to prevent. Identical-burst overflow is instead safely shed and retried into a cache hit (~1s added latency on the overflow, no heap risk).

This PR spells that out in the comment so the number isn't mistaken for a pure heap figure and bumped.

Not in scope

Actually admitting more identical-burst traffic would require not charging a heap slot for a join/hit — i.e. knowing the cache-key outcome before consuming a slot, which conflicts with the gate's before-body-parse cheapness. Tracked as a design option in CS-12916 only if identical-burst 429s show up as painful in telemetry.

Test plan

Comment-only; no runtime surface. Typecheck/lint on the changed file only.

🤖 Generated with Claude Code

[github-actions]

Host Test Results

    1 files      1 suites   2h 40m 52s ⏱️
4 734 tests 4 720 ✅ 14 💤 0 ❌
4 749 runs  4 735 ✅ 14 💤 0 ❌

Results for commit 046a7a3.

Realm Server Test Results

    1 files  ± 0    210 suites  +3   1h 14m 46s ⏱️ - 2m 36s
2 764 tests +60  2 764 ✅ +61  0 💤 ±0  0 ❌  - 1 
2 803 runs  +60  2 803 ✅ +61  0 💤 ±0  0 ❌  - 1 

Results for commit 046a7a3. ± Comparison against earlier commit 2df2a3d.

[Copilot]

🟡 Changes recommended

Address the two outstanding nit findings in the search-bound comments.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This comment-only PR clarifies the dual concurrency and heap-protection role of SERVER_MAX_IN_FLIGHT_SEARCHES while preserving the default of 30 and runtime behavior.

Changes:

• Documents distinct-query heap protection.
• Explains interaction with live-search coalescing.
• Clarifies why identical bursts should not raise the ceiling.

File summaries

File	Summary and review findings
packages/runtime-common/search-bounds.ts	Updates search-bound rationale comments. Two outstanding nits: qualify cache scope to live /_federated-search traffic (3 votes), and describe retries as only possibly becoming cache hits when retained (3 votes).

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[lukemelia]

[Claude Code 🤖] Good catch — fixed in 046a7a3. The parenthetical now reads "identical live /_federated-search requests can share one document via the live-search cache, which is wired into that handler only; per-realm /_search calls, also gated here, assemble independently," so the heap rationale stays correct for the non-federated gated path.

[lukemelia]

[Claude Code 🤖] Fixed in 046a7a3. Reworded to a possible hit: overflow "is instead shed and retried, which lands as a cache hit only while the first response is still retained (a non-zero LIVE_SEARCH_CACHE_TTL_MS, body within LIVE_SEARCH_CACHE_MAX_BYTES); otherwise the retry re-assembles the document" — so it no longer overstates the default protection.

[backspace]

[Claude Code 🤖] This review checked each claim in the new comment against the mechanism it describes: the admission middleware (searchAdmission, SEARCH_PATH_PATTERN), the live-cache branch of respondWithJobScopedSearchCache, the per-realm /_search route in realm.ts, the LiveSearchCache retention rules, and the host's shed-retry loop in StoreService. It did not re-derive the heap figures.

No blocking issues. The comment's central claim holds on the current code: the gate charges every admitted request for its full lifetime and never sees the cache outcome, so the ceiling is the distinct-query heap bound, and raising it for identical bursts would raise that worst case with it.

1. Say "~one computation" rather than "~one document" for what the live cache saves, and allow for a retry that joins a running compute. See the inline comment on SERVER_MAX_IN_FLIGHT_SEARCHES. Non-blocking.

Overlap with the slot-release change. Let coalesced live searches hand back their admission slot rewrites this same comment block, plus the other two homes of the rationale (the search-inflight.ts header and the searchAdmission comment), and it changes the premise this comment rests on: a join or a hit hands its slot back the moment the cache decides, so the ceiling becomes a bound on concurrent computations plus the brief admission-to-lookup window, rather than on requests. It takes the route this description's "not in scope" section rules out: the slot is still charged at admission, before the body is parsed, and is released early, so the cheap shed is kept. It also removes the Response round trip the inline comment describes. That PR will be adapted after this one merges: it will keep this PR's distinct-query heap rationale and the /_search qualification, and replace the "charged for the request's lifetime" framing with the early-release semantics.

Adjacent, not asked here. The search-inflight.ts header still says every in-flight search holds tens of MB, which no longer matches this PR's "each distinct in-flight search" qualification. The slot-release adaptation rewrites that header, so leaving it alone in this PR is the right call.

[backspace]

[Claude Code 🤖] Say "~one computation" here rather than "~one document": on the response path a join or a hit still materialises its own copy of the body, and this is the sentence a reader would cite to argue identical bursts are heap-free.

The sharing ends where respondWithJobScopedSearchCache wraps the cached string in new Response(body). That encodes it into a stream, and setContextResponse decodes it back through webStreamToText into a fresh per-request string before Koa writes it. What the cache removes is the compute (SQL, loadLinks, the serialization intermediates), which is the larger cost, so "computation" is the accurate unit and stays accurate once the slot-release change drops the Response round trip on this path.

In the same passage, a retry admitted while the first compute is still running joins it rather than re-assembling, so "otherwise the retry re-assembles the document" wants "or joins it if the compute is still running" ahead of it.

Wording introduced here; non-blocking.