From 27462cab1279232e22f0d2ea648248f6ba205784 Mon Sep 17 00:00:00 2001 From: Dune Desormeaux Date: Mon, 10 Aug 2026 14:17:12 +0000 Subject: [PATCH 1/3] fix build by removing 77 from flairs.yml --- tools/sitegen/src/curation/flairs.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/tools/sitegen/src/curation/flairs.yml b/tools/sitegen/src/curation/flairs.yml index fa7b8b7c..4b36eb5f 100644 --- a/tools/sitegen/src/curation/flairs.yml +++ b/tools/sitegen/src/curation/flairs.yml @@ -203,7 +203,6 @@ assignments: "74_Wild_Pebble": [] "75_Turing_Clouds": [] "76_hot_fuzz": [] - "77_Placeholder": [] "78_Talker": [] "81_West_Coast_LPG": [] "82_Computer_Grids": [] From c1258a2ab2f614f5882afbebc1c2759f4dec58e1 Mon Sep 17 00:00:00 2001 From: Dune Desormeaux Date: Mon, 10 Aug 2026 14:25:16 +0000 Subject: [PATCH 2/3] fix npm audit error --- tools/sitegen/package-lock.json | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/sitegen/package-lock.json b/tools/sitegen/package-lock.json index 5fdbc93f..76e4cbb6 100644 --- a/tools/sitegen/package-lock.json +++ b/tools/sitegen/package-lock.json @@ -1622,9 +1622,9 @@ } }, "node_modules/nanoid": { - "version": "3.3.16", - "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.16.tgz", - "integrity": "sha512-bzlKTyNJ7+LdGIIwy8ijFpIqEQIvafahV7eYykJ8Cvh42EdJeODoJ6gUJXpQJvej1BddH8OqTXZNE/KfbWAu8Q==", + "version": "3.3.18", + "resolved": "https://registry.npmjs.org/nanoid/-/nanoid-3.3.18.tgz", + "integrity": "sha512-DTg4MJbGMWkfi6VZFdNt2/caMbQy4Ou+Op/hJQvGEWcnVfoA1QA+xzRKAzw9jD6+GVOOeYr/mIcuDSdug6F6+w==", "funding": [ { "type": "github", From c89efff287b2feb605a17ebc0f808c1e76655def Mon Sep 17 00:00:00 2001 From: Dune Desormeaux Date: Mon, 10 Aug 2026 14:28:08 +0000 Subject: [PATCH 3/3] Enhance curation synchronization and validation logic - Update synchronization to remove stale empty assignments for deleted cards. - Improve validation to handle stale empty assignments and provide warnings. - Add tests for synchronization behavior and validation rules regarding empty assignments. --- documentation/curating-program-card-index.md | 10 ++--- tools/sitegen/src/curation/cli.js | 42 ++++++++++++++------ tools/sitegen/src/validate/prRules.js | 40 ++++++++++++++----- tools/sitegen/test/curation-cli.test.js | 39 ++++++++++++++++++ tools/sitegen/test/pr-rules.test.js | 25 ++++++++++++ 5 files changed, 129 insertions(+), 27 deletions(-) create mode 100644 tools/sitegen/test/curation-cli.test.js diff --git a/documentation/curating-program-card-index.md b/documentation/curating-program-card-index.md index 66128e9c..e98d6993 100644 --- a/documentation/curating-program-card-index.md +++ b/documentation/curating-program-card-index.md @@ -22,9 +22,9 @@ npm install --prefix tools/sitegen npm run sync-curation ``` -This adds an empty assignment for every new card. It does not award a flair: an empty list means that the card has been registered but has not been given an editorial badge. +This adds an empty assignment for every new card and removes empty assignments for deleted cards. It does not award a flair: an empty list means that the card has been registered but has not been given an editorial badge. -The command preserves existing assignments and reports stale entries instead of deleting them. A stale entry usually means that a release folder was renamed or removed and should be investigated before editing `flairs.yml`. +The command preserves stale entries that have editorial flairs so a release deletion cannot silently discard curation. These assigned stale entries must be investigated and cleaned up manually. ## Curating flairs @@ -202,7 +202,7 @@ npm run build The curation check catches: - cards missing from `flairs.yml` -- stale card assignments +- stale card assignments that still have editorial flairs - unknown or duplicate flairs - invalid assignment shapes - missing cards referenced by the hero, embeds, or shelves @@ -219,13 +219,13 @@ Then serve the generated `site/` directory with the usual local preview and insp - the **Browse all cards** archive - tag filtering under **Advanced search** -The GitHub Pages workflow synchronizes missing empty assignments before checking and building. After a card is merged to `main`, the curation workflow commits the synchronized `flairs.yml`. Invalid references, stale assignments, unknown flairs, and malformed curation still fail with an actionable error. +The GitHub Pages workflow adds missing empty assignments and removes stale empty assignments before checking and building. After a card addition or deletion is merged to `main`, the curation workflow commits the synchronized `flairs.yml`. Invalid references, stale assignments with editorial flairs, unknown flairs, and malformed curation still fail with an actionable error. ## Safe editing rules - Change only curation files when making an editorial decision. - Never rename a release folder merely to alter its displayed title. -- Never delete a stale assignment until the corresponding release rename or deletion is understood. +- Never delete a stale assignment with editorial flairs until the corresponding release rename or deletion is understood. - Prefer flair IDs, not visible labels, in assignments and shelves. - Keep empty assignment entries. - Treat `New`, `Updated`, `Tom's Pick`, and `Chris's Pick` as editorial choices, not objective card metadata. diff --git a/tools/sitegen/src/curation/cli.js b/tools/sitegen/src/curation/cli.js index bc55fd1e..0c4f799e 100644 --- a/tools/sitegen/src/curation/cli.js +++ b/tools/sitegen/src/curation/cli.js @@ -41,7 +41,7 @@ function releaseCards() { .sort((a, b) => a.id.localeCompare(b.id, undefined, { numeric: true })); } -function validate(flairs, discovery, cards) { +export function validate(flairs, discovery, cards) { const errors = []; const warnings = []; const available = Array.isArray(flairs.available_flairs) ? flairs.available_flairs : []; @@ -100,7 +100,12 @@ function validate(flairs, discovery, cards) { if (!assignmentIds.has(card.id)) errors.push(`flairs.yml: missing assignment for ${card.id}`); } for (const id of assignmentIds) { - if (!cardIds.has(id)) errors.push(`flairs.yml: stale assignment for ${id}`); + if (cardIds.has(id)) continue; + if (Array.isArray(assignments[id]) && assignments[id].length === 0) { + warnings.push(`flairs.yml: stale empty assignment for ${id}; run \`npm run sync-curation\` to remove it`); + } else { + errors.push(`flairs.yml: stale assignment for ${id}`); + } } const referencedCards = []; @@ -145,9 +150,18 @@ function validate(flairs, discovery, cards) { return { errors, warnings }; } -function synchronize(flairsDocument, cards) { +export function synchronize(flairsDocument, cards) { let assignments = flairsDocument.get('assignments', true); if (!(assignments instanceof YAMLMap)) throw new Error('flairs.yml: assignments must be a mapping before it can be synchronized'); + const cardIds = new Set(cards.map(card => card.id)); + const removed = []; + assignments.items = assignments.items.filter(pair => { + const id = String(pair.key?.value ?? pair.key); + const assigned = pair.value?.toJSON?.() ?? pair.value; + if (cardIds.has(id) || !Array.isArray(assigned) || assigned.length !== 0) return true; + removed.push(id); + return false; + }); const existing = new Set(assignments.items.map(pair => String(pair.key?.value ?? pair.key))); const added = []; for (const card of cards) { @@ -158,8 +172,7 @@ function synchronize(flairsDocument, cards) { existing.add(card.id); added.push(card.id); } - if (added.length) fs.writeFileSync(FLAIRS_FILE, String(flairsDocument)); - return added; + return { added, removed }; } function report(result) { @@ -174,8 +187,11 @@ function main() { const cards = releaseCards(); const flairsDocument = readDocument(FLAIRS_FILE); if (mode === 'sync') { - const added = synchronize(flairsDocument, cards); - console.log(added.length ? `Added ${added.length} curation assignment(s): ${added.join(', ')}` : 'Curation assignments are already synchronized.'); + const { added, removed } = synchronize(flairsDocument, cards); + if (added.length || removed.length) fs.writeFileSync(FLAIRS_FILE, String(flairsDocument)); + if (added.length) console.log(`Added ${added.length} curation assignment(s): ${added.join(', ')}`); + if (removed.length) console.log(`Removed ${removed.length} stale empty curation assignment(s): ${removed.join(', ')}`); + if (!added.length && !removed.length) console.log('Curation assignments are already synchronized.'); } const currentFlairs = readDocument(FLAIRS_FILE).toJS() || {}; @@ -194,9 +210,11 @@ function main() { } } -try { - main(); -} catch (error) { - console.error(`error: ${error.message}`); - process.exitCode = 1; +if (process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url)) { + try { + main(); + } catch (error) { + console.error(`error: ${error.message}`); + process.exitCode = 1; + } } diff --git a/tools/sitegen/src/validate/prRules.js b/tools/sitegen/src/validate/prRules.js index 89cdbd74..ca565b3f 100644 --- a/tools/sitegen/src/validate/prRules.js +++ b/tools/sitegen/src/validate/prRules.js @@ -57,23 +57,43 @@ export function summarizePrTrigger(changes) { function allowsSynchronizedFlairs(changes, currentFlairs, baseFlairs) { if (!currentFlairs || !baseFlairs) return false; - const addedCards = new Set(changes - .filter(change => change.status.startsWith('A')) - .map(change => posix(change.path).match(/^releases\/([^/]+)\/info\.yaml$/)?.[1]) - .filter(Boolean)); + const addedCards = new Set(); + const deletedCards = new Set(); + const cardFromInfoPath = file => posix(file).match(/^releases\/([^/]+)\/info\.yaml$/)?.[1]; + for (const change of changes) { + if (change.oldPath) { + const deleted = cardFromInfoPath(change.oldPath); + const added = cardFromInfoPath(change.path); + if (deleted) deletedCards.add(deleted); + if (added) addedCards.add(added); + continue; + } + const card = cardFromInfoPath(change.path); + if (!card) continue; + if (change.status.startsWith('A')) addedCards.add(card); + if (change.status.startsWith('D')) deletedCards.add(card); + } const currentAssignments = currentFlairs.assignments; const baseAssignments = baseFlairs.assignments; if (!currentAssignments || typeof currentAssignments !== 'object' || Array.isArray(currentAssignments) || !baseAssignments || typeof baseAssignments !== 'object' || Array.isArray(baseAssignments)) return false; const withoutAssignments = value => Object.fromEntries(Object.entries(value).filter(([key]) => key !== 'assignments')); if (!isDeepStrictEqual(withoutAssignments(currentFlairs), withoutAssignments(baseFlairs))) return false; - for (const [card, assignment] of Object.entries(baseAssignments)) { - if (!Object.hasOwn(currentAssignments, card) || !isDeepStrictEqual(currentAssignments[card], assignment)) return false; + let synchronizedChanges = 0; + for (const card of new Set([...Object.keys(baseAssignments), ...Object.keys(currentAssignments)])) { + const inBase = Object.hasOwn(baseAssignments, card); + const inCurrent = Object.hasOwn(currentAssignments, card); + if (inBase && inCurrent) { + if (!isDeepStrictEqual(currentAssignments[card], baseAssignments[card])) return false; + } else if (inCurrent) { + if (!addedCards.has(card) || !Array.isArray(currentAssignments[card]) || currentAssignments[card].length !== 0) return false; + synchronizedChanges += 1; + } else { + if (!deletedCards.has(card) || !Array.isArray(baseAssignments[card]) || baseAssignments[card].length !== 0) return false; + synchronizedChanges += 1; + } } - const additions = Object.entries(currentAssignments).filter(([card]) => !Object.hasOwn(baseAssignments, card)); - return additions.length > 0 && additions.every(([card, assignment]) => - addedCards.has(card) && Array.isArray(assignment) && assignment.length === 0 - ); + return synchronizedChanges > 0; } function walkFiles(dir) { diff --git a/tools/sitegen/test/curation-cli.test.js b/tools/sitegen/test/curation-cli.test.js new file mode 100644 index 00000000..1a0bb18f --- /dev/null +++ b/tools/sitegen/test/curation-cli.test.js @@ -0,0 +1,39 @@ +import assert from 'node:assert/strict'; +import test from 'node:test'; +import YAML from 'yaml'; +import { synchronize, validate } from '../src/curation/cli.js'; + +const discovery = { shelves: [] }; + +test('curation validation permits a stale empty assignment but rejects an assigned one', () => { + const result = validate({ + available_flairs: [{ id: 'new' }], + assignments: { current: [], deleted_empty: [], deleted_curated: ['new'] }, + }, discovery, [{ id: 'current', title: 'Current' }]); + + assert.ok(result.warnings.some(message => message.includes('stale empty assignment for deleted_empty'))); + assert.ok(!result.errors.some(message => message.includes('deleted_empty'))); + assert.ok(result.errors.some(message => message === 'flairs.yml: stale assignment for deleted_curated')); +}); + +test('curation synchronization removes only stale empty assignments', () => { + const document = YAML.parseDocument(` +available_flairs: + - id: new +assignments: + current: [] + deleted_empty: [] + deleted_curated: [new] +`); + const changes = synchronize(document, [ + { id: 'current', title: 'Current' }, + { id: 'added', title: 'Added card' }, + ]); + + assert.deepEqual(changes, { added: ['added'], removed: ['deleted_empty'] }); + assert.deepEqual(document.toJS().assignments, { + current: [], + deleted_curated: ['new'], + added: [], + }); +}); \ No newline at end of file diff --git a/tools/sitegen/test/pr-rules.test.js b/tools/sitegen/test/pr-rules.test.js index 0c9234ec..cdf47e2a 100644 --- a/tools/sitegen/test/pr-rules.test.js +++ b/tools/sitegen/test/pr-rules.test.js @@ -165,6 +165,31 @@ assignments: && item.file === 'tools/sitegen/src/curation/flairs.yml')); }); +test('sync-curation may remove an empty assignment for a deleted card', async t => { + const root = await fixture(t); + const changes = [ + { status: 'D', path: 'releases/42_deleted/info.yaml' }, + { status: 'M', path: 'tools/sitegen/src/curation/flairs.yml' }, + ]; + const baseFlairs = { available_flairs: [{ id: 'new' }], assignments: { '03_old': ['new'], '42_deleted': [] } }; + await write(root, 'tools/sitegen/src/curation/flairs.yml', ` +available_flairs: + - id: new +assignments: + 03_old: [new] +`); + const allowed = await evaluatePrRules(changes, { root, baseFlairs }); + assert.ok(!allowed.some(item => + item.ruleId === 'change-outside-release-directory' + && item.file === 'tools/sitegen/src/curation/flairs.yml')); + + baseFlairs.assignments['42_deleted'] = ['new']; + const assignedDeletion = await evaluatePrRules(changes, { root, baseFlairs }); + assert.ok(assignedDeletion.some(item => + item.ruleId === 'change-outside-release-directory' + && item.file === 'tools/sitegen/src/curation/flairs.yml')); +}); + test('deleting a complete release reports an explicit warning', async t => { const root = await fixture(t); const diagnostics = await evaluatePrRules([