commit a8c66c780b41c336dabbea5d5ec6c6f8b65c0969
parent c1c69a59fece812f57a089c6a04c5cde549b5672
Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st>
Date: Mon, 21 Sep 2026 13:43:04 -0400
tags review: refuse an undefined tag, and stop leaking the vocabulary into git
Three fixes from the review of the slice.
applyTagAssignments never checked that the tag EXISTS. A typo — eva-collabs —
would ride on records for ever: no /tags.json can show a tag with no
definition, so nothing would ever surface the mistake. `add` and `suppress` now
refuse with the fix in the message ("define it first"); `remove` and
`unsuppress` deliberately do not check, because they only take a claim away and
an operator must be able to clean up after a deleted definition. Both halves
tested.
.gitignore gained /export/public/tags.json beside duplicates.json — a compose
run was leaving the published vocabulary as untracked noise in the public repo.
dedupeRefs was quadratic (keys.includes) on a path whose whole point is bulk:
Set-backed now, order preserved.
Also written down where the next reader will need them: that the store's
read-modify-write is atomic in-process only because the function is
synchronous — an await between the read and the write would make concurrent
bulk tagging lossy — and that a site-layer RULE never tags a record. `ruleApplies`
is exported from curatedTags.ts for the index build to reuse instead of keeping
a second copy of the scope predicate.
Co-Authored-By: Claude Opus <noreply@anthropic.com>
Diffstat:
4 files changed, 96 insertions(+), 8 deletions(-)
diff --git a/.gitignore b/.gitignore
@@ -63,6 +63,7 @@ yarn-error.log*
/export/public/chart-templates.json
/export/public/search-aliases.json
/export/public/duplicates.json
+/export/public/tags.json
/export/public/site.json
/export/public/_headers
/export/public/sw.js
diff --git a/common/lib/curatedTags.ts b/common/lib/curatedTags.ts
@@ -469,7 +469,14 @@ export function compileTagRules(defs: CuratedTagDef[]): CompiledTagRules {
// Channel scope and date range are cheap string work and are applied BEFORE any
// regex runs — the whole point of scoping a rule to one channel is not paying
// for it on the other twenty-nine.
-function ruleApplies(rule: CompiledTagRule, input: TagRuleInput): boolean {
+// Exported because the index build asks the same question one step earlier — to
+// decide whether a video's cues are worth decoding AT ALL
+// (curatedTagsIndex.ts). A second copy of this predicate could disagree with
+// the evaluation it is supposed to be predicting.
+export function ruleApplies(
+ rule: CompiledTagRule,
+ input: { channelSlug: string; uploadDate?: string },
+): boolean {
if (rule.channels && !rule.channels.has(input.channelSlug)) return false;
if (rule.dateFrom || rule.dateTo) {
const date = (input.uploadDate ?? "").replace(/\D/g, "");
diff --git a/common/lib/curatedTagsStore.test.ts b/common/lib/curatedTagsStore.test.ts
@@ -57,6 +57,17 @@ const COLLAB: CuratedTagsConfig = {
assignments: {},
};
+// applyTagAssignments refuses to pin or suppress a tag the vocabulary does not
+// define, so most of these tests need one. Kept explicit per test rather than
+// seeded globally — the refusal itself is a test below.
+function seedVocab(paths: Paths, ...ids: string[]): void {
+ writeGlobalTags(paths, {
+ version: 1,
+ tags: ids.map((id) => ({ id, label: id })),
+ assignments: {},
+ });
+}
+
const V1 = { channelSlug: "legal-mindset", id: "XZqL6k9IHGA" };
const V2 = { channelSlug: "legal-mindset", id: "AAAAAAAAAAA" };
@@ -214,8 +225,44 @@ test("applyTagAssignments rejects a bad op, tag id or video ref", () => {
});
});
+test("add and suppress refuse a tag the vocabulary does not define", () => {
+ // The typo guard. Without it `eva-collabs` would ride on records for ever:
+ // no /tags.json can show a tag that has no definition, so nothing would
+ // surface the mistake.
+ withPaths((paths) => {
+ seedVocab(paths, "eva-collab");
+ for (const op of ["add", "suppress"] as const) {
+ assert.throws(
+ () => applyTagAssignments(paths, { op, tag: "eva-collabs", videos: [V1] }),
+ /unknown tag "eva-collabs" — define it first/,
+ op,
+ );
+ }
+ assert.deepEqual(readGlobalTags(paths).assignments, {});
+ });
+});
+
+test("remove and unsuppress still work for a tag whose def was deleted", () => {
+ // The other half of the guard: those two only ever take a claim away, and an
+ // operator must be able to clean up after deleting a definition.
+ withPaths((paths) => {
+ writeGlobalTags(paths, {
+ version: 1,
+ tags: [],
+ assignments: {
+ "legal-mindset/XZqL6k9IHGA": { manual: ["gone"] },
+ "legal-mindset/AAAAAAAAAAA": { suppressed: ["gone"] },
+ },
+ });
+ applyTagAssignments(paths, { op: "remove", tag: "gone", videos: [V1] });
+ applyTagAssignments(paths, { op: "unsuppress", tag: "gone", videos: [V2] });
+ assert.deepEqual(readGlobalTags(paths).assignments, {});
+ });
+});
+
test("applyTagAssignments lowercases the tag id it stores", () => {
withPaths((paths) => {
+ seedVocab(paths, "eva-collab");
applyTagAssignments(paths, { op: "add", tag: " EVA-Collab ", videos: [V1] });
assert.deepEqual(readGlobalTags(paths).assignments["legal-mindset/XZqL6k9IHGA"].manual, [
"eva-collab",
@@ -227,6 +274,7 @@ test("applyTagAssignments lowercases the tag id it stores", () => {
test("add pins with provenance and is idempotent", () => {
withPaths((paths) => {
+ seedVocab(paths, "eva-collab");
const first = applyTagAssignments(paths, {
op: "add",
tag: "eva-collab",
@@ -266,6 +314,7 @@ test("add pins with provenance and is idempotent", () => {
test("add clears a suppression of the same tag", () => {
withPaths((paths) => {
+ seedVocab(paths, "eva-collab");
applyTagAssignments(paths, { op: "suppress", tag: "eva-collab", videos: [V1] });
applyTagAssignments(paths, { op: "add", tag: "eva-collab", videos: [V1] });
const a = assignmentFor(readGlobalTags(paths), V1.channelSlug, V1.id)!;
@@ -276,6 +325,7 @@ test("add clears a suppression of the same tag", () => {
test("remove unpins only — a rule hit survives it, which is why suppress exists", () => {
withPaths((paths) => {
+ seedVocab(paths, "eva-collab");
applyTagAssignments(paths, { op: "add", tag: "eva-collab", videos: [V1] });
applyTagAssignments(paths, { op: "remove", tag: "eva-collab", videos: [V1] });
const cfg = readGlobalTags(paths);
@@ -287,6 +337,7 @@ test("remove unpins only — a rule hit survives it, which is why suppress exist
test("suppress rejects a rule-derived tag and records provenance", () => {
withPaths((paths) => {
+ seedVocab(paths, "eva-topic");
const res = applyTagAssignments(paths, {
op: "suppress",
tag: "eva-topic",
@@ -307,6 +358,7 @@ test("suppress rejects a rule-derived tag and records provenance", () => {
test("suppress also unpins, and unsuppress clears the rejection", () => {
withPaths((paths) => {
+ seedVocab(paths, "eva-collab");
applyTagAssignments(paths, { op: "add", tag: "eva-collab", videos: [V1] });
applyTagAssignments(paths, { op: "suppress", tag: "eva-collab", videos: [V1] });
let a = assignmentFor(readGlobalTags(paths), V1.channelSlug, V1.id)!;
@@ -333,6 +385,7 @@ test("an op that changes nothing writes no file at all", () => {
test("other tags on the same video, and other videos, are untouched", () => {
withPaths((paths) => {
+ seedVocab(paths, "eva-collab", "eva-topic");
applyTagAssignments(paths, { op: "add", tag: "eva-collab", videos: [V1, V2] });
applyTagAssignments(paths, { op: "add", tag: "eva-topic", videos: [V1] });
applyTagAssignments(paths, { op: "suppress", tag: "eva-collab", videos: [V1] });
@@ -356,6 +409,7 @@ test("the tag vocabulary survives an assignment write", () => {
test("a bulk call is ONE write, and reports exactly the keys it changed", () => {
withPaths((paths) => {
+ seedVocab(paths, "eva-collab");
const many = Array.from({ length: 50 }, (_, i) => ({
channelSlug: "legal-mindset",
id: `v${i}`,
@@ -380,7 +434,10 @@ test("provenance for a tag that is neither pinned nor suppressed is pruned", ()
paths.globalTagsFile,
JSON.stringify({
version: 1,
- tags: [],
+ tags: [
+ { id: "eva-collab", label: "Collab" },
+ { id: "eva-topic", label: "Discussed" },
+ ],
assignments: {
"legal-mindset/XZqL6k9IHGA": {
manual: ["eva-collab"],
diff --git a/common/lib/curatedTagsStore.ts b/common/lib/curatedTagsStore.ts
@@ -149,8 +149,11 @@ export type ApplyTagAssignmentsResult = {
touched: string[];
};
+// Order-preserving de-dupe, Set-backed: a bulk "tag selected" can carry
+// thousands of refs, and `keys.includes` would make that quadratic.
function dedupeRefs(videos: TagVideoRef[]): string[] {
const keys: string[] = [];
+ const seen = new Set<string>();
for (const v of videos) {
if (!v || typeof v.channelSlug !== "string" || typeof v.id !== "string") {
throw new Error("each video needs a channelSlug and an id");
@@ -163,7 +166,9 @@ function dedupeRefs(videos: TagVideoRef[]): string[] {
);
}
const key = assignmentKey(slug, id);
- if (!keys.includes(key)) keys.push(key);
+ if (seen.has(key)) continue;
+ seen.add(key);
+ keys.push(key);
}
return keys;
}
@@ -173,11 +178,21 @@ function withoutTag(list: string[] | undefined, tag: string): string[] {
}
// Apply one op for one tag to any number of videos, in a SINGLE atomic write of
-// the global file. Validates the tag id and the op, and records provenance for
-// every pin and suppression it writes. Throws on invalid input (an unknown op,
-// a malformed tag id or video ref) — this is the one write path all three
-// writers (editor UI, ops route, umtool) funnel through, so it refuses rather
-// than silently storing junk a sanitize pass would drop later.
+// the global file. Validates the tag id, the op and the video refs, checks the
+// tag EXISTS in the vocabulary for the two ops that add a claim, and records
+// provenance for every pin and suppression it writes. It throws rather than
+// silently storing junk a sanitize pass would drop later: this is the one write
+// path all three writers (editor UI, ops route, umtool) funnel through, and a
+// mistyped id here would put a tag on records that no /tags.json can ever show.
+//
+// `remove` and `unsuppress` deliberately skip the vocabulary check — they only
+// ever take a claim away, and an operator must be able to clean up after a tag
+// whose definition has already been deleted.
+//
+// The read-modify-write below is atomic against other writers in THIS process
+// only because this function is synchronous end to end — there is no await
+// between readGlobalTags and writeGlobalTags for another handler to interleave
+// in. Introducing one would silently make concurrent bulk tagging lossy.
//
// Returns which keys changed, so the caller can dirty exactly those videos.
export function applyTagAssignments(
@@ -202,6 +217,14 @@ export function applyTagAssignments(
const at = input.at ?? new Date().toISOString();
const config = readGlobalTags(paths);
+ if (
+ (op === "add" || op === "suppress") &&
+ !config.tags.some((def) => def.id === tag)
+ ) {
+ throw new Error(
+ `unknown tag "${tag}" — define it first (pnpm ops tags / the /tags page)`,
+ );
+ }
const changed: string[] = [];
for (const key of keys) {
