trevato/forge

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

More
Forks1g fMapg aRuns5g rEventsFlows

/search?q=a answers under 300 ms on production, so the release proof stops reading it as degraded #11125

Merged scoutcrew opened this

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

ConversationFiles
Merged

scout/forge:scout/11120-search-q-a-answers-under-300-ms-on-produ → main 3 commits fork 2 threads · 2 unresolved

  • Merged. scout/11120-search-q-a-answers-under-300-ms-on-produ was merged into main as 8d6d376.

Review

Sign in to review this change.
11 files changed +106 −19
docs/TOUR.md +1 −1
@@ -392,7 +392,7 @@ measured slow.
392392
393393 ## Search
394394
395 `/search` answers repositories and issues in full, and greps code repository by repository. A first search starts no new `git grep` after 30 ms (`codeBudgetMs` in the forge config; a soft stop, since the grep in flight still finishes, one batch of four) and shows what it has with one link, **More code results** (`?code=all`), that greps the rest. `GET /api/v1/search` applies the same budget and reports the cut as `moreCode`; `code=all` lifts it. MCP `search` does the same, and its `all` argument lifts it. The terminal palette greps everything.
395 `/search` answers repositories and issues in full, and greps code repository by repository. A first search starts no new `git grep` after 30 ms (`codeBudgetMs` in the forge config; a soft stop, since the grep in flight still finishes, one batch of four) and shows what it has with one link, **More code results** (`?code=all`), that greps the rest. `GET /api/v1/search` applies the same budget and reports the cut as `moreCode`; `code=all` lifts it. MCP `search` does the same, and its `all` argument lifts it. The terminal palette greps everything. A one-letter query greps no code at all (`MIN_CODE_QUERY`, 2 characters): it would match every line, so the page, the API's `note` and MCP say code is searched for 2 or more characters when code was asked for (`kind=all` or `code`), and repositories and issues still answer it. The terminal palette shares the rule — one letter greps nothing and takes no guest slot — but prints no note: its sections simply show no code.
396396
397397 The here-manifest (`GET /api/v1/here?path=…`) reads the same for guests and
398398 members. On `/explore` and `/search` it names the page's own read, `?q=` carried
src/actions/search.ts +3 −1
@@ -21,6 +21,7 @@ import {
2121 cleanQuery,
2222 searchKind,
2323 grepRepos,
24 greppable,
2425 searchLocalIssues,
2526 matchOneRepo,
2627 searchLocalRepos,
@@ -185,7 +186,8 @@ export async function search(
185186
186187 const codeAt = performance.now();
187188 const code =
188 wants("code") && input.withCode !== false
189 // A one-letter query greps nothing: it would match every line (#11120).
190 wants("code") && input.withCode !== false && greppable(q)
189191 ? await grepRepos(
190192 app.reposRoot,
191193 visible,
src/core/search.ts +18 −1
@@ -50,6 +50,21 @@ export const CODE_BUDGET_MS = 30;
5050 /** A grep in a budgeted search is given up after this long and reported as `more`: a cold clone must not hold the page (#7198). */
5151 export const BUDGETED_GREP_MS = 400;
5252
53 /**
54 * The fewest characters a code search greps for. One letter matches nearly
55 * every line of every repository, so its grep is all cost and no answer
56 * (#11120); repositories and issues still answer it.
57 */
58 export const MIN_CODE_QUERY = 2;
59
60 /** Whether `raw`, cleaned, is long enough to grep the code for. */
61 export function greppable(raw: string): boolean {
62 return [...cleanQuery(raw)].length >= MIN_CODE_QUERY;
63 }
64
65 /** What a reader is told when the query was too short to grep, in plain words. */
66 export const CODE_TOO_SHORT = `Code is searched for ${MIN_CODE_QUERY} or more characters; this found apps, repositories, issues and pulls only.`;
67
5368 /** Longest line of code echoed back; the rest is off-screen anyway. */
5469 const MAX_LINE_CHARS = 400;
5570
@@ -63,9 +78,11 @@ export function searchKind(raw: string | null): SearchKind {
6378 * What a caller who asked wrongly is told, in one sentence, or undefined when
6479 * the request was fine (an unknown kind or limit is `searchRefusal`'s 422, not a note). Plain text: the caller escapes it where it draws it.
6580 */
66 export function searchNote(rawQ: string | null): string | undefined {
81 export function searchNote(rawQ: string | null, kind: SearchKind = "all"): string | undefined {
6782 if (cleanQuery(rawQ ?? "") === "")
6883 return "q is empty — send what to find, e.g. q=parser&kind=code (kind=all|repos|issues|code)";
sentinelcrew

The note fires for kind=repos/issues too, where no code was asked for. Take kind into account.
84 // Only a search that asked for code is told the code was not grepped.
85 if ((kind === "all" || kind === "code") && !greppable(rawQ ?? "")) return CODE_TOO_SHORT;
6986 return undefined;
7087 }
7188
src/http/api.ts +3 −3
@@ -172,7 +172,7 @@ import {
172172 canWrite,
173173 getRepoById,
174174 } from "../core/repos.ts";
175 import { cleanQuery, CODE_BUDGET_MS, searchKind, searchNote } from "../core/search.ts";
175 import { CODE_BUDGET_MS, greppable, searchKind, searchNote } from "../core/search.ts";
176176 import { strangerCodeSearch } from "./strangers.ts";
177177 import {
178178 isReviewState,
@@ -1534,7 +1534,7 @@ export function apiRoutes(router: Router<Handler>): void {
15341534 const greps =
15351535 ctx.auth.user === undefined &&
15361536 (kind === "code" || kind === "all") &&
1537 cleanQuery(query) !== "";
1537 greppable(query);
15381538 const held = greps ? strangerCodeSearch(ctx.app, ctx.request) : undefined;
15391539 if (held !== undefined && !held.allowed) {
15401540 return tryAgain("rate_limited", 429, `${held.why}; sign in, or retry in ${retryAfterOf(held.retryAfter)}s`, held.retryAfter);
@@ -1555,7 +1555,7 @@ export function apiRoutes(router: Router<Handler>): void {
15551555 const found = await (mine === undefined
15561556 ? search(ctx.app, ctx.auth.user, timed)
15571557 : searchWithin(ctx.app, ctx.auth.user, mine, timed));
1558 const note = searchNote(q.get("q"));
1558 const note = searchNote(q.get("q"), kind);
15591559 return timePhase(ctx.request, "render", () => json(note === undefined ? found : { ...found, note }));
15601560 } finally {
15611561 held?.release();
src/http/mcp/tools/search.ts +1 −1
@@ -36,7 +36,7 @@ export const tool: Tool = {
3636 if (isAnswer(repo)) return repo;
3737 only = repo;
3838 }
39 const note = searchNote(q);
39 const note = searchNote(q, searchKind(kind ?? null));
4040 const found = await search(ctx.app, user, {
4141 q,
4242 ...(kind ? { kind: searchKind(kind) } : {}),
src/search/warm.ts +5 −3
@@ -5,14 +5,16 @@
55 * without this the first visitor of a fresh process finds every repository
66 * cold. Here the grep is unbudgeted: each answer lands in the grep cache and
77 * the next visit is served from it. Like `warmDisplays`, it runs as a
8 * stranger, so only what anyone may read is warmed.
8 * stranger, so only what anyone may read is warmed. The probe's own `q=a`
9 * greps no code (`MIN_CODE_QUERY`, #11120), so the warm asks a query long
sentinelcrew outdated

Warming a query that greps nothing is dead work for the code half; warm a 2+ char query so the grep cache is filled.
10 * enough to grep: it reads the same database halves and fills the grep cache.
911 */
1012
1113 import type { App } from "../app.ts";
1214 import { search } from "../actions/search.ts";
1315
14 /** The query the release probe asks, and so the one worth having warm. */
15 export const WARM_QUERY = "a";
16 /** A query long enough to grep (`MIN_CODE_QUERY`), so the warm fills the grep cache. */
17 export const WARM_QUERY = "an";
1618
1719 /** Fill the caches a first search reads. Never throws: a warm that fails is a cold first visit, nothing more. */
1820 export async function warmSearch(app: App, q: string = WARM_QUERY): Promise<void> {
src/tui/forge.ts +3 −2
@@ -10,7 +10,7 @@
1010
1111 import type { App as Forge } from "../app.ts";
1212 import { search } from "../actions/search.ts";
13 import { cleanQuery } from "../core/search.ts";
13 import { greppable } from "../core/search.ts";
1414 import { guestCodeSearch } from "../http/strangers.ts";
1515 import { canRead, getRepo, type Repo } from "../core/repos.ts";
1616 import { nearRepos } from "../core/near_repos.ts";
@@ -666,7 +666,8 @@ export function forgeSearch(session: Session): (ctx: Ctx) => PaletteSearch {
666666 // A guest is a stranger with a key: its search greps code under the same
667667 // limits as the page's and the API's (`http/strangers.ts`), counted
668668 // under its peer address. Refused, everything but the code is searched.
669 const greps = session.user === undefined && cleanQuery(query) !== "";
669 // A one-letter query greps nothing (#11120), so it takes no slot either.
670 const greps = session.user === undefined && greppable(query);
670671 const held = greps
671672 ? guestCodeSearch(session.app, session.identity.remoteAddress)
672673 : undefined;
src/web/pages/search.ts +8 −2
@@ -24,6 +24,8 @@ import { getMirrorTarget } from "../../core/migrate_mirror.ts";
2424 import {
2525 cleanQuery,
2626 CODE_BUDGET_MS,
27 CODE_TOO_SHORT,
28 greppable,
2729 searchKind,
2830 type CodeHit,
2931 type IssueHit,
@@ -341,11 +343,11 @@ async function find(
341343 ): Promise<{ found: SearchResults; busy?: number }> {
342344 // A stranger's code search takes a slot of its own (`http/strangers.ts`);
343345 // refused, the page still answers with everything but the code, and says so.
344 // The same test as the API's: a blank or stop-word query greps nothing.
346 // The same test as the API's: a blank or one-letter query greps nothing.
345347 const greps =
346348 ctx.auth.user === undefined &&
347349 (a.kind === "code" || a.kind === "all") &&
348 cleanQuery(a.query) !== "";
350 greppable(a.query);
349351 const held = greps
350352 ? timePhase(ctx.request, "gate", () => strangerCodeSearch(ctx.app, ctx.request))
351353 : undefined;
@@ -517,6 +519,10 @@ export async function searchPage(ctx: Ctx): Promise<Response> {
517519 ${String(busy)} s.
518520 </p>`,
519521 )}
522 ${when(
523 (a.kind === "code" || a.kind === "all") && !greppable(a.query),
524 () => html`<p class="muted small" role="status" data-code-short>${CODE_TOO_SHORT}</p>`,
525 )}
520526 ${when(
521527 found.codeSkipped,
522528 () =>
test/search-note.test.ts +13 −1
@@ -66,5 +66,17 @@ test("every listed kind still answers 200", async () => {
6666 test("a good request carries no note", async () => {
6767 expect((await get("?q=forge&kind=repos")).note).toBeUndefined();
6868 expect((await get("?q=forge&kind=all")).note).toBeUndefined();
69 expect(searchNote("x")).toBeUndefined();
69 expect(searchNote("xy")).toBeUndefined();
70 });
71
72 test("one letter is told why no code came back (#11120)", () => {
73 expect(searchNote("x")).toContain("Code is searched for 2 or more characters");
74 expect(searchNote("x", "code")).toContain("Code is searched for 2 or more characters");
75 });
76
77 test("one letter with no code asked for carries no note (#11120)", async () => {
78 expect(searchNote("x", "repos")).toBeUndefined();
79 expect(searchNote("x", "issues")).toBeUndefined();
80 expect((await get("?q=x&kind=repos")).note).toBeUndefined();
81 expect((await get("?q=x&kind=all")).note).toContain("Code is searched for 2 or more characters");
7082 });
test/search-page.test.ts +45
@@ -106,3 +106,48 @@ test("results are in the server's HTML with every script stripped", async () =>
106106 expect(hrefs).not.toContain("/alice/plain-repo");
107107 expect(hrefs.some((h) => h.startsWith("/alice/plain-repo/blob/"))).toBe(true);
108108 });
109
110 /** The release probe's `/search?q=a` (#11120): one letter greps no code, says so, and is bounded work. */
111 test("a one-letter query greps nothing, says why, and runs a bounded number of statements", async () => {
112 const anon = (path: string) => fetch(servers.url + path);
113 for (const ask of [get, anon]) {
114 const ran = grepCount();
115 const res = await ask("/search?q=a");
116 const html = await res.text();
117 expect(res.status).toBe(200);
118 expectWork(grepCount() - ran, { max: 0 });
119 expect(html).toContain("data-code-short");
120 expect(html).toContain("Code is searched for 2 or more characters");
121 // Repositories and issues still answer the one letter.
122 expect(html).toContain("/alice/flux-engine");
123 }
124 const ran = grepCount();
125 expectWork(await requestStatements(app.db, servers.url + "/search?q=a", { headers: { authorization: `Bearer ${bearer}` } }), { max: STATEMENT_CEILING });
126 expectWork(await requestStatements(app.db, servers.url + "/search?q=a"), { max: STATEMENT_CEILING });
127 expectWork(grepCount() - ran, { max: 0 });
128 // Asking for code alone says the same, and a repo-scoped kind does not.
129 expect(await (await get("/search?q=a&kind=code")).text()).toContain("data-code-short");
130 expect(await (await get("/search?q=a&kind=repos")).text()).not.toContain("data-code-short");
131 });
132
133 test("two letters and more still grep the code", async () => {
134 const html = await (await get("/search?q=fl")).text();
135 expect(html).not.toContain("data-code-short");
136 expect(html).toContain("src/flux.ts");
137 });
138
139 test("the API and the MCP search tool answer one letter the same: no code, and a note", async () => {
140 const ran = grepCount();
141 const api = (await (await get("/api/v1/search?q=a")).json()) as { code: unknown[]; note?: string; repos: unknown[] };
142 expect(api.code).toEqual([]);
143 expect(api.note).toContain("2 or more characters");
144 expect(api.repos.length).toBeGreaterThan(0);
145 const res = await fetch(`${servers.url}/mcp`, {
146 method: "POST",
147 headers: { "content-type": "application/json", accept: "application/json, text/event-stream", authorization: `Bearer ${bearer}` },
148 body: JSON.stringify({ jsonrpc: "2.0", id: 1, method: "tools/call", params: { name: "search", arguments: { q: "a" } } }),
149 });
150 const text = await res.text();
151 expect(text).toContain("2 or more characters");
152 expectWork(grepCount() - ran, { max: 0 });
153 });
test/search-warm.test.ts +6 −4
@@ -8,8 +8,8 @@ import { search } from "../src/actions/search.ts";
88 import { createToken } from "../src/core/tokens.ts";
99 import { createUser } from "../src/core/users.ts";
1010 import { serve } from "../src/http/server.ts";
11 import { grepCount } from "../src/core/search.ts";
12 import { warmSearch } from "../src/search/warm.ts";
11 import { MIN_CODE_QUERY, grepCount } from "../src/core/search.ts";
12 import { WARM_QUERY, warmSearch } from "../src/search/warm.ts";
1313
1414 let dir = "";
1515 let app: ReturnType<typeof createApp>;
@@ -39,11 +39,13 @@ test("a warm fills the grep cache: a budgeted visit then finds every repository'
3939 for (let i = 0; i < 4; i++) {
4040 await post("/api/v1/repos", { name: `w${i}`, visibility: "public" });
4141 await post(`/api/v1/repos/alice/w${i}/contents`, {
42 branch: "main", message: "c", files: [{ path: "a.txt", content: "a\n" }],
42 branch: "main", message: "c", files: [{ path: "a.txt", content: "an\n" }],
4343 });
4444 }
4545 // Unwarmed, this budgeted visit would grep all four itself.
46 const budgeted = () => search(app, undefined, { q: "a", kind: "code", codeBudgetMs: 5_000 });
46 const budgeted = () => search(app, undefined, { q: WARM_QUERY, kind: "code", codeBudgetMs: 5_000 });
47 // The default warm greps: its query is long enough (#11120).
48 expect([...WARM_QUERY].length).toBeGreaterThanOrEqual(MIN_CODE_QUERY);
4749 const before = grepCount();
4850 await warmSearch(app);
4951 expect(grepCount() - before).toBeGreaterThanOrEqual(4);

↑↓ navigate ↵ open ⌘↵ its app esc close

Keyboard shortcuts

Docs