commit 314d48162e881e53d52deb7433006831d7292d3f parent 3096e352ba2c3424065986aa1a15cc3e1b8df4c3 Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st> Date: Mon, 21 Sep 2026 14:10:26 -0400 tags S3 review: the compressed read, the renamed overlay, and six smaller ones BLOCKER — THE PREVIEW OPENED THE INDEX UNCOMPRESSED. buildIndex writes index.mdb with `compression: true`; lmdb-js only interprets the compressed status byte when the READING store carries a compression object, so every value over its ~1 KB threshold threw "Data read, but end of buffer not reached". That is every real description and every transcript — i.e. exactly what a tag rule exists to match — and because the video page awaits ruleHitsForVideo during its render, ONE rule was enough to 500 every video-detail page. Fixed on both opens, with the reason written down beside them. Belt and braces, because a reader this far from its writer will drift again: one record's failure now costs one record (counted, and said out loud in the preview note), an iteration that dies keeps what it found, ruleHitsForVideo degrades to "not indexed", and loadVideoTags cannot throw into a page render at all. The regression test is a real LMDB file, not a stand-in — a fake would not have had the bug. common/controller/curatedTagsPreview.test.ts writes the way buildIndex writes and reads back a fat record; removing `compression: true` fails it. The e2e fixture grew a video whose description (1.8 KB) and transcript (41 cues) both clear the threshold, asserted through Preview and through the video page. While writing that fixture: a plain VTT indexes as ZERO cues. parseVtt keeps only body lines carrying inline timing tags (YouTube's auto-caption shape) and skips a bare text line, so the fixture's transcripts are written that way — and no caption rule could ever have fired against the old fixture. HIGH — A COLOUR-ONLY SITE OVERLAY RENAMED THE TAG. mergeTagDefs applies every field the overlay carries and sanitizeTagsConfig fills a missing label with the id, so a site that set only a colour published "eva-collab" where the corpus says "Collab". Stripping the invented label on the way out does not work — the store sanitizes on write AND on read — so a row that does not override the label is now written carrying the CORPUS label, and the tab says what that costs (a later corpus rename does not follow until this tab is saved again). The clean fix belongs in the model: sanitize needs to coerce a SITE layer without defaulting presentation fields. Five more, each with a test: * Pin all SKIPPED THE SUPPRESSED ROWS. `add` clears a suppression, so pinning a preview page silently reversed every rejection the operator had just made. The button now says how many it skips. * "Suppress on selected" joins the bulk menu, with the sentence that makes the pair intelligible: untag unpins, and a rule that still matches keeps the tag. * A deleted definition's pins are no longer invisible: /tags names the orphans, and removing a definition that carries facts asks first. * The bulk path resolves directory names through the index's own `mtimes` map (one ranged read) and reads a metadata file only for what it misses. * toggleVideoTagAction revalidated `/videos/<recordId>` — a path that does not exist on Rumble or Twitch, where the segment is the DIRECTORY name. And three small ones: the optimistic panel row keeps its colour and group, the cue decode is gated per RECORD (not per rule-kind) so a channel-scoped caption rule stops decoding transcripts it cannot fire on, and Pin is disabled until the definition is saved rather than offering a click the store will refuse. Co-Authored-By: Claude Opus <noreply@anthropic.com> Diffstat:
21 files changed, 1080 insertions(+), 75 deletions(-)
diff --git a/common/controller/curatedTagsPreview.test.ts b/common/controller/curatedTagsPreview.test.ts @@ -0,0 +1,361 @@ +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { mkdtempSync, rmSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; +import { open } from "lmdb"; +import type { Paths } from "../lib/paths"; +import type { TranscriptSummary } from "../lib/transcripts"; +import type { CuratedTagDef } from "../lib/curatedTags"; +import { + previewTagRule, + recordIdsByVideoDir, + ruleHitsForVideo, +} from "./curatedTagsPreview"; + +// Run with: pnpm -C common exec tsx --test "controller/curatedTagsPreview.test.ts" +// +// A REAL LMDB FILE, not a stand-in — which is the whole point of this file. +// The bug this suite exists to stop was invisible to a fake: the preview opened +// the index WITHOUT `compression: true` while buildIndex writes with it, and +// lmdb-js only reads the compressed status byte when the reading store carries +// a compression object. Every value over its ~1 KB threshold — that is, every +// real description and every transcript — then threw "Data read, but end of +// buffer not reached", and because the video page awaits ruleHitsForVideo +// during its render, one rule was enough to 500 every video-detail page. +// +// So the fixture writes the way buildIndex writes, and one record is +// deliberately fat. + +const KEEP = process.env.KEEP_TAG_PREVIEW_FIXTURE === "1"; + +// Comfortably over lmdb-js's 1000-byte compression threshold. +const FAT = "a fat description that keeps going. ".repeat(60); + +type IndexKey = [string, string, string]; + +type Fixture = { + paths: Paths; + cleanup(): void; +}; + +async function buildIndexFixture( + records: { + slug: string; + dir?: string; + id: string; + uploadDate: string; + title?: string; + description?: string; + cues?: { start: number; end: number; text: string }[]; + chat?: { start: number; end: number; text: string }[]; + }[], +): Promise<Fixture> { + const dir = mkdtempSync(path.join(tmpdir(), "tag-preview-")); + const lmdbPath = path.join(dir, "index.mdb"); + // maxDbs and compression EXACTLY as buildIndex opens it. + const root = open({ path: lmdbPath, maxDbs: 18, compression: true }); + const sums = root.openDB<TranscriptSummary, IndexKey>({ + name: "sums", + encoding: "msgpack", + }); + const cues = root.openDB({ name: "cues", encoding: "msgpack" }); + const subs = root.openDB({ name: "subs", encoding: "msgpack" }); + const byChannel = root.openDB({ name: "byChannel", encoding: "msgpack" }); + const mtimes = root.openDB({ name: "mtimes", encoding: "msgpack" }); + + let last: Promise<boolean> | undefined; + for (const r of records) { + const key: IndexKey = [r.uploadDate, r.slug, r.id]; + const summary = { + slug: `${r.slug}/${r.id}`, + id: r.id, + channelSlug: r.slug, + title: r.title ?? r.id, + uploadDate: r.uploadDate, + duration: 60, + channel: r.slug, + description: r.description ?? "", + tags: [], + isLivestream: false, + ageRestricted: false, + platform: "youtube", + webpageUrl: `https://example.invalid/${r.id}`, + } as unknown as TranscriptSummary; + last = sums.put(key, summary); + byChannel.put([r.slug, r.uploadDate, r.id], 1); + mtimes.put([r.slug, r.dir ?? r.id], { metaMs: 1, indexKey: key }); + if (r.cues) last = cues.put(key, r.cues); + if (r.chat) last = subs.put(key, [{ track: "live_chat", cues: r.chat }]); + } + await last; + await root.close(); + + return { + paths: { lmdbPath } as Paths, + cleanup: () => { + if (!KEEP) rmSync(dir, { recursive: true, force: true }); + }, + }; +} + +function def( + id: string, + rule: Partial<CuratedTagDef["rules"] extends (infer R)[] | undefined ? R : never> & { + kind: "metadata" | "caption" | "chat-author"; + pattern: string; + }, +): CuratedTagDef { + return { + id, + label: id, + rules: [{ id: "r1", enabled: true, ...rule }], + }; +} + +test("a record far over the compression threshold is read, not thrown", async () => { + const f = await buildIndexFixture([ + { + slug: "lm", + id: "big", + uploadDate: "20260101", + title: "An ordinary title", + description: `${FAT} elfpire guested here`, + cues: Array.from({ length: 400 }, (_, i) => ({ + start: i, + end: i + 1, + text: `a caption line number ${i} with enough words to matter`, + })), + }, + ]); + try { + const meta = previewTagRule(f.paths, { + def: def("t", { kind: "metadata", pattern: "elfpire" }), + channels: [], + allSlugs: ["lm"], + }); + assert.equal(meta.indexAvailable, true); + // THE ASSERTION THAT WOULD HAVE CAUGHT IT: without compression on the read + // side this is 0 matches and 1 unreadable, not 1 and 0. + assert.equal(meta.unreadable, 0); + assert.deepEqual( + meta.matches.map((m) => m.id), + ["big"], + ); + + // And the caption path, which decodes a four-hundred-cue value. + const caption = previewTagRule(f.paths, { + def: def("t", { kind: "caption", pattern: "caption line number 399" }), + channels: [], + allSlugs: ["lm"], + }); + assert.equal(caption.unreadable, 0); + assert.equal(caption.matches.length, 1); + + // Same file, the single-record path the video page uses. + const hits = ruleHitsForVideo( + f.paths, + [def("t", { kind: "metadata", pattern: "elfpire" })], + "lm", + "big", + "20260101", + ); + assert.deepEqual(hits, { hits: ["t"], indexAvailable: true, indexed: true }); + } finally { + f.cleanup(); + } +}); + +test("a chat-author rule reads the live_chat track and no other", async () => { + const f = await buildIndexFixture([ + { + slug: "lm", + id: "chatty", + uploadDate: "20260101", + chat: [ + { start: 1, end: 2, text: "SomeoneElse: hello" }, + { start: 3, end: 4, text: "ElfpireEva: hi" }, + ], + }, + { + slug: "lm", + id: "quiet", + uploadDate: "20260102", + // The NAME IS IN THE MESSAGE, not in the author prefix — a chat-author + // rule must not match it. + chat: [{ start: 1, end: 2, text: "SomeoneElse: elfpire was here" }], + }, + ]); + try { + const r = previewTagRule(f.paths, { + def: def("t", { kind: "chat-author", pattern: "elfpire" }), + channels: [], + allSlugs: ["lm"], + }); + assert.deepEqual( + r.matches.map((m) => m.id), + ["chatty"], + ); + } finally { + f.cleanup(); + } +}); + +test("the scan cap stops the call, and the cursor resumes past the boundary key", async () => { + // 300 records, all matching, with a CAPTION rule — whose per-call cap is 250. + const records = Array.from({ length: 300 }, (_, i) => ({ + slug: "lm", + id: `v${String(i).padStart(3, "0")}`, + uploadDate: `2026${String(100 + i).padStart(4, "0")}`, + cues: [{ start: 0, end: 1, text: "the needle is here" }], + })); + const f = await buildIndexFixture(records); + try { + const first = previewTagRule(f.paths, { + def: def("t", { kind: "caption", pattern: "needle" }), + channels: [], + allSlugs: ["lm"], + }); + // The match cap (100) bites before the scan cap here, which is itself the + // contract: a preview is for judging a rule, not for enumerating its hits. + assert.equal(first.stoppedBy, "match-cap"); + assert.equal(first.matches.length, 100); + assert.ok(first.nextCursor); + + const second = previewTagRule(f.paths, { + def: def("t", { kind: "caption", pattern: "needle" }), + channels: [], + allSlugs: ["lm"], + cursor: first.nextCursor, + }); + // NO OVERLAP. getRange's start is inclusive, so a resume that did not skip + // the boundary key would re-list the row the operator has just acted on. + const firstIds = new Set(first.matches.map((m) => m.id)); + assert.equal( + second.matches.some((m) => firstIds.has(m.id)), + false, + "the resumed page re-listed a record from the first page", + ); + assert.equal(second.matches[0].id, "v100"); + } finally { + f.cleanup(); + } +}); + +test("the scan cap is reported when the matches do not fill a page", async () => { + // 300 records, only the last one matching, caption kind -> the 250-record + // scan cap stops the call before the match is reached. + const records = Array.from({ length: 300 }, (_, i) => ({ + slug: "lm", + id: `v${String(i).padStart(3, "0")}`, + uploadDate: `2026${String(100 + i).padStart(4, "0")}`, + cues: [{ start: 0, end: 1, text: i === 299 ? "the needle" : "hay" }], + })); + const f = await buildIndexFixture(records); + try { + const first = previewTagRule(f.paths, { + def: def("t", { kind: "caption", pattern: "needle" }), + channels: [], + allSlugs: ["lm"], + }); + assert.equal(first.stoppedBy, "scan-cap"); + assert.equal(first.scanned, 250); + assert.equal(first.matches.length, 0); + const second = previewTagRule(f.paths, { + def: def("t", { kind: "caption", pattern: "needle" }), + channels: [], + allSlugs: ["lm"], + cursor: first.nextCursor, + }); + assert.equal(second.scanned, 50); + assert.deepEqual( + second.matches.map((m) => m.id), + ["v299"], + ); + assert.equal(second.stoppedBy, "end"); + assert.equal(second.nextCursor, null); + } finally { + f.cleanup(); + } +}); + +test("a channel-scoped rule scans only its own channel", async () => { + const f = await buildIndexFixture([ + { slug: "lm", id: "a1", uploadDate: "20260101", title: "needle here" }, + { slug: "other", id: "b1", uploadDate: "20260101", title: "needle here" }, + { slug: "other", id: "b2", uploadDate: "20260102", title: "needle here" }, + ]); + try { + const scoped = previewTagRule(f.paths, { + def: { + id: "t", + label: "t", + rules: [ + { + id: "r1", + kind: "metadata", + pattern: "needle", + enabled: true, + channels: ["lm"], + }, + ], + }, + channels: ["lm"], + allSlugs: ["lm", "other"], + }); + assert.deepEqual( + scoped.matches.map((m) => m.id), + ["a1"], + ); + // SCANNED, not merely unmatched: the other channel's two records were never + // looked at, which is the entire point of scoping a rule to a channel. + assert.equal(scoped.scanned, 1); + + const unscoped = previewTagRule(f.paths, { + def: def("t", { kind: "metadata", pattern: "needle" }), + channels: [], + allSlugs: ["lm", "other"], + }); + assert.equal(unscoped.scanned, 3); + assert.equal(unscoped.matches.length, 3); + } finally { + f.cleanup(); + } +}); + +test("recordIdsByVideoDir maps a directory name to the record id", async () => { + const f = await buildIndexFixture([ + { slug: "rum", dir: "a-url-slug", id: "v2embedid", uploadDate: "20260101" }, + { slug: "rum", id: "plain", uploadDate: "20260102" }, + { slug: "other", dir: "elsewhere", id: "nope", uploadDate: "20260103" }, + ]); + try { + const map = recordIdsByVideoDir(f.paths, "rum"); + assert.equal(map.get("a-url-slug"), "v2embedid"); + // A directory whose name IS the id still appears — callers fall back to the + // name only when the index has never seen the video at all. + assert.equal(map.get("plain"), "plain"); + // Another channel's directories are not in this channel's map. + assert.equal(map.get("elsewhere"), undefined); + assert.equal(map.size, 2); + } finally { + f.cleanup(); + } +}); + +test("no index is an empty answer, never a throw", () => { + const paths = { lmdbPath: path.join(tmpdir(), "definitely-not-here.mdb") } as Paths; + const r = previewTagRule(paths, { + def: def("t", { kind: "metadata", pattern: "x" }), + channels: [], + allSlugs: ["lm"], + }); + assert.equal(r.indexAvailable, false); + assert.deepEqual(r.matches, []); + assert.deepEqual(recordIdsByVideoDir(paths, "lm").size, 0); + assert.deepEqual(ruleHitsForVideo(paths, [def("t", { kind: "metadata", pattern: "x" })], "lm", "v1"), { + hits: [], + indexAvailable: false, + indexed: false, + }); +}); diff --git a/common/controller/curatedTagsPreview.ts b/common/controller/curatedTagsPreview.ts Binary files differ. diff --git a/editor/app/channels/[slug]/bulkVideoActions.ts b/editor/app/channels/[slug]/bulkVideoActions.ts @@ -22,6 +22,7 @@ import { loadRawMetadataFromDir, summarize, } from "yt-dlp-transcript-common/lib/transcripts-server"; +import { recordIdsByVideoDir } from "yt-dlp-transcript-common/controller/curatedTagsPreview"; import { applyTagAssignmentsAction } from "../../tags/actions"; import { deleteOneVideoDir, @@ -235,21 +236,34 @@ export async function bulkClearFailedMarkersAction( // // THE KEYS ARE RECORD IDS, NOT DIRECTORY NAMES. The list holds directory names; // assignments are keyed by the id the index and the export use, and the two -// differ on Rumble, Odysee and Twitch. Resolving them costs one metadata read -// per SELECTED video (bounded by the selection, not by the channel), which is -// the same file the video page reads to draw its own panel. +// differ on Rumble, Odysee and Twitch. The index's own directory -> id map +// answers that for the whole selection in one ranged read; only a video the +// index has never seen costs a metadata read. +// +// `op` is add | remove | suppress, and the three are not interchangeable: +// remove UNPINS (a rule that still matches keeps the tag), suppress REJECTS. export async function bulkApplyTagAction( slug: string, videoIds: string[], tag: string, - op: "add" | "remove", + op: "add" | "remove" | "suppress", ): Promise<BulkActionSummary> { const paths = getPaths(); + // THE MAP FIRST, THE FILES ONLY FOR WHAT IT MISSES. The index already stores + // directory -> record id for every indexed video (the `mtimes` sub-DB), so + // one ranged read answers the whole selection; reading a metadata.info.json + // per selected video was four thousand serial opens for a four-thousand-video + // bulk. A video the index has never seen (never built, just downloaded) still + // falls back to its own metadata, and then to its directory name. + const byDir = recordIdsByVideoDir(paths, slug); const channelDataDir = path.join(paths.channelsDir, slug, "data"); const videos: { channelSlug: string; id: string }[] = []; for (const dirId of videoIds) { - const meta = await loadRawMetadataFromDir(path.join(channelDataDir, dirId)); - const id = meta ? summarize(slug, dirId, meta).id : dirId; + let id = byDir.get(dirId); + if (!id) { + const meta = await loadRawMetadataFromDir(path.join(channelDataDir, dirId)); + id = meta ? summarize(slug, dirId, meta).id : dirId; + } videos.push({ channelSlug: slug, id }); } const result = await applyTagAssignmentsAction({ op, tag, videos }); diff --git a/editor/app/channels/[slug]/components/VideoListPane.tsx b/editor/app/channels/[slug]/components/VideoListPane.tsx @@ -37,6 +37,7 @@ type BulkAction = | "remove_wrong_format" | "tag" | "untag" + | "suppress_tag" | "delete"; const BULK_ACTION_OPTIONS: { value: BulkAction; label: string }[] = [ @@ -51,6 +52,7 @@ const BULK_ACTION_OPTIONS: { value: BulkAction; label: string }[] = [ { value: "remove_wrong_format", label: "Remove wrong-format audio" }, { value: "tag", label: "Tag selected" }, { value: "untag", label: "Untag selected" }, + { value: "suppress_tag", label: "Suppress on selected" }, { value: "delete", label: "Delete directories" }, ]; @@ -379,7 +381,10 @@ export function VideoListPane({ const applyDisabled = pending || (action === "delete" && !deleteArmed) || - ((action === "tag" || action === "untag") && !bulkTag); + ((action === "tag" || + action === "untag" || + action === "suppress_tag") && + !bulkTag); function handleApply() { switch (action) { @@ -443,9 +448,11 @@ export function VideoListPane({ doSummaryBulk(bulkRemoveWrongFormatAudioAction); break; case "tag": - case "untag": { + case "untag": + case "suppress_tag": { if (!bulkTag) return; - const op = action === "tag" ? "add" : "remove"; + const op = + action === "tag" ? "add" : action === "untag" ? "remove" : "suppress"; doSummaryBulk((s, ids) => bulkApplyTagAction(s, ids, bulkTag, op)); break; } @@ -733,7 +740,9 @@ export function VideoListPane({ actionLabel="bulk re-download incomplete" /> )} - {(action === "tag" || action === "untag") && ( + {(action === "tag" || + action === "untag" || + action === "suppress_tag") && ( <label className="flex items-center gap-1.5 text-xs text-muted-foreground"> <span>Tag</span> <select @@ -785,6 +794,21 @@ export function VideoListPane({ Apply </button> </div> + {/* UNTAG IS NOT THE OPPOSITE OF TAG, and the chips cannot show the + difference: they list PINS, while a rule's hit is derived at + index build and never stored. So an untagged video whose rule + still matches looks clear here and ships tagged. */} + {(action === "tag" || + action === "untag" || + action === "suppress_tag") && ( + <p + className="text-[11px] text-muted-foreground" + aria-label="bulk tag hint" + > + Untag unpins; a rule that still matches keeps the tag — use + Suppress to reject it. + </p> + )} </div> {error && ( <p diff --git a/editor/app/channels/[slug]/videos/[id]/components/TagsPanel.tsx b/editor/app/channels/[slug]/videos/[id]/components/TagsPanel.tsx @@ -31,6 +31,7 @@ export function TagsPanel({ view }: { view: VideoTagsView }) { view.videoId, tag, op, + view.directoryId ?? undefined, ); if (!result.ok) { setError(result.error); @@ -38,9 +39,15 @@ export function TagsPanel({ view }: { view: VideoTagsView }) { } setRows((rs) => { const existing = rs.find((r) => r.id === tag); + // SPREAD THE ROW WE HAD. Rebuilding it field by field dropped the + // colour, the group and the group label, so pinning a tag visibly + // restyled its own chip until the next reload. + const def = view.defs.find((d) => d.id === tag); const next = { + ...(def ?? {}), + ...(existing ?? {}), id: tag, - label: existing?.label ?? tag, + label: existing?.label ?? def?.label ?? tag, ruleHit: existing?.ruleHit ?? false, pinned: op === "add", suppressed: op === "suppress", diff --git a/editor/app/channels/[slug]/videos/[id]/lib/videoTags.ts b/editor/app/channels/[slug]/videos/[id]/lib/videoTags.ts @@ -48,8 +48,16 @@ export type VideoTagsView = { // Set only when it differs from the directory name, so the panel can say so. directoryId: string | null; rows: VideoTagRow[]; - // Every defined tag, for the "add tag" picker. - defs: { id: string; label: string }[]; + // Every defined tag, for the "add tag" picker — with the presentation fields, + // so a row the panel adds optimistically looks like the one the server would + // have rendered instead of losing its colour until the next reload. + defs: { + id: string; + label: string; + color?: string; + group?: string; + groupLabel?: string; + }[]; // The index has never been built (or is unreadable): rules cannot be // evaluated, so the panel shows pins and suppressions only and says why. indexAvailable: boolean; @@ -58,10 +66,34 @@ export type VideoTagsView = { indexed: boolean; }; +// NOTHING HERE MAY THROW. The video-detail page awaits this during its render, +// and a tag panel is the least important thing on that page: a corpus read that +// fails — an unreadable index value, a half-written tags.json — must cost the +// PANEL, never the page. Every failure below degrades to "no tags, and here is +// why", which is a state the panel already draws. export async function loadVideoTags( slug: string, dirId: string, ): Promise<VideoTagsView> { + try { + return await readVideoTags(slug, dirId); + } catch { + return { + channelSlug: slug, + videoId: dirId, + directoryId: null, + rows: [], + defs: [], + indexAvailable: false, + indexed: false, + }; + } +} + +async function readVideoTags( + slug: string, + dirId: string, +): Promise<VideoTagsView> { const paths = getPaths(); const videoDir = path.join(paths.channelsDir, slug, "data", dirId); const meta = await loadRawMetadataFromDir(videoDir); @@ -112,7 +144,13 @@ export async function loadVideoTags( videoId, directoryId: videoId === dirId ? null : dirId, rows, - defs: defs.map((d) => ({ id: d.id, label: d.label })), + defs: defs.map((d) => ({ + id: d.id, + label: d.label, + ...(d.color ? { color: d.color } : {}), + ...(d.group ? { group: d.group } : {}), + ...(d.groupLabel ? { groupLabel: d.groupLabel } : {}), + })), indexAvailable, indexed, }; diff --git a/editor/app/channels/[slug]/videos/[id]/videoActions.ts b/editor/app/channels/[slug]/videos/[id]/videoActions.ts @@ -681,6 +681,9 @@ export async function toggleVideoTagAction( videoId: string, tag: string, op: "add" | "remove" | "suppress" | "unsuppress", + // The DIRECTORY name, when the caller has it. Only used to revalidate the + // page the operator is on; the write never touches it. + dirId?: string, ): Promise<{ ok: true; changed: number } | { ok: false; error: string }> { const result = await applyTagAssignmentsAction({ op, @@ -688,7 +691,12 @@ export async function toggleVideoTagAction( videos: [{ channelSlug: slug, id: videoId }], }); if (!result.ok) return result; - revalidatePath(`/channels/${slug}/videos/${videoId}`); + // THE ROUTE SEGMENT IS THE DIRECTORY NAME, and `videoId` here is the RECORD + // id — the two differ on Rumble, Odysee and Twitch, so revalidating + // `/videos/<recordId>` would refresh a path that does not exist while the + // page the operator is looking at kept its cached panel. The caller passes + // the directory name when it knows it; the list path covers both either way. + revalidatePath(`/channels/${slug}/videos/${dirId ?? videoId}`); revalidatePath(`/channels/${slug}/videos`); return result; } diff --git a/editor/app/sites/[siteId]/tags/page.tsx b/editor/app/sites/[siteId]/tags/page.tsx @@ -41,7 +41,9 @@ export default async function SiteTagsPage({ assignments stay in the corpus vocabulary: a record is shared by every site carrying its channel, so what a tag MEANS is decided once. Corpus rules are listed read-only for reference. Changes bake into the - site's next export build. + site's next export build. A tag you overlay here keeps the label it + had when you saved: rename it in the corpus and save this tab again to + follow the new wording. </p> <SiteTagsClient key={siteId} diff --git a/editor/app/tags/actions.ts b/editor/app/tags/actions.ts @@ -79,10 +79,36 @@ export async function saveSiteTagDefsAction( } const paths = getPaths(); const current = readSiteTags(paths, siteId); + // AN OVERLAY MUST NOT RENAME THE TAG IT IS ONLY RECOLOURING. + // + // mergeTagDefs applies every field the overlay CARRIES, and + // sanitizeTagsConfig fills a missing `label` with the id — so a site that set + // nothing but a colour would publish "eva-collab" where the corpus says + // "Collab". Stripping the invented label on the way out does not help: the + // store sanitizes on write AND on read (curatedTagsStore.ts), so the default + // comes back before compose ever sees the file. + // + // So a row that does not override the label is written carrying the CORPUS + // label — the overlay then says exactly what this site publishes, and the + // merge is a no-op for that field. The cost is stated rather than hidden: a + // later change to the corpus label does not follow into a site that already + // has an overlay until this tab is saved again, and the tab says so. The + // clean fix belongs in the model (sanitize must be able to coerce a SITE + // layer without defaulting presentation fields); this is the editor half. + const globalById = new Map( + readGlobalTags(paths).tags.map((t) => [t.id, t]), + ); + const mirrored = tags.map((t) => { + const id = (t.id ?? "").trim().toLowerCase(); + if (Object.prototype.hasOwnProperty.call(t, "label") && t.label?.trim()) { + return t; + } + return { ...t, label: globalById.get(id)?.label ?? id }; + }); writeSiteTags(paths, siteId, { ...current, assignments: {}, - tags: sanitizeTagsConfig({ tags }).tags, + tags: sanitizeTagsConfig({ tags: mirrored }).tags, }); revalidatePath(`/sites/${siteId}/tags`); return { ok: true }; diff --git a/editor/app/tags/components/EditorTagsClient.tsx b/editor/app/tags/components/EditorTagsClient.tsx @@ -121,6 +121,7 @@ function regexError(pattern: string): string | null { type PreviewState = { rows: TagPreviewRow[]; scanned: number; + unreadable: number; nextCursor: string | null; stoppedBy: "end" | "scan-cap" | "match-cap"; indexAvailable: boolean; @@ -130,12 +131,24 @@ export function EditorTagsClient({ initialTags, counts, channels, + orphans, }: { initialTags: CuratedTagDef[]; counts: TagCounts; channels: string[]; + // Tag ids carrying pins or suppressions that no definition claims — a def + // deleted after the facts were made. They keep riding on records and appear + // in no published /tags.json, so the only place they can be seen is here. + orphans: string[]; }) { const [rows, setRows] = useState<TagRow[]>(() => initialTags.map(toRow)); + // The ids that EXIST in the file right now. Pinning under an id the store has + // never heard of is refused by the store (it would put a tag on records that + // no /tags.json can show), so the buttons that pin are disabled until the + // definition is saved rather than offering a click that can only fail. + const [savedIds, setSavedIds] = useState<Set<string>>( + () => new Set(initialTags.map((t) => t.id)), + ); const [dirty, setDirty] = useState(false); const [saved, setSaved] = useState(false); const [error, setError] = useState<string | null>(null); @@ -175,6 +188,23 @@ export function EditorTagsClient({ }; const removeTag = (key: string) => { + const row = rows.find((r) => r.key === key); + const count = row ? counts[row.id.trim().toLowerCase()] : undefined; + const facts = (count?.pinned ?? 0) + (count?.suppressed ?? 0); + // DELETING A DEFINITION DOES NOT DELETE ITS FACTS, and the operator should + // hear that before saving, not discover it in a file. The pins survive (an + // assignment is a fact about a video) and stop appearing anywhere until the + // id is defined again. + if ( + facts > 0 && + !confirm( + `"${row?.id}" carries ${facts} pin${facts === 1 ? "" : "s"}/suppression${ + facts === 1 ? "" : "s" + }. Removing the definition keeps them on those videos — they will show as undefined here and on no site. Remove it?`, + ) + ) { + return; + } setRows((rs) => rs.filter((r) => r.key !== key)); touch(); }; @@ -188,11 +218,13 @@ export function EditorTagsClient({ return; } startSave(async () => { - const result = await saveTagDefsAction(rows.map(toDef)); + const defs = rows.map(toDef); + const result = await saveTagDefsAction(defs); if (result.ok) { setDirty(false); setSaved(true); setError(null); + setSavedIds(new Set(defs.map((d) => d.id))); } else { setError(result.error); } @@ -212,6 +244,8 @@ export function EditorTagsClient({ [row.key]: { rows: cursor ? [...(p[row.key]?.rows ?? []), ...result.rows] : result.rows, scanned: (cursor ? (p[row.key]?.scanned ?? 0) : 0) + result.scanned, + unreadable: + (cursor ? (p[row.key]?.unreadable ?? 0) : 0) + result.unreadable, nextCursor: result.nextCursor, stoppedBy: result.stoppedBy, indexAvailable: result.indexAvailable, @@ -299,6 +333,19 @@ export function EditorTagsClient({ )} </div> + {orphans.length > 0 && ( + <p + className="rounded-md border border-warning/30 bg-warning-soft px-3 py-2 text-xs text-warning" + data-testid="tags-orphans" + > + {orphans.length} tag{orphans.length === 1 ? "" : "s"} carry pins but no + definition: <code className="font-mono">{orphans.join(", ")}</code>. + Those videos keep the tag and no site can show it — define the id + again to bring it back, or untag the videos from{" "} + <code className="font-mono">pnpm ops tag-videos</code>. + </p> + )} + {rows.length === 0 ? ( <p className="rounded-md border border-dashed px-3 py-6 text-center text-sm text-muted-foreground"> No tags yet. Add one, give it an id like <code>eva-collab</code>, and @@ -312,6 +359,8 @@ export function EditorTagsClient({ row={row} counts={counts[row.id.trim().toLowerCase()]} channels={channels} + // A row whose id is not in the file yet cannot carry assignments. + defined={savedIds.has(row.id.trim().toLowerCase())} busy={previewBusy === row.key} preview={preview[row.key]} note={previewNote[row.key] ?? ""} @@ -331,6 +380,7 @@ function TagCard({ row, counts, channels, + defined, busy, preview, note, @@ -342,6 +392,9 @@ function TagCard({ row: TagRow; counts: { pinned: number; suppressed: number } | undefined; channels: string[]; + // The id exists in the saved file. Pinning under an unsaved id is refused by + // the store, so the pin controls are disabled rather than offered. + defined: boolean; busy: boolean; preview: PreviewState | undefined; note: string; @@ -356,6 +409,14 @@ function TagCard({ const tagId = row.id.trim().toLowerCase(); const idValid = TAG_ID_RE.test(tagId); + // PIN ALL SKIPS WHAT WAS ALREADY REJECTED. `add` clears a suppression of the + // same tag (a later pin beats an older rejection), so pinning the whole page + // would silently undo every judgement the operator had already made against + // this rule — which is the exact work a preview exists to collect. + const previewRows = preview?.rows ?? []; + const pinnable = previewRows.filter((r) => !r.suppressed); + const suppressedInPreview = previewRows.length - pinnable.length; + const patchRule = (key: string, next: Partial<RuleRow>) => onPatch({ rules: row.rules.map((r) => (r.key === key ? { ...r, ...next } : r)), @@ -627,23 +688,33 @@ function TagCard({ > {busy ? "Working…" : "Preview"} </button> - {preview && preview.rows.length > 0 && ( + {preview && pinnable.length > 0 && ( <button type="button" - disabled={busy} + disabled={busy || !defined} onClick={() => onApply( "add", - preview.rows.map((r) => ({ + pinnable.map((r) => ({ channelSlug: r.channelSlug, id: r.id, })), ) } aria-label={`pin all ${tagId}`} + title={ + defined + ? suppressedInPreview > 0 + ? `Skips ${suppressedInPreview} row(s) you already suppressed — a pin would undo that rejection.` + : "Pins every row listed below." + : "Save the definition first." + } className="rounded border border-border px-2 py-1 text-xs hover:bg-muted disabled:opacity-50" > - Pin all ({preview.rows.length}) + Pin all ({pinnable.length}) + {suppressedInPreview > 0 + ? ` — skips ${suppressedInPreview} suppressed` + : ""} </button> )} {note && ( @@ -673,6 +744,9 @@ function TagCard({ : preview.stoppedBy === "scan-cap" ? " — stopped at the scan cap" : " — stopped at the match cap"} + {preview.unreadable > 0 + ? ` · ${preview.unreadable} record(s) could not be read` + : ""} </p> <ul className="flex flex-col divide-y divide-border rounded border border-border"> {preview.rows.map((r) => ( @@ -700,7 +774,8 @@ function TagCard({ )} <button type="button" - disabled={busy} + disabled={busy || !defined} + title={defined ? undefined : "Save the definition first."} onClick={() => onApply("add", [ { channelSlug: r.channelSlug, id: r.id }, @@ -726,7 +801,8 @@ function TagCard({ </button> <button type="button" - disabled={busy} + disabled={busy || !defined} + title={defined ? undefined : "Save the definition first."} onClick={() => onApply("suppress", [ { channelSlug: r.channelSlug, id: r.id }, diff --git a/editor/app/tags/page.tsx b/editor/app/tags/page.tsx @@ -36,6 +36,15 @@ export default async function TagsPage() { } } + // Ids carrying facts that no definition claims — a def deleted after the pins + // were made. effectiveTagsFor deliberately KEEPS such a tag on its videos + // (losing an operator's fact silently would be worse), and nothing else in + // the app can show it: a published /tags.json is built from defs. So this + // page is the only place it can surface, and it does. + const orphans = Object.keys(counts) + .filter((id) => !config.tags.some((t) => t.id === id)) + .sort(); + return ( <section className="flex flex-col gap-4"> <header className="flex flex-col gap-1"> @@ -53,6 +62,7 @@ export default async function TagsPage() { initialTags={config.tags} counts={counts} channels={channels} + orphans={orphans} /> </section> ); diff --git a/editor/e2e/fixtures/test-transcripts/curated-tags-channel/channels/tagchan/config.json b/editor/e2e/fixtures/test-transcripts/curated-tags-channel/channels/tagchan/config.json @@ -0,0 +1,5 @@ +{ + "handling": "youtube", + "name": "Tag Fixture Channel", + "url": "https://www.youtube.com/@tagfixture/videos" +} +\ No newline at end of file diff --git a/editor/e2e/fixtures/test-transcripts/curated-tags-channel/channels/tagchan/data/20240101_small0000/metadata.info.json b/editor/e2e/fixtures/test-transcripts/curated-tags-channel/channels/tagchan/data/20240101_small0000/metadata.info.json @@ -0,0 +1,17 @@ +{ + "channel": "Tag Fixture Channel", + "channel_id": "UCtagfixture", + "channel_url": "https://www.youtube.com/channel/UCtagfixture", + "uploader": "Tag Fixture Channel", + "duration": 120, + "is_live": false, + "was_live": false, + "live_status": "not_live", + "age_limit": 0, + "id": "20240101_small0000", + "title": "A Small Synthetic Video", + "upload_date": "20240101", + "description": "A short synthetic description.", + "extractor_key": "Youtube", + "webpage_url": "https://www.youtube.com/watch?v=20240101_small0000" +} +\ No newline at end of file diff --git a/editor/e2e/fixtures/test-transcripts/curated-tags-channel/channels/tagchan/data/20240101_small0000/transcript.en.vtt b/editor/e2e/fixtures/test-transcripts/curated-tags-channel/channels/tagchan/data/20240101_small0000/transcript.en.vtt @@ -0,0 +1,9 @@ +WEBVTT +Kind: captions +Language: en + +00:00:00.000 --> 00:00:05.000 +<00:00:00.000><c>Hello, this is a small synthetic transcript.</c> + +00:00:05.000 --> 00:00:10.000 +<00:00:05.000><c>The second cue covers the next five seconds.</c> diff --git a/editor/e2e/fixtures/test-transcripts/curated-tags-channel/channels/tagchan/data/20240102_bigdesc000/metadata.info.json b/editor/e2e/fixtures/test-transcripts/curated-tags-channel/channels/tagchan/data/20240102_bigdesc000/metadata.info.json @@ -0,0 +1,17 @@ +{ + "channel": "Tag Fixture Channel", + "channel_id": "UCtagfixture", + "channel_url": "https://www.youtube.com/channel/UCtagfixture", + "uploader": "Tag Fixture Channel", + "duration": 120, + "is_live": false, + "was_live": false, + "live_status": "not_live", + "age_limit": 0, + "id": "20240102_bigdesc000", + "title": "A Long Synthetic Video", + "upload_date": "20240102", + "description": "A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. A long synthetic description. The guest tonight is Elfpire Eva.", + "extractor_key": "Youtube", + "webpage_url": "https://www.youtube.com/watch?v=20240102_bigdesc000" +} +\ No newline at end of file diff --git a/editor/e2e/fixtures/test-transcripts/curated-tags-channel/channels/tagchan/data/20240102_bigdesc000/transcript.en.vtt b/editor/e2e/fixtures/test-transcripts/curated-tags-channel/channels/tagchan/data/20240102_bigdesc000/transcript.en.vtt @@ -0,0 +1,126 @@ +WEBVTT +Kind: captions +Language: en + +00:00:00.000 --> 00:00:05.000 +<00:00:00.000><c>Caption line number 0 of a transcript long enough to be compressed.</c> + +00:00:05.000 --> 00:00:10.000 +<00:00:05.000><c>Caption line number 1 of a transcript long enough to be compressed.</c> + +00:00:10.000 --> 00:00:15.000 +<00:00:10.000><c>Caption line number 2 of a transcript long enough to be compressed.</c> + +00:00:15.000 --> 00:00:20.000 +<00:00:15.000><c>Caption line number 3 of a transcript long enough to be compressed.</c> + +00:00:20.000 --> 00:00:25.000 +<00:00:20.000><c>Caption line number 4 of a transcript long enough to be compressed.</c> + +00:00:25.000 --> 00:00:30.000 +<00:00:25.000><c>Caption line number 5 of a transcript long enough to be compressed.</c> + +00:00:30.000 --> 00:00:35.000 +<00:00:30.000><c>Caption line number 6 of a transcript long enough to be compressed.</c> + +00:00:35.000 --> 00:00:40.000 +<00:00:35.000><c>Caption line number 7 of a transcript long enough to be compressed.</c> + +00:00:40.000 --> 00:00:45.000 +<00:00:40.000><c>Caption line number 8 of a transcript long enough to be compressed.</c> + +00:00:45.000 --> 00:00:50.000 +<00:00:45.000><c>Caption line number 9 of a transcript long enough to be compressed.</c> + +00:00:50.000 --> 00:00:55.000 +<00:00:50.000><c>Caption line number 10 of a transcript long enough to be compressed.</c> + +00:00:55.000 --> 00:01:00.000 +<00:00:55.000><c>Caption line number 11 of a transcript long enough to be compressed.</c> + +00:01:00.000 --> 00:01:05.000 +<00:01:00.000><c>Caption line number 12 of a transcript long enough to be compressed.</c> + +00:01:05.000 --> 00:01:10.000 +<00:01:05.000><c>Caption line number 13 of a transcript long enough to be compressed.</c> + +00:01:10.000 --> 00:01:15.000 +<00:01:10.000><c>Caption line number 14 of a transcript long enough to be compressed.</c> + +00:01:15.000 --> 00:01:20.000 +<00:01:15.000><c>Caption line number 15 of a transcript long enough to be compressed.</c> + +00:01:20.000 --> 00:01:25.000 +<00:01:20.000><c>Caption line number 16 of a transcript long enough to be compressed.</c> + +00:01:25.000 --> 00:01:30.000 +<00:01:25.000><c>Caption line number 17 of a transcript long enough to be compressed.</c> + +00:01:30.000 --> 00:01:35.000 +<00:01:30.000><c>Caption line number 18 of a transcript long enough to be compressed.</c> + +00:01:35.000 --> 00:01:40.000 +<00:01:35.000><c>Caption line number 19 of a transcript long enough to be compressed.</c> + +00:01:40.000 --> 00:01:45.000 +<00:01:40.000><c>Caption line number 20 of a transcript long enough to be compressed.</c> + +00:01:45.000 --> 00:01:50.000 +<00:01:45.000><c>Caption line number 21 of a transcript long enough to be compressed.</c> + +00:01:50.000 --> 00:01:55.000 +<00:01:50.000><c>Caption line number 22 of a transcript long enough to be compressed.</c> + +00:01:55.000 --> 00:02:00.000 +<00:01:55.000><c>Caption line number 23 of a transcript long enough to be compressed.</c> + +00:02:00.000 --> 00:02:05.000 +<00:02:00.000><c>Caption line number 24 of a transcript long enough to be compressed.</c> + +00:02:05.000 --> 00:02:10.000 +<00:02:05.000><c>Caption line number 25 of a transcript long enough to be compressed.</c> + +00:02:10.000 --> 00:02:15.000 +<00:02:10.000><c>Caption line number 26 of a transcript long enough to be compressed.</c> + +00:02:15.000 --> 00:02:20.000 +<00:02:15.000><c>Caption line number 27 of a transcript long enough to be compressed.</c> + +00:02:20.000 --> 00:02:25.000 +<00:02:20.000><c>Caption line number 28 of a transcript long enough to be compressed.</c> + +00:02:25.000 --> 00:02:30.000 +<00:02:25.000><c>Caption line number 29 of a transcript long enough to be compressed.</c> + +00:02:30.000 --> 00:02:35.000 +<00:02:30.000><c>Caption line number 30 of a transcript long enough to be compressed.</c> + +00:02:35.000 --> 00:02:40.000 +<00:02:35.000><c>Caption line number 31 of a transcript long enough to be compressed.</c> + +00:02:40.000 --> 00:02:45.000 +<00:02:40.000><c>Caption line number 32 of a transcript long enough to be compressed.</c> + +00:02:45.000 --> 00:02:50.000 +<00:02:45.000><c>Caption line number 33 of a transcript long enough to be compressed.</c> + +00:02:50.000 --> 00:02:55.000 +<00:02:50.000><c>Caption line number 34 of a transcript long enough to be compressed.</c> + +00:02:55.000 --> 00:03:00.000 +<00:02:55.000><c>Caption line number 35 of a transcript long enough to be compressed.</c> + +00:03:00.000 --> 00:03:05.000 +<00:03:00.000><c>Caption line number 36 of a transcript long enough to be compressed.</c> + +00:03:05.000 --> 00:03:10.000 +<00:03:05.000><c>Caption line number 37 of a transcript long enough to be compressed.</c> + +00:03:10.000 --> 00:03:15.000 +<00:03:10.000><c>Caption line number 38 of a transcript long enough to be compressed.</c> + +00:03:15.000 --> 00:03:20.000 +<00:03:15.000><c>Caption line number 39 of a transcript long enough to be compressed.</c> + +00:03:20.000 --> 00:03:25.000 +<00:03:20.000><c>And near the end somebody finally says elfpire out loud.</c> diff --git a/editor/e2e/fixtures/test-transcripts/curated-tags-channel/channels/tagchan/data/a-rumble-url-slug/metadata.info.json b/editor/e2e/fixtures/test-transcripts/curated-tags-channel/channels/tagchan/data/a-rumble-url-slug/metadata.info.json @@ -0,0 +1,17 @@ +{ + "channel": "Tag Fixture Channel", + "channel_id": "UCtagfixture", + "channel_url": "https://www.youtube.com/channel/UCtagfixture", + "uploader": "Tag Fixture Channel", + "duration": 120, + "is_live": false, + "was_live": false, + "live_status": "not_live", + "age_limit": 0, + "id": "v2embedid", + "title": "A Rumble Synthetic Video", + "upload_date": "20240103", + "description": "A rumble synthetic description mentioning elfpire once.", + "extractor_key": "RumbleEmbed", + "webpage_url": "https://rumble.com/embed/v2embedid/" +} +\ No newline at end of file diff --git a/editor/e2e/fixtures/test-transcripts/curated-tags-channel/channels/tagchan/data/a-rumble-url-slug/transcript.en.vtt b/editor/e2e/fixtures/test-transcripts/curated-tags-channel/channels/tagchan/data/a-rumble-url-slug/transcript.en.vtt @@ -0,0 +1,6 @@ +WEBVTT +Kind: captions +Language: en + +00:00:00.000 --> 00:00:05.000 +<00:00:00.000><c>A rumble transcript cue.</c> diff --git a/editor/e2e/tags.spec.ts b/editor/e2e/tags.spec.ts @@ -10,17 +10,34 @@ import { buildIndex, readJson, resetData, writeSite } from "./helpers"; // its pins, its suppressions and the provenance beside each one — rather than // about the controls that produced it. // -// The corpus fixture is the one-video YouTube channel, and the index is built -// once per test that needs a rule: a rule is evaluated against the transcript -// index, so "preview" against a corpus that was never built is a legitimate but -// uninteresting empty state. +// THE FIXTURE IS THREE VIDEOS, AND EACH ONE IS THERE FOR A REASON: +// +// 20240101_small0000 ordinary, two short cues +// 20240102_bigdesc000 a description AND a transcript both over lmdb-js's +// ~1 KB compression threshold. A reader that opens the +// index without `compression: true` throws on exactly +// these values and on no others — which is how a preview +// that "worked" against a three-line fixture took down +// every video-detail page on the real corpus. +// a-rumble-url-slug directory named for the URL slug, record keyed by the +// embed id (v2embedid) — Rumble's two ids. Every +// assignment must land under the RECORD id. -const SLUG = "test-youtube"; -const VIDEO = "20240101_test1234567"; +const SLUG = "tagchan"; +const SMALL = "20240101_small0000"; +const BIG = "20240102_bigdesc000"; +const RUMBLE_DIR = "a-rumble-url-slug"; +const RUMBLE_ID = "v2embedid"; type TagsFile = { version: number; - tags: { id: string; label: string; rules?: { id: string; kind: string }[] }[]; + tags: { + id: string; + label: string; + color?: string; + hidden?: boolean; + rules?: { id: string; kind: string }[]; + }[]; assignments: Record< string, { @@ -33,6 +50,13 @@ type TagsFile = { const tagsFile = () => readJson<TagsFile>("test-transcripts/tags.json"); +async function withIndex(page: Page) { + await writeSite("testsite", { + channels: [{ slug: SLUG, groupId: "default" }], + }); + await buildIndex(page); +} + async function addTag( page: Page, id: string, @@ -54,10 +78,16 @@ async function addTag( await expect(page.getByTestId("tags-saved")).toBeVisible(); } +async function pinFromPanel(page: Page, tag: string) { + const panel = page.getByTestId("video-tags-panel"); + await panel.getByLabel("add tag").selectOption(tag); + await panel.getByRole("button", { name: "pin selected tag" }).click(); +} + test("a tag is defined, edited and removed, and the file holds what the form said", async ({ page, }) => { - await resetData("one-youtube-channel-with-data"); + await resetData("curated-tags-channel"); await addTag(page, "eva-collab", "Collab"); let file = await tagsFile(); @@ -87,7 +117,7 @@ test("a tag is defined, edited and removed, and the file holds what the form sai test("an invalid id is refused by name, and a broken regex is kept with its reason", async ({ page, }) => { - await resetData("one-youtube-channel-with-data"); + await resetData("curated-tags-channel"); await page.goto("/tags"); await page.getByRole("button", { name: "add tag", exact: true }).click(); const card = page.getByTestId("tag-card").last(); @@ -117,45 +147,89 @@ test("an invalid id is refused by name, and a broken regex is kept with its reas test("a rule previews against the index, and Pin writes the operator's provenance", async ({ page, }) => { - await resetData("one-youtube-channel-with-data"); - await writeSite("testsite", { - channels: [{ slug: SLUG, groupId: "default" }], - }); - await buildIndex(page); - await addTag(page, "synthetic", "Synthetic", { pattern: "synthetic" }); + 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(); const row = page.getByTestId("preview-row").first(); - await expect(row).toContainText("Synthetic Test Video"); + await expect(row).toContainText("A Small Synthetic Video"); await expect(row).toContainText(SLUG); // A preview PERSISTS NOTHING — rule hits are derived at build time. expect((await tagsFile()).assignments).toEqual({}); - await row - .getByRole("button", { name: `pin ${VIDEO}`, exact: true }) - .click(); + await row.getByRole("button", { name: `pin ${SMALL}`, exact: true }).click(); await expect(row).toContainText("pinned"); const file = await tagsFile(); - expect(file.assignments[`${SLUG}/${VIDEO}`].manual).toEqual(["synthetic"]); - expect(file.assignments[`${SLUG}/${VIDEO}`].sources?.synthetic.source).toBe( + expect(file.assignments[`${SLUG}/${SMALL}`].manual).toEqual(["synthetic"]); + expect(file.assignments[`${SLUG}/${SMALL}`].sources?.synthetic.source).toBe( "operator", ); }); -test("the video page shows provenance and turns a rule hit into a suppression", async ({ +// THE COMPRESSION REGRESSION, from both ends. +// +// buildIndex writes the index compressed; a reader that opens it without +// `compression: true` throws "Data read, but end of buffer not reached" for +// every value over ~1 KB. That is every real description and every transcript — +// so the preview found nothing, and because the video page awaits the same +// reader during its render, one rule was enough to 500 every video-detail page. +test("a video whose description and transcript are large previews and renders", async ({ page, }) => { - await resetData("one-youtube-channel-with-data"); - await writeSite("testsite", { - channels: [{ slug: SLUG, groupId: "default" }], + await resetData("curated-tags-channel"); + await withIndex(page); + + // The caption side: the needle is in the last cue of a forty-one-cue + // transcript, so a match proves the whole compressed cue array decoded. + await addTag(page, "eva-topic", "Discussed", { + pattern: "elfpire out loud", + kind: "caption", }); - await buildIndex(page); - await addTag(page, "synthetic", "Synthetic", { pattern: "synthetic" }); + await page.goto("/tags"); + await page + .getByTestId("tag-card") + .first() + .getByRole("button", { name: "preview eva-topic" }) + .click(); + const preview = page.getByTestId("tag-preview"); + await expect(preview.getByTestId("preview-row")).toHaveCount(1); + await expect(preview).toContainText("A Long Synthetic Video"); + // A record that could not be read is COUNTED and said out loud. There is + // nothing to say here, and if there were, this is where it would appear. + await expect(preview).not.toContainText("could not be read"); + + // The metadata side: the same video's 1.8 KB description. + await addTag(page, "eva-meta", "Metadata", { pattern: "guest tonight" }); + await page.goto("/tags"); + const metaCard = page.locator( + '[data-testid="tag-card"][data-tag="eva-meta"]', + ); + await metaCard.getByRole("button", { name: "preview eva-meta" }).click(); + await expect( + metaCard.getByTestId("preview-row").filter({ hasText: "A Long Synthetic" }), + ).toHaveCount(1); + + // And the page that awaits the same reader for one key. + await page.goto(`/channels/${SLUG}/videos/${BIG}`); + const panel = page.getByTestId("video-tags-panel"); + await expect(panel).toBeVisible(); + await expect( + panel.getByTestId("video-tag").filter({ hasText: "Discussed" }), + ).toHaveAttribute("data-state", "rule"); +}); + +test("the video page shows provenance and turns a rule hit into a suppression", async ({ + page, +}) => { + await resetData("curated-tags-channel"); + await withIndex(page); + await addTag(page, "synthetic", "Synthetic", { pattern: "small synthetic" }); - await page.goto(`/channels/${SLUG}/videos/${VIDEO}`); + await page.goto(`/channels/${SLUG}/videos/${SMALL}`); const panel = page.getByTestId("video-tags-panel"); const tag = panel.getByTestId("video-tag").filter({ hasText: "Synthetic" }); // No pin yet: it is here because the RULE fires, and the panel says so. @@ -168,9 +242,9 @@ test("the video page shows provenance and turns a rule hit into a suppression", ).toHaveAttribute("data-state", "suppressed"); const file = await tagsFile(); - expect(file.assignments[`${SLUG}/${VIDEO}`].suppressed).toEqual(["synthetic"]); - expect(file.assignments[`${SLUG}/${VIDEO}`].manual ?? []).toEqual([]); - expect(file.assignments[`${SLUG}/${VIDEO}`].sources?.synthetic.source).toBe( + expect(file.assignments[`${SLUG}/${SMALL}`].suppressed).toEqual(["synthetic"]); + expect(file.assignments[`${SLUG}/${SMALL}`].manual ?? []).toEqual([]); + expect(file.assignments[`${SLUG}/${SMALL}`].sources?.synthetic.source).toBe( "operator", ); @@ -181,26 +255,69 @@ test("the video page shows provenance and turns a rule hit into a suppression", .getByRole("button", { name: "restore synthetic" }) .click(); await expect - .poll(async () => (await tagsFile()).assignments[`${SLUG}/${VIDEO}`]) + .poll(async () => (await tagsFile()).assignments[`${SLUG}/${SMALL}`]) .toBeUndefined(); }); +// THE TWO-ID VIDEO, through all three surfaces. +// +// Its directory is `a-rumble-url-slug`; its record is `v2embedid`. Every +// assignment must be keyed by the record id — a pin under the directory name is +// a key the index, the export and every other reader will never look up — while +// every URL and every list row still uses the directory name. +test("a video whose record id is not its directory name is tagged by the record id", async ({ + page, +}) => { + await resetData("curated-tags-channel"); + await withIndex(page); + await addTag(page, "eva-collab", "Collab"); + + // 1. the video page. + await page.goto(`/channels/${SLUG}/videos/${RUMBLE_DIR}`); + await expect(page.getByTestId("video-tags-panel")).toContainText(RUMBLE_ID); + await pinFromPanel(page, "eva-collab"); + await expect + .poll(async () => Object.keys((await tagsFile()).assignments)) + .toEqual([`${SLUG}/${RUMBLE_ID}`]); + + // 2. the list: the chip counts it, and the row it filters to is the DIRECTORY. + await page.goto(`/channels/${SLUG}/videos`); + const chip = page.getByRole("button", { name: "tag eva-collab", exact: true }); + await expect(chip).toContainText("Collab 1"); + await chip.click(); + const list = page.getByRole("list", { name: "videos" }); + await expect(list).toContainText(RUMBLE_DIR); + await expect(list).not.toContainText(SMALL); + + // 3. the bulk bar, which is handed directory names and must send record ids. + await page.getByLabel(`select ${RUMBLE_DIR}`).check(); + await page.getByLabel("bulk action", { exact: true }).selectOption("untag"); + await page.getByLabel("bulk tag", { exact: true }).selectOption("eva-collab"); + await page.getByLabel("apply bulk action").click(); + await expect.poll(async () => (await tagsFile()).assignments).toEqual({}); +}); + test("the bulk bar tags the selection in one write, and the chip filters the list", async ({ page, }) => { - await resetData("one-youtube-channel-with-data"); + await resetData("curated-tags-channel"); await addTag(page, "eva-collab", "Collab"); await page.goto(`/channels/${SLUG}/videos`); - await page.getByLabel(`select ${VIDEO}`).check(); + await page.getByLabel(`select ${SMALL}`).check(); await page.getByLabel("bulk action", { exact: true }).selectOption("tag"); - await page.getByLabel("bulk tag").selectOption("eva-collab"); + await page.getByLabel("bulk tag", { exact: true }).selectOption("eva-collab"); + // UNTAG IS NOT THE OPPOSITE OF TAG, and the bar says so wherever a tag op is + // selected: the chips show pins, and a rule's hit is not one. + await expect(page.getByLabel("bulk tag hint")).toContainText( + "Untag unpins; a rule that still matches keeps the tag", + ); await page.getByLabel("apply bulk action").click(); // THE FILE IS THE ASSERTION, not the bar: a summary bulk clears the selection // on success, and the bar (with its result line) is rendered only while // something is selected. That is how every other bulk action here behaves. await expect - .poll(async () => (await tagsFile()).assignments[`${SLUG}/${VIDEO}`]?.manual) + .poll(async () => (await tagsFile()).assignments[`${SLUG}/${SMALL}`]?.manual) .toEqual(["eva-collab"]); await expect(page.getByLabel("bulk action bar")).toBeHidden(); @@ -209,25 +326,104 @@ test("the bulk bar tags the selection in one write, and the chip filters the lis const chip = page.getByRole("button", { name: "tag eva-collab", exact: true }); await expect(chip).toContainText("Collab 1"); await chip.click(); - await expect(page.getByRole("list", { name: "videos" })).toContainText( - VIDEO, - ); + await expect(page.getByRole("list", { name: "videos" })).toContainText(SMALL); await expect(page).toHaveURL(/tg=eva-collab/); - // Untag, and the chip has nothing left to offer. - await page.getByLabel(`select ${VIDEO}`).check(); - await page.getByLabel("bulk action", { exact: true }).selectOption("untag"); - await page.getByLabel("bulk tag").selectOption("eva-collab"); + // Suppress is its own bulk op, and it is not "untag twice": it REJECTS, so a + // rule that matches this video stops putting the tag on it. + await page.getByLabel(`select ${SMALL}`).check(); + await page + .getByLabel("bulk action", { exact: true }) + .selectOption("suppress_tag"); + await page.getByLabel("bulk tag", { exact: true }).selectOption("eva-collab"); await page.getByLabel("apply bulk action").click(); await expect - .poll(async () => (await tagsFile()).assignments) - .toEqual({}); + .poll( + async () => + (await tagsFile()).assignments[`${SLUG}/${SMALL}`]?.suppressed, + ) + .toEqual(["eva-collab"]); + expect( + (await tagsFile()).assignments[`${SLUG}/${SMALL}`].manual ?? [], + ).toEqual([]); +}); + +test("Pin all skips the rows the operator already suppressed", async ({ + page, +}) => { + await resetData("curated-tags-channel"); + await withIndex(page); + // Matches the long video and the rumble one — both descriptions say elfpire. + await addTag(page, "eva-collab", "Collab", { pattern: "elfpire" }); + + await page.goto("/tags"); + const card = page.getByTestId("tag-card").first(); + await card.getByRole("button", { name: "preview eva-collab" }).click(); + await expect(page.getByTestId("preview-row")).toHaveCount(2); + + // Reject one of them. + await page + .getByTestId("preview-row") + .filter({ hasText: "Rumble" }) + .getByRole("button", { name: `suppress ${RUMBLE_ID}`, exact: true }) + .click(); + await expect + .poll( + async () => + (await tagsFile()).assignments[`${SLUG}/${RUMBLE_ID}`]?.suppressed, + ) + .toEqual(["eva-collab"]); + + // PIN ALL MUST NOT UNDO THAT. `add` clears a suppression of the same tag, so + // pinning the whole page would silently reverse every judgement the operator + // had just made — which is the work a preview exists to collect. + const pinAll = card.getByRole("button", { name: "pin all eva-collab" }); + await expect(pinAll).toContainText("skips 1 suppressed"); + await pinAll.click(); + await expect + .poll(async () => (await tagsFile()).assignments[`${SLUG}/${BIG}`]?.manual) + .toEqual(["eva-collab"]); + expect( + (await tagsFile()).assignments[`${SLUG}/${RUMBLE_ID}`].suppressed, + ).toEqual(["eva-collab"]); + expect( + (await tagsFile()).assignments[`${SLUG}/${RUMBLE_ID}`].manual ?? [], + ).toEqual([]); +}); + +test("a definition deleted while it carries pins is confirmed, and its orphans are named", async ({ + page, +}) => { + await resetData("curated-tags-channel"); + await addTag(page, "eva-collab", "Collab"); + await page.goto(`/channels/${SLUG}/videos/${SMALL}`); + await pinFromPanel(page, "eva-collab"); + await expect + .poll(async () => (await tagsFile()).assignments[`${SLUG}/${SMALL}`]?.manual) + .toEqual(["eva-collab"]); + + await page.goto("/tags"); + page.once("dialog", (d) => { + expect(d.message()).toContain("keeps them on those videos"); + void d.accept(); + }); + await page.getByRole("button", { name: "remove tag eva-collab" }).click(); + await page.getByRole("button", { name: "save tags" }).click(); + await expect(page.getByTestId("tags-saved")).toBeVisible(); + + // The pin survives — an assignment is a fact about a video — and /tags is the + // only surface that can still show it, so it does. + expect((await tagsFile()).assignments[`${SLUG}/${SMALL}`].manual).toEqual([ + "eva-collab", + ]); + await page.reload(); + await expect(page.getByTestId("tags-orphans")).toContainText("eva-collab"); }); -test("a site overlays presentation and cannot touch rules or assignments", async ({ +test("a site overlays presentation and cannot touch rules, assignments or a label it did not set", async ({ page, }) => { - await resetData("one-youtube-channel-with-data"); + await resetData("curated-tags-channel"); await writeSite("testsite", { channels: [{ slug: SLUG, groupId: "default" }], }); @@ -235,9 +431,9 @@ test("a site overlays presentation and cannot touch rules or assignments", async await page.goto("/sites/testsite/tags"); const overlay = page.getByTestId("tag-overlay").first(); - await overlay - .getByLabel("label for eva-collab", { exact: true }) - .fill("On mic"); + // COLOUR ONLY. Nothing here says anything about the label, and the tag must + // not come out of the merge renamed to its id. + await overlay.getByLabel("colour for eva-collab").fill("#b48ead"); await overlay.getByLabel("visibility for eva-collab").selectOption("true"); await page.getByRole("button", { name: "save site tags" }).click(); await expect(page.getByTestId("site-tags-saved")).toBeVisible(); @@ -246,7 +442,7 @@ test("a site overlays presentation and cannot touch rules or assignments", async "test-transcripts/sites/testsite/tags.json", ); expect(site.tags).toEqual([ - { id: "eva-collab", label: "On mic", hidden: true }, + { id: "eva-collab", label: "Collab", color: "#b48ead", hidden: true }, ]); // The corpus rule is untouched, and the site file carries no rules and no // assignments at all — a record is shared by every site that carries its diff --git a/umtool/e2e/fixtures/make-fixture.mjs b/umtool/e2e/fixtures/make-fixture.mjs @@ -1035,6 +1035,17 @@ writeProject( // slug a cue directory is named for, while siteVideo/siteChannel say // what the published archive calls it. Rumble's two ids, and the // editor must be asked with the archive's. +// A project whose manifest names NO channel — neither on the clip nor in its +// provenance. The tag route must say which clips it could not name rather than +// quietly tagging a smaller set than the operator asked for. +writeProject( + "editor-tag-nochannel-fixture", + manifest("editor-tag-nochannel-fixture", "The Unnamed Channel Fixture", { channelSlug: null, siteOrigin: "https://archive.example" }, [ + { type: "clip", id: "a01", video: "vid1", channel: "testchan", start: 3.0, end: 6.0, cite: 3, section: 0, quote: "this one is named" }, + { type: "clip", id: "a02", video: "vid2", start: 1.0, end: 4.0, cite: 1, section: 0, quote: "this one is not" }, + ]), +); + writeProject( "editor-tag-fixture", manifest("editor-tag-fixture", "The Editor Tag Fixture", { siteOrigin: "https://archive.example" }, [ diff --git a/umtool/e2e/report-tag-via-editor.spec.ts b/umtool/e2e/report-tag-via-editor.spec.ts @@ -132,3 +132,34 @@ test("a section is its own scope, and the control on the project page uses it", expect(ui[0].tag).toBe("eva-collab"); expect(ui[0].videos).toEqual([{ slug: "testchan", id: "vid1" }]); }); + +// A CLIP THE TOOL CANNOT NAME IS NAMED BACK. +// +// A clip carries its channel, or the manifest's provenance does. When neither +// does there is no archive key to send, and the honest answer is to say which +// clips those were — a tag the operator believes covers the whole report, +// quietly covering less of it, is the failure this reports instead. +test("clips with no channel are listed, not silently dropped", async ({ + request, + baseURL, +}) => { + const project = "reports/editor-tag-nochannel-fixture"; + const read = await request.get( + `${baseURL}/api/report/tag?project=${encodeURIComponent(project)}`, + ); + expect(read.status(), await read.text()).toBe(200); + const state = (await read.json()) as { videos: number; missing: string[] }; + expect(state.videos).toBe(1); + expect(state.missing).toEqual(["a02"]); + + const post = await request.post(`${baseURL}/api/report/tag`, { + data: { project, tag: "eva-collab" }, + }); + expect(post.status(), await post.text()).toBe(200); + const answer = (await post.json()) as { + videos: number; + missing?: string[]; + }; + expect(answer.videos).toBe(1); + expect(answer.missing).toEqual(["a02"]); +});