Archilyzer · Source

archilyzer

Archilyzer
git clone https://archilyzer.pages.dev/source/archilyzer.git
Log | Files | Refs | README | LICENSE

commit 8c080780da1041b730c6addd9bf86859b9adc3a3
parent 9fde71d250691e541784dfde8bf00fc280d2284a
Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st>
Date:   Tue, 22 Sep 2026 16:43:16 -0400

Merge branch 'main' into editor/debts

Diffstat:
Mcommon/components/FiltersPanel.tsx | 186++++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------------
Mcommon/components/QueryLeafView.tsx | 6+++++-
Mcommon/components/SearchResults.tsx | 13++++++-------
Mcommon/controller/autoRunner.test.ts | 64++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Mcommon/controller/autoRunner.ts | 21+++++++++++++++++++--
Mcommon/controller/curatedTagsIndex.test.ts | 21+++++++++++++++++++++
Mcommon/controller/curatedTagsIndex.ts | 9+++++++++
Mcommon/controller/curatedTagsPreview.ts | 0
Mcommon/lib/archive/contract.test.ts | 21+++++++++++++++++++++
Mcommon/lib/archive/contract.ts | 34+++++++++++++++++++++++-----------
Mcommon/lib/curatedTags.test.ts | 126+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
Mcommon/lib/curatedTags.ts | 71++++++++++++++++++++++++++++++++++++++++++++++++++++++-----------------
Mcommon/lib/curatedTagsStore.ts | 15+++++++++------
Meditor/app/api/ops/_lib.ts | 42++++++++++++++++++++++++++++++++++++++++++
Meditor/app/api/ops/build-deploy/route.ts | 100++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------
Meditor/app/api/ops/build-site/route.ts | 64+++++++++++++++++++++++++++++++++++++---------------------------
Meditor/app/operations/status.ts | 11+++++++++--
Meditor/app/tags/actions.ts | 18++++++++++++++++--
Meditor/app/tags/components/EditorTagsClient.tsx | 35++++++++++++++++++++++++++++++-----
Meditor/e2e/ops-api.spec.ts | 84++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
Meditor/e2e/tags.spec.ts | 42+++++++++++++++++++++++++++++++++++++++++-
Meditor/scripts/measure-nav.mjs | 2+-
Mexport/app/ask/AskHub.tsx | 29++++++++++++++++++++++++++---
Aexport/e2e-hub/ask.spec.ts | 55+++++++++++++++++++++++++++++++++++++++++++++++++++++++
Mexport/e2e/inline-channel-chips.spec.ts | 11+++++++++++
Mexport/e2e/query-tree.spec.ts | 36++++++++++++++++++++++++++++++++++++
Mexport/e2e/tag-chips.spec.ts | 74++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Mmcp/src/protocol.test.ts | 9+++++++++
Mmcp/src/search.test.ts | 45+++++++++++++++++++++++++++++++++++++++++++++
Mmcp/src/search.ts | 30+++++++++++++++++++++++++++---
Mmcp/src/server.ts | 8++++++++
Mscripts/archilyzer-ops.mjs | 185++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------
Mscripts/archilyzer-ops.test.mjs | 137++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
Mscripts/worktree.mjs | 39++++++++++++++++++++++++++++++---------
Ascripts/worktree.test.mjs | 46++++++++++++++++++++++++++++++++++++++++++++++
35 files changed, 1502 insertions(+), 187 deletions(-)

diff --git a/common/components/FiltersPanel.tsx b/common/components/FiltersPanel.tsx @@ -13,7 +13,7 @@ // to be the outer one. It also doubles as the inline collapse, driven by the // session's persisted `filtersCollapsed`. -import { Fragment, useCallback, useMemo } from "react"; +import { Fragment, useCallback, useMemo, useState } from "react"; import { ChevronRightIcon } from "lucide-react"; import { MISSING_STATES, @@ -36,6 +36,7 @@ import { groupPublishedTags, selectableTags, } from "../lib/publishedTags"; +import type { PublishedTag } from "../lib/curatedTags"; export default function FiltersPanel({ // Inside the sheet the sheet itself IS the disclosure, so the wrapper stays @@ -777,6 +778,62 @@ function TagChipRow() { .sort(); }, [publishable, draftTags]); + // Collapse is per-group and LOCAL, unlike the channel groups' persisted + // `collapsedGroups`: a tag row is a handful of chips a reader folds away to + // get the results back on screen, not a filter shape worth carrying into + // every future session — and a group that came back folded would hide the + // vocabulary this site is built around. Every group starts open. + const [collapsedTagGroups, setCollapsedTagGroups] = useState<Set<string>>( + () => new Set(), + ); + const toggleTagGroup = useCallback((key: string) => { + setCollapsedTagGroups((prev) => { + const next = new Set(prev); + if (next.has(key)) next.delete(key); + else next.add(key); + return next; + }); + }, []); + + const renderChip = (tag: PublishedTag) => { + const selected = draftTags.has(tag.id); + return ( + <button + key={tag.id} + type="button" + aria-pressed={selected} + data-testid="tag-chip" + data-tag-id={tag.id} + title={ + selected + ? `Clear "${tag.label}"` + : `Keep only videos tagged "${tag.label}"` + } + onClick={() => toggleDraftTag(tag.id)} + className={cn( + "flex items-center gap-1.5 rounded border px-2.5 py-1.5 text-sm select-none min-w-0 transition-colors", + selected + ? "border-primary bg-primary/10 text-foreground" + : "border-border bg-card/60 hover:bg-accent hover:text-accent-foreground", + )} + > + {tag.color && ( + <span + aria-hidden="true" + className="inline-block size-2 shrink-0 rounded-full" + style={{ background: tag.color }} + /> + )} + <span className="truncate">{tag.label}</span> + {/* The count is this site's, computed at index time — the same number + the chip's filter will produce. */} + <span className="text-xs text-muted-foreground shrink-0"> + {tag.count} + </span> + </button> + ); + }; + if (groups.length === 0 && unpublished.length === 0) return null; return ( @@ -794,62 +851,85 @@ function TagChipRow() { : `${draftTags.size} selected — a video with ANY of them`} </span> </div> - {/* One wrapping flow, phone-first: a group is a label plus its chips and - wraps as a unit, so a narrow viewport gets one group per line rather - than a torn row. */} + {/* One wrapping flow, phone-first: a group is a disclosure — its label + over its chips — and wraps as a unit, so a narrow viewport gets one + group per line rather than a torn row, and a reader on a phone can + fold a group away to get the results back on screen. */} <div className="flex flex-wrap items-center gap-x-4 gap-y-2"> - {groups.map((group) => ( - <div - key={group.id || "ungrouped"} - data-testid="tag-chip-group" - data-group-id={group.id} - className="flex flex-wrap items-center gap-1.5 min-w-0" - > - {group.label && ( - <span className="text-xs text-muted-foreground shrink-0"> - {group.label}: - </span> - )} - {group.tags.map((tag) => { - const selected = draftTags.has(tag.id); - return ( - <button - key={tag.id} - type="button" - aria-pressed={selected} - data-testid="tag-chip" - data-tag-id={tag.id} - title={ - selected - ? `Clear "${tag.label}"` - : `Keep only videos tagged "${tag.label}"` - } - onClick={() => toggleDraftTag(tag.id)} + {groups.map((group) => { + const key = group.id || "ungrouped"; + const chips = group.tags.map(renderChip); + // The trailing ungrouped bucket has no label, so there is nothing to + // put in a summary and nothing to name what folding it away hides: + // it stays a plain wrapping row. + if (!group.label) { + return ( + <div + key={key} + data-testid="tag-chip-group" + data-group-id={group.id} + className="flex flex-wrap items-center gap-1.5 min-w-0" + > + {chips} + </div> + ); + } + const isOpen = !collapsedTagGroups.has(key); + // What the summary has to say when the chips are hidden: a folded + // group that is narrowing the results must still admit it, or the + // reader is left with a short list and no visible cause. + const selectedCount = group.tags.reduce( + (n, tag) => (draftTags.has(tag.id) ? n + 1 : n), + 0, + ); + return ( + <details + key={key} + open={isOpen} + data-testid="tag-chip-group" + data-group-id={group.id} + className="min-w-0 max-w-full" + > + <summary + onClick={(e) => { + // Drive the open state from React rather than the browser's + // default toggle — same rationale as the channel group + // chips above. + e.preventDefault(); + toggleTagGroup(key); + }} + className="cursor-pointer select-none flex items-center gap-1.5 py-1 text-xs text-muted-foreground min-w-0" + title={ + isOpen + ? `Hide the ${group.label} tags` + : `Show the ${group.label} tags` + } + > + <ChevronRightIcon + aria-hidden="true" className={cn( - "flex items-center gap-1.5 rounded border px-2.5 py-1.5 text-sm select-none min-w-0 transition-colors", - selected - ? "border-primary bg-primary/10 text-foreground" - : "border-border bg-card/60 hover:bg-accent hover:text-accent-foreground", - )} - > - {tag.color && ( - <span - aria-hidden="true" - className="inline-block size-2 shrink-0 rounded-full" - style={{ background: tag.color }} - /> + "size-3.5 shrink-0 transition-transform", + isOpen && "rotate-90", )} - <span className="truncate">{tag.label}</span> - {/* The count is this site's, computed at index time — the - same number the chip's filter will produce. */} - <span className="text-xs text-muted-foreground shrink-0"> - {tag.count} + /> + <span className="font-medium text-foreground truncate"> + {group.label} + </span> + {selectedCount > 0 && ( + <span + data-testid="tag-group-selected" + className="shrink-0" + > + {selectedCount} selected </span> - </button> - ); - })} - </div> - ))} + )} + </summary> + <div className="flex flex-wrap items-center gap-1.5 min-w-0 pt-1.5"> + {chips} + </div> + </details> + ); + })} {unpublished.length > 0 && ( <div data-testid="tag-chip-group" diff --git a/common/components/QueryLeafView.tsx b/common/components/QueryLeafView.tsx @@ -43,7 +43,11 @@ type Props = { // (lib/curatedTags.ts) — the chip row in the filter panel. The code token, the // URL and the MCP `scopes` enum stay `"tags"`: renaming those would break every // saved query and share link to fix a word on a screen. -const SCOPE_LABELS: Record<LayerScope, string> = { +// Exported because the result list labels its per-leaf sections with the same +// words (SearchResults.tsx): a reader picks "Keywords" in the builder and has +// to find "Keywords" over the hits it produced, so there is one table, not two +// that drift. +export const SCOPE_LABELS: Record<LayerScope, string> = { transcripts: "Transcripts", chat: "Live chat", posts: "Posts", diff --git a/common/components/SearchResults.tsx b/common/components/SearchResults.tsx @@ -33,6 +33,7 @@ import { formatTimestamp } from "../lib/vtt"; import { vodExpiry } from "../lib/vodExpiry"; import { Button } from "./ui/button"; import { LayerSwatch } from "./LayerSwatch"; +import { SCOPE_LABELS } from "./QueryLeafView"; import { ChartShapeControls } from "./charts/ChartShapeControls"; import { SearchChartPanel } from "./charts/SearchChartPanel"; import { @@ -697,14 +698,12 @@ const ResultCard = memo(function ResultCard({ data-leaf-section={leafId} > <LayerSwatch leafId={leafId} size="xs" /> + {/* The builder's table, not a second copy of it: this chain + predated the description and tags scopes and called both + of them "Transcripts", which is where the hits did NOT come + from. */} <span className="text-[10px] uppercase tracking-wide text-muted-foreground"> - {leafInfo.scope === "metadata" - ? "Title / channel" - : leafInfo.scope === "chat" - ? "Live chat" - : leafInfo.scope === "posts" - ? "Posts" - : "Transcripts"} + {SCOPE_LABELS[leafInfo.scope]} </span> <span className="font-mono text-xs text-muted-foreground truncate"> {leafInfo.query} diff --git a/common/controller/autoRunner.test.ts b/common/controller/autoRunner.test.ts @@ -4,6 +4,7 @@ import { mkdirSync, mkdtempSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import path from "node:path"; import { + computeLeafPending, focusHoldLine, laneDispatchRoot, makeFocusHoldReporter, @@ -19,6 +20,7 @@ import { type FocusSummary, } from "../lib/channelPriority"; import { LANES } from "../lib/autoQueueTypes"; +import { emptyAutoQueueState } from "../jobs/autoQueueState"; import type { AutoQueueGroup, AutoQueuePolicy, @@ -479,3 +481,65 @@ test("a focus holds the scan the same way it holds every download pick", () => { ); }); + +// --- computeLeafPending's `shared`: the four-lane poll reads once ----------- +// +// /operations asks all four lanes for their pending work on a ~3 s poll, and +// each call used to list every channel's config off disk and re-parse the +// auto-queue state — both of which the caller had already read for the rest of +// the payload, and neither of which is per-lane. + +function pendingFixture(): Paths { + const dir = mkdtempSync(path.join(tmpdir(), "leaf-pending-")); + const channelsDir = path.join(dir, "channels"); + // A channel that IS on disk. An injected listing has to be believed over it. + mkdirSync(path.join(channelsDir, "on-disk"), { recursive: true }); + writeFileSync( + path.join(channelsDir, "on-disk", "config.json"), + JSON.stringify({ url: "https://www.youtube.com/@ondisk" }), + ); + return { + channelsDir, + sitesDir: path.join(dir, "sites"), + autoQueueStateFile: path.join(dir, "auto-queue.json"), + } as Paths; +} + +test("computeLeafPending takes its channel listing from `shared`", async () => { + const paths = pendingFixture(); + const result = await computeLeafPending(LANES[0], paths, { + configs: [], + state: emptyAutoQueueState(), + }); + // The leaves the compiled root defines, all at zero: an empty listing is no + // channels, so there is no work to attribute and nothing to pick. + assert.deepEqual(Object.values(result.counts), [0]); + assert.deepEqual(Object.values(result.head), [[]]); + assert.deepEqual(result.owner, {}); + assert.equal(result.nextUp, null); +}); + +test("an injected auto-queue state changes nothing about the answer", async () => { + const paths = pendingFixture(); + // The state carries SWRR weights, which only matter once there is work to + // pick between — so what `shared.state` must never do is change the result. + // It saves a read; it is not an input to the numbers. + const state = emptyAutoQueueState(); + state[LANES[0]].runtime.currentWeights = { "prio-all": 17 }; + const injected = await computeLeafPending(LANES[0], paths, { + configs: [], + state, + }); + const read = await computeLeafPending(LANES[0], paths, { configs: [] }); + assert.deepEqual(injected.counts, read.counts); + assert.deepEqual(injected.head, read.head); + assert.deepEqual(injected.owner, read.owner); + assert.equal(injected.nextUp, read.nextUp); + + // AND THE CALLER'S OBJECT IS NOT TOUCHED. selectNextWork advances fairness by + // mutating currentWeights in place, and this runs on a 3-second poll: the + // deep clone that stops merely HAVING the page open from skewing a + // round-robin group now has to protect a state the caller still holds and + // will hand to the next lane. + assert.deepEqual(state[LANES[0]].runtime.currentWeights, { "prio-all": 17 }); +}); diff --git a/common/controller/autoRunner.ts b/common/controller/autoRunner.ts @@ -354,8 +354,12 @@ async function listChannelMeta( paths: Paths, kind: AutoQueueKind, priority: ChannelPriority, + // A listing the CALLER already has. The status poll reads every channel's + // config once and then asked four lanes for their pending work, each of which + // re-read the whole directory — see computeLeafPending's `shared`. + sharedConfigs?: readonly { slug: string; config: ChannelConfig }[], ): Promise<{ meta: ChannelMeta[]; slugs: string[] }> { - const configs = await listChannelConfigs(paths); + const configs = sharedConfigs ?? (await listChannelConfigs(paths)); return { meta: configs .filter(({ slug }) => !isChannelPaused(priority, slug, kind)) @@ -904,9 +908,21 @@ export type LeafPending = { // operator sees as "cornbreadman: 12 pending". Uses the same matching AND the // same ordering as the runner, so the numbers and the drill-down line up with // what would actually be picked. +// +// `shared` IS THE FOUR-LANE POLL'S WAY OUT OF READING EVERYTHING FOUR TIMES. +// /operations asks all four lanes on a ~3 s poll, and each call listed every +// channel's config (a readdir plus a read per channel — 30 on Jeralyzer) and +// parsed the auto-queue state document again, having already read both itself +// for the rest of the payload. Nothing about either read is per-lane, so a +// caller that holds them passes them in and the poll pays once. Omitted, each +// is read here exactly as before, so every other caller is unchanged. export async function computeLeafPending( kind: AutoQueueKind, paths: Paths = getPaths(), + shared?: { + configs?: readonly { slug: string; config: ChannelConfig }[]; + state?: AutoQueueState; + }, ): Promise<LeafPending> { const settings = getSettings(); const policy = settings.autoQueue[kind]; @@ -920,6 +936,7 @@ export async function computeLeafPending( paths, kind, settings.channelPriority, + shared?.configs, ); const ctx = priorityContextFor(paths, settings, slugs); const root = laneDispatchRoot(kind, policy, ctx, meta.map((m) => m.slug)); @@ -974,7 +991,7 @@ export async function computeLeafPending( // runtime.currentWeights in place. This runs on a 3-second status poll, so // asking the live runtime would let merely HAVING the page open skew a // round-robin group's rotation. Deep-clone first; the clone is discarded. - const state = await readAutoQueueState(paths); + const state = shared?.state ?? (await readAutoQueueState(paths)); const runtime = { currentWeights: { ...state[kind].runtime.currentWeights }, }; diff --git a/common/controller/curatedTagsIndex.test.ts b/common/controller/curatedTagsIndex.test.ts @@ -144,6 +144,27 @@ test("hashCuratedRules ignores presentation, reacts to derivation", () => { ]), base, ); + // An exclusion changes which records the rule derives, so it is in the hash + // — and the empty list it replaces must not be, or the field's arrival would + // re-derive the corpus a second time. + assert.notEqual( + hashCuratedRules([ + { + ...COLLAB_RULE, + rules: [{ ...COLLAB_RULE.rules![0], channelsExclude: ["mommaocco"] }], + }, + ]), + base, + ); + assert.equal( + hashCuratedRules([ + { + ...COLLAB_RULE, + rules: [{ ...COLLAB_RULE.rules![0], channelsExclude: [] }], + }, + ]), + base, + ); // A disabled rule is not part of the derivation at all. assert.equal( hashCuratedRules([ diff --git a/common/controller/curatedTagsIndex.ts b/common/controller/curatedTagsIndex.ts @@ -64,6 +64,14 @@ function sha1(s: string): string { // The shape of the vocabulary that can change a record. Presentation fields // (label, colour, group, hidden) are deliberately absent: relabelling a tag // must not re-page 30,000 videos. +// +// ONE-TIME COST WHEN THIS SHAPE CHANGES: the hash is stored per build, so +// adding a field to it (channelsExclude, 2026-09-22) makes every stored +// `curatedRulesHash` stale and the first build-index after the change +// re-derives the whole corpus once — ~80 s, read back out of LMDB with no cue +// re-read. The log line below says `rules <hash8> (changed)` when that happens; +// that sentence is the operator's signal that the pass was the migration and +// not a rule they edited. export function hashCuratedRules(defs: CuratedTagDef[]): string { const shape = defs.map((def, i) => [ def.id, @@ -75,6 +83,7 @@ export function hashCuratedRules(defs: CuratedTagDef[]): string { r.kind, r.pattern, [...(r.channels ?? [])].sort(), + [...(r.channelsExclude ?? [])].sort(), r.dateFrom ?? null, r.dateTo ?? null, ]), diff --git a/common/controller/curatedTagsPreview.ts b/common/controller/curatedTagsPreview.ts Binary files differ. diff --git a/common/lib/archive/contract.test.ts b/common/lib/archive/contract.test.ts @@ -261,3 +261,24 @@ test("the search index worker's private pageFileName matches CONTRACT.pagePad", assert.ok(src.includes("`/transcripts/${slug}/${pageFileName(p)}`")); assert.equal(pageUrl("transcripts", "alpha", 12), "/transcripts/alpha/page-0012.json"); }); + +// shipsPwa is the one function in this module that reads the ambient +// environment, and the guard in front of that read is not observable from its +// return value — `typeof process` is true in every runtime the test suite has. +// So the assertion is on the SOURCE, the same way the service-worker checks +// above are: what is being pinned is that the read cannot throw at import time +// in a browser, not what it answers. +test("shipsPwa guards its process.env read", () => { + const src = readSource("common/lib/archive/contract.ts"); + const fn = /export function shipsPwa\([\s\S]*?\n}/.exec(src); + assert.ok(fn, "shipsPwa not found — did it move or get renamed?"); + assert.match(fn[0], /typeof process !== "undefined"/); + // The guard must come BEFORE the dereference, which is the only arrangement + // that stops `ReferenceError: process is not defined`. + assert.ok( + fn[0].indexOf('typeof process !== "undefined"') < + fn[0].indexOf("process.env.INSTANCE_MODE"), + "the guard must precede the read", + ); + // What it ANSWERS is unchanged, and is pinned by the behaviour test above. +}); diff --git a/common/lib/archive/contract.ts b/common/lib/archive/contract.ts @@ -180,18 +180,30 @@ export function pageUrl( // Was two copies with "keep in sync" comments on each (compose-site.ts and // export/app/lib/mode.ts); S2c deleted both, and this is the one. // -// `process.env.INSTANCE_MODE` is read bare, exactly as both copies read it, and -// the reason that is safe is NOT that Next inlines it — it does not: -// INSTANCE_MODE is neither `NEXT_PUBLIC_` nor listed in a next.config `env:` -// block, so a client bundle would read `undefined` here. (An earlier draft of -// this comment claimed the opposite; the S1 review checked.) It is safe because -// no browser evaluates it: the only caller in the export app is -// export/app/lib/mode.ts, whose only caller is export/app/layout.tsx, a SERVER -// component, and the other caller is compose-site.ts, a build script. If this -// ever becomes reachable from a client module it needs a -// `typeof process !== "undefined"` guard AND an env var the client can see. +// THE `typeof process` GUARD STOPS A CRASH. IT DOES NOT MAKE HUB DETECTION +// WORK CLIENT-SIDE, and that distinction is the whole comment. +// +// `INSTANCE_MODE` is neither `NEXT_PUBLIC_` nor listed in a next.config `env:` +// block, so Next does not inline it and a client bundle reads `undefined` here. +// (An earlier draft of this comment claimed the opposite; the S1 review +// checked.) Hub detection is therefore SERVER-ONLY, guard or no guard, until +// somebody ships the value to the client deliberately — which would need an env +// var the client can actually see, not a change to this line. +// +// Today that costs nothing: the export app reaches this only through +// export/app/lib/mode.ts ← export/app/layout.tsx, a SERVER component, and the +// other caller is compose-site.ts, a build script. What the guard buys is the +// failure mode when that stops being true. An unguarded `process.env` in a +// module some client component pulls in throws `ReferenceError: process is not +// defined` at import time and takes the page down with it; guarded, the same +// import yields `false` — no PWA, which is the right answer for every site but +// a hub and a recoverable one for a hub. A wrong answer nobody notices beats a +// white screen. export function shipsPwa(site: { pwa?: boolean }): boolean { - return site.pwa === true || process.env.INSTANCE_MODE === "hub"; + return ( + site.pwa === true || + (typeof process !== "undefined" && process.env.INSTANCE_MODE === "hub") + ); } // ─── The hub's member entry, once ─── diff --git a/common/lib/curatedTags.test.ts b/common/lib/curatedTags.test.ts @@ -226,7 +226,40 @@ test("sanitizeTagsConfig is idempotent", () => { // ─── mergeTagDefs ─── -test("mergeTagDefs overlays presentation fields and appends site rules", () => { +test("the site layer carries no rules, on read and on write", () => { + const raw = { + tags: [ + { + id: "eva-collab", + rules: [{ id: "site", kind: "metadata", pattern: "extra" }], + }, + { + id: "site-only", + label: "Site only", + rules: [{ id: "r1", kind: "caption", pattern: "x" }], + }, + ], + }; + const site = sanitizeTagsConfig(raw, { layer: "site" }); + // Not emptied — DROPPED: no `rules` key at all, so what a site writes has + // none and nothing downstream is ever handed a rule that cannot fire. + for (const def of site.tags) { + assert.equal(Object.prototype.hasOwnProperty.call(def, "rules"), false); + } + // The same coercion runs on read, so a hand-edited site file cannot smuggle + // one back in. + assert.deepEqual( + sanitizeTagsConfig(JSON.parse(JSON.stringify(site)), { layer: "site" }), + site, + ); + // And the corpus layer is untouched by this: the identical input keeps them. + assert.deepEqual( + (sanitizeTagsConfig(raw).tags[0].rules ?? []).map((r) => r.id), + ["site"], + ); +}); + +test("mergeTagDefs overlays presentation fields and cannot add a rule", () => { const merged = mergeTagDefs(DEFS, [ { id: "eva-collab", @@ -235,6 +268,9 @@ test("mergeTagDefs overlays presentation fields and appends site rules", () => { color: "#b48ead", order: 9, hidden: true, + // A rule reaching the merge at all means someone bypassed the site + // coercion; it is still not appended, because a site rule can never tag + // a record and "harmless on a published def" is how it looks like it can. rules: [{ id: "site", kind: "metadata", pattern: "extra", enabled: true }], }, ]); @@ -246,7 +282,7 @@ test("mergeTagDefs overlays presentation fields and appends site rules", () => { assert.equal(collab.hidden, true); assert.deepEqual( (collab.rules ?? []).map((r) => r.id), - ["meta", "site"], + ["meta"], ); // Untouched fields survive, and the global input is not mutated. assert.equal(collab.group, "eva"); @@ -432,6 +468,92 @@ test("channel scope and date range are applied before any regex", () => { assert.deepEqual(evaluateTagRules({ ...hit, uploadDate: undefined }, [scoped]), []); }); +test("an excluded channel never fires, and beats the allow-list", () => { + // The shape this exists for: every channel EXCEPT the one that floods the + // tag, with no allow-list to keep in sync as channels are added. + const wide: CuratedTagDef = { + id: "wide", + label: "Wide", + rules: [ + { + id: "r1", + kind: "metadata", + pattern: "elfpire", + channelsExclude: ["mommaocco"], + enabled: true, + }, + ], + }; + const hit = { ...base, title: "elfpire" }; + assert.deepEqual(evaluateTagRules(hit, [wide]), ["wide"]); + assert.deepEqual(evaluateTagRules({ ...hit, channelSlug: "mommaocco" }, [wide]), []); + + // A slug in BOTH lists is excluded — the exclusion is the more specific + // statement, so it wins rather than the allow-list re-admitting the channel. + const both: CuratedTagDef = { + ...wide, + rules: [ + { + ...wide.rules![0], + channels: [base.channelSlug, "mommaocco"], + channelsExclude: ["mommaocco"], + }, + ], + }; + assert.deepEqual(evaluateTagRules(hit, [both]), ["wide"]); + assert.deepEqual(evaluateTagRules({ ...hit, channelSlug: "mommaocco" }, [both]), []); + // And the allow-list still bounds the rule on its own. + assert.deepEqual(evaluateTagRules({ ...hit, channelSlug: "other" }, [both]), []); + + // The exclusion applies to the cue kinds too — they route through the same + // ruleApplies, which is exactly what needsCaptionCues/needsChatCues predict. + const compiled = compileTagRules([ + { + id: "cap", + label: "Cap", + rules: [ + { + id: "r1", + kind: "caption", + pattern: "elfpire", + channelsExclude: ["mommaocco"], + enabled: true, + }, + ], + }, + ]); + const captionCues = [{ text: "and then elfpire said" }]; + assert.deepEqual(evaluateCompiledRules({ ...base, captionCues }, compiled), ["cap"]); + assert.deepEqual( + evaluateCompiledRules( + { ...base, channelSlug: "mommaocco", captionCues }, + compiled, + ), + [], + ); +}); + +test("sanitizeTagsConfig cleans channelsExclude like channels, and omits it when empty", () => { + const read = (raw: object) => + (sanitizeTagsConfig({ + tags: [{ id: "t", rules: [{ kind: "metadata", pattern: "x", ...raw }] }], + }).tags[0].rules ?? [])[0]; + + const rule = read({ channelsExclude: ["a", " ", 3, " b "] }); + assert.deepEqual(rule.channelsExclude, ["a", "b"]); + // Idempotent: sanitizing an already-sanitized rule changes nothing. + assert.deepEqual(read(rule), rule); + + // An empty (or all-junk, or absent) exclude emits no key at all, so a rule + // nobody touched round-trips through the store byte-identically. + for (const raw of [{}, { channelsExclude: [] }, { channelsExclude: [" ", 7] }]) { + assert.equal( + Object.prototype.hasOwnProperty.call(read(raw), "channelsExclude"), + false, + ); + } +}); + test("a disabled rule never fires, and neither does a broken one", () => { const defs = sanitizeTagsConfig({ tags: [ diff --git a/common/lib/curatedTags.ts b/common/lib/curatedTags.ts @@ -47,6 +47,11 @@ export type CuratedTagRule = { pattern: string; // Channel slugs this rule may fire on. Absent or empty = every channel. channels?: string[]; + // Channel slugs this rule may NEVER fire on, applied AFTER `channels`. A slug + // in both lists is excluded: naming a channel to skip is the more specific + // statement, and the shape this exists for — "every channel except that one" + // — needs no allow-list at all. Absent or empty = exclude nothing. + channelsExclude?: string[]; // Inclusive upload-date bounds, compared digit-wise so both "20260101" and // "2026-01-01" work. null/absent = unbounded. dateFrom?: string | null; @@ -164,6 +169,16 @@ function tryCompile(pattern: string): { re: RegExp } | { error: string } { } } +// A rule's channel scope, either side of it. Shared so the allow-list and the +// exclude list can never drift into sanitizing differently. +function channelList(v: unknown): string[] { + return Array.isArray(v) + ? v + .filter((c): c is string => typeof c === "string" && c.trim() !== "") + .map((c) => c.trim()) + : []; +} + function coerceRule(raw: unknown, index: number): CuratedTagRule | null { if (!raw || typeof raw !== "object") return null; const r = raw as Record<string, unknown>; @@ -173,11 +188,8 @@ function coerceRule(raw: unknown, index: number): CuratedTagRule | null { // A rule with no pattern cannot mean anything; drop it. if (pattern.trim() === "") return null; const id = trimmedString(r.id) ?? `r${index + 1}`; - const channels = Array.isArray(r.channels) - ? r.channels - .filter((c): c is string => typeof c === "string" && c.trim() !== "") - .map((c) => c.trim()) - : []; + const channels = channelList(r.channels); + const channelsExclude = channelList(r.channelsExclude); const dateFrom = dateBound(r.dateFrom); const dateTo = dateBound(r.dateTo); const rule: CuratedTagRule = { @@ -186,6 +198,7 @@ function coerceRule(raw: unknown, index: number): CuratedTagRule | null { pattern, enabled: r.enabled !== false, // default true ...(channels.length > 0 ? { channels } : {}), + ...(channelsExclude.length > 0 ? { channelsExclude } : {}), ...(dateFrom ? { dateFrom } : {}), ...(dateTo ? { dateTo } : {}), }; @@ -209,11 +222,21 @@ function coerceTagDef(raw: unknown, layer: TagLayer): CuratedTagDef | null { // override", and inventing one here would rename the tag on that site, since // mergeTagDefs applies every field the overlay carries. const label = trimmedString(r.label) ?? (layer === "site" ? undefined : id); - const rules = Array.isArray(r.rules) - ? r.rules - .map((rule, i) => coerceRule(rule, i)) - .filter((rule): rule is CuratedTagRule => rule !== null) - : []; + // THE SECOND FIELD THE LAYERS TREAT DIFFERENTLY: a site layer carries no + // rules at all, and one written there is dropped — on read AND on write, so + // it can never reach a file, a merge or a published def. + // + // A rule is evaluated once at index time, over records SHARED by every site + // that carries the channel. There is no per-site record for a per-site hit to + // live in, so a site rule could only ever LOOK like it worked. Dropping it in + // the coercion is the one place that covers every writer. Rules bind from + // transcripts/tags.json alone; promote one there to make it fire. + const rules = + layer === "site" || !Array.isArray(r.rules) + ? [] + : r.rules + .map((rule, i) => coerceRule(rule, i)) + .filter((rule): rule is CuratedTagRule => rule !== null); const order = typeof r.order === "number" && Number.isFinite(r.order) ? r.order @@ -335,10 +358,13 @@ export function sanitizeTagsConfig( // Layer a site's tags.json over the corpus one. // // For an id the corpus already defines, the site entry is a FIELD-WISE OVERLAY -// of label/groupLabel/color/order/hidden and may APPEND rules. It can never -// delete a corpus rule (and assignments do not live at the site layer at all — -// an assignment is a fact about a video, not a presentation choice). A site id -// the corpus does not define becomes a full site-only tag. +// of label/groupLabel/color/order/hidden — presentation, and nothing else. It +// can never delete a corpus rule, and it cannot add one: coerceTagDef drops +// rules from a site layer entirely (see there for why a per-site rule cannot +// tag anything), so there is nothing here to append. Assignments do not live at +// the site layer at all — an assignment is a fact about a video, not a +// presentation choice. A site id the corpus does not define becomes a full +// site-only tag, rule-less like every other site row. // // Deliberately NOT mergeAliases' wholesale replacement: aliases are suggestions, // tags are a shared vocabulary that a site may dress up but not gut. @@ -362,9 +388,8 @@ export function mergeTagDefs( if (overlay.color !== undefined) base.color = overlay.color; if (overlay.order !== undefined) base.order = overlay.order; if (overlay.hidden !== undefined) base.hidden = overlay.hidden; - if (overlay.rules && overlay.rules.length > 0) { - base.rules = [...(base.rules ?? []), ...overlay.rules]; - } + // No rule branch: a sanitized site def has none, and the corpus rules on + // `base` are left exactly as the corpus wrote them. } return out; } @@ -430,6 +455,8 @@ export type CompiledTagRule = { re: RegExp; // null = every channel. channels: Set<string> | null; + // null = exclude nothing. Applied after `channels` — see ruleApplies. + channelsExclude: Set<string> | null; dateFrom: string | null; dateTo: string | null; }; @@ -473,6 +500,10 @@ export function compileTagRules(defs: CuratedTagDef[]): CompiledTagRules { rule.channels && rule.channels.length > 0 ? new Set(rule.channels) : null, + channelsExclude: + rule.channelsExclude && rule.channelsExclude.length > 0 + ? new Set(rule.channelsExclude) + : null, dateFrom: dateBound(rule.dateFrom), dateTo: dateBound(rule.dateTo), }; @@ -502,6 +533,12 @@ export function ruleApplies( input: { channelSlug: string; uploadDate?: string }, ): boolean { if (rule.channels && !rule.channels.has(input.channelSlug)) return false; + // The exclude wins over the allow-list on purpose: a slug in both is + // excluded, so "this whole group, minus the one that floods it" is one rule + // rather than an allow-list the operator has to keep in sync by hand. + if (rule.channelsExclude && rule.channelsExclude.has(input.channelSlug)) { + return false; + } if (rule.dateFrom || rule.dateTo) { const date = (input.uploadDate ?? "").replace(/\D/g, ""); if (!date) return false; diff --git a/common/lib/curatedTagsStore.ts b/common/lib/curatedTagsStore.ts @@ -7,12 +7,15 @@ // site-only tags. See common/lib/curatedTags.ts for the pure model, the // coercion and mergeTagDefs. // -// **A site-layer RULE never tags a record.** mergeTagDefs will append one (it -// is harmless on a published /tags.json def), but rules are evaluated once at -// index time over records SHARED by every site that carries the channel — -// there is no per-site record for a per-site hit to live in. Rules bind only -// from transcripts/tags.json; promote one there to make it fire. This is why -// the editor offers no site-side rule editor. +// **A site-layer RULE never tags a record, so a site layer carries none.** +// Rules are evaluated once at index time over records SHARED by every site that +// carries the channel — there is no per-site record for a per-site hit to live +// in, so a rule written at the site layer could only ever LOOK like it worked. +// The site coercion therefore DROPS rules outright: on READ, so a hand-edited +// sites/<id>/tags.json cannot smuggle one in, and on WRITE, so neither can a +// caller — and mergeTagDefs has no rule branch left to append through. Rules +// bind from transcripts/tags.json alone; promote one there to make it fire. +// This is why the editor offers no site-side rule editor. // // Unlike aliasesStore there are NO seeded defaults: a fresh install has no // tags, and an absent global file reads as an empty config. diff --git a/editor/app/api/ops/_lib.ts b/editor/app/api/ops/_lib.ts @@ -1,6 +1,7 @@ import { NextResponse } from "next/server"; import { authorizeWorkerRequest } from "yt-dlp-transcript-common/lib/workerToken"; import { isValidChannelSlug } from "yt-dlp-transcript-common/controller/channels"; +import { isValidSiteId } from "yt-dlp-transcript-common/lib/site"; import type { StreamActionResult } from "yt-dlp-transcript-common/jobs/streamCommand"; import type { QueueOutcome } from "../../channels/lib/queueForSlugs"; @@ -158,6 +159,47 @@ export function reqStringArray(body: OpsBody, key: string): string[] { return (v as string[]).map((s) => s.trim()); } +// ONE SITE OR SEVERAL, SPELLED EITHER WAY. The two build routes disagreed — +// build-site took `siteIds` (a list), build-deploy took `siteId` (one) — so the +// same body worked on one and 400'd on the other, and the fix people reached +// for was to guess, which costs a round trip every time. Both routes now accept +// both keys, and this reader is the single place that says what that means. +// +// Both keys at once is still a 400 rather than a merge: a caller that sent both +// has two ideas about what it wants, and picking one on its behalf is how a +// deploy of the wrong site becomes somebody's afternoon. Neither key is a 400 +// naming both, because "which did you mean" is the actual question. +// +// A SITE ID, NOT MERELY A STRING, and checked BEFORE any job starts — the same +// reason reqSlug exists, with a worse failure behind it. `getSite` throws a +// plain Error on an id failing SITE_ID_RE and `ops()` maps that to a 500, so +// ["good", "BAD"] used to queue the first build, throw on the second and answer +// 500 with no jobs and no ids: a real build running that nothing was watching, +// and a `--wait` exiting 1 about it. +export function reqSiteIds(body: OpsBody): string[] { + const one = body.siteId; + const many = body.siteIds; + if (one !== undefined && many !== undefined) { + throw new OpsInputError('send either "siteId" or "siteIds", not both'); + } + let ids: string[]; + if (one !== undefined) ids = [reqString(body, "siteId")]; + else if (many !== undefined) ids = reqStringArray(body, "siteIds"); + else { + throw new OpsInputError( + '"siteId" (a string) or "siteIds" (a non-empty array of strings) is required', + ); + } + for (const id of ids) { + if (!isValidSiteId(id)) { + throw new OpsInputError( + `"${id}" is not a valid site id (lowercase letters, digits and "-"; must start with a letter or digit)`, + ); + } + } + return ids; +} + export function oneOf<T extends string>( body: OpsBody, key: string, diff --git a/editor/app/api/ops/build-deploy/route.ts b/editor/app/api/ops/build-deploy/route.ts @@ -3,29 +3,93 @@ import { buildAndDeployAction, buildAndDeployAllSitesAction, } from "../../../sites/lib/buildAction"; -import { jobResponse, OpsInputError, ops, optBool, optString } from "../_lib"; +import { + jobResponse, + OpsInputError, + ops, + opsFail, + optBool, + reqSiteIds, +} from "../_lib"; export const dynamic = "force-dynamic"; -// POST { siteId: string, skipArchives? } | { all: true, skipArchives? } -// -> { ok: true, jobId } +// POST { siteId: string | siteIds: string[], skipArchives? } +// | { all: true, skipArchives? } +// -> { ok: true, jobs: [{ siteId, jobId }], skipped: [{ siteId, reason }], +// jobId? } +// +// Build THEN deploy: one managed job per site (one log, one Cancel each), so +// the caller polls /api/jobs/<jobId>/log exactly as it does for build-site. +// +// `siteId` and `siteIds` are the same key (reqSiteIds), and the response is +// build-site's: this route took one id and that one a list, so the two halves +// of the same sentence in a runbook needed different JSON. `jobId` is still +// there when exactly one job started, so every existing single-site caller — +// and jobResponse's own shape — is unchanged; `jobs`/`skipped` are additive, +// and `pnpm ops --wait` already reads `jobs[]`. // -// One managed job either way (build then deploy, one log, one Cancel), so the -// caller polls /api/jobs/<jobId>/log exactly as for a single site. +// Asking for builds and getting NONE is still a 400 carrying the reason, not a +// cheerful `{ ok: true, jobs: [] }`: --wait would exit 0 on it and report +// success about a deploy that never started. export async function POST(request: Request) { - return ops(request, ["siteId", "all", "skipArchives"], async (body) => { - const skipArchives = optBool(body, "skipArchives"); - const all = optBool(body, "all"); - const siteId = optString(body, "siteId"); - if (all) { - if (siteId) throw new OpsInputError('send either "siteId" or "all", not both'); - return jobResponse(await buildAndDeployAllSitesAction(skipArchives)); - } - if (!siteId?.trim()) { - throw new OpsInputError('"siteId" is required (or send { "all": true })'); - } - return jobResponse(await buildAndDeployAction(siteId.trim(), skipArchives)); - }); + return ops( + request, + ["siteId", "siteIds", "all", "skipArchives"], + async (body) => { + const skipArchives = optBool(body, "skipArchives"); + if (optBool(body, "all")) { + if (body.siteId !== undefined || body.siteIds !== undefined) { + throw new OpsInputError( + 'send either "siteId"/"siteIds" or "all", not both', + ); + } + return jobResponse(await buildAndDeployAllSitesAction(skipArchives)); + } + const jobs: { siteId: string; jobId: string }[] = []; + const skipped: { siteId: string; reason: string }[] = []; + let info = false; + for (const siteId of reqSiteIds(body)) { + // ONE SITE'S THROW CANNOT COST THE OTHERS THEIR JOB IDS. An exception + // out of here becomes a 500 carrying no `jobs` at all, while the builds + // already queued run on with nobody holding their ids. reqSiteIds + // rejects the malformed-id case before any job starts; this catches + // whatever else the action can raise. + let result: Awaited<ReturnType<typeof buildAndDeployAction>>; + try { + result = await buildAndDeployAction(siteId, skipArchives); + } catch (e) { + skipped.push({ siteId, reason: (e as Error).message }); + continue; + } + if (!result.ok) { + skipped.push({ siteId, reason: result.error }); + info = info || result.info === true; + continue; + } + // The stream is cancelled, never returned — see _lib's header. + void result.stream.cancel(); + jobs.push({ siteId, jobId: result.jobId }); + } + if (jobs.length === 0) { + // One site asked for, one reason: the bare sentence the action gave, + // exactly as jobResponse has always returned it. + return opsFail( + skipped.length === 1 + ? skipped[0].reason + : skipped.map((s) => `${s.siteId}: ${s.reason}`).join("; "), + 400, + info ? { info: true } : undefined, + ); + } + return NextResponse.json({ + ok: true, + jobs, + skipped, + ...(jobs.length === 1 ? { jobId: jobs[0].jobId } : {}), + }); + }, + ); } export function GET() { diff --git a/editor/app/api/ops/build-site/route.ts b/editor/app/api/ops/build-site/route.ts @@ -3,49 +3,59 @@ import { buildAllSitesAction, buildExportAction, } from "../../../sites/lib/buildAction"; -import { jobResponse, OpsInputError, ops, optBool } from "../_lib"; +import { + jobResponse, + OpsInputError, + ops, + optBool, + reqSiteIds, +} from "../_lib"; export const dynamic = "force-dynamic"; -// POST { siteIds: string[], skipData?, skipArchives? } | { all: true, skipArchives? } +// POST { siteIds: string[] | siteId: string, skipData?, skipArchives? } +// | { all: true, skipArchives? } +// +// BUILD WITHOUT DEPLOYING. One build-export job per site on the shared build +// queue (they run one at a time, as they do from /sites), returning +// `{ jobs: [{ siteId, jobId }] }` — a list, because there is a job per site and +// a caller waiting on them needs all the ids. `{ all: true }` is the single +// build-all job instead, which is the docker fan-out. // -// BUILD WITHOUT DEPLOYING. `siteIds` queues one build-export job per site on the -// shared build queue (they run one at a time, as they do from /sites) and -// returns `{ jobs: [{ siteId, jobId }] }` — a list, because there is a job per -// site and a caller waiting on them needs all the ids. `{ all: true }` is the -// single build-all job instead, which is the docker fan-out. +// `siteId` and `siteIds` both work, via reqSiteIds: this route and build-deploy +// used to disagree about the spelling, which made the pair unguessable. export async function POST(request: Request) { return ops( request, - ["siteIds", "all", "skipData", "skipArchives"], + ["siteId", "siteIds", "all", "skipData", "skipArchives"], async (body) => { const skipArchives = optBool(body, "skipArchives"); if (optBool(body, "all")) { - if (body.siteIds !== undefined) { - throw new OpsInputError('send either "siteIds" or "all", not both'); + if (body.siteIds !== undefined || body.siteId !== undefined) { + throw new OpsInputError( + 'send either "siteId"/"siteIds" or "all", not both', + ); } return jobResponse(await buildAllSitesAction(skipArchives)); } - const raw = body.siteIds; - if ( - !Array.isArray(raw) || - raw.length === 0 || - raw.some((s) => typeof s !== "string" || !s.trim()) - ) { - throw new OpsInputError( - '"siteIds" is required and must be a non-empty array of strings (or send { "all": true })', - ); - } const skipData = optBool(body, "skipData"); const jobs: { siteId: string; jobId: string }[] = []; const skipped: { siteId: string; reason: string }[] = []; - for (const siteId of (raw as string[]).map((s) => s.trim())) { - const result = await buildExportAction( - siteId, - undefined, - skipData, - skipArchives, - ); + for (const siteId of reqSiteIds(body)) { + // One site's throw cannot cost the others their job ids — see the same + // guard in build-deploy. + let result: Awaited<ReturnType<typeof buildExportAction>>; + try { + result = await buildExportAction( + siteId, + undefined, + skipData, + skipArchives, + ); + } catch (e) { + skipped.push({ siteId, reason: (e as Error).message }); + continue; + } if (!result.ok) { skipped.push({ siteId, reason: result.error }); continue; diff --git a/editor/app/operations/status.ts b/editor/app/operations/status.ts @@ -27,7 +27,8 @@ import { readPriorityView } from "./channelPriorityView"; // set costs a channel listing and, for a site focus, a sites read; the // auto-queue state document is one JSON parse — it was read once PER LANE, four // times per poll, because `buildKind` did its own reading; and the channel -// briefs are shared with the lanes builder through the per-request cache. +// briefs are shared with the lanes builder through the per-request cache, and +// with the four computeLeafPending calls through their `shared` argument. export async function buildAutoQueueStatusPayload(): Promise<AutoQueueStatusPayload> { const paths = getPaths(); const settings = getSettings(); @@ -37,8 +38,14 @@ export async function buildAutoQueueStatusPayload(): Promise<AutoQueueStatusPayl readAutoQueueState(paths), getChannelBriefs(paths), ]); + // …and the four lanes' pending work re-reads NEITHER. `briefs` is the channel + // listing and `state` the auto-queue document, both already in hand one line + // up; without them each of the four calls listed every channel's config off + // disk again and re-parsed the state, on a ~3 s poll. const pendingByKind = await Promise.all( - LANES.map((lane) => computeLeafPending(lane, paths)), + LANES.map((lane) => + computeLeafPending(lane, paths, { configs: briefs, state }), + ), ); const byLane = <T>(values: readonly T[]): Record<AutoQueueKind, T> => Object.fromEntries(LANES.map((lane, i) => [lane, values[i]])) as Record< diff --git a/editor/app/tags/actions.ts b/editor/app/tags/actions.ts @@ -160,9 +160,23 @@ export async function previewTagRuleAction(input: { // "every channel", and then so does the tag. const rules = input.def.rules ?? []; const unscoped = rules.some((r) => !r.channels || r.channels.length === 0); - const channels = unscoped - ? [] + const scoped = unscoped + ? allSlugs : Array.from(new Set(rules.flatMap((r) => r.channels ?? []))); + // A channel leaves the scan set only when EVERY rule excludes it — one rule's + // exclusion is not the tag's, and the preview has to scan what the build + // would. (ruleApplies gates each rule per video regardless, so this is about + // not reading channels nothing can match, never about correctness.) + const excluded = new Set( + (rules[0]?.channelsExclude ?? []).filter((slug) => + rules.every((r) => (r.channelsExclude ?? []).includes(slug)), + ), + ); + const kept = scoped.filter((s) => !excluded.has(s)); + // An empty list means "every slug" to previewTagRule, so a tag that excluded + // everything must not fall through to a full scan — but it also cannot scan + // nothing, so hand it the scoped set and let ruleApplies reject each video. + const channels = kept.length > 0 ? kept : scoped; const result = previewTagRule(paths, { def: input.def, channels, diff --git a/editor/app/tags/components/EditorTagsClient.tsx b/editor/app/tags/components/EditorTagsClient.tsx @@ -38,7 +38,11 @@ const KINDS: { value: CuratedTagRuleKind; label: string; hint: string }[] = [ { value: "caption", label: "Caption", hint: "the transcript's cue text" }, ]; -type RuleRow = CuratedTagRule & { key: string; channelsText: string }; +type RuleRow = CuratedTagRule & { + key: string; + channelsText: string; + channelsExcludeText: string; +}; type TagRow = { key: string; @@ -71,6 +75,7 @@ function toRow(def: CuratedTagDef): TagRow { ...r, key: mkKey(), channelsText: (r.channels ?? []).join(", "), + channelsExcludeText: (r.channelsExclude ?? []).join(", "), })), }; } @@ -89,16 +94,20 @@ function toDef(row: TagRow): CuratedTagDef { ...(row.rules.length > 0 ? { rules: row.rules.map((r) => { - const channels = r.channelsText - .split(",") - .map((c) => c.trim()) - .filter(Boolean); + const slugs = (text: string) => + text + .split(",") + .map((c) => c.trim()) + .filter(Boolean); + const channels = slugs(r.channelsText); + const channelsExclude = slugs(r.channelsExcludeText); return { id: r.id.trim() || "rule", kind: r.kind, pattern: r.pattern, enabled: r.enabled !== false, ...(channels.length > 0 ? { channels } : {}), + ...(channelsExclude.length > 0 ? { channelsExclude } : {}), ...(r.dateFrom ? { dateFrom: r.dateFrom } : {}), ...(r.dateTo ? { dateTo: r.dateTo } : {}), } satisfies CuratedTagRule; @@ -615,6 +624,21 @@ function TagCard({ className="rounded border border-border bg-card px-1.5 py-1 font-mono text-xs text-foreground" /> </label> + {/* Applied AFTER the allow-list, and it wins: a slug in both is + excluded. "Every channel except that one" is then one rule + with an empty Channels box. */} + <label className="flex min-w-[14rem] flex-1 flex-col gap-1 text-[11px] text-muted-foreground"> + Except (comma-separated; wins over Channels) + <input + value={rule.channelsExcludeText} + onChange={(e) => + patchRule(rule.key, { channelsExcludeText: e.target.value }) + } + list="tag-rule-channels" + aria-label="rule exclude channels" + className="rounded border border-border bg-card px-1.5 py-1 font-mono text-xs text-foreground" + /> + </label> <label className="flex w-28 flex-col gap-1 text-[11px] text-muted-foreground"> From <input @@ -670,6 +694,7 @@ function TagCard({ pattern: "", enabled: true, channelsText: "", + channelsExcludeText: "", }, ], }) diff --git a/editor/e2e/ops-api.spec.ts b/editor/e2e/ops-api.spec.ts @@ -37,6 +37,7 @@ import { resolvePath, writeChannelConfig, writeSettings, + writeSite, } from "./helpers"; const TOKEN = "test-worker-token"; @@ -48,7 +49,8 @@ type OpsResponse = { jobId?: string; queued?: string[]; jobIds?: string[]; - skipped?: { slug: string; reason: string }[]; + skipped?: { slug?: string; siteId?: string; reason: string }[]; + jobs?: { siteId: string; jobId: string }[]; }; async function ops( @@ -714,3 +716,83 @@ test("tag-videos refuses a traversing slug, a bad op and an unknown key", async // Nothing was written by any of the three. expect(await pathExists("test-transcripts/tags.json")).toBe(false); }); + +// --- the two build routes speak the same body --------------------------------- +// +// They did not. build-site took `siteIds` (a list) and build-deploy took +// `siteId` (one), so the two halves of the same runbook sentence needed +// different JSON and the only way to learn which was to get a 400. Both now +// accept both keys; sending BOTH is still refused, because a caller with two +// ideas about what to build should not have one picked for it. +// Job sidecars on disk — the only way to say "and nothing was queued". +async function listJobIds(): Promise<string[]> { + return (await readdir(resolvePath("test-transcripts/.jobs")).catch(() => [])) + .filter((f) => f.endsWith(".meta.json")) + .sort(); +} + +test("build-site and build-deploy each take siteId or siteIds, and refuse both or neither", async ({ + request, +}) => { + await resetData("title-filter-channel"); + await settings(); + + for (const action of ["build-site", "build-deploy"]) { + const both = await ops(request, action, { + siteId: "a", + siteIds: ["a"], + }); + expect(both.status, action).toBe(400); + expect(both.body.error, action).toContain("not both"); + + const neither = await ops(request, action, {}); + expect(neither.status, action).toBe(400); + expect(neither.body.error, action).toContain("siteId"); + + // `all` is still exclusive of either spelling. + const withAll = await ops(request, action, { all: true, siteId: "a" }); + expect(withAll.status, action).toBe(400); + expect(withAll.body.error, action).toContain("not both"); + + // A MALFORMED ID IS A 400 BEFORE ANY JOB STARTS, and that is the point of + // validating in reqSiteIds rather than letting getSite throw: an id list of + // ["good", "BAD"] used to queue the first build, throw on the second and + // answer 500 with no ids at all — a real build running that nothing was + // watching, and a --wait exiting 1 about it. + const before = await listJobIds(); + const bad = await ops(request, action, { siteIds: ["buildsite", "BAD ID"] }); + expect(bad.status, action).toBe(400); + expect(bad.body.error, action).toContain("not a valid site id"); + expect(bad.body.jobs, action).toBeUndefined(); + expect(await listJobIds(), action).toEqual(before); + } +}); + +test("build-site with a bare siteId starts one build-export job", async ({ + request, +}) => { + test.setTimeout(120_000); + await resetData("title-filter-channel"); + await settings(); + await writeSite("buildsite", {}); + + const { status, body } = await ops(request, "build-site", { + siteId: "buildsite", + }); + expect(status).toBe(200); + expect(body.ok).toBe(true); + // The response is the LIST shape whichever key was used — one job, named. + expect(body.jobs?.length).toBe(1); + expect(body.jobs?.[0].siteId).toBe("buildsite"); + expect(body.skipped).toEqual([]); + + const jobId = body.jobs![0].jobId; + await expect + .poll(async () => { + const meta = await readJson<{ kind: string }>( + `test-transcripts/.jobs/${jobId}.meta.json`, + ).catch(() => null); + return meta?.kind ?? null; + }) + .toBe("build-export"); +}); diff --git a/editor/e2e/tags.spec.ts b/editor/e2e/tags.spec.ts @@ -36,7 +36,7 @@ type TagsFile = { label: string; color?: string; hidden?: boolean; - rules?: { id: string; kind: string }[]; + rules?: { id: string; kind: string; channelsExclude?: string[] }[]; }[]; assignments: Record< string, @@ -170,6 +170,46 @@ test("a rule previews against the index, and Pin writes the operator's provenanc ); }); +test("an excluded channel is saved on the rule and drops out of the preview", async ({ + page, +}) => { + await resetData("curated-tags-channel"); + await withIndex(page); + await addTag(page, "synthetic", "Synthetic", { pattern: "small synthetic" }); + + await page.goto("/tags"); + const card = page.getByTestId("tag-card").first(); + await card.getByRole("button", { name: "preview synthetic" }).click(); + await expect(page.getByTestId("preview-row")).toHaveCount(1); + + // Excluding the only channel in the fixture is the whole assertion: the + // preview has to predict the build, and the build will not fire this rule + // there any more. + const rule = card.getByTestId("tag-rule").first(); + await rule.getByLabel("rule exclude channels").fill(SLUG); + await card.getByRole("button", { name: "preview synthetic" }).click(); + await expect(page.getByTestId("preview-row")).toHaveCount(0); + await expect(page.getByTestId("tag-preview")).toContainText("0 matches"); + + await page.getByRole("button", { name: "save tags" }).click(); + await expect(page.getByTestId("tags-saved")).toBeVisible(); + expect((await tagsFile()).tags[0].rules?.[0].channelsExclude).toEqual([SLUG]); + + // Cleared, it leaves no key behind — a rule nobody scoped round-trips as it + // was written. + await page.goto("/tags"); + await page + .getByTestId("tag-rule") + .first() + .getByLabel("rule exclude channels") + .fill(""); + await page.getByRole("button", { name: "save tags" }).click(); + await expect(page.getByTestId("tags-saved")).toBeVisible(); + expect((await tagsFile()).tags[0].rules?.[0]).not.toHaveProperty( + "channelsExclude", + ); +}); + // THE COMPRESSION REGRESSION, from both ends. // // buildIndex writes the index compressed; a reader that opens it without diff --git a/editor/scripts/measure-nav.mjs b/editor/scripts/measure-nav.mjs @@ -36,7 +36,7 @@ const BASE = argOf("base", "http://localhost:3001").replace(/\/$/, ""); const RUNS = Number(argOf("runs", "3")); // Routes worth timing, with the segment name used to build a state tree. THE -// TWELVE SINGLE-SEGMENT PAGES IN THE SIDEBAR (editor/app/lib/nav.ts), and +// THIRTEEN SINGLE-SEGMENT PAGES IN THE SIDEBAR (editor/app/lib/nav.ts), and // only those: stateTree() below encodes ONE segment, so /sites/<id>/charts // and /operations/<id> cannot be listed — each would need the nested // [segment, {children: [param, …]}] form, a different tree per route. A diff --git a/export/app/ask/AskHub.tsx b/export/app/ask/AskHub.tsx @@ -5,10 +5,14 @@ // across every shelved archive. Mirrors HubHome's MultiSiteDataProvider wiring. import { useMemo } from "react"; +import { PlayerProvider } from "yt-dlp-transcript-common/components/PlayerProvider"; +import TranscriptModal from "yt-dlp-transcript-common/components/TranscriptModal"; +import PostModal from "yt-dlp-transcript-common/components/PostModal"; import { MultiSiteDataProvider, type FederatedSite, } from "yt-dlp-transcript-common/components/SearchDataContext"; +import { SearchSessionProvider } from "yt-dlp-transcript-common/components/SearchSessionContext"; import { useRegistry } from "yt-dlp-transcript-common/components/siteRegistry"; import AskChat from "./AskChat"; @@ -24,9 +28,28 @@ export default function AskHub() { [sites], ); + // The whole provider stack, in the order SiteWorkspace mounts it for a single + // site — PlayerProvider, then the data source, then the session — because + // hub mode bypasses that shell entirely and /ask is on its own here. + // + // None of these are optional: AskChat's retrieval reads the committed query + // tree, the filters and runQueryTree out of useSearchSession, and the session + // itself calls usePlayer (it opens a transcript at a cited timestamp). A + // missing provider THROWS, so the hub build's prerender of /ask failed and + // took the whole route with it — which is why e2e:2origin was red. The fix is + // the stack, never an opt-out of prerendering. return ( - <MultiSiteDataProvider sites={federated}> - <AskChat /> - </MultiSiteDataProvider> + <PlayerProvider> + <MultiSiteDataProvider sites={federated}> + <SearchSessionProvider> + <AskChat /> + </SearchSessionProvider> + </MultiSiteDataProvider> + {/* The viewers a citation opens into, siblings of the session exactly as + HubHome and SiteWorkspace mount them — a cited link with nothing to + open is the failure this avoids. */} + <TranscriptModal /> + <PostModal /> + </PlayerProvider> ); } diff --git a/export/e2e-hub/ask.spec.ts b/export/e2e-hub/ask.spec.ts @@ -0,0 +1,55 @@ +import { expect, test, type Page } from "@playwright/test"; + +// The hub's /ask route. +// +// Hub mode bypasses the workspace shell that mounts the search session for a +// single site, so AskHub has to supply its own — AskChat's retrieval reads the +// committed query tree and filters out of it. When it did not, `next build` +// with INSTANCE_MODE=hub threw while prerendering /ask ("useSearchSession must +// be used within a SearchSessionProvider") and took the whole route with it. +// This spec is the cheap guard on the rendered page; the build itself is the +// other half, covered by the 2-origin suite which builds the hub for real. + +// No built-in pool: this route has to stand up on a hub with an empty shelf, +// which is what a fresh hub is. +async function stubBuiltins(page: Page) { + await page.route("**/hub-sites.json", (r) => + r.fulfill({ + status: 200, + contentType: "application/json", + headers: { "access-control-allow-origin": "*" }, + body: "[]", + }), + ); +} + +test.describe("hub /ask", () => { + test("renders the chat with its composer", async ({ page }) => { + const errors: string[] = []; + page.on("pageerror", (e) => errors.push(String(e))); + + await stubBuiltins(page); + await page.goto("/ask"); + + // The route's own header — hub mode says "the federation", not + // "the transcripts". + await expect( + page.getByRole("heading", { name: /Ask a question about the federation/ }), + ).toBeVisible(); + + // The composer is AskChat's whole point, and it only renders once the + // provider stack AskHub now mounts is there. Located by its placeholder, + // the way ask-chat.spec.ts does it — the first textbox on the page is the + // provider panel's API-key field, not this. + const box = page.getByPlaceholder( + /Ask about the transcripts|Loading transcripts/, + ); + await expect(box).toBeVisible(); + await expect( + page.getByRole("button", { name: "Ask", exact: true }), + ).toBeVisible(); + + // A missing provider surfaces as a client-side throw, not a blank page. + expect(errors).toEqual([]); + }); +}); diff --git a/export/e2e/inline-channel-chips.spec.ts b/export/e2e/inline-channel-chips.spec.ts @@ -116,6 +116,17 @@ async function installRoutes(page: Page) { generatedAt: new Date().toISOString(), }); }); + // No curated tags: this fixture is about channel chips, and the tag groups + // are <details> too — a real /tags.json from the dev server's public/ would + // put open tag disclosures inside the same "details details" the + // expand-nothing assertions below count. + await page.route("**/tags.json", async (route) => { + await route.fulfill({ + status: 404, + contentType: "application/json", + body: "{}", + }); + }); } async function waitForHydration(page: Page) { diff --git a/export/e2e/query-tree.spec.ts b/export/e2e/query-tree.spec.ts @@ -208,6 +208,42 @@ test.describe("composite search — query tree", () => { await expectResultSlugs(page, [CHAT_LARGE_SLUG]); }); + test("a leaf section is labelled with its own scope", async ({ page }) => { + // The section bar over a card's hits used to be a four-way ternary that + // predated the description and tags scopes, so hits from either were + // filed under "Transcripts" — the one place they demonstrably did not + // come from. It reads the builder's own table now, which is also why + // `tags` says "Keywords": the curated vocabulary is what "Tags" means. + const tree: SGroup = { + k: "g", + o: "OR", + c: [ + { k: "l", q: "zebra", s: "description" }, + { k: "l", q: "gaming", s: "tags" }, + ], + }; + await page.goto(`/?qt=${qt(tree)}`); + await expectResultSlugs(page, [TRANSCRIPT_ONLY_SLUG, CHAT_LARGE_SLUG]); + + await expect( + page.locator( + `[data-result-slug="${TRANSCRIPT_ONLY_SLUG}"] [data-leaf-section]`, + ), + ).toContainText("Description"); + await expect( + page.locator( + `[data-result-slug="${CHAT_LARGE_SLUG}"] [data-leaf-section]`, + ), + ).toContainText("Keywords"); + // Neither says Transcripts, which is what the old chain said for both. + // textContent, not innerText: the bar is CSS-uppercased. + const sections = page.locator("[data-leaf-section]"); + await expect(sections).toHaveCount(2); + expect((await sections.allTextContents()).join(" ")).not.toContain( + "Transcripts", + ); + }); + test("legacy URL: ?q=alpha auto-migrates to a single-leaf transcripts query", async ({ page, }) => { diff --git a/export/e2e/tag-chips.spec.ts b/export/e2e/tag-chips.spec.ts @@ -50,6 +50,21 @@ function chip(page: Page, id: string) { return page.locator(`[data-testid="tag-chip"][data-tag-id="${id}"]`); } +function tagGroup(page: Page, groupId: string) { + return page.locator( + `[data-testid="tag-chip-group"][data-group-id="${groupId}"]`, + ); +} + +// Fold/unfold by clicking the group label in its summary — the same gesture +// the channel group chips take (channel-group-chips.spec.ts). +async function collapseTagGroup(page: Page, groupId: string, label: string) { + await tagGroup(page, groupId) + .locator("summary") + .getByText(label, { exact: true }) + .click(); +} + function card(page: Page, id: string) { return page.locator(`[data-result-slug="${slugOf(id)}"]`); } @@ -90,6 +105,44 @@ test.describe("curated tag chips", () => { await expect(chip(page, TAG_COLLAB)).toHaveAttribute("aria-pressed", "false"); }); + test("a tag group collapses and its selection count survives on the summary", async ({ + page, + }) => { + // A site can publish several groups of several chips each, and on a + // phone that row is the whole viewport before a single result. Each + // group is a <details>, open by default — folding one away must not + // hide that it is still narrowing the list, so the count moves to the + // summary. + const group = tagGroup(page, "eva"); + await expect(group).toHaveAttribute("open", ""); + await expect(chip(page, TAG_COLLAB)).toBeVisible(); + // Nothing selected: no count on the summary, not a zero. + await expect(group.getByTestId("tag-group-selected")).toHaveCount(0); + + await chip(page, TAG_COLLAB).click(); + await expect(group.getByTestId("tag-group-selected")).toHaveText( + "1 selected", + ); + + await collapseTagGroup(page, "eva", "Eva"); + await expect(group).not.toHaveAttribute("open"); + await expect(chip(page, TAG_COLLAB)).toBeHidden(); + // Still visible, still saying what it is doing to the results. + await expect(group.getByTestId("tag-group-selected")).toHaveText( + "1 selected", + ); + + // ...and the selection survives the fold, chip state included. + await collapseTagGroup(page, "eva", "Eva"); + await expect(group).toHaveAttribute("open", ""); + await expect(chip(page, TAG_COLLAB)).toHaveAttribute( + "aria-pressed", + "true", + ); + await apply(page); + await expect(card(page, VIDEO_TRANSCRIPT_ONLY)).toHaveCount(0); + }); + test("a tagged card shows its tags; an untagged one shows none", async ({ page, }) => { @@ -293,6 +346,27 @@ test.describe("curated tag chips", () => { await expect(trigger).toContainText("1"); await expect(card(page, VIDEO_TRANSCRIPT_ONLY)).toHaveCount(0); }); + + test("a group folds away inside the sheet", async ({ page }) => { + // 390px is where a multi-group row costs the most, so the fold has to + // work in the sheet and not only in the inline panel. + await installRoutes(page); + await installTagRoutes(page); + await page.goto("/"); + await waitForHydration(page); + + await page.getByTestId("filters-trigger").click(); + const group = tagGroup(page, "eva"); + await expect(chip(page, TAG_COLLAB)).toBeVisible(); + + await collapseTagGroup(page, "eva", "Eva"); + await expect(group).not.toHaveAttribute("open"); + await expect(chip(page, TAG_COLLAB)).toBeHidden(); + // The summary is still one line inside the sheet, not a torn row. + const box = await group.locator("summary").boundingBox(); + expect(box).not.toBeNull(); + expect(box!.width).toBeLessThanOrEqual(390); + }); }); test.describe("on a site that publishes none", () => { diff --git a/mcp/src/protocol.test.ts b/mcp/src/protocol.test.ts @@ -259,6 +259,7 @@ test("stdio: a site with no /tags.json says so instead of returning nothing", as }), ); assert.match(search, /No matches for "coffee"/); + assert.match(search, /posts: skipped — a tag filter was given/); assert.match(search, /publishes no \/tags\.json/); assert.match(search, /not evidence of absence/); }); @@ -309,6 +310,14 @@ test("stdio: enumerate_matches filters by tag and names the filter", async (t) = assert.match(tagged, /filters — tags: eva-collab/); // …and the complete-set line is still the honest one. assert.match(tagged, /complete set: yes/); + // The posts corpus is not part of that count and the footer says why: a post + // carries no curated tags, so a tag filter drops the corpus whole. Without + // the sentence "no post matches" and "posts were never searched" read the + // same on the wire. + assert.match( + tagged, + /posts: skipped — a tag filter was given and posts carry no curated tags \(the export UI does the same\)/, + ); }); test("stdio: prompts are served on both eras", async (t) => { diff --git a/mcp/src/search.test.ts b/mcp/src/search.test.ts @@ -1117,6 +1117,51 @@ test("posts: a posts-scope spec leaf matches post text", async () => { assert.deepEqual(r.hits.map((h) => h.videoId).sort(), ["p1", "p2"]); }); +test("posts: a tag filter takes the posts corpus out of the search", async () => { + // Curated tags live on video records, so every post fails a tag filter. + // Scanning them to drop them all costs a manifest probe and a shard read per + // posting channel — and counting them before the drop is the bug the export + // viewer had (tag-chips.spec.ts). The corpus is skipped, and the result says + // so rather than leaving a caller to read "0 posts" as "searched, unmatched". + const src = new StubSource(); + const r = await searchTranscripts(src, { + query: "zephyrpost", + filters: { ...KEEP_ALL, curatedTags: ["eva-collab"] }, + }); + assert.equal(r.total, 0); + assert.equal(r.postsScanned.skippedForTagFilter, true); + assert.equal(r.postsScanned.requested, false); + assert.equal(r.postsScanned.channels, 0, "no manifest was even probed"); +}); + +test("posts: without a tag filter the same query still searches them", async () => { + // The control: posts are silenced BY the tag filter, never by this change. + const src = new StubSource(); + const r = await searchTranscripts(src, { + query: "zephyrpost", + filters: { ...KEEP_ALL }, + }); + assert.equal(r.total, 2); + assert.equal(r.postsScanned.skippedForTagFilter, false); + assert.equal(r.postsScanned.requested, true); +}); + +test("posts: a posts-scope spec leaf under a tag filter matches nothing", async () => { + // The spec path is the other half: an explicit posts leaf carries its own + // slug set, so the default-scope exclusion above does not cover it. + const src = new StubSource(); + const root = newGroup({ + op: "AND", + children: [newLeaf({ id: "l1", query: "zephyrpost", scope: "posts" })], + }); + const channels = await src.listChannels(); + const r = await runSearchSpec(src, channels, { + tree: root, + filters: { ...KEEP_ALL, curatedTags: ["eva-collab"] }, + }); + assert.equal(r.total, 0); +}); + test("posts: findPost resolves by id and getThread returns the whole thread", async () => { const src = new StubSource(); const found = await findPost(src, "p2"); diff --git a/mcp/src/search.ts b/mcp/src/search.ts @@ -234,7 +234,16 @@ export type SearchResult = { // across 0 channel(s)" — indistinguishable from "searched everything, found // nothing". `channels` counts the channels that actually HAVE a posts index, // so 0 with `requested` true means the post corpus is empty here. - postsScanned: { requested: boolean; channels: number; pages: number }; + // `skippedForTagFilter` is the one way `requested: false` is worth saying out + // loud: a post carries no curated tags, so a tag filter drops the whole posts + // corpus rather than reporting it as searched-and-unmatched. The export UI + // makes the same call (SearchSessionContext's globalScopeSlugs). + postsScanned: { + requested: boolean; + channels: number; + pages: number; + skippedForTagFilter: boolean; + }; // What duplicate collapsing did to the count. `available: false` means this // corpus ships no duplicates.json, so no claim about mirrors can be made // either way — distinct from "checked, found none". @@ -541,8 +550,18 @@ export async function searchTranscripts( // content_types:["video"] must not be widened by a posts scope. const wantVideos = contentTypes.includes("video") && (!scopes || scopes.some((s) => s !== "posts")); - const wantPosts = + const postsAsked = contentTypes.includes("post") && (!scopes || scopes.includes("posts")); + // A tag filter takes the posts corpus out of the search entirely: curated + // tags live on VIDEO records (lib/curatedTags.ts), so every post would fail + // the filter, and scanning them to drop them all is a manifest probe and a + // shard read per posting channel spent on a foregone conclusion. Counting + // them as hits would be worse: the export viewer hit exactly that bug (see + // tag-chips.spec.ts) — rows dropped by the display fold that the header had + // already counted. + const postsSkippedForTagFilter = + postsAsked && (opts.filters?.curatedTags?.length ?? 0) > 0; + const wantPosts = postsAsked && !postsSkippedForTagFilter; // Counted apart from the video pass so "no posts index anywhere in scope" is // distinguishable from "searched the posts and found nothing". let postChannelsScanned = 0; @@ -774,6 +793,7 @@ export async function searchTranscripts( requested: wantPosts, channels: postChannelsScanned, pages: postPagesScanned, + skippedForTagFilter: postsSkippedForTagFilter, }, duplicates, truncated, @@ -1131,7 +1151,11 @@ export async function runSearchSpec( // Only run when the tree actually has a posts leaf: a video-only spec must // not pay a manifest probe per channel. Post records reuse the same evalNode // with `postText` set and no cues, so AND/OR/negate semantics are identical. - const wantsPosts = [...matchers.values()].some((m) => m.scope === "posts"); + // …and the same exclusion as the plain path: a posts leaf under a tag filter + // can only ever match nothing, because a post carries no curated tags. + const wantsPosts = + [...matchers.values()].some((m) => m.scope === "posts") && + (filters?.curatedTags?.length ?? 0) === 0; if (wantsPosts) { postsOuter: for (const ch of channels) { let pm; diff --git a/mcp/src/server.ts b/mcp/src/server.ts @@ -1366,6 +1366,14 @@ function incompletePageBanner( // `scanned 0 page(s) across 0 channel(s)`, which reads like nothing ran. function describePostsPass(result: SearchResult): string { const p = result.postsScanned; + // Said out loud, because the alternative is a caller concluding from silence + // that the posts were searched and matched nothing. + if (p.skippedForTagFilter) { + return ( + "posts: skipped — a tag filter was given and posts carry no curated " + + "tags (the export UI does the same)" + ); + } if (!p.requested) return ""; if (p.channels === 0) { return ( diff --git a/scripts/archilyzer-ops.mjs b/scripts/archilyzer-ops.mjs @@ -8,7 +8,8 @@ // // USAGE // -// pnpm ops <action> [--json '<body>' | --file <path>] [--wait] [--quiet] +// pnpm ops <action> [--json '<body>' | --file <path>] [--wait] +// [--wait-timeout <seconds>] [--quiet] // pnpm ops get channel <slug> [--counts] // pnpm ops get tags [<tagId>] // pnpm ops list @@ -30,6 +31,8 @@ // pnpm ops lane --json '{"lane":"download","held":true}' // pnpm ops refresh-report --json '{"all":true}' // pnpm ops relocate --json '{"slugs":["x"],"locationId":"platter"}' +// pnpm ops build-site --json '{"siteId":"anilyzer"}' --wait +// pnpm ops build-deploy --json '{"siteIds":["anilyzer","jeralyzer"]}' --wait // pnpm ops get channel the-quartering // pnpm ops tags --json '{"op":"define","tag":{"id":"eva-collab","label":"Collab"}}' // pnpm ops tag-videos --file ids.json @@ -42,15 +45,24 @@ // --wait follows /api/jobs/<jobId>/log to the end for a job-starting action and // exits 0 only if the job finished `done`. Without it the command returns as // soon as the job is QUEUED, which is the honest answer: the queue may hold it -// behind other work for hours. +// behind other work for hours. A poll that fails does NOT end the follow — see +// followJob — and --wait-timeout <seconds> is there for a caller that cannot +// wait indefinitely. // // The response JSON is printed verbatim on stdout (log lines from --wait go to // stderr), so `pnpm ops … | jq` works. import { readFile } from "node:fs/promises"; +import { pathToFileURL } from "node:url"; const DEFAULT_URL = "http://localhost:3001"; +// Consecutive polls where NEITHER the job's log NOR the active list answered, +// after which --wait gives up. At the 30 s backoff ceiling that is ~5 minutes +// of an editor saying nothing at all, which is not a busy server — it is a +// server that is gone. +const MAX_PROBE_FAILURES = 10; + // The read-side routes, reachable as `get <noun> <arg>`. Kept tiny and explicit: // an ops API that let a caller assemble arbitrary GET paths would be a proxy, // not an adapter. @@ -104,10 +116,32 @@ export function parseArgs(argv) { let wait = false; let quiet = false; let counts = false; + let waitTimeout = null; + const readTimeout = (raw) => { + const n = Number(raw); + if (!Number.isFinite(n) || n <= 0) { + return { error: "--wait-timeout needs a positive number of seconds" }; + } + waitTimeout = n; + // A timeout on a wait nobody asked for is not a preference, it is a typo + // with no effect — so it IMPLIES --wait rather than being ignored. + wait = true; + return null; + }; for (let i = 0; i < argv.length; i++) { const arg = argv[i]; if (arg === "--wait") { wait = true; + } else if (arg === "--wait-timeout") { + const raw = argv[++i]; + if (raw === undefined) { + return { error: "--wait-timeout needs a number of seconds" }; + } + const err = readTimeout(raw); + if (err) return err; + } else if (arg.startsWith("--wait-timeout=")) { + const err = readTimeout(arg.slice("--wait-timeout=".length)); + if (err) return err; } else if (arg === "--quiet") { quiet = true; } else if (arg === "--counts") { @@ -167,6 +201,7 @@ export function parseArgs(argv) { path: GETTERS[noun](positional[2], counts), wait: false, quiet, + waitTimeout, }; } const action = positional[0]; @@ -194,18 +229,28 @@ export function parseArgs(argv) { ...(action === "tag-videos" ? { defaultSource: agentSource() } : {}), wait, quiet, + waitTimeout, }; } export function usage() { return [ "Usage: pnpm ops <action> [--json '<body>' | --file <path>] [--wait]", + " [--wait-timeout <seconds>] [--quiet]", " pnpm ops get channel <slug> [--counts]", " pnpm ops get tags [<tagId>]", " pnpm ops list", "", `Actions: ${ACTIONS.join(", ")}`, "", + "--wait follows the job's log and survives a poll that fails (a busy", + " in-process build starves the server): it backs off and, after three", + " failures, asks /api/jobs/active whether the job is still there.", + "--wait-timeout <seconds> gives up and exits 1 instead of waiting forever.", + " Default: no timeout — the queue may legitimately hold a job for hours.", + "", + 'build-site and build-deploy both take "siteId" (one) or "siteIds" (a list).', + "", "Env: ARCHILYZER_EDITOR_URL (default http://localhost:3001), WORKER_TOKEN,", " ARCHILYZER_AGENT (provenance of a tag write; default \"cli\")", ].join("\n"); @@ -229,19 +274,127 @@ function authHeaders() { // the browser polls it same-origin with no token. Sending one here implied a // gate that does not exist, which is worse than sending nothing: the next // person to read this would conclude the endpoint was protected. -async function followJob(jobId, quiet) { +// +// A POLL FAILURE IS NOT A JOB FAILURE, and this used to treat them as the same +// thing. The editor is single-process: a busy in-process build-index starves +// the event loop for long enough that `fetch` rejects outright, and one +// rejection ended the follow with a stack trace about a job that was running +// fine and went on to finish. So every poll is caught, the wait backs off +// 1→2→4…→30 s instead of hammering a server that is already struggling, and +// `from` is kept across the failure so not one line of log text is lost. +// +// After three consecutive failures the endpoint is no longer trusted to answer +// at all, and the question becomes a different one — IS THE JOB STILL THERE? +// `/api/jobs/active` is cheap and is what the editor's own head polls. Listed +// ⇒ keep waiting, however long that takes. Absent ⇒ it ended while we could not +// see it, so one last log poll reads the terminal status; if even that fails, +// the follow gives up rather than claiming an outcome it never read. +// +// `--wait-timeout` bounds the whole thing for a caller that cannot hang (CI, +// an agent). Default none, because the honest default for a queue that may hold +// a job behind hours of other work is to wait. +export async function followJob(jobId, quiet, opts = {}) { + const doFetch = opts.fetch ?? fetch; + const sleep = opts.sleep ?? ((ms) => new Promise((r) => setTimeout(r, ms))); + const now = opts.now ?? (() => Date.now()); + const deadline = + opts.timeoutSeconds > 0 ? now() + opts.timeoutSeconds * 1000 : null; + const base = opts.baseUrl ?? baseUrl(); + const id = encodeURIComponent(jobId); + let from = 0; + let failures = 0; + let probeFailures = 0; + let backoff = 1000; + + // One log poll. Returns the status, or null when the poll itself failed — + // never throws, so a transient fetch rejection cannot end the follow. + const pollLog = async () => { + try { + const res = await doFetch(`${base}/api/jobs/${id}/log?from=${from}`); + if (!res.ok) return null; + const payload = await res.json(); + if (payload.content && !quiet) process.stderr.write(payload.content); + // Only advance once the chunk is in hand: a poll that failed halfway + // must re-ask for the same offset. + from = payload.nextOffset ?? from; + return payload.status ?? null; + } catch { + return null; + } + }; + + const stillListed = async () => { + try { + const res = await doFetch(`${base}/api/jobs/active`); + if (!res.ok) return null; + const payload = await res.json(); + const jobs = Array.isArray(payload.jobs) ? payload.jobs : []; + return jobs.some((j) => j && j.id === jobId); + } catch { + return null; + } + }; + + // "queued" and "running" are the two NON-answers. Everything else is the job + // having ended, which is the only thing worth returning. + const terminal = (s) => s !== null && s !== "queued" && s !== "running"; + for (;;) { - const res = await fetch( - `${baseUrl()}/api/jobs/${encodeURIComponent(jobId)}/log?from=${from}`, - ); - if (!res.ok) throw new Error(`log poll failed: HTTP ${res.status}`); - const payload = await res.json(); - if (payload.content && !quiet) process.stderr.write(payload.content); - from = payload.nextOffset ?? from; - const status = payload.status; - if (status !== "queued" && status !== "running") return status; - await new Promise((r) => setTimeout(r, 1000)); + const status = await pollLog(); + if (status !== null) { + failures = 0; + probeFailures = 0; + backoff = 1000; + if (terminal(status)) return status; + } else { + failures++; + if (failures >= 3) { + const listed = await stillListed(); + if (listed === false) { + // Gone from the active list: it went terminal while the log endpoint + // was unreachable. One more try at the status it ended with — and + // ONLY a terminal one is an outcome. A log endpoint that came back + // answering "running" means the active list was stale, not that the + // job finished; returning that printed "running" as the result and + // exited 1 for a job that was fine. + const final = await pollLog(); + if (terminal(final)) return final; + if (final === null) { + throw new Error( + `lost contact with job ${jobId}: it is no longer active and its log could not be read`, + ); + } + // Readable again and still going: back to waiting, from scratch. + failures = 0; + probeFailures = 0; + backoff = 1000; + } else if (listed === true) { + // A job we can still see is a job to wait for. + failures = 0; + probeFailures = 0; + backoff = 1000; + } else { + // The probe failed too, so we now know nothing at all. Without a + // bound this waits forever on an editor that has gone away; with one + // it says so. Only CONSECUTIVE failures count — a single answer of + // either kind resets it. + probeFailures++; + if (probeFailures >= MAX_PROBE_FAILURES) { + throw new Error( + `lost contact with the editor at ${base}: ${MAX_PROBE_FAILURES} consecutive failed polls of job ${jobId} and of /api/jobs/active`, + ); + } + } + } + backoff = Math.min(backoff * 2, 30_000); + } + if (deadline !== null && now() >= deadline) { + throw new Error( + `--wait-timeout: gave up after ${opts.timeoutSeconds}s waiting for job ${jobId}; it is still running and can be followed on /jobs`, + ); + } + await sleep(status !== null ? 1000 : backoff); } } @@ -325,7 +478,9 @@ async function main() { } let worst = 0; for (const jobId of jobIds) { - const status = await followJob(jobId, parsed.quiet); + const status = await followJob(jobId, parsed.quiet, { + timeoutSeconds: parsed.waitTimeout ?? 0, + }); console.error(`[${jobId}] ${status}`); if (status !== "done") worst = 1; } @@ -333,7 +488,7 @@ async function main() { } // Importable for the arg-parsing tests; only the CLI entry point runs main(). -if (process.argv[1] && import.meta.url === `file://${process.argv[1]}`) { +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { main().then( (code) => process.exit(code), (e) => { diff --git a/scripts/archilyzer-ops.test.mjs b/scripts/archilyzer-ops.test.mjs @@ -5,7 +5,7 @@ // Run with: pnpm test:scripts import assert from "node:assert/strict"; import test from "node:test"; -import { parseArgs, usage } from "./archilyzer-ops.mjs"; +import { followJob, parseArgs, usage } from "./archilyzer-ops.mjs"; test("no arguments prints usage", () => { assert.equal(parseArgs([]).help, true); @@ -118,3 +118,138 @@ test("get tags reads the whole vocabulary, or one tag's assignments", () => { // The optional argument is per-noun: `get channel` still demands a slug. assert.match(parseArgs(["get", "channel"]).error, /needs an argument/); }); + +// ─── --wait, and the poll that fails ─── + +test("--wait-timeout is parsed, and refuses a non-number", () => { + assert.equal(parseArgs(["build-index", "--wait", "--wait-timeout", "90"]).waitTimeout, 90); + assert.equal(parseArgs(["build-index", "--wait-timeout=90"]).waitTimeout, 90); + // Default is NONE: the queue may legitimately hold a job for hours. + assert.equal(parseArgs(["build-index", "--wait"]).waitTimeout, null); + assert.match(parseArgs(["build-index", "--wait-timeout"]).error, /needs a number/); + assert.match(parseArgs(["build-index", "--wait-timeout", "soon"]).error, /positive number/); + assert.match(parseArgs(["build-index", "--wait-timeout", "0"]).error, /positive number/); + assert.match(usage(), /--wait-timeout/); +}); + +// A fake editor. `log` is the sequence of log-poll outcomes — an Error is +// thrown at the caller the way a starved server makes `fetch` reject. +function fakeEditor({ log = [], active = [] } = {}) { + const calls = { log: 0, active: 0 }; + const reply = (body) => ({ ok: true, json: async () => body }); + const doFetch = async (url) => { + if (url.includes("/api/jobs/active")) { + const next = active[Math.min(calls.active, active.length - 1)]; + calls.active++; + if (next instanceof Error) throw next; + return reply({ jobs: next }); + } + const next = log[Math.min(calls.log, log.length - 1)]; + calls.log++; + if (next instanceof Error) throw next; + return reply(next); + }; + return { doFetch, calls }; +} + +const follow = (jobId, editor, opts = {}) => + followJob(jobId, true, { + fetch: editor.doFetch, + sleep: async () => {}, + baseUrl: "http://editor", + ...opts, + }); + +test("a poll that rejects does not end the follow", async () => { + // THE BUG THIS FIXES: one `fetch failed` while an in-process build-index + // starved the server used to abort --wait and report a job that was fine. + const editor = fakeEditor({ + log: [ + { content: "a", nextOffset: 1, status: "running" }, + new Error("fetch failed"), + new Error("fetch failed"), + { content: "b", nextOffset: 2, status: "done" }, + ], + }); + assert.equal(await follow("j1", editor), "done"); + // Two failures is below the probe threshold, so it never asked. + assert.equal(editor.calls.active, 0); +}); + +test("after three failures it asks whether the job is still there", async () => { + const boom = new Error("fetch failed"); + // Still listed ⇒ keep waiting, and the follow ends on the status it finally + // reads rather than on a guess. + const waiting = fakeEditor({ + log: [boom, boom, boom, { content: "", nextOffset: 0, status: "done" }], + active: [[{ id: "j1" }]], + }); + assert.equal(await follow("j1", waiting), "done"); + assert.equal(waiting.calls.active, 1); + + // Absent from the active list ⇒ it ended while the log was unreachable, so + // one final poll reads the terminal status. `failed` is reported, not hidden. + const gone = fakeEditor({ + log: [boom, boom, boom, { content: "", nextOffset: 0, status: "failed" }], + active: [[]], + }); + assert.equal(await follow("j1", gone), "failed"); +}); + +test("a job that is gone AND unreadable refuses to claim an outcome", async () => { + const boom = new Error("fetch failed"); + const editor = fakeEditor({ log: [boom], active: [[]] }); + await assert.rejects(follow("j1", editor), /lost contact with job j1/); +}); + +test("--wait-timeout gives up on a job that never ends", async () => { + const editor = fakeEditor({ + log: [{ content: "", nextOffset: 0, status: "running" }], + }); + let clock = 0; + await assert.rejects( + follow("j1", editor, { timeoutSeconds: 5, now: () => (clock += 3000) }), + /--wait-timeout: gave up after 5s/, + ); +}); + +// The two build routes used to disagree about the spelling of their one +// argument, so the usage text is where a reader finds out they no longer do. +test("usage says both build routes take siteId or siteIds", () => { + assert.match(usage(), /build-site and build-deploy both take "siteId".*"siteIds"/); +}); + +test("a recovered log endpoint answering 'running' is not an outcome", async () => { + // THE BUG: the absent-from-active branch returned whatever the final poll + // said. A stale active list plus a recovered log endpoint therefore printed + // "running" as the job's result and exited 1 for a job that was fine. + const boom = new Error("fetch failed"); + const editor = fakeEditor({ + log: [ + boom, + boom, + boom, + { content: "", nextOffset: 0, status: "running" }, + { content: "", nextOffset: 0, status: "done" }, + ], + active: [[]], + }); + assert.equal(await follow("j1", editor), "done"); +}); + +test("an editor that answers nothing at all is given up on, not waited on", async () => { + // Without --wait-timeout the null-probe path used to wait forever: the log + // never answers AND the active list never answers, so nothing ever resets + // the failure count and nothing ever concludes. + const boom = new Error("fetch failed"); + const editor = fakeEditor({ log: [boom], active: [boom] }); + await assert.rejects(follow("j1", editor), /lost contact with the editor/); +}); + +test("--wait-timeout implies --wait", () => { + // A timeout on a wait nobody asked for is a typo with no effect, not a + // preference to honour silently. + const p = parseArgs(["build-index", "--wait-timeout", "30"]); + assert.equal(p.wait, true); + assert.equal(p.waitTimeout, 30); +}); diff --git a/scripts/worktree.mjs b/scripts/worktree.mjs @@ -9,6 +9,7 @@ import { execFileSync, spawn } from "node:child_process"; import fs from "node:fs"; import path from "node:path"; +import { pathToFileURL } from "node:url"; // Base ports (offset 0 == main worktree). Mirrors the hardcoded defaults in // editor/export package.json scripts and the Playwright configs. @@ -67,6 +68,21 @@ function mainRoot(trees = listWorktrees()) { return trees.length ? trees[0].path : git(["rev-parse", "--show-toplevel"]); } +// WHERE A WORKTREE LIVES, asked the same way by `add` and by `rm`. +// +// A branch name may contain "/" and a sibling directory name may not, so `add` +// has always flattened `tags/site` to `tags-site`. `rm` did not, so +// `pnpm wt rm tags/site` pointed git at a path nothing had ever created — and +// that string is exactly what somebody who just ran `add` will type. One +// function, both callers, so the two spellings cannot drift apart again. +// +// An absolute path passes through untouched: `rm` accepts one (it is what +// `list` prints), and sanitizing it would eat its separators. +export function worktreeDirFor(main, name) { + if (path.isAbsolute(name)) return name; + return path.join(path.dirname(main), name.replace(/[^A-Za-z0-9._-]/g, "-")); +} + function offsetForIndex(index) { return index * OFFSET_STEP; } @@ -211,8 +227,7 @@ function cmdAdd(args) { } const main = mainRoot(); - const name = branch.replace(/[^A-Za-z0-9._-]/g, "-"); - const dir = path.join(path.dirname(main), name); + const dir = worktreeDirFor(main, branch); if (branchExists(branch)) { git(["worktree", "add", dir, branch], { stdio: "inherit" }); @@ -280,7 +295,9 @@ function cmdRm(args) { return 1; } const main = mainRoot(); - const dir = path.isAbsolute(name) ? name : path.join(path.dirname(main), name); + // Sanitized exactly as `add` sanitized it, so `rm <the branch you added>` + // finds the directory `add` actually made. + const dir = worktreeDirFor(main, name); // --share-data worktrees always carry an untracked .worktree-env, so git // refuses a plain remove; --force handles that (and any other local files). const rmArgs = ["worktree", "remove", ...(force ? ["--force"] : []), dir]; @@ -328,9 +345,13 @@ async function main() { } } -main() - .then((code) => process.exit(code ?? 0)) - .catch((err) => { - process.stderr.write(`${err?.stack ?? err}\n`); - process.exit(1); - }); +// Importable for the unit tests (worktreeDirFor is pure and needs no repo); +// only the CLI entry point runs main(), which shells out to git. +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + main() + .then((code) => process.exit(code ?? 0)) + .catch((err) => { + process.stderr.write(`${err?.stack ?? err}\n`); + process.exit(1); + }); +} diff --git a/scripts/worktree.test.mjs b/scripts/worktree.test.mjs @@ -0,0 +1,46 @@ +// Where `pnpm wt` puts a worktree — the one thing `add` and `rm` have to agree +// about. Pure: no repo, no git, no filesystem. +// +// Run with: pnpm test:scripts +import assert from "node:assert/strict"; +import test from "node:test"; +import path from "node:path"; +import { worktreeDirFor } from "./worktree.mjs"; + +const MAIN = "/home/u/Projects/yt-dlp-transcript-browser"; +const SIBLING = path.dirname(MAIN); + +test("a branch with a slash becomes one sibling directory", () => { + // THE BUG: `add` flattened and `rm` did not, so `pnpm wt rm tags/site` — + // the exact string the person had just passed to `add` — pointed git at a + // path that had never existed. + assert.equal(worktreeDirFor(MAIN, "tags/site"), path.join(SIBLING, "tags-site")); + assert.equal( + worktreeDirFor(MAIN, "one-core/phase-2/gate-a"), + path.join(SIBLING, "one-core-phase-2-gate-a"), + ); +}); + +test("a plain name is unchanged, dots and dashes survive", () => { + assert.equal(worktreeDirFor(MAIN, "hotfix"), path.join(SIBLING, "hotfix")); + assert.equal( + worktreeDirFor(MAIN, "release-1.2_rc"), + path.join(SIBLING, "release-1.2_rc"), + ); +}); + +test("an absolute path passes through untouched", () => { + // `rm` accepts one because it is what `list` prints; sanitizing it would eat + // its separators and aim git at a directory that does not exist. + const abs = path.join(SIBLING, "tags-site"); + assert.equal(worktreeDirFor(MAIN, abs), abs); + assert.equal(worktreeDirFor(MAIN, "/tmp/wt/some thing"), "/tmp/wt/some thing"); +}); + +test("nothing escapes the sibling directory", () => { + // A relative name cannot climb out: every "/" and "." run that could form a + // traversal is flattened to dashes first. + const dir = worktreeDirFor(MAIN, "../../etc/passwd"); + assert.equal(dir, path.join(SIBLING, "..-..-etc-passwd")); + assert.equal(path.dirname(dir), SIBLING); +});