commit 6bb8e59468c31cbd636463027dc04ad20fc36601
parent 883ca8ed31054cdd3b468b0876a15abd46ef00f9
Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st>
Date: Mon, 21 Sep 2026 13:37:27 -0400
tags S2 review: the posts leaf, the orphan chip, and a validated tg
Three things the review caught, each a way the UI could say one thing while
doing another.
1. A tag filter now takes an explicit scopes:['posts'] leaf out of the
search. Gating only globalScopeSlugs was half the job: a posts leaf is fed
its own slug set, so the posts corpus was still being scanned, its rows
dropped by the display fold — while `totalHits` sums the pipeline's hits
BEFORE that fold. The header could read "3 videos, 8 hits" with hits no
card on the page accounted for. `tagFilteredPostScope` hands the pipeline
`null` — the value it already uses for "no posts in scope" — so the leaf
is never run rather than run and discarded.
2. A `?tg=` naming a valid id this site does not publish now gets its own
dismissible chip ("<id> (not on this site) ×"). It was being hydrated into
the selection while the row rendered chips only from /tags.json, so the
Filters badge and the "N selected" line said a filter was on with nothing
lit — and if it was the only tag, the results went to zero with nothing to
click off. The selection always has a chip now, so the badge and the row
cannot disagree. The row also renders for an orphan alone, which is the
same dead end on a site that publishes nothing.
3. `tg` is validated against TAG_ID_RE at both URL entry points (urlState's
parse/write and the session's hydration), so the URL agrees with the MCP's
parseSearchArgs about what an id is. A malformed token is dropped: the only
thing it can do downstream is turn a filter into a silent empty result. A
well-formed id the site does not publish is the DIFFERENT case above and is
kept, because it can be cleared.
Tests: six new e2e cases (the posts leaf under a tag filter and its control,
the orphan chip and its dismissal, a malformed token, and the Filters badge
counting a tag selection on a phone — the badge only exists below xl), and an
enumerate_matches + tags case in protocol.test.ts, whose fixture records now
carry the curatedTags its /tags.json counts claim.
Co-Authored-By: Claude Opus <noreply@anthropic.com>
Diffstat:
5 files changed, 251 insertions(+), 13 deletions(-)
diff --git a/common/components/FiltersPanel.tsx b/common/components/FiltersPanel.tsx
@@ -760,11 +760,24 @@ function TagChipRow() {
// selectableTags is the belt to compose-site's braces: a stale or
// hand-written document must not be able to offer a chip that finds nothing.
- const groups = useMemo(
- () => groupPublishedTags(selectableTags(curatedTags)),
- [curatedTags],
- );
- if (groups.length === 0) return null;
+ const publishable = useMemo(() => selectableTags(curatedTags), [curatedTags]);
+ const groups = useMemo(() => groupPublishedTags(publishable), [publishable]);
+
+ // Tags the SELECTION carries that this site does not publish. A `?tg=` link
+ // written against another site in the family, or against this one before a
+ // rebuild, arrives with a perfectly valid id that has no chip — and without
+ // this the Filters badge and the "N selected" line would say a filter is on
+ // while nothing in the row is lit, and (if it is the only tag) the results
+ // would go to zero with nothing to click off. So the selection ALWAYS has a
+ // chip, even when the vocabulary does not have the tag.
+ const unpublished = useMemo(() => {
+ const known = new Set(publishable.map((t) => t.id));
+ return Array.from(draftTags)
+ .filter((id) => !known.has(id))
+ .sort();
+ }, [publishable, draftTags]);
+
+ if (groups.length === 0 && unpublished.length === 0) return null;
return (
<div
@@ -837,6 +850,35 @@ function TagChipRow() {
})}
</div>
))}
+ {unpublished.length > 0 && (
+ <div
+ data-testid="tag-chip-group"
+ data-group-id="__unpublished"
+ className="flex flex-wrap items-center gap-1.5 min-w-0"
+ >
+ {unpublished.map((id) => (
+ <button
+ key={id}
+ type="button"
+ aria-pressed={true}
+ data-testid="tag-chip"
+ data-tag-id={id}
+ data-unpublished="true"
+ title={`"${id}" is not published by this site — click to clear it`}
+ onClick={() => toggleDraftTag(id)}
+ className="flex items-center gap-1.5 rounded border border-dashed border-primary px-2.5 py-1.5 text-sm select-none min-w-0 bg-primary/5 transition-colors hover:bg-accent hover:text-accent-foreground"
+ >
+ <span className="truncate">{id}</span>
+ <span className="text-xs text-muted-foreground shrink-0">
+ (not on this site)
+ </span>
+ <span aria-hidden="true" className="text-xs shrink-0">
+ ×
+ </span>
+ </button>
+ ))}
+ </div>
+ )}
</div>
</div>
);
diff --git a/common/components/SearchSessionContext.tsx b/common/components/SearchSessionContext.tsx
@@ -78,6 +78,7 @@ import {
type ChartShape,
} from "../lib/chartShare";
import type { DisplaySummary, Platform } from "../lib/transcripts";
+import { isTagId } from "../lib/curatedTags";
import type { Post } from "../lib/posts";
import { makeId, splitId } from "./originId";
import { sortGroups, type ChannelGroup } from "../lib/channelGroups";
@@ -764,7 +765,9 @@ function useSearchSessionState() {
for (const t of transcripts) if (passesFilter(t)) out.push(t.slug);
// A post carries no curated tags, so a tag filter drops the whole posts
// corpus rather than letting it through un-filtered — the same answer
- // passesFilter gives an untagged video.
+ // passesFilter gives an untagged video. See tagFilteredPostScope below:
+ // the DEFAULT scope is only half of it, because a `posts`-scope leaf
+ // carries its own slug set past this.
if (!committedNop && committedTags.size === 0) {
for (const slug of postScopeSlugs) out.push(slug);
}
@@ -772,6 +775,20 @@ function useSearchSessionState() {
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [transcripts, filterKey, postScopeSlugs, committedNop]);
+ // The posts slug set as the PIPELINE sees it. A `posts`-scope leaf is fed
+ // from here and not from the global scope above, so gating only the global
+ // build left an explicit scopes:['posts'] leaf searching the whole posts
+ // corpus while a tag chip was on: the rows were then dropped by the display
+ // fold, but `totalHits` sums the pipeline's hits BEFORE that fold, so the
+ // header read "3 videos, 8 hits" with hits nobody could see.
+ //
+ // `null` (not `[]`) is the "no posts in scope" signal the pipeline already
+ // understands — the same value it gets when no leaf needs posts at all.
+ const tagFilteredPostScope = useMemo<ReadonlySet<string> | null>(
+ () => (committedTags.size > 0 ? null : postScopeSlugs),
+ [committedTags, postScopeSlugs],
+ );
+
const hasActiveQuery = useMemo(
() => isNodeActive(committedRoot),
[committedRoot],
@@ -800,7 +817,7 @@ function useSearchSessionState() {
globalScope: globalScopeSlugs,
summaries: transcripts,
chatScopeSlugs: needsChatManifests ? chatScopeSlugs : null,
- postScopeSlugs: needsPostsManifests ? postScopeSlugs : null,
+ postScopeSlugs: needsPostsManifests ? tagFilteredPostScope : null,
initialHitLimit: hitLimit,
concurrency: fetchConcurrency,
flushIntervalMs,
@@ -1144,7 +1161,10 @@ function useSearchSessionState() {
// legacy branch below, because it is not part of either schema: it is the
// live representation of the committed tag selection, so its presence on
// the URL always wins and its absence falls through to the snapshot.
- const tagsFromUrl = params.getAll("tg").filter((t) => t !== "");
+ // isTagId, not a bare non-empty check: the URL is an entry point like the
+ // MCP's parseSearchArgs and must agree with it about what an id is. A
+ // malformed token can only ever produce a silent empty result.
+ const tagsFromUrl = params.getAll("tg").filter((t) => isTagId(t));
let initialTags: ReadonlySet<string> | null =
tagsFromUrl.length > 0 ? new Set(tagsFromUrl) : null;
diff --git a/common/components/urlState.ts b/common/components/urlState.ts
@@ -7,6 +7,7 @@ import { useMemo, useSyncExternalStore } from "react";
// unchanged.
export type { SearchMode } from "../lib/searchQuery";
import type { SearchMode } from "../lib/searchQuery";
+import { isTagId } from "../lib/curatedTags";
// Per-video modal content mode. Independent from the search page's `mode` so
// the modal can be toggled without disturbing search state. Absence on the
@@ -103,7 +104,14 @@ function parse(search: string): UrlParams {
nu: p.get("nu") === "1",
mode,
tracks: p.getAll("tk"),
- tags: p.getAll("tg"),
+ // Validated against TAG_ID_RE, so this entry point agrees with the MCP's
+ // parseSearchArgs: a lowercase [a-z0-9._-] id or nothing. A malformed
+ // token is DROPPED rather than carried, because the only thing a
+ // never-matching id can do downstream is turn a filter into a silent
+ // empty result. (A well-formed id this site does not publish is a
+ // different case and is kept — the chip row renders it so it can be
+ // cleared.)
+ tags: p.getAll("tg").filter((t) => isTagId(t)),
};
}
@@ -200,7 +208,7 @@ export function writeUrlParams(patch: Patch) {
}
if (patch.tags !== undefined) {
params.delete("tg");
- for (const v of patch.tags) params.append("tg", v);
+ for (const v of patch.tags) if (isTagId(v)) params.append("tg", v);
}
const qs = params.toString();
const next = `${window.location.pathname}${qs ? `?${qs}` : ""}`;
diff --git a/export/e2e/tag-chips.spec.ts b/export/e2e/tag-chips.spec.ts
@@ -9,6 +9,21 @@ import {
VIDEO_TRANSCRIPT_ONLY,
} from "./fixtures/data";
+// A composite query tree, seeded through `?qt=` exactly as posts-search.spec
+// does. Only the two leaf kinds this file needs.
+function qt(
+ children: { q: string; s: "metadata" | "posts" }[],
+ op: "AND" | "OR" = "OR",
+): string {
+ return encodeURIComponent(
+ JSON.stringify({
+ k: "g",
+ o: op,
+ c: children.map((c) => ({ k: "l", q: c.q, s: c.s })),
+ }),
+ );
+}
+
// Curated per-video tags (common/lib/curatedTags.ts) in the export viewer: the
// filter panel's chip row, the `tg` URL param, and the tags a result card
// carries. Modelled on channel-group-chips.spec.ts, but driven from the shared
@@ -158,6 +173,92 @@ test.describe("curated tag chips", () => {
.toEqual([]);
});
+ test("a tag filter takes an explicit posts-scope leaf out of the search", async ({
+ page,
+ }) => {
+ // A post carries no curated tags. The DEFAULT scope build already knew
+ // that, but a scopes:['posts'] leaf is fed its own slug set, so the posts
+ // corpus was still being searched under a tag filter: the rows were then
+ // dropped by the display fold while `totalHits` — which sums the
+ // pipeline's hits BEFORE that fold — still counted them. The header is
+ // the assertion because the header is where it showed: N videos and a
+ // hit count that no card on the page accounts for.
+ //
+ // "chat" matches all three video titles; "alpha" matches two posts.
+ const tree = qt([
+ { q: "chat", s: "metadata" },
+ { q: "alpha", s: "posts" },
+ ]);
+ await page.goto(`/?qt=${tree}&tg=${TAG_COLLAB}`);
+ await waitForHydration(page);
+
+ // Only the two tagged videos, and only their two hits.
+ await expect(page.getByTestId("results-summary")).toHaveText(
+ "Matching videos (2 videos, 2 hits)",
+ );
+ await expect(card(page, VIDEO_CHAT_SMALL)).toBeVisible();
+ await expect(card(page, VIDEO_CHAT_LARGE)).toBeVisible();
+ await expect(card(page, VIDEO_TRANSCRIPT_ONLY)).toHaveCount(0);
+ });
+
+ test("the same query without a tag filter does search the posts leaf", async ({
+ page,
+ }) => {
+ // The control for the case above: the posts leaf is only silenced BY the
+ // tag filter, never by this change.
+ const tree = qt([
+ { q: "chat", s: "metadata" },
+ { q: "alpha", s: "posts" },
+ ]);
+ await page.goto(`/?qt=${tree}`);
+ await waitForHydration(page);
+
+ await expect(page.getByTestId("results-summary")).toHaveText(
+ "Matching videos (5 videos, 5 hits)",
+ );
+ });
+
+ test("a tg naming an id this site does not publish still gets a chip", async ({
+ page,
+ }) => {
+ // A valid id from another site in the family, or from this one before a
+ // rebuild. Without a chip the Filters badge would say a filter is on
+ // while nothing in the row was lit, and the reader would have no way to
+ // clear the thing that emptied their results.
+ await page.goto("/?tg=not-on-this-site");
+ await waitForHydration(page);
+
+ const orphan = chip(page, "not-on-this-site");
+ await expect(orphan).toBeVisible();
+ await expect(orphan).toHaveAttribute("data-unpublished", "true");
+ await expect(orphan).toContainText("not on this site");
+ await expect(orphan).toHaveAttribute("aria-pressed", "true");
+ // It really is filtering: nothing carries it.
+ await expect(page.getByTestId("results-summary")).toHaveText("All videos (0)");
+
+ // One click clears it, and the param goes with it.
+ await orphan.click();
+ await apply(page);
+ await expect(page.getByTestId("tag-chip")).toHaveCount(2);
+ await expect(card(page, VIDEO_TRANSCRIPT_ONLY)).toBeVisible();
+ await expect
+ .poll(() => new URL(page.url()).searchParams.getAll("tg"))
+ .toEqual([]);
+ });
+
+ test("a malformed tg token is dropped at the URL, not carried", async ({
+ page,
+ }) => {
+ // The URL is an entry point like the MCP's parseSearchArgs and agrees
+ // with it about what an id is. A token that can never match must not
+ // become a filter that silently returns nothing.
+ await page.goto("/?tg=Not%20An%20Id");
+ await waitForHydration(page);
+
+ await expect(card(page, VIDEO_TRANSCRIPT_ONLY)).toBeVisible();
+ await expect(page.locator("[data-unpublished]")).toHaveCount(0);
+ });
+
test("a tg link arrives already filtered", async ({ page }) => {
await page.goto(`/?tg=${TAG_TOPIC}`);
await waitForHydration(page);
@@ -168,6 +269,32 @@ test.describe("curated tag chips", () => {
});
});
+ // The Filters chip only exists below xl — from xl the panel is inline under
+ // the bar and a chip would be a second copy of a control already on screen.
+ test.describe("on a phone", () => {
+ test.use({ viewport: { width: 390, height: 844 } });
+
+ test("the Filters badge counts a tag selection", async ({ page }) => {
+ await installRoutes(page);
+ await installTagRoutes(page);
+ await page.goto("/");
+ await waitForHydration(page);
+
+ const trigger = page.getByTestId("filters-trigger");
+ // Nothing narrowed yet: the badge is absent, not zero.
+ await expect(trigger).toHaveText("Filters");
+
+ await trigger.click();
+ await chip(page, TAG_COLLAB).click();
+ await page.getByRole("button", { name: "Apply filters" }).click();
+
+ // Without this the reader could apply a filter from the sheet and see no
+ // sign of it on the chip that opens the sheet.
+ await expect(trigger).toContainText("1");
+ await expect(card(page, VIDEO_TRANSCRIPT_ONLY)).toHaveCount(0);
+ });
+ });
+
test.describe("on a site that publishes none", () => {
test("there is no chip row at all", async ({ page }) => {
// installRoutes alone: /tags.json 404s, which is the real default for a
diff --git a/mcp/src/protocol.test.ts b/mcp/src/protocol.test.ts
@@ -98,7 +98,12 @@ async function writeFixture(opts: { tags?: boolean } = {}): Promise<string> {
}),
);
}
- const rec = (id: string, title: string, text: string) => ({
+ const rec = (
+ id: string,
+ title: string,
+ text: string,
+ curatedTags?: string[],
+ ) => ({
slug: `chan-a/${id}`,
id,
channelSlug: "chan-a",
@@ -112,13 +117,18 @@ async function writeFixture(opts: { tags?: boolean } = {}): Promise<string> {
ageRestricted: false,
platform: "youtube",
webpageUrl: `https://example.test/${id}`,
+ // Omitted when empty, exactly as the index build writes it — so the
+ // no-tags fixture's records are byte-identical to a pre-spec-4 site's.
+ ...(curatedTags && curatedTags.length > 0 ? { curatedTags } : {}),
cues: [{ start: 12, end: 15, text }],
});
+ // The records agree with the /tags.json above: one video per tag, which is
+ // what its counts claim.
await writeFile(
path.join(chDir, pageFileName(0)),
JSON.stringify([
- rec("a1", "Coffee one", "i love coffee"),
- rec("a2", "Tea two", "i love tea"),
+ rec("a1", "Coffee one", "i love coffee", opts.tags ? ["eva-collab"] : undefined),
+ rec("a2", "Tea two", "i love tea", opts.tags ? ["loose-tag"] : undefined),
]),
);
return dir;
@@ -270,6 +280,37 @@ test("stdio: a tag the site does not publish is named, not silently empty", asyn
assert.match(out, /tags: eva-collab OR not-a-real-tag/);
});
+test("stdio: enumerate_matches filters by tag and names the filter", async (t) => {
+ const dir = await writeFixture({ tags: true });
+ t.after(() => rm(dir, { recursive: true, force: true }));
+ const s = await connect(dir, { mode: "auto" });
+ t.after(() => s.close());
+
+ // Both records match "i love"; only a1 carries eva-collab.
+ const all = firstText(
+ await s.client.callTool({
+ name: "enumerate_matches",
+ arguments: { query: "i love" },
+ }),
+ );
+ assert.match(all, /2 match\(es\)/);
+
+ const tagged = firstText(
+ await s.client.callTool({
+ name: "enumerate_matches",
+ arguments: { query: "i love", tags: ["eva-collab"] },
+ }),
+ );
+ assert.match(tagged, /1 match\(es\)/);
+ assert.match(tagged, /- a1 \| video/);
+ assert.doesNotMatch(tagged, /- a2 \| video/);
+ // The worklist's footer names the filter that produced it, so a count can
+ // never be quoted without the scope that made it.
+ assert.match(tagged, /filters — tags: eva-collab/);
+ // …and the complete-set line is still the honest one.
+ assert.match(tagged, /complete set: yes/);
+});
+
test("stdio: prompts are served on both eras", async (t) => {
const dir = await writeFixture();
t.after(() => rm(dir, { recursive: true, force: true }));
