commit 29bfb7c42d2c4b1f110c0848fb0fa73c61189a24
parent 314d48162e881e53d52deb7433006831d7292d3f
Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st>
Date: Mon, 21 Sep 2026 14:24:44 -0400
tags: a site overlay that only recolours must not rename the tag
The model fix the S3 review is owed. `coerceTagDef` filled a missing `label`
with the id, and because the store sanitizes on write AND on read, a site row
that set nothing but a colour arrived at mergeTagDefs carrying `label:
"eva-collab"` and renamed the tag on that site — where the corpus says
"Collab". S3 worked around it by mirroring the corpus label into the site row,
which bought correctness today at the price of a rename that never followed
until someone re-saved the tab.
So `sanitizeTagsConfig(raw, {layer})`: at "global" a definition still ends up
named after its id, at "site" a missing presentation field STAYS missing.
readSiteTags/writeSiteTags pass the site layer in both directions, because the
file goes through sanitize twice. `CuratedTagDef.label` is optional now and the
display fallback is `label ?? id` at every consumer; the one place it is baked
in is publishedTagsFrom, because /tags.json is read, not layered.
The editor's mirroring is gone, and so is the site tab's "keeps the label it
had when you saved" copy — a blank field now follows the corpus for ever, which
is what the tab says. SiteTagsClient loses the cast it needed when `label` was
required.
Tests: sanitize at the site layer keeps a label absent and round-trips;
a colour-only overlay keeps the corpus label through mergeTagDefs and follows a
later rename; the same through the store's two sanitize passes; and the
integration test composes a colour-only overlay and asserts /tags.json
publishes "Collab", then renames in the corpus and asserts the site follows
with no site edit. The editor spec asserted the workaround — it now asserts the
site file carries no label at all, and that the rename reaches the tab.
Co-Authored-By: Claude Opus <noreply@anthropic.com>
Diffstat:
13 files changed, 217 insertions(+), 47 deletions(-)
diff --git a/common/controller/curatedTagsBuild.test.ts b/common/controller/curatedTagsBuild.test.ts
@@ -29,7 +29,7 @@ process.env.SETTINGS_FILE = path.join(ROOT, "settings.json");
const { getPaths } = await import("../lib/paths");
const { buildIndex } = await import("./buildIndex");
-const { writeGlobalTags } = await import("../lib/curatedTagsStore");
+const { writeGlobalTags, writeSiteTags } = await import("../lib/curatedTagsStore");
const {
curatedTagsRuntime,
reapplyCuratedTags,
@@ -226,6 +226,50 @@ test("compose 1: the site publishes /tags.json and corpus.json points at it", as
);
});
+test("compose 1b: a colour-only site overlay publishes the CORPUS label", async () => {
+ // The site file is written through the same store the editor uses, so this
+ // covers the whole path the overlay takes: sanitize-on-write, sanitize-on-
+ // read, mergeTagDefs, publishedTagsFrom. A row that only recolours must not
+ // rename the tag to its id on this site.
+ writeSiteTags(paths, SITE, {
+ version: 1,
+ tags: [{ id: "eva-collab", color: "#b48ead", order: 3 }],
+ assignments: {},
+ });
+ await composeSite();
+ const published = JSON.parse(
+ readFileSync(path.join(paths.exportPublicDir, "tags.json"), "utf8"),
+ );
+ assert.deepEqual(published.tags, [
+ {
+ id: "eva-collab",
+ label: "Collab",
+ group: "eva",
+ groupLabel: "Eva",
+ color: "#b48ead",
+ order: 3,
+ count: 1,
+ channels: { [CHANNEL]: 1 },
+ },
+ ]);
+
+ // …and a corpus rename follows onto the site with no site edit at all.
+ writeGlobalTags(paths, {
+ ...EVA,
+ tags: [{ ...EVA.tags[0], label: "On mic" }],
+ });
+ await composeSite();
+ assert.equal(
+ JSON.parse(
+ readFileSync(path.join(paths.exportPublicDir, "tags.json"), "utf8"),
+ ).tags[0].label,
+ "On mic",
+ );
+ // Put the fixture back for the tests below.
+ writeGlobalTags(paths, EVA);
+ writeSiteTags(paths, SITE, { version: 1, tags: [], assignments: {} });
+});
+
test("build 3 + compose 2: dropping the vocabulary removes the file and the pointer", async () => {
writeGlobalTags(paths, { version: 1, tags: [], assignments: {} });
await build();
diff --git a/common/controller/curatedTagsIndex.ts b/common/controller/curatedTagsIndex.ts
@@ -498,7 +498,10 @@ export function publishedTagsFrom(
})
.map(({ def, entry }) => ({
id: def.id,
- label: def.label,
+ // The published document always carries a label: the wire format is read,
+ // not layered, so an un-overridden tag falls back to its id exactly once —
+ // here — and never by a sanitize pass inventing one on the site file.
+ label: def.label ?? def.id,
...(def.group ? { group: def.group } : {}),
...(def.groupLabel ? { groupLabel: def.groupLabel } : {}),
...(def.color ? { color: def.color } : {}),
diff --git a/common/lib/curatedTags.test.ts b/common/lib/curatedTags.test.ts
@@ -108,6 +108,32 @@ test("sanitizeTagsConfig defaults label to the id and drops empty extras", () =>
assert.deepEqual(cfg.tags[0], { id: "solo", label: "solo" });
});
+test("the SITE layer leaves a missing label ABSENT", () => {
+ // An overlay that only recolours must carry no label. Defaulting it to the id
+ // here would make mergeTagDefs rename the tag on that site — the site would
+ // publish "eva-collab" where the corpus says "Collab" — and because the store
+ // sanitizes on read as well as on write, stripping it later cannot help.
+ const site = sanitizeTagsConfig(
+ { tags: [{ id: "eva-collab", color: "#b48ead" }] },
+ { layer: "site" },
+ );
+ assert.deepEqual(site.tags[0], { id: "eva-collab", color: "#b48ead" });
+ assert.equal("label" in site.tags[0], false);
+ // A site row that DOES set a label keeps it, and is still coerced.
+ const named = sanitizeTagsConfig(
+ { tags: [{ id: "eva-collab", label: " On mic " }] },
+ { layer: "site" },
+ );
+ assert.equal(named.tags[0].label, "On mic");
+ // The global layer is unchanged: a definition always ends up named.
+ assert.equal(
+ sanitizeTagsConfig({ tags: [{ id: "eva-collab" }] }).tags[0].label,
+ "eva-collab",
+ );
+ // And it survives a round trip at its own layer (the store sanitizes twice).
+ assert.deepEqual(sanitizeTagsConfig(site, { layer: "site" }), site);
+});
+
test("sanitizeTagsConfig drops unusable rules but keeps a non-compiling one disabled", () => {
const cfg = sanitizeTagsConfig({
tags: [
@@ -243,6 +269,27 @@ test("mergeTagDefs cannot delete a global rule or a global tag", () => {
);
});
+test("a colour-only overlay keeps the corpus label", () => {
+ // The bug this optionality exists for, end to end through both functions.
+ const site = sanitizeTagsConfig(
+ { tags: [{ id: "eva-collab", color: "#b48ead", order: 9 }] },
+ { layer: "site" },
+ );
+ const merged = mergeTagDefs(DEFS, site.tags);
+ const collab = merged.find((t) => t.id === "eva-collab")!;
+ assert.equal(collab.label, "Collab", "the corpus wording, not the id");
+ assert.equal(collab.color, "#b48ead");
+ assert.equal(collab.order, 9);
+ // A later corpus rename follows with no site edit at all.
+ const renamed = DEFS.map((d) =>
+ d.id === "eva-collab" ? { ...d, label: "On mic" } : d,
+ );
+ assert.equal(
+ mergeTagDefs(renamed, site.tags).find((t) => t.id === "eva-collab")!.label,
+ "On mic",
+ );
+});
+
test("mergeTagDefs appends a site-only tag whole", () => {
const siteOnly: CuratedTagDef = {
id: "site-thing",
diff --git a/common/lib/curatedTags.ts b/common/lib/curatedTags.ts
@@ -25,6 +25,10 @@ export const TAG_ID_RE = /^[a-z0-9][a-z0-9._-]*$/;
export const CURATED_TAGS_VERSION = 1;
+// Which file a config came out of. The two layers differ in exactly one way —
+// see coerceTagDef — and naming it beats a boolean at every call site.
+export type TagLayer = "global" | "site";
+
export type CuratedTagRuleKind = "metadata" | "chat-author" | "caption";
const RULE_KINDS: readonly CuratedTagRuleKind[] = [
@@ -56,7 +60,11 @@ export type CuratedTagRule = {
export type CuratedTagDef = {
id: string;
- label: string;
+ // OPTIONAL, and the optionality is load-bearing at the SITE layer: an overlay
+ // that only recolours a tag must carry no label, or mergeTagDefs would apply
+ // one and rename the tag on that site. Absent means "whatever the layer below
+ // says", and the display fallback everywhere is `label ?? id`.
+ label?: string;
// Optional grouping so a chip row can read "Eva: Collab · In chat · Discussed".
group?: string;
groupLabel?: string;
@@ -190,11 +198,17 @@ function coerceRule(raw: unknown, index: number): CuratedTagRule | null {
return rule;
}
-function coerceTagDef(raw: unknown): CuratedTagDef | null {
+function coerceTagDef(raw: unknown, layer: TagLayer): CuratedTagDef | null {
if (!raw || typeof raw !== "object") return null;
const r = raw as Record<string, unknown>;
const id = normalizeTagId(r.id);
if (!id) return null;
+ // The one field the two layers treat differently. A corpus definition is the
+ // thing being named, so it always ends up with a label (the id, if nobody
+ // wrote one). A site row is an OVERLAY: a missing label means "do not
+ // 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))
@@ -206,7 +220,7 @@ function coerceTagDef(raw: unknown): CuratedTagDef | null {
: undefined;
return {
id,
- label: trimmedString(r.label) ?? id,
+ ...(label !== undefined ? { label } : {}),
...(trimmedString(r.group) ? { group: trimmedString(r.group) } : {}),
...(trimmedString(r.groupLabel)
? { groupLabel: trimmedString(r.groupLabel) }
@@ -265,7 +279,17 @@ function coerceAssignment(raw: unknown): CuratedTagAssignment | null {
// NEVER throws, and missing/invalid input yields an empty config. The one
// deliberate exception to "drop what is malformed" is a rule whose regex does
// not compile: that rule is kept, disabled, with a reason.
-export function sanitizeTagsConfig(raw: unknown): CuratedTagsConfig {
+//
+// `layer` decides how a missing presentation field is read: at "global" a
+// definition without a label is named after its id, at "site" it stays unnamed
+// so the merge below leaves the corpus wording alone. Callers that hold a
+// site file — readSiteTags/writeSiteTags, the site overlay action — must pass
+// it in BOTH directions, because the file is sanitized on write and on read.
+export function sanitizeTagsConfig(
+ raw: unknown,
+ opts: { layer?: TagLayer } = {},
+): CuratedTagsConfig {
+ const layer: TagLayer = opts.layer ?? "global";
const empty: CuratedTagsConfig = {
version: CURATED_TAGS_VERSION,
tags: [],
@@ -282,7 +306,7 @@ export function sanitizeTagsConfig(raw: unknown): CuratedTagsConfig {
const seen = new Set<string>();
if (Array.isArray(r.tags)) {
for (const entry of r.tags) {
- const def = coerceTagDef(entry);
+ const def = coerceTagDef(entry, layer);
if (!def) continue;
if (seen.has(def.id)) continue; // first definition of an id wins
seen.add(def.id);
diff --git a/common/lib/curatedTagsStore.test.ts b/common/lib/curatedTagsStore.test.ts
@@ -151,6 +151,31 @@ test("effectiveSiteTags layers the site overlay over the corpus vocabulary", ()
});
});
+test("a colour-only overlay survives the store's two sanitize passes", () => {
+ // writeSiteTags sanitizes, readSiteTags sanitizes again — both at the SITE
+ // layer, so the row that set only a colour still carries no label and the
+ // site keeps publishing the corpus wording. A later corpus rename follows
+ // with no site edit.
+ withPaths((paths) => {
+ writeGlobalTags(paths, COLLAB);
+ writeSiteTags(paths, "anilyzer", {
+ version: 1,
+ tags: [{ id: "eva-collab", color: "#b48ead" }],
+ assignments: {},
+ });
+ assert.equal("label" in readSiteTags(paths, "anilyzer").tags[0], false);
+ const defs = effectiveSiteTags(paths, "anilyzer");
+ assert.equal(defs[0].label, "Collab");
+ assert.equal(defs[0].color, "#b48ead");
+
+ writeGlobalTags(paths, {
+ ...COLLAB,
+ tags: [{ ...COLLAB.tags[0], label: "On mic" }],
+ });
+ assert.equal(effectiveSiteTags(paths, "anilyzer")[0].label, "On mic");
+ });
+});
+
test("a site with no overlay ships the corpus vocabulary unchanged", () => {
withPaths((paths) => {
writeGlobalTags(paths, COLLAB);
diff --git a/common/lib/curatedTagsStore.ts b/common/lib/curatedTagsStore.ts
@@ -81,12 +81,18 @@ export function writeGlobalTags(paths: Paths, config: CuratedTagsConfig): void {
writeJsonAtomic(paths.globalTagsFile, sanitizeTagsConfig(config));
}
-// Per-site presentation overlay + site-only rules/tags. Absent/unreadable →
-// empty (no overlay).
+// Per-site presentation overlay + site-only tags. Absent/unreadable → empty
+// (no overlay).
+//
+// Coerced as the SITE layer, on read AND on write: an overlay row that carries
+// no label must keep carrying none, or mergeTagDefs would apply an invented one
+// and rename the tag on this site. Both directions matter because the file goes
+// through sanitize twice — the write, and every read after it.
export function readSiteTags(paths: Paths, siteId: string): CuratedTagsConfig {
try {
return sanitizeTagsConfig(
JSON.parse(readFileSync(siteTagsFile(paths, siteId), "utf8")),
+ { layer: "site" },
);
} catch {
return emptyTagsConfig();
@@ -98,7 +104,10 @@ export function writeSiteTags(
siteId: string,
config: CuratedTagsConfig,
): void {
- writeJsonAtomic(siteTagsFile(paths, siteId), sanitizeTagsConfig(config));
+ writeJsonAtomic(
+ siteTagsFile(paths, siteId),
+ sanitizeTagsConfig(config, { layer: "site" }),
+ );
}
// The tag definitions a site actually ships: the corpus vocabulary with the
diff --git a/editor/app/channels/[slug]/lib/videoTagRows.ts b/editor/app/channels/[slug]/lib/videoTagRows.ts
@@ -28,7 +28,12 @@ export function attachCuratedTags(
rows: VideoRow[],
): { rows: VideoRow[]; defs: TagDef[] } {
const config = readGlobalTags(paths);
- const defs: TagDef[] = config.tags.map((t) => ({ id: t.id, label: t.label }));
+ // `label` is optional on a definition (a site overlay may carry none); a row
+ // in the UI always needs one, and the id is the fallback everywhere.
+ const defs: TagDef[] = config.tags.map((t) => ({
+ id: t.id,
+ label: t.label ?? t.id,
+ }));
const prefix = `${slug}/`;
const mine = new Map<string, string[]>();
for (const [key, assignment] of Object.entries(config.assignments)) {
diff --git a/editor/app/channels/[slug]/videos/[id]/lib/videoTags.ts b/editor/app/channels/[slug]/videos/[id]/lib/videoTags.ts
@@ -146,7 +146,7 @@ async function readVideoTags(
rows,
defs: defs.map((d) => ({
id: d.id,
- label: d.label,
+ label: d.label ?? d.id,
...(d.color ? { color: d.color } : {}),
...(d.group ? { group: d.group } : {}),
...(d.groupLabel ? { groupLabel: d.groupLabel } : {}),
diff --git a/editor/app/sites/[siteId]/tags/page.tsx b/editor/app/sites/[siteId]/tags/page.tsx
@@ -41,9 +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. 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.
+ site's next export build. A field you leave blank keeps following
+ the corpus: rename a tag there and this site follows it, unless you have
+ set your own wording here.
</p>
<SiteTagsClient
key={siteId}
diff --git a/editor/app/sites/components/SiteTagsClient.tsx b/editor/app/sites/components/SiteTagsClient.tsx
@@ -56,14 +56,15 @@ function overlayToDef(row: OverlayRow): CuratedTagDef | null {
}
// mergeTagDefs applies only the fields that are PRESENT, so an overlay with
// no label must not carry `label: ""` — that would blank the corpus label.
- // The type requires `label`, hence the cast on the way out.
- const def: Record<string, unknown> = { id: row.id };
+ // `label` is optional on a definition precisely so this row can omit it and
+ // keep following the corpus wording.
+ const def: CuratedTagDef = { id: row.id };
if (label) def.label = label;
if (groupLabel) def.groupLabel = groupLabel;
if (color) def.color = color;
if (hasOrder) def.order = order;
if (row.hidden !== "") def.hidden = row.hidden === "true";
- return def as unknown as CuratedTagDef;
+ return def;
}
export function SiteTagsClient({
@@ -160,7 +161,7 @@ export function SiteTagsClient({
<input
value={row.label}
onChange={(e) => patch(g.id, { label: e.target.value })}
- placeholder={g.label}
+ placeholder={g.label ?? g.id}
aria-label={`label for ${g.id}`}
className="rounded border border-border bg-card px-2 py-1 text-sm text-foreground"
/>
diff --git a/editor/app/tags/actions.ts b/editor/app/tags/actions.ts
@@ -79,36 +79,16 @@ 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 };
- });
+ // AN OVERLAY MUST NOT RENAME THE TAG IT IS ONLY RECOLOURING — and that is now
+ // the model's job, not this action's. `sanitizeTagsConfig(..., {layer:
+ // "site"})` leaves a missing label ABSENT instead of defaulting it to the id,
+ // and readSiteTags/writeSiteTags coerce at that layer in both directions, so
+ // a row saved with no label of its own keeps following the corpus label for
+ // ever — no mirroring, and nothing that goes stale until the tab is re-saved.
writeSiteTags(paths, siteId, {
...current,
assignments: {},
- tags: sanitizeTagsConfig({ tags: mirrored }).tags,
+ tags: sanitizeTagsConfig({ tags }, { layer: "site" }).tags,
});
revalidatePath(`/sites/${siteId}/tags`);
return { ok: true };
diff --git a/editor/e2e/tags.spec.ts b/editor/e2e/tags.spec.ts
@@ -441,8 +441,12 @@ test("a site overlays presentation and cannot touch rules, assignments or a labe
const site = await readJson<TagsFile>(
"test-transcripts/sites/testsite/tags.json",
);
+ // The row holds ONLY what this site said. No label — not even a copy of the
+ // corpus one — because the site file is coerced at the site layer, where a
+ // missing presentation field stays missing. That is what lets a later corpus
+ // rename follow onto this site with no edit here.
expect(site.tags).toEqual([
- { id: "eva-collab", label: "Collab", color: "#b48ead", hidden: true },
+ { id: "eva-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
@@ -450,4 +454,23 @@ test("a site overlays presentation and cannot touch rules, assignments or a labe
expect(site.assignments).toEqual({});
expect((await tagsFile()).tags[0].rules?.length).toBe(1);
await expect(page.getByText("Corpus rules:")).toBeVisible();
+
+ // And the corpus wording still reaches this site: rename the tag on /tags and
+ // the overlay — which never claimed a label — follows it.
+ await page.goto("/tags");
+ await page.getByTestId("tag-card").first().getByLabel("tag label").fill("On mic");
+ await page.getByRole("button", { name: "save tags" }).click();
+ await expect(page.getByTestId("tags-saved")).toBeVisible();
+ await page.goto("/sites/testsite/tags");
+ await expect(
+ page
+ .getByTestId("tag-overlay")
+ .first()
+ // exact, or this also matches "group label for eva-collab".
+ .getByLabel("label for eva-collab", { exact: true }),
+ ).toHaveAttribute("placeholder", "On mic");
+ expect(
+ (await readJson<TagsFile>("test-transcripts/sites/testsite/tags.json"))
+ .tags[0].label,
+ ).toBeUndefined();
});
diff --git a/plans/FACTS.md b/plans/FACTS.md
@@ -265,6 +265,15 @@ every site carrying the channel, so there is no per-site record for a per-site h
in. Rules bind only from `transcripts/tags.json` — promote one there to make it fire. The
editor therefore offers no site-side rule editor.
+**`CuratedTagDef.label` is OPTIONAL, and that is what makes the overlay work.**
+`sanitizeTagsConfig(raw, {layer})` defaults a missing label to the id at the `global` layer
+and leaves it ABSENT at the `site` layer (`readSiteTags`/`writeSiteTags` pass `site`, in both
+directions, because the file is sanitized on write AND on every read). Without that, a site
+row that set only a colour would arrive at `mergeTagDefs` carrying `label: <id>` and rename
+the tag on that site — publishing "eva-collab" where the corpus says "Collab". Every reader
+therefore renders `label ?? id`; the one place the fallback is baked in is
+`publishedTagsFrom`, because `/tags.json` is read, not layered.
+
**A chat cue's author is a string prefix, not a field.** `common/lib/liveChat.ts:100-105`
emits every live-chat cue as ``text: author ? `${author}: ${text}` : text`` (`:104`) — `Cue` is
`{start, end, text}` (`vtt.ts:1`) and has no `author`. So a `chat-author` rule matches
