From 48da7d8350c870049bf5a6bfc1fb669cb646396c Mon Sep 17 00:00:00 2001 From: Thomas Jung Date: Fri, 16 Oct 2026 15:31:04 -0400 Subject: [PATCH 1/4] fix(#2805): backfill-images.ts accepts comma-list --slug/--slugs --- scripts/backfill-images.ts | 49 ++++++++++++---- test/unit/backfill-images-slug-filter.test.js | 56 +++++++++++++++++++ 2 files changed, 93 insertions(+), 12 deletions(-) create mode 100644 test/unit/backfill-images-slug-filter.test.js diff --git a/scripts/backfill-images.ts b/scripts/backfill-images.ts index 2b12ba4d6..e99c3188e 100644 --- a/scripts/backfill-images.ts +++ b/scripts/backfill-images.ts @@ -11,7 +11,8 @@ // // Usage: // CAP_BASE_URL= CONTENT_API_KEY= npm run backfill-images -// ... [--slug ] # only images referenced by one tutorial (#2288) +// ... [--slug ] # only images for one tutorial (#2288); comma-separated or multi +// ... [--slugs ] # comma-list of slugs (#2805); union with --slug // ... [--channel qa] # QA: reads CAP_QA_BASE_URL/CONTENT_API_KEY_QA + hugo/public-qa // ... [--limit N] # only the first N unique images (smoke) // ... [--concurrency N] # parallel fetch+push (default 12) @@ -25,7 +26,7 @@ import { readdirSync, readFileSync, existsSync } from 'node:fs'; import { join } from 'node:path'; import { extractImgCdnUrls, channelFor } from '../srv/lib/image-warm-utils.js'; -interface Args { limit: number; concurrency: number; dryRun: boolean; force: boolean; slug?: string; channel: 'prod' | 'qa'; } +interface Args { limit: number; concurrency: number; dryRun: boolean; force: boolean; slugs: string[]; channel: 'prod' | 'qa'; } /** Read `--slug ` from argv; undefined when absent or blank. */ export function parseSlug(argv: string[]): string | undefined { @@ -34,6 +35,26 @@ export function parseSlug(argv: string[]): string | undefined { return v || undefined; } +/** + * Read `--slug` and `--slugs` from argv, union them, comma-split each, + * trim, drop blanks, de-duplicate preserving first-seen order. + * Returns [] when neither flag is present or both are blank (whole-catalog default). + * Mirrors publish-content.ts filterToSlugs / PUBLISH_SLUGS pattern (#2805). + */ +export function parseSlugs(argv: string[]): string[] { + const get = (flag: string): string => { + const i = argv.indexOf(flag); + return i >= 0 ? (argv[i + 1] ?? '') : ''; + }; + const raw = [get('--slug'), get('--slugs')].join(','); + const seen = new Set(); + const result: string[] = []; + for (const s of raw.split(',').map(v => v.trim()).filter(Boolean)) { + if (!seen.has(s)) { seen.add(s); result.push(s); } + } + return result; +} + /** Read `--channel qa|prod` from argv; defaults to 'prod'. Mirrors publish-content.ts. */ export function parseChannel(argv: string[]): 'prod' | 'qa' { const i = argv.indexOf('--channel'); @@ -50,7 +71,7 @@ function parseArgs(argv: string[]): Args { concurrency: Number(get('--concurrency')) || 12, dryRun: argv.includes('--dry-run'), force: argv.includes('--force'), - slug: parseSlug(argv), + slugs: parseSlugs(argv), channel: parseChannel(argv), }; } @@ -58,19 +79,21 @@ function parseArgs(argv: string[]): Args { /** * Walk hugo/public/tutorials//index.html → Map. * - * When `slug` is given, only that one tutorial's built HTML is scanned — used - * by the slug-targeted rebuild path (#2288) to re-warm just the changed - * tutorial's images without re-fetching the whole catalog. An unknown slug - * yields an empty map (no throw): nothing to warm. + * When `slugs` is a non-empty array, only those tutorials' built HTML is + * scanned — used by the slug-targeted rebuild path (#2288, #2805) to re-warm + * just the changed tutorials' images without re-fetching the whole catalog. + * An unknown slug yields no entries (no throw): best-effort warm. + * When `slugs` is undefined or empty, the whole tutorials dir is scanned + * (the original default, intentional for full/catalog rebuilds). */ -export function collectImageUrls(publicDir: string, slug?: string): Map { +export function collectImageUrls(publicDir: string, slugs?: string[]): Map { const urlToSlug = new Map(); const tutorialsDir = join(publicDir, 'tutorials'); if (!existsSync(tutorialsDir)) { throw new Error(`Not found: ${tutorialsDir} — run the Hugo build first (npm run build:all).`); } - const slugs = slug ? [slug] : readdirSync(tutorialsDir); - for (const s of slugs) { + const list = slugs?.length ? slugs : readdirSync(tutorialsDir); + for (const s of list) { const indexPath = join(tutorialsDir, s, 'index.html'); if (!existsSync(indexPath)) continue; let html: string; @@ -126,8 +149,10 @@ async function main() { if (!apiKey && !args.dryRun) { console.error(`Fatal: ${isQa ? 'CONTENT_API_KEY_QA' : 'CONTENT_API_KEY'} not set (${envHint})`); process.exit(1); } const publicDir = join(process.cwd(), 'hugo', isQa ? 'public-qa' : 'public'); - console.log(`Enumerating image URLs under ${publicDir}/tutorials ${args.slug ? `(slug: ${args.slug}) ` : ''}...`); - let urlToSlug = collectImageUrls(publicDir, args.slug); + const slugList = args.slugs; + const slugLabel = slugList.length ? `(slugs: ${slugList.join(',')}) ` : ''; + console.log(`Enumerating image URLs under ${publicDir}/tutorials ${slugLabel}...`); + let urlToSlug = collectImageUrls(publicDir, slugList.length ? slugList : undefined); let entries = [...urlToSlug.entries()]; const totalUnique = entries.length; if (args.limit > 0) entries = entries.slice(0, args.limit); diff --git a/test/unit/backfill-images-slug-filter.test.js b/test/unit/backfill-images-slug-filter.test.js new file mode 100644 index 000000000..4f6d96846 --- /dev/null +++ b/test/unit/backfill-images-slug-filter.test.js @@ -0,0 +1,56 @@ +import { describe, it, expect } from 'vitest'; +import { parseSlugs } from '../../scripts/backfill-images.ts'; + +describe('parseSlugs (#2805)', () => { + it('splits a comma list from --slug', () => { + expect(parseSlugs(['--slug', 'a,b,c'])).toEqual(['a', 'b', 'c']); + }); + it('reads --slugs the same way', () => { + expect(parseSlugs(['--slugs', 'a,b'])).toEqual(['a', 'b']); + }); + it('unions --slug and --slugs, de-duping and preserving order', () => { + expect(parseSlugs(['--slug', 'a', '--slugs', 'b,a,c'])).toEqual(['a', 'b', 'c']); + }); + it('trims whitespace and drops blanks', () => { + expect(parseSlugs(['--slug', ' a , , b '])).toEqual(['a', 'b']); + }); + it('returns [] when neither flag is present', () => { + expect(parseSlugs([])).toEqual([]); + }); + it('returns [] for an empty value (whole-catalog default)', () => { + expect(parseSlugs(['--slug', ''])).toEqual([]); + }); +}); + +import { collectImageUrls } from '../../scripts/backfill-images.ts'; +import { mkdtempSync, mkdirSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; + +function fixture() { + const root = mkdtempSync(join(tmpdir(), 'bf-')); + const mk = (slug, u) => { + mkdirSync(join(root, 'tutorials', slug), { recursive: true }); + writeFileSync(join(root, 'tutorials', slug, 'index.html'), + ``); + }; + mk('slug-a', 'https://raw.githubusercontent.com/x/a.png'); + mk('slug-b', 'https://raw.githubusercontent.com/x/b.png'); + mk('slug-c', 'https://raw.githubusercontent.com/x/c.png'); + return root; +} + +describe('collectImageUrls multi-slug (#2805)', () => { + it('scans only the requested slugs when given an array', () => { + const m = collectImageUrls(fixture(), ['slug-a', 'slug-b']); + expect([...m.values()].sort()).toEqual(['slug-a', 'slug-b']); + }); + it('scans the whole dir when slugs is empty/undefined', () => { + expect(collectImageUrls(fixture(), []).size).toBe(3); + expect(collectImageUrls(fixture()).size).toBe(3); + }); + it('yields nothing (no throw) for an unknown slug — best-effort warm', () => { + const m = collectImageUrls(fixture(), ['does-not-exist']); + expect(m.size).toBe(0); + }); +}); From 123c6d409791b623e5fed7b837d37f8cf9cabc4a Mon Sep 17 00:00:00 2001 From: Thomas Jung Date: Fri, 16 Oct 2026 15:38:05 -0400 Subject: [PATCH 2/4] fix(#2805): image-refresh step honours plural slugs input (no whole-catalog fallback) --- .github/workflows/rebuild-content.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/rebuild-content.yml b/.github/workflows/rebuild-content.yml index d74cd043f..5403c6e89 100644 --- a/.github/workflows/rebuild-content.yml +++ b/.github/workflows/rebuild-content.yml @@ -1017,6 +1017,9 @@ jobs: # force-publish is set — that is exactly the "push my stubborn change # through" case). POST /content/image dedups on image bytes; changed # bytes are re-stored, unchanged report 'unchanged'. + # #2805: REFRESH_SLUG (inputs.slug / dispatch_slug) and REFRESH_SLUGS + # (inputs.slugs / dispatch_slugs) are both passed so a plural-slugs + # dispatch scopes the warm to those N tutorials, not the whole catalog. # # 2. full: re-warm the whole catalog (warm-images input, default true). # No full-catalog warm existed before #2346; the prior comment claiming @@ -1036,7 +1039,8 @@ jobs: CONTENT_API_KEY: ${{ steps.env.outputs.target == 'qa' && secrets.CONTENT_API_KEY_QA || secrets.CONTENT_API_KEY }} GITHUB_TOKEN: ${{ steps.app-token.outputs.token || secrets.TUTORIALS_GITHUB_TOKEN }} REFRESH_SLUG: ${{ inputs.slug || steps.mode.outputs.dispatch_slug }} - run: npx tsx scripts/backfill-images.ts --slug "$REFRESH_SLUG" + REFRESH_SLUGS: ${{ inputs.slugs || steps.mode.outputs.dispatch_slugs }} + run: npx tsx scripts/backfill-images.ts --slug "$REFRESH_SLUG" --slugs "$REFRESH_SLUGS" - name: Refresh tutorial images (full catalog) if: ${{ steps.publish.outcome == 'success' && steps.mode.outputs.effective_mode == 'full' && inputs.warm-images != false }} From bb7dd36310e6a05554fc20071512afcb4f6cf2ab Mon Sep 17 00:00:00 2001 From: Thomas Jung Date: Fri, 16 Oct 2026 15:43:49 -0400 Subject: [PATCH 3/4] docs(#2805): record image-refresh root cause + fix in spike Finding 3 --- .../reference/publish-performance-spike-2802.md | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/docs/developers/reference/publish-performance-spike-2802.md b/docs/developers/reference/publish-performance-spike-2802.md index 1228d49aa..53760d81f 100644 --- a/docs/developers/reference/publish-performance-spike-2802.md +++ b/docs/developers/reference/publish-performance-spike-2802.md @@ -23,7 +23,7 @@ dominant phases, each ~100–165s, that the issue did not separate: Secondary: the issue's guessed defaults (4/25) are wrong — actual is **concurrency=6, batch-size=50**. The `carryForwardUnchanged` OOM fix (#1932) **still holds** — no remaining -whole-corpus BLOB path. Image-refresh is **not** always 0–2s (one run was 361s). +whole-corpus BLOB path. Image-refresh is **not** always 0–2s (one run was 361s) — root-caused and fixed in #2805 (plural `slugs` input not threaded to the image step). ## Evidence — per-phase wall-clock from real PROD runs (2026-10-15) @@ -158,7 +158,15 @@ These are HANA-only wins (SQLite unit tests won't show them). Measure with the e response or run the three in parallel. Low value (seconds), low risk. - **Image-refresh is NOT innocent.** The issue says it is "always 0–2s"; run 38816499609 spent **361s** on `Refresh tutorial images (slug-targeted)` — co-equal with publish as a - wall-clock cost. Worth its own look (separate from this spike, but flag it). + wall-clock cost. **Root cause (#2805): the `slugs` plural input was not threaded to the + image step.** That run was a `workflow_dispatch` with `mode=slug-targeted` and the plural + `slugs=` input set (singular `slug` empty), but the step env was + `REFRESH_SLUG: inputs.slug || dispatch_slug` — it never read `inputs.slugs`, so + `REFRESH_SLUG` was empty and `backfill-images.ts --slug ""` enumerated the whole catalog + (21,712 images). **Fixed in #2805:** `backfill-images.ts` now accepts a comma-list + `--slug`/`--slugs`, and the step also passes + `REFRESH_SLUGS: inputs.slugs || dispatch_slugs` (mirroring the publish step's + `PUBLISH_SLUGS`). The same plural-input gap had already been fixed on the publish side. ## Suggested implementation order (separate PRs) @@ -167,7 +175,8 @@ These are HANA-only wins (SQLite unit tests won't show them). Measure with the e 2. **Finding 2 — batch metadata reads then writes** in `appendToSession` (HANA round-trip reduction). ~tens of seconds; higher blast radius, needs hybrid/PROD timing proof. 3. Finding 3 micro-wins (parallel/fold the pre-session GETs; A/B concurrency) only after 1 & 2. -4. Open a sibling issue for the image-refresh 361s outlier. +4. ~~Open a sibling issue for the image-refresh 361s outlier.~~ Done — #2805, root-caused + and fixed (see Finding 3 above: plural `slugs` input not threaded to the image step). ## Not changed by this spike From 45c2c0aa492ed4d9893586dd99971fb46da2dc08 Mon Sep 17 00:00:00 2001 From: Thomas Jung Date: Fri, 16 Oct 2026 15:44:30 -0400 Subject: [PATCH 4/4] docs(#2805): design + implementation plan for plural-slugs image fix --- ...16-slug-targeted-plural-slugs-image-fix.md | 221 ++++++++++++++++++ ...6-slug-targeted-plural-slugs-fix-design.md | 179 ++++++++++++++ 2 files changed, 400 insertions(+) create mode 100644 docs/superpowers/plans/2026-10-16-slug-targeted-plural-slugs-image-fix.md create mode 100644 docs/superpowers/specs/2026-10-16-slug-targeted-plural-slugs-fix-design.md diff --git a/docs/superpowers/plans/2026-10-16-slug-targeted-plural-slugs-image-fix.md b/docs/superpowers/plans/2026-10-16-slug-targeted-plural-slugs-image-fix.md new file mode 100644 index 000000000..3af694b79 --- /dev/null +++ b/docs/superpowers/plans/2026-10-16-slug-targeted-plural-slugs-image-fix.md @@ -0,0 +1,221 @@ +# Slug-targeted image refresh honours plural `slugs` (#2805) — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** A `slug-targeted` content rebuild dispatched with the plural `slugs` input scopes the image-refresh step to exactly those slug(s) instead of silently warming the whole 21,712-image catalog. + +**Architecture:** Mirror the already-shipped publish-side multi-slug pattern onto the image path. `backfill-images.ts` gains a comma-list `--slug`/`--slugs` + `REFRESH_SLUGS` env fallback and `collectImageUrls` takes a slug array; the workflow image step passes a plural `REFRESH_SLUGS` built from `inputs.slugs || dispatch_slugs`, exactly as the publish step already passes `PUBLISH_SLUGS`. + +**Tech Stack:** TypeScript (tsx), Vitest unit tests, GitHub Actions YAML. + +**Spec:** `docs/superpowers/specs/2026-10-16-slug-targeted-plural-slugs-fix-design.md` + +## Global Constraints + +- Branch base: `origin/DEV` (PR targets DEV; `main` is protected, no main-hotfix path). +- Image warm step keeps `continue-on-error: true` — a warm failure must never fail the rebuild. +- Scripts stay importable-without-run (the `isMainModule` guard in `backfill-images.ts`) so unit tests import pure functions without executing `main()`. +- Backward compatible: a single-value `--slug "a"` and an empty/absent slug (whole-catalog default) must behave exactly as today. +- Mirror the publish precedent verbatim where it exists: `filterToSlugs` (`scripts/publish-content.ts:800`), `slugs: get('--slugs', process.env.PUBLISH_SLUGS ?? '')` (`:752`), workflow `PUBLISH_SLUGS` env (`.github/workflows/rebuild-content.yml:955`). + +## Review Focus + +- **Empty `REFRESH_SLUGS` + empty `REFRESH_SLUG`** (no filter at all) must still enumerate the whole catalog — the default/`full`-mode behaviour is intentional and must not become a no-op. → Task 1 test. +- **A typo'd / unknown slug** in the list must NOT silently warm nothing or everything; `collectImageUrls` yields an empty map for an unknown slug (no index.html), so the warm reports 0 images rather than falling back to the catalog — assert the count, not a throw (image warm is best-effort, unlike publish which throws). → Task 1 test. +- **Whitespace / duplicate entries** in the comma list (`" a , a , "`) must be trimmed, de-duped, and blanks dropped. → Task 1 test. +- **Single-slug `--slug "a"`** (existing CI call shape) must keep working unchanged. → Task 1 test. +- **`--slug` and `--slugs` both present** (singular + plural combined) must union, not override. → Task 1 test. + +--- + +### Task 1: `backfill-images.ts` accepts multiple slugs + +**Files:** +- Modify: `scripts/backfill-images.ts` (`parseSlug`/`parseArgs` ~`:31-56`, `collectImageUrls` ~`:66-83`, `main` enumerate/log ~`:117-134`) +- Test: `test/unit/backfill-images-slug-filter.test.js` (create) + +**Interfaces:** +- Consumes: nothing new. +- Produces: + - `export function parseSlugs(argv: string[]): string[]` — union of comma-split `--slug` and `--slugs` values, trimmed, blanks dropped, de-duplicated, order-preserving. `[]` when neither present. + - `export function collectImageUrls(publicDir: string, slugs?: string[]): Map` — when `slugs` is a non-empty array, scan only those `tutorials//index.html`; when `undefined`/empty, scan the whole `tutorials/` dir (unchanged default). An unknown slug contributes nothing (its index.html is absent) — no throw. + - `Args.slugs: string[]` replaces `Args.slug?: string` on the parsed-args object. + +- [ ] **Step 1: Write the failing test** `test/unit/backfill-images-slug-filter.test.js` + +```js +import { describe, it, expect } from 'vitest'; +import { parseSlugs } from '../../scripts/backfill-images.ts'; + +describe('parseSlugs (#2805)', () => { + it('splits a comma list from --slug', () => { + expect(parseSlugs(['--slug', 'a,b,c'])).toEqual(['a', 'b', 'c']); + }); + it('reads --slugs the same way', () => { + expect(parseSlugs(['--slugs', 'a,b'])).toEqual(['a', 'b']); + }); + it('unions --slug and --slugs, de-duping and preserving order', () => { + expect(parseSlugs(['--slug', 'a', '--slugs', 'b,a,c'])).toEqual(['a', 'b', 'c']); + }); + it('trims whitespace and drops blanks', () => { + expect(parseSlugs(['--slug', ' a , , b '])).toEqual(['a', 'b']); + }); + it('returns [] when neither flag is present', () => { + expect(parseSlugs([])).toEqual([]); + }); + it('returns [] for an empty value (whole-catalog default)', () => { + expect(parseSlugs(['--slug', ''])).toEqual([]); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `npx vitest run --project unit test/unit/backfill-images-slug-filter.test.js` +Expected: FAIL — `parseSlugs` is not exported. + +- [ ] **Step 3: Implement `parseSlugs` and thread it through** + +In `scripts/backfill-images.ts`: +- Add `export function parseSlugs(argv: string[]): string[]` that reads BOTH `--slug` and `--slugs` values (each may be comma-separated), concatenates, `.split(',')`, `.map(trim)`, `.filter(Boolean)`, de-dupes via `new Set` while preserving first-seen order. +- Keep `parseSlug` exported too (unused by main after this, but a thin wrapper or leave as-is — do not break any importer; grep shows none besides tests). +- Change `Args.slug?: string` → `Args.slugs: string[]`; set `slugs: parseSlugs(argv)` in `parseArgs`. + +- [ ] **Step 4: Run test to verify it passes** + +Run: `npx vitest run --project unit test/unit/backfill-images-slug-filter.test.js` +Expected: PASS (parseSlugs block). + +- [ ] **Step 5: Write the failing `collectImageUrls` multi-slug test** + +Append to the same test file. Build a temp fixture dir (use `node:fs` `mkdtempSync` + write two `tutorials//index.html` files each containing one distinct `/img-cdn?u=…` URL and a third slug dir NOT requested): + +```js +import { collectImageUrls } from '../../scripts/backfill-images.ts'; +import { mkdtempSync, mkdirSync, writeFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { tmpdir } from 'node:os'; + +function fixture() { + const root = mkdtempSync(join(tmpdir(), 'bf-')); + const mk = (slug, u) => { + mkdirSync(join(root, 'tutorials', slug), { recursive: true }); + writeFileSync(join(root, 'tutorials', slug, 'index.html'), + ``); + }; + mk('slug-a', 'https://raw.githubusercontent.com/x/a.png'); + mk('slug-b', 'https://raw.githubusercontent.com/x/b.png'); + mk('slug-c', 'https://raw.githubusercontent.com/x/c.png'); + return root; +} + +describe('collectImageUrls multi-slug (#2805)', () => { + it('scans only the requested slugs when given an array', () => { + const m = collectImageUrls(fixture(), ['slug-a', 'slug-b']); + expect([...m.values()].sort()).toEqual(['slug-a', 'slug-b']); + }); + it('scans the whole dir when slugs is empty/undefined', () => { + expect(collectImageUrls(fixture(), []).size).toBe(3); + expect(collectImageUrls(fixture()).size).toBe(3); + }); + it('yields nothing (no throw) for an unknown slug — best-effort warm', () => { + const m = collectImageUrls(fixture(), ['does-not-exist']); + expect(m.size).toBe(0); + }); +}); +``` + +- [ ] **Step 6: Run to verify it fails** + +Run: `npx vitest run --project unit test/unit/backfill-images-slug-filter.test.js` +Expected: FAIL — `collectImageUrls` still takes a single `slug?: string`. + +- [ ] **Step 7: Implement `collectImageUrls(publicDir, slugs?: string[])`** + +Change the signature and the one branch (`:72`): `const list = slugs?.length ? slugs : readdirSync(tutorialsDir);`. Rest of the loop is unchanged (it already iterates a list). Update `main` (`:129-130`): `const slugList = args.slugs; let urlToSlug = collectImageUrls(publicDir, slugList.length ? slugList : undefined);` and the enumerate log to show `(slugs: a,b)` when scoped. + +- [ ] **Step 8: Run the file + the full unit project** + +Run: `npx vitest run --project unit test/unit/backfill-images-slug-filter.test.js` +Expected: PASS (all). +Run: `npx vitest run --project unit` +Expected: PASS, no regressions (especially `publish-content-slug-filter.test.js`, `publish-image-warm.test.js`). + +- [ ] **Step 9: Dry-run proof against built HTML (manual, if `hugo/public/tutorials` exists)** + +Run: `CAP_BASE_URL=http://x npx tsx scripts/backfill-images.ts --slug "a,b" --dry-run` +Expected: log reads `(slugs: a,b)` and a small `Found N unique image URL(s)`, NOT 21,712. (Skip if no local Hugo build; note in PR that CI proves it.) + +- [ ] **Step 10: Commit** + +```bash +git add scripts/backfill-images.ts test/unit/backfill-images-slug-filter.test.js +git commit -m "fix(#2805): backfill-images.ts accepts comma-list --slug/--slugs" +``` + +--- + +### Task 2: Workflow image step passes the plural slug list + +**Files:** +- Modify: `.github/workflows/rebuild-content.yml` — image step `env` + `run` (~`:1031-1039`); comment block (~`:1013-1029`). + +**Interfaces:** +- Consumes: `steps.mode.outputs.dispatch_slugs` (already emitted, `:290`), `inputs.slugs`, `inputs.slug`, `steps.mode.outputs.dispatch_slug`. Task 1's `backfill-images.ts` `--slug`/`--slugs` CLI. +- Produces: nothing downstream depends on this step. + +- [ ] **Step 1: Add `REFRESH_SLUGS` env and pass it to the script** + +In the `Refresh tutorial images (slug-targeted)` step, add alongside the existing `REFRESH_SLUG`: + +```yaml + REFRESH_SLUG: ${{ inputs.slug || steps.mode.outputs.dispatch_slug }} + REFRESH_SLUGS: ${{ inputs.slugs || steps.mode.outputs.dispatch_slugs }} + run: npx tsx scripts/backfill-images.ts --slug "$REFRESH_SLUG" --slugs "$REFRESH_SLUGS" +``` + +(Mirrors the publish step's `PUBLISH_SLUG` + `PUBLISH_SLUGS` pairing at `:948`/`:955`. `backfill-images.ts` unions both and ignores blanks, so passing both empty = whole-catalog, exactly as before; passing either populates the scope.) + +- [ ] **Step 2: Update the comment block** + +In the `#2288 / #2346` comment block above the step, add one line noting `REFRESH_SLUG`+`REFRESH_SLUGS` now derive from `inputs.slug`+`inputs.slugs` (and the `dispatch_slug`/`dispatch_slugs` equivalents), so a plural-`slugs` dispatch scopes the warm instead of falling back to the full catalog (the #2805 361s regression). + +- [ ] **Step 3: Validate the workflow YAML parses** + +Run: `npx yaml-lint .github/workflows/rebuild-content.yml 2>/dev/null || python -c "import yaml,sys; yaml.safe_load(open('.github/workflows/rebuild-content.yml')); print('YAML OK')"` +Expected: `YAML OK` (or yaml-lint clean). If neither tool is available, confirm with `yq '.jobs.rebuild.steps[] | select(.name == "Refresh tutorial images (slug-targeted)")' .github/workflows/rebuild-content.yml` prints the step with both env keys. + +- [ ] **Step 4: Commit** + +```bash +git add .github/workflows/rebuild-content.yml +git commit -m "fix(#2805): image-refresh step honours plural slugs input (no whole-catalog fallback)" +``` + +--- + +### Task 3: Doc touch-ups + +**Files:** +- Modify: `docs/developers/reference/publish-performance-spike-2802.md` (Finding 3 image bullet). +- Modify: `CLAUDE.md` Top Gotchas (optional one-liner) — SKIP unless a reviewer asks; keep the diff tight. + +**Interfaces:** none. + +- [ ] **Step 1: Update the spike doc's image bullet** + +Change the Finding 3 "Image-refresh is NOT innocent" bullet to note the root cause (plural `slugs` input not threaded to `REFRESH_SLUG`) and that #2805 fixed it by passing `REFRESH_SLUGS`. One or two sentences; don't rewrite the finding. + +- [ ] **Step 2: Commit** + +```bash +git add docs/developers/reference/publish-performance-spike-2802.md +git commit -m "docs(#2805): record image-refresh root cause + fix in spike Finding 3" +``` + +--- + +## Notes for the whole-branch review + +- The publish half of the design doc's scope was found already shipped on `origin/DEV`; this plan implements only the remaining image half plus docs. The design doc records that. +- CI proof: after merge to DEV, dispatch `rebuild-content.yml -f mode=slug-targeted -f slugs=` and confirm the image step logs `(slugs: …)` + a small count, step wall-clock ≈ seconds (not 361s). Attach to the PR. diff --git a/docs/superpowers/specs/2026-10-16-slug-targeted-plural-slugs-fix-design.md b/docs/superpowers/specs/2026-10-16-slug-targeted-plural-slugs-fix-design.md new file mode 100644 index 000000000..6e3b02ae5 --- /dev/null +++ b/docs/superpowers/specs/2026-10-16-slug-targeted-plural-slugs-fix-design.md @@ -0,0 +1,179 @@ +# Design — slug-targeted rebuild honours the plural `slugs` input (#2805) + +**Date:** 2026-10-16 +**Issue:** sap-tutorials/tutorials-ims#2805 (follow-up to #2802, related #2801) +**Repo:** sap-tutorials/tutorials-ims (local folder `tutorials-poc`) + +## Intent / the why + +Issue #2805 reports the `Refresh tutorial images (slug-targeted)` step took **361s** on +run 38816499609 — co-equal with publish, contradicting the "always 0–2s" assumption. The +spike (#2802, Finding 3) flagged it as an outlier but did not root-cause it. + +### Root cause (confirmed from the run log) + +Run 38816499609 was a **`workflow_dispatch`** with: + +- `mode = slug-targeted` +- `slug` (singular) = **empty** +- `slugs` (plural) = `devtoberfest2026-fun-week4-validation` (one slug) + +The mode step (`Determine effective rebuild mode`) correctly resolved `slug-targeted` +because its inference at `rebuild-content.yml:247-252` honours **either** `INPUT_SLUG` +**or** `INPUT_SLUGS`. But the two downstream fast-path steps resolve the slug from only +the **singular** input + the repository_dispatch slug: + +- Image step env (`:997`): `REFRESH_SLUG: ${{ inputs.slug || steps.mode.outputs.dispatch_slug }}` +- Publish step env (`:914`): `PUBLISH_SLUG: ${{ … (inputs.slug || steps.mode.outputs.dispatch_slug) … }}` + +Neither reads `inputs.slugs`. So `REFRESH_SLUG` resolved to **empty**, and +`backfill-images.ts --slug ""` → `parseSlug` returns `undefined` → `collectImageUrls` +enumerated the **whole catalog (21,712 images)** = 361s. The log is unambiguous: + +``` +REFRESH_SLUG: +Enumerating image URLs under …/tutorials ... ← no "(slug: …)" scope +Found 21712 unique image URL(s). +``` + +The same latent bug sat in the **publish** step: on this run publish ran 365s +(whole-catalog delta) for the same reason. The `slugs`-plural operator path silently +defeated *both* fast paths. + +This is a **silent correctness/performance fallback**, not a slow slug. The fix makes the +plural input actually scope the work, and removes the silent whole-catalog surprise. + +### Already-fixed on origin/DEV (publish half) + +Investigation on `origin/DEV` (ahead of `main`) shows the **publish** half is already done: +`publish-content.ts` has `opts.slugs` (env `PUBLISH_SLUGS` / `--slugs`), `filterToSlugs`, +and combines `slug + slugs` at the call site; the workflow mode step emits a +`dispatch_slugs` output and the publish step passes `PUBLISH_SLUGS: inputs.slugs || +dispatch_slugs`. **The only remaining gap for #2805 is the image-refresh path**, which was +never updated and still reads singular-only (`REFRESH_SLUG: inputs.slug || +dispatch_slug`, `backfill-images.ts --slug` single). This plan mirrors the existing publish +pattern onto the image path. + +## Goals + +1. A `workflow_dispatch` with `mode=slug-targeted` and the **plural** `slugs` input scopes + both the publish and the image-refresh steps to exactly those slug(s) — no whole-catalog + fallback. +2. Support **multiple** slugs (comma-separated), since `slugs` is explicitly a multi-slug + input (`#2801` multi-slug dispatch). +3. One source of truth for the resolved slug filter, consumed by both steps. + +## Non-goals + +- Do **not** change the mode-inference logic (`:247-269`) — it already honours plural. +- Do **not** change `repository_dispatch` behaviour — `dispatch_slug` stays single-slug + (payload is one changed tutorial); it just folds into the central filter. +- Do **not** touch the `--allow-revert` path: it stays **single-slug-only** (its guard + `resolveAllowRevertSlugs` at `publish-content.ts:574` intentionally requires exactly + one `--slug`). Multi-slug + allow-revert is refused, unchanged. +- No change to `catalog-only`/`full` modes. + +## Anti-goals (would make this a failure even if green) + +- A fix that makes an **unknown/typo'd** slug silently warm/publish nothing or everything. + `filterToSlug` must still fail loudly on a slug that isn't in the Hugo output. +- Introducing a second slug-resolution expression that can drift from the first (the exact + class of bug we're fixing). + +## Constraints + +- CLAUDE.md: PR over direct merge; feature PR targets **DEV**, not main. Branch from fresh + `origin/DEV`. +- `continue-on-error: true` on the image step stays (best-effort warm must never fail the + rebuild). +- Scripts are invoked via `npx tsx` in CI; keep them importable-without-run (the + `isMainModule` guard) so unit tests keep working. +- Slug charset already validated upstream for dispatch payloads; `slugs` is a + `workflow_dispatch` string input (operator-supplied, trusted) but we still split/trim + defensively. + +## The how — decided + +### 1. Central resolved slug filter (workflow) + +In `Determine effective rebuild mode` (`rebuild-content.yml`), after `effective_mode` is +computed, emit a new output `slug_filter`: + +- Start from `INPUT_SLUG` (singular). +- Append `INPUT_SLUGS` (plural) if set (comma-join, matching the existing step-summary + logic at `:1064-1070`). +- For `repository_dispatch`, `slug_filter` = the validated `PAYLOAD_SLUG` (same value that + sets `dispatch_slug` today). +- Deduplicate + strip blanks + trim; emit as a single comma-separated `slug_filter=…` line. +- Only meaningful when `effective_mode == 'slug-targeted'`; empty otherwise. + +Both consumer steps change to read it: + +- Image (`:997`): `REFRESH_SLUG: ${{ steps.mode.outputs.slug_filter }}` +- Publish (`:914`): `PUBLISH_SLUG: ${{ steps.mode.outputs.effective_mode == 'slug-targeted' && steps.mode.outputs.slug_filter || '' }}` + +`dispatch_slug` output is retained (other steps/summary may reference it) — `slug_filter` +supersedes it only at these two consumers. + +### 2. Multi-slug in the scripts (comma list via existing `--slug`) + +**backfill-images.ts** +- `parseSlug` → `parseSlugs(argv): string[]` returning the comma-split, trimmed, non-empty + list from `--slug` (keep a single-value call compatible: `"a"` → `["a"]`, `""`/absent → + `[]`). +- `collectImageUrls(publicDir, slugs?: string[])` — change the `slug ? [slug] : readdir` + line (`:72`) to `slugs?.length ? slugs : readdirSync(...)`. The rest already loops a slug + array. +- Log line shows the joined slug list. + +**publish-content.ts** +- `opts.slug` (single) → keep the field, but add `opts.slugs: string[]` derived the same + comma-split way from `--slug` / `PUBLISH_SLUG`. +- Replace the single `filterToSlug(tutorials, opts.slug)` call (`:1254-1257`) with a + `filterToSlugs(tutorials, opts.slugs)` that: + - no-op (returns false) when the list is empty, + - throws loudly (same message shape) if **any** listed slug is absent from the Hugo + output, + - otherwise reduces `tutorials` to exactly the listed slugs. +- `--allow-revert`: keep `resolveAllowRevertSlugs` reading a **single** slug. If + `opts.slugs.length !== 1` while `allowRevert` is set, it throws (tighten the existing + "requires --slug" guard to "requires exactly one --slug"). Single-slug behaviour + unchanged. + +### 3. Tests (TDD — failing first) + +- **Unit (`test/unit/…`)** for `parseSlugs` + `filterToSlugs`: + - `"a,b,c"` → 3 slugs; `" a , b "` → `["a","b"]`; `""`/absent → `[]`. + - `filterToSlugs` reduces the Map to listed slugs; throws on an unknown slug; no-op on + empty. + - `collectImageUrls(dir, ["slug-a","slug-b"])` reads only those two index.html files + (fixture-backed), not the whole dir. +- **allow-revert guard** test: `allowRevert` + 2 slugs → throws; + 1 slug → ok. +- Reuse the existing `test/unit/publish-image-warm.test.js` harness/fixtures where + possible. + +### 4. Doc / gotcha + +Add a one-line note to the image-step comment block (`:973-988`) that `REFRESH_SLUG` now +derives from the central `slug_filter` (slug + slugs + dispatch_slug), so the plural input +scopes the warm. Update `publish-performance-spike-2802.md` Finding 3's image bullet to +point at this fix (it currently just says "worth its own look"). + +## What's left to the builder + +- Exact shell for the comma-join/dedupe in the mode step (sketch above; mirror `:1064-1070`). +- Whether `filterToSlugs` lives beside `filterToSlug` or replaces it (keep both if any + other caller uses the single form; grep shows only the one call site + tests). +- Test file placement under `test/unit/` consistent with siblings. + +## Verification (before "done") + +- `npm test` green (new unit tests + no regressions). +- Dry-run proof: `backfill-images.ts --slug "a,b" --dry-run` enumerates only a+b's images; + `--slug ""` still enumerates all (default) — i.e. the fallback is intact and *intended* + only when no filter is given. +- Re-dispatch (or `act`/manual trace) that a `slugs=`-only slug-targeted run now logs + `(slug: …)` scope and a small image count, not 21,712. (CI proof noted in PR.) +``` +Line numbers are against `main` @ 31e34a08f (current checkout) / to be rebased on origin/DEV. +```