commit 96205fd3f63a65f92bae6712eb203543e8984bff
parent 4a71c45cba4d69016b8dc9a5c05d455bf23e01c1
Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st>
Date: Sat, 12 Sep 2026 11:56:23 -0400
common: lib/search holds no caller's budget as a default
Review finding F3. `truncate(text, max = MCP_POLICY.snippetChars)` and
`ctx.snippetChars ?? MCP_POLICY.snippetChars` baked ONE caller's 240 into
the shared module. The failure mode is the bad kind: a viewer adopter
that forgets to pass its policy does not break, it silently clips every
excerpt at 240 with the whole suite green.
So the width is now a required parameter — `truncate`'s `max`,
`windowedTranscript`'s `maxLines`, `RecordCtx.snippetChars` — and
`MCP_POLICY` is no longer imported anywhere under `lib/search/` except by
`policy.ts`, which defines it. tsc is the enforcement; two tests pin the
observable the default was hiding, by running the same input under
MCP_POLICY and VIEWER_POLICY and asserting the widths differ (240 vs
uncapped). VIEWER_POLICY now has consumers.
The ripple was four lines outside the tests: mcp already passed
`policy.snippetChars` at every scanner site and `opts.maxLines ??
MCP_POLICY.windowLineCap` at getWindowedTranscript, and the two post
literals (120 / 480) were never the policy's to set. **No read path
changed — the bench counters cannot move.**
common 1128 → 1129 (+1 window case; the evalTree default-vs-policy case
was rewritten in place, not added). mcp 205 unchanged.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Diffstat:
4 files changed, 52 insertions(+), 27 deletions(-)
diff --git a/common/lib/search/evalTree.test.ts b/common/lib/search/evalTree.test.ts
@@ -3,6 +3,7 @@ import assert from "node:assert/strict";
import { newGroup, newLeaf } from "../searchQuery";
import type { SearchAlias } from "../searchAliases";
import { VIDEO_STATES, type VideoState } from "../availability";
+import { MCP_POLICY, VIEWER_POLICY } from "./policy";
import {
buildMatcher,
buildLeafMatchers,
@@ -24,6 +25,7 @@ const ctx = (over: Partial<RecordCtx> = {}): RecordCtx => ({
chatCues: [],
snippetsPerVideo: 4,
includeSnippets: true,
+ snippetChars: MCP_POLICY.snippetChars,
...over,
});
@@ -182,20 +184,20 @@ test("a chat leaf reads chatCues and tags the live_chat track", () => {
assert.equal(r.hits[0].seconds, 30);
});
+// `RecordCtx.snippetChars` is REQUIRED — no MCP default survives in lib/search,
+// because a viewer adopter that omitted it used to get 240-character excerpts
+// with nothing failing. tsc enforces that it is passed; this pins that passing
+// the viewer's policy actually widens the excerpt.
test("snippetChars comes from the policy the caller passes", () => {
const long = "y".repeat(300);
- const wide = evalLeaf(
- newLeaf({ id: "l", query: "y" }),
- { scope: "transcripts", test: () => true },
- ctx({ cues: [cue(0, long)], snippetChars: Number.POSITIVE_INFINITY }),
- );
- assert.equal(wide.hits[0].text.length, 300);
- const narrow = evalLeaf(
- newLeaf({ id: "l", query: "y" }),
- { scope: "transcripts", test: () => true },
- ctx({ cues: [cue(0, long)] }),
- );
- assert.equal(narrow.hits[0].text.length, 240, "…defaulting to the MCP's 240");
+ const run = (snippetChars: number) =>
+ evalLeaf(
+ newLeaf({ id: "l", query: "y" }),
+ { scope: "transcripts", test: () => true },
+ ctx({ cues: [cue(0, long)], snippetChars }),
+ );
+ assert.equal(run(VIEWER_POLICY.snippetChars).hits[0].text.length, 300);
+ assert.equal(run(MCP_POLICY.snippetChars).hits[0].text.length, 240);
});
// ─── the tree algebra, per record ───
diff --git a/common/lib/search/evalTree.ts b/common/lib/search/evalTree.ts
@@ -33,7 +33,6 @@ import { matchAliases, type SearchAlias } from "../searchAliases";
import { VIDEO_STATES, type VideoState } from "../availability";
import type { Cue } from "../vtt";
import { clock, truncate, type Matcher } from "./window";
-import { MCP_POLICY } from "./policy";
export type { Matcher };
@@ -138,9 +137,10 @@ export type RecordCtx = {
postText?: string;
snippetsPerVideo: number;
includeSnippets: boolean;
- // Snippet truncation width — `SearchPolicy.snippetChars`. Defaults to the
- // MCP's 240 so an existing caller's output is byte-identical.
- snippetChars?: number;
+ // Snippet truncation width — `SearchPolicy.snippetChars`. REQUIRED: a default
+ // here would be one caller's budget imposed on every other, and the way a
+ // viewer adopter discovers it is by shipping quietly clipped excerpts.
+ snippetChars: number;
};
export type LeafOutcome = { matched: boolean; count: number; hits: ScopedSnippet[] };
@@ -151,7 +151,7 @@ export function evalLeaf(
ctx: RecordCtx,
): LeafOutcome {
if (!isLeaf(leaf)) return { matched: false, count: 0, hits: [] };
- const max = ctx.snippetChars ?? MCP_POLICY.snippetChars;
+ const max = ctx.snippetChars;
const hits: ScopedSnippet[] = [];
const push = (s: ScopedSnippet): void => {
if (ctx.includeSnippets && hits.length < ctx.snippetsPerVideo) hits.push(s);
diff --git a/common/lib/search/window.test.ts b/common/lib/search/window.test.ts
@@ -9,6 +9,7 @@ import {
findFirstMatchInRange,
} from "./window";
import type { Cue } from "../vtt";
+import { MCP_POLICY, VIEWER_POLICY } from "./policy";
const cue = (start: number, text: string): Cue => ({ start, end: start + 3, text });
@@ -20,11 +21,22 @@ test("clock renders 0 as 0:00 rather than the empty string", () => {
});
test("truncate collapses whitespace and clips with the ellipsis inside the budget", () => {
- assert.equal(truncate(" a b\n c "), "a b c");
+ assert.equal(truncate(" a b\n c ", MCP_POLICY.snippetChars), "a b c");
assert.equal(truncate("abcdef", 4), "abc…");
assert.equal(truncate("abcd", 4), "abcd");
});
+// The regression this guards: `max` used to default to MCP_POLICY.snippetChars,
+// so a viewer path that forgot to pass its width clipped at 240 with every test
+// green. The default is gone (tsc now requires the argument); this pins that the
+// two policies actually produce different widths, which is the observable the
+// silent default hid.
+test("the width is the CALLER's, and the two policies differ", () => {
+ const long = "z".repeat(600);
+ assert.equal(truncate(long, MCP_POLICY.snippetChars).length, 240);
+ assert.equal(truncate(long, VIEWER_POLICY.snippetChars), long);
+});
+
test("windowedTranscript merges overlapping windows and stamps each line", () => {
const cues = [
cue(0, "one"),
@@ -36,6 +48,7 @@ test("windowedTranscript merges overlapping windows and stamps each line", () =>
const r = windowedTranscript(cues, (t) => t.includes("needle"), {
before: 15,
after: 15,
+ maxLines: MCP_POLICY.windowLineCap,
});
assert.equal(r.matchCount, 2);
// Both windows overlap, so every cue appears exactly once.
@@ -51,11 +64,13 @@ test("windowedTranscript honours maxLines and the timestamps/stamp options", ()
const bare = windowedTranscript([cue(7, "hit")], () => true, {
timestamps: false,
+ maxLines: MCP_POLICY.windowLineCap,
});
assert.deepEqual(bare.lines, ["hit"]);
const linked = windowedTranscript([cue(7, "hit")], () => true, {
stamp: (c, s) => `${c}|${s}`,
+ maxLines: MCP_POLICY.windowLineCap,
});
assert.deepEqual(linked.lines, ["[0:07|7] hit"]);
});
diff --git a/common/lib/search/window.ts b/common/lib/search/window.ts
@@ -30,7 +30,6 @@ import {
mergeSnippets,
type WindowSnippet,
} from "../transcriptWindow";
-import { MCP_POLICY } from "./policy";
// A zero-second cue reads as "0:00", not as the empty string formatDuration
// returns for a falsy input.
@@ -40,9 +39,17 @@ export function clock(seconds: number): string {
}
// Collapse whitespace and clip to `max` characters, ellipsis included in the
-// budget. `max` comes from a SearchPolicy (`snippetChars`); the default is the
-// MCP's, which is what every existing caller passed implicitly.
-export function truncate(text: string, max = MCP_POLICY.snippetChars): string {
+// budget.
+//
+// `max` is REQUIRED, and that is the point. It used to default to
+// `MCP_POLICY.snippetChars`, which meant a viewer adopter that simply forgot to
+// pass its policy would silently clip every excerpt at the MCP's 240 characters
+// with every test still green — the failure mode where nothing is broken, the
+// output is just quietly wrong. A shared module does not get to hold one
+// caller's budget as its default. Pass `policy.snippetChars`, or a literal when
+// the width is genuinely not the policy's to set (a post's 120-character
+// stand-in title).
+export function truncate(text: string, max: number): string {
const t = text.trim().replace(/\s+/g, " ");
return t.length > max ? t.slice(0, max - 1) + "…" : t;
}
@@ -65,10 +72,11 @@ export function windowedTranscript(
// Markdown link "[m:ss](url)" or the compact base form "m:ss|156"), given
// the line's clock + start seconds. Bare clock when omitted.
stamp?: (clock: string, seconds: number) => string;
- // Cap on merged excerpt lines emitted for this video. `maxCues` above stays
- // the per-window bound.
- maxLines?: number;
- } = {},
+ // Cap on merged excerpt lines emitted for this video (`policy.windowLineCap`).
+ // REQUIRED, for the same reason `truncate`'s `max` is. `maxCues` above
+ // stays the per-window bound.
+ maxLines: number;
+ },
): { lines: string[]; matchCount: number } {
const list = cues as Cue[];
const timestamps = opts.timestamps !== false;
@@ -84,7 +92,7 @@ export function windowedTranscript(
maxCues: opts.maxCues,
}),
);
- merged = mergeSnippets(merged, win, opts.maxLines ?? MCP_POLICY.windowLineCap);
+ merged = mergeSnippets(merged, win, opts.maxLines);
}
const lines = merged.map((s) => {
if (!timestamps) return s.text;
