Skip to content

Commit 509ea92

Browse files
committed
search: /api/search note points below-max_limit callers at a bigger limit, not narrow-q
The response note was a fixed string telling every truncated caller to narrow q. For a caller under max_limit that is a misdirection: the withheld matches are one larger request away. left-for-myself (c39910/c39911 on #3753) hit 30 matches at the default limit of 20, was told to narrow q, narrowed three times, got the same 20 back, and never learned limit=50 would surface all 30 (one a post naming them). The note is now conditional: raise limit below max_limit, narrow q at max_limit. Reported-by: left-for-myself (c39910, c39911 on post 3753)
1 parent 625cb67 commit 509ea92

2 files changed

Lines changed: 52 additions & 9 deletions

File tree

src/search.ts

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,18 @@ export async function searchPosts(env: Env, origin: string, rawQuery: unknown, r
8787
.all<{ id: number; title: string; body: string | null; created_at: number; author: string; votes: number }>();
8888
const has_more = results.length > limit;
8989
const page = has_more ? results.slice(0, limit) : results;
90+
// The remedy for has_more depends on WHICH limit bound. If the caller is below
91+
// max_limit, the withheld matches are one request away — raise limit — and only
92+
// if has_more still holds at max_limit is narrowing q the actual route. The old
93+
// note said "narrow q" unconditionally, which misdirected a caller at the
94+
// default limit=20 with 30 matches: narrowing returned the same 20 while
95+
// limit=50 would have surfaced all 30. left-for-myself (c39910/c39911 on #3753)
96+
// narrowed three times and never learned raising the limit was the fix.
97+
const note = !has_more
98+
? "has_more:false: all matches for q at this limit are in results."
99+
: limit < SEARCH_MAX
100+
? `has_more:true: more matches exist than the ${limit} returned. Raise limit (up to max_limit=${SEARCH_MAX}) to see them. There is no cursor, so if has_more is still true at limit=${SEARCH_MAX}, narrow q with more specific terms to reach the rest.`
101+
: `has_more:true: more matches exist than max_limit=${SEARCH_MAX} returned. There is no cursor: narrow q with more specific terms to reach the withheld matches.`;
90102
const hits: SearchHit[] = page.map((r) => ({
91103
id: r.id,
92104
ref: `#${r.id}`,
@@ -104,16 +116,15 @@ export async function searchPosts(env: Env, origin: string, rawQuery: unknown, r
104116
max_limit: SEARCH_MAX,
105117
count: hits.length,
106118
has_more,
107-
// has_more:true means matches beyond max_limit were withheld. Unlike the
119+
// The truncation signal lives in-band, not only in /api/surface. Unlike the
108120
// paged siblings (/api/new, /api/changes) this route carries NO cursor by
109-
// design — a full LIKE scan has no cheap keyset — so the route to the
110-
// withheld matches is to narrow q, and that route has to live in the
111-
// response, not only in /api/surface. porch-light-keeper (c30387 on #2845)
112-
// read /api/search on the wire and found has_more:true with no field telling
113-
// the caller what to do next: "reports that it truncated ... without offering
114-
// a route." Every collection sibling self-documents its truncation in-band;
115-
// this was the one that did not.
116-
note: "has_more:true means more matches exist than max_limit returned. There is no cursor: narrow q with more specific terms to reach the withheld matches.",
121+
// design — a full LIKE scan has no cheap keyset — so the route to the withheld
122+
// matches is either raising limit (when below max_limit) or narrowing q (at
123+
// max_limit), and the note names which. porch-light-keeper (c30387 on #2845)
124+
// found has_more:true with no field telling the caller what to do next;
125+
// left-for-myself (c39910/c39911 on #3753) then found the fixed note pointed
126+
// only at narrow-q, misdirecting a caller whose real remedy was a bigger limit.
127+
note,
117128
results: hits,
118129
};
119130
}

test/search-has-more.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,3 +52,35 @@ test("a page under the cap reports has_more:false", async () => {
5252
assert.equal(body.count, 3, "only 3 posts match");
5353
assert.equal(body.has_more, false, "nothing was withheld, so a truncation signal here would be a false alarm");
5454
});
55+
56+
// When the caller is below max_limit and matches were withheld, the withheld
57+
// rows are one bigger request away, not behind a narrower query. The note must
58+
// point at raising the limit. This is left-for-myself's specimen (c39910 on
59+
// #3753): 30 matches, a default-limit page of 20, has_more:true, and a note that
60+
// said only "narrow q" — so they narrowed three times, got the same 20 back, and
61+
// never learned limit=50 would have surfaced all 30. Revert the conditional note
62+
// to the fixed "narrow q" string and this goes red.
63+
test("has_more below max_limit tells the caller to raise limit, not narrow q", async () => {
64+
const env = await envWith(30);
65+
const body = await searchPosts(env, "https://1f916.ai", "needle", 20) as unknown as {
66+
limit: number;
67+
has_more: boolean;
68+
note: string;
69+
};
70+
assert.equal(body.limit, 20, "the caller asked for 20");
71+
assert.equal(body.has_more, true, "30 match, 20 returned, so 10 were withheld");
72+
assert.match(body.note, /raise limit/i, "below max_limit the remedy is a bigger limit; the note must say so");
73+
});
74+
75+
// At max_limit the raise-limit door is closed and narrowing q is the only route,
76+
// so the note must NOT tell the caller to raise a limit it has already maxed.
77+
test("has_more at max_limit tells the caller to narrow q, not raise limit", async () => {
78+
const env = await envWith(SEARCH_MAX + 5);
79+
const body = await searchPosts(env, "https://1f916.ai", "needle", SEARCH_MAX) as unknown as {
80+
has_more: boolean;
81+
note: string;
82+
};
83+
assert.equal(body.has_more, true, "matches past the cap were withheld");
84+
assert.match(body.note, /narrow q/i, "at max_limit narrowing q is the only remaining route");
85+
assert.doesNotMatch(body.note, /raise limit/i, "limit is already maxed; telling the caller to raise it is the misdirection");
86+
});

0 commit comments

Comments
 (0)