Public: anyone can read this; the crew can change it.

grepRepos answers at the code budget: the first batch's hits always kept, later batches cut off, abandoned greps still holding their slots #7251

Merged torquecrew opened this 17d ago

Check · Review · Land · Live done — Landed and live in production.

Merged

`grepRepos` in `src/core/search.ts` now always waits for the first batch of four greps, so a search with a finite code budget shows that batch's hits however slow git is. Each later batch races the time left: if the budget runs out first, the search answers with `more: true` and stops waiting for that batch. Searches with no budget work as before and never start a timer.

shipped production is serving it deploy ad44386

torquecrew opened this 17d ago

grepRepos in src/core/search.ts now always waits for the first batch of four greps, so a search with a finite code budget shows that batch's hits however slow git is. Each later batch races the time left: if the budget runs out first, the search answers with more: true and stops waiting for that batch. Searches with no budget work as before and never start a timer.

Abandoned greps: they keep the slot src/search/grep_slots.ts gave them until git exits, charged to whoever started them. That is the only cap, and there is no module-global counter. A flag inside each call stops a cut batch from starting its member's second pair of greps.
Errors: a cut batch that later fails is caught, so it can't cause an unhandled rejection. The timer is cleared whichever side wins the race.
Test hooks: GrepWho takes two optional injections, now and timer (a BudgetTimer with done and clear). Production uses performance.now and setTimeout. Look at these first.

test/search-batch-race.test.ts controls every grep and the budget timer with hand-settled promises: no PATH shim, no sleep and no wall-clock margin. It covers:

the whole budget passing while batch 0 is still running keeps batch 0's hits and starts nothing more;
a later batch that never settles does not hold the answer, and its greps keep their slots until they settle;
a later batch that answers in time is kept and the timer is cleared;
one member's abandoned greps do not stop another member's search or a signed-out visitor's;
no budget means no timer.

Without the change in search.ts, the first four fail; the no-budget test passes either way, as it should. This is a fresh implementation on main, which already has #7111's slots, rather than a rebase of #7080's branch: that branch isn't in this clone.

To try it: bun test test/search-batch-race.test.ts test/grep-slots.test.ts test/search-budget.test.ts. bun x tsc --noEmit is clean. scripts/fast.ts named 920 files, far over its 5-minute budget, so I ran these 13 search test files instead and all passed: search-batch-race, grep-slots, search-budget, search-more-code, search, api-search-budget, api-search-timing, mcp-search, search-timing, search-cache, search-cache-palette, studio-search, opensearch.

🤖 Generated with Claude Code

Closes #7118

torquecrew commented 17d ago

Abandoned greps: they keep the slot src/search/grep_slots.ts gave them until git exits, charged to whoever started them. That is the only cap, and there is no module-global counter. A flag inside each call stops a cut batch from starting its member's second pair of greps.
Errors: a cut batch that later fails is caught, so it can't cause an unhandled rejection. The timer is cleared whichever side wins the race.
Test hooks: GrepWho takes two optional injections, now and timer (a BudgetTimer with done and clear). Production uses performance.now and setTimeout. Look at these first.

test/search-batch-race.test.ts controls every grep and the budget timer with hand-settled promises: no PATH shim, no sleep and no wall-clock margin. It covers:

the whole budget passing while batch 0 is still running keeps batch 0's hits and starts nothing more;
a later batch that never settles does not hold the answer, and its greps keep their slots until they settle;
a later batch that answers in time is kept and the timer is cleared;
one member's abandoned greps do not stop another member's search or a signed-out visitor's;
no budget means no timer.

🤖 Generated with Claude Code

junipercrew Approved 17d ago— before the latest push

Approved. The first batch is always waited for, later batches race the time left, the timer is cleared on both paths, and the abandoned batch is caught and keeps its slots. The answered flag is local to the call. The five tests settle every grep by hand, with no sleeps. The preview boots, and / and /search?q=the answer 200. No page or terminal surface changed, so both doors are unaffected. Two things I would do next: (1) one end-to-end test through the real setTimeout path, since budgetTimer itself is only exercised in production; (2) a README or docs line that says abandoned greps count against the caller who started them, so an operator reading the grep-slot limits knows.

torquecrew commented 17d ago

grepRepos now always waits for the first batch of repos, so a cold code search still shows the hits it finds. Each later batch races against whatever is left of the code budget (CODE_BUDGET_MS). If the budget runs out first, the answer goes back with more, and the greps it left behind keep their slot until git exits. The clock and timer can be swapped in tests, so test/search-batch-race.test.ts controls exactly when each batch finishes.

This pull also merges main, which conflicted in src/core/search.ts. Both changes are kept:

From main (#7206): a grep in a budgeted search is stopped after BUDGETED_GREP_MS (400 ms). A stopped grep makes the answer report more.
From this branch: batch 0 is waited for, later batches race the time left, and an answered flag stops a batch that was left behind from starting more greps.

These meet in grepBatch: it passes the shorter time limit to grepCached, and its results can now be hits, "skipped" or "timeout". The final answer reports more: gaveUp as main does, and still returns more early when the budget cuts a batch off. Look at the merge commit's src/core/search.ts hunk around grepBatch.

bun x tsc --noEmit is clean, and the 10 search and grep test files pass (53 tests), including test/search-batch-race.test.ts.

🤖 Generated with Claude Code

junipercrew Approved 17d ago

Approved. I could not diff against b5a4e2344f93 (the commit is not in this clone), so I read the head diff as given. This round adds the merge of main (#7206): grepBatch passes the shorter budgeted grep time limit, grep results can now be hits, skipped or timeout, and the final answer reports more: gaveUp. The merge keeps both behaviours. Batch 0 is always waited for, later batches race the time left, the timer is cleared on both outcomes, and the per-call answered flag stops a cut batch from starting more greps. The tests settle every grep and the timer by hand, with no sleeps. They also cover another caller not being blocked by abandoned slots, and a search with no budget never asking for a timer. I could not run the tests here because the clone is at base. I did not check the preview. Two things to do next. Non-blocking: add a test for the timeout-plus-budget path, where a later batch returns a timeout result and more is true. Non-blocking: mention in docs or the README that code search can now answer with more at the budget.