From 88d1b58f1fa2529a3e6f3d017046f3771154d2b1 Mon Sep 17 00:00:00 2001 From: CRT Date: Wed, 21 Oct 2026 18:09:06 +0700 Subject: [PATCH] shots: render refuses missing frames and fails on load errors; shots check errors on renders with failed loads (pitfall 93) --- cli/shots/lib.ts | 17 ++++++++++++++ cli/shots/render.ts | 10 +++++++- cli/shots/report.ts | 1 + cli/shots/spec.ts | 2 +- docs/pitfalls.md | 1 + skills/store-pitfalls/SKILL.md | 12 ++++++++++ tests/shots-missing-frames.test.ts | 37 ++++++++++++++++++++++++++++++ 7 files changed, 78 insertions(+), 2 deletions(-) create mode 100644 tests/shots-missing-frames.test.ts diff --git a/cli/shots/lib.ts b/cli/shots/lib.ts index b440186..8f56cde 100644 --- a/cli/shots/lib.ts +++ b/cli/shots/lib.ts @@ -305,3 +305,20 @@ export function boxOutside(b: DeviceBox, W: number, H: number, tol = 1): string[ if (b.x + b.w > W + tol) out.push(`right ${Math.round(b.x + b.w - W)} px`); return out; } + +/** Frame files (relative to shots.frames: //) a render of these scenes and sizes + * loads. A missing one renders an image with no app or game content (only a 404 in the page), so `shots render` + * refuses to start without them. Landscape layouts use each scene's first frame; the feature graphic a phone frame. */ +export function framesForRender(scenes: { frames: { src: string }[] }[], sizes: string[], langs: string[], + shots: Pick & { feature?: { frame?: string } }): string[] { + const out = new Set(); + for (const lang of langs) for (const size of sizes) { + const s = SIZES[size]; + if (!s) continue; + if (s.feature) { const f = shots.feature?.frame ?? scenes[0]?.frames[0]?.src; if (f) out.add(`${lang}/phone/${f}`); continue; } + const d = renderDims(size, shots); + const kind = d.landscape ? 'landscape' : s.device ? 'tablet' : 'phone'; + for (const sc of scenes) for (const f of d.landscape ? sc.frames.slice(0, 1) : sc.frames) out.add(`${lang}/${kind}/${f.src}`); + } + return [...out].sort(); +} diff --git a/cli/shots/render.ts b/cli/shots/render.ts index 4aa751d..fff6517 100644 --- a/cli/shots/render.ts +++ b/cli/shots/render.ts @@ -25,7 +25,7 @@ import { done, log, warn } from '../lib/log'; import { closeBrowser, launch } from './browser'; import { defaultConcurrency, pool, progressLine } from './capture-web'; import { shotsStyle } from '../lib/config'; -import { type CopyFile, type DeviceBox, type TextBox, SIZES, renderDims, sheetTimeoutMs, featureLayout, copyText, defaultSizes, frameDims, mergeCopy, palette, renderDir, scriptFont, sheetLayout, validateCopy, variantScenes } from './lib'; +import { type CopyFile, type DeviceBox, type TextBox, SIZES, renderDims, sheetTimeoutMs, featureLayout, copyText, defaultSizes, frameDims, framesForRender, mergeCopy, palette, renderDir, scriptFont, sheetLayout, validateCopy, variantScenes } from './lib'; import { bgCss, bgTone, type ContrastSample, contrastIssues, parseColour, photoBackground, photoCss, type RGB, worstContrast } from './palette'; import { type WeightSample, weightIssues } from './weights'; import type { Palette } from '../lib/config'; @@ -70,6 +70,12 @@ export async function run(ctx: Ctx): Promise { if (bad.length) throw new Error(`--scenes: unknown scene id${bad.length > 1 ? 's' : ''} ${bad.join(', ')}${v ? ` (variant ${v})` : ''}; scene ids: ${job.scenes.map(s => s.id).join(', ')}`); } const framesDir = resolve(shots.frames), outDir = resolve(shots.out); + // every frame the scenes load must exist: a missing one renders a store image with no app or game in it + const gone = [...jobs.values()].flatMap(job => { + const sel = a.list('scenes').length ? job.scenes.filter(s => a.list('scenes').includes(s.id)) : job.scenes; + return framesForRender(sel, sizes, langs, shots); + }).filter((f, i, all) => all.indexOf(f) === i && !existsSync(join(framesDir, f))); + if (gone.length) throw new Error(`shots render: ${gone.length} frame file${gone.length > 1 ? 's' : ''} missing under ${relative(root, framesDir) || '.'} (run \`storebound shots capture\` first, or restore the frames folder; it is often git-ignored):\n ${gone.slice(0, 20).join('\n ')}${gone.length > 20 ? `\n ... ${gone.length - 20} more` : ''}`); if (a.has('dry')) { let n = 0; for (const [v, job] of jobs) for (const lang of langs) { @@ -290,6 +296,8 @@ export async function run(ctx: Ctx): Promise { server.stop(true); if (Object.keys(report).length) writeFileSync(join(outDir, 'report.json'), JSON.stringify(mergeReport(readReport(outDir), report), null, 1) + '\n'); } + const broken = Object.entries(report).filter(([, r]) => r.missing?.length); + if (broken.length) throw new Error(`shots render: ${broken.length} image${broken.length > 1 ? 's' : ''} rendered with files that failed to load (frame, logo or background; the image has holes): ${broken.slice(0, 5).map(([k, r]) => `${k} (${r.missing!.join(', ')})`).join('; ')}${broken.length > 5 ? ' ...' : ''}`); const flagged = Object.values(report).filter(r => r.overflow.length).length; if (flagged) warn('shots', `${flagged} images with text overflow flags: storebound shots check / shots gallery`); done('shots', `render done${warnings ? `, ${warnings} with warnings (missing frames or fonts?)` : ''}`); diff --git a/cli/shots/report.ts b/cli/shots/report.ts index f197ada..04d6c18 100644 --- a/cli/shots/report.ts +++ b/cli/shots/report.ts @@ -88,6 +88,7 @@ export function scanRenders(dir: string, o: { langs: string[]; variants?: string const fromStore = !variant && !!pulledEntry(pulled, dir, file, bytes); const issues = fromStore ? [] : checkImage(info, size); const entry = report[rel.replace(/\.jpe?g$/i, '.png')]; + if (entry?.missing?.length) issues.push({ level: 'error', code: 'frame-missing', msg: `rendered while ${entry.missing.length} file${entry.missing.length > 1 ? 's' : ''} failed to load (${entry.missing.join(', ')}): the image lacks its frame / logo / background; restore the frames (shots capture) and render again` }); for (const msg of entry?.overflow ?? []) issues.push({ level: 'warn', code: 'overflow', msg }); issues.push(...fontIssues(entry?.fonts)); issues.push(...deviceIssues(entry, size, info.w, info.h)); diff --git a/cli/shots/spec.ts b/cli/shots/spec.ts index 10d9bd1..40ae395 100644 --- a/cli/shots/spec.ts +++ b/cli/shots/spec.ts @@ -122,7 +122,7 @@ export function imageInfo(b: Uint8Array): ImageInfo { } export type IssueLevel = 'error' | 'warn'; -export interface Issue { level: IssueLevel; code: 'size' | 'alpha' | 'format' | 'bytes' | 'count' | 'overflow' | 'video' | 'font-fallback' | 'glyph-missing' | 'device-crop' | 'fonts' | 'margin' | 'font-weight' | 'contrast' | 'background'; msg: string } +export interface Issue { level: IssueLevel; code: 'size' | 'alpha' | 'format' | 'bytes' | 'count' | 'overflow' | 'video' | 'font-fallback' | 'glyph-missing' | 'device-crop' | 'fonts' | 'margin' | 'font-weight' | 'contrast' | 'background' | 'frame-missing'; msg: string } /** Problems with one image for a size key. */ export function checkImage(info: ImageInfo, sizeKey: string): Issue[] { diff --git a/docs/pitfalls.md b/docs/pitfalls.md index b1248cc..8056ceb 100644 --- a/docs/pitfalls.md +++ b/docs/pitfalls.md @@ -9,6 +9,7 @@ fails without the fix. | # | date | lesson | guard | |---|---|---|---| +| 93 | 2026-10 | A git-ignored, never-captured frames folder rendered store images with no app or game in them; render only logged 404s and `shots check` said 0 errors. | `shots render` refuses to start while a scene frame is missing (`framesForRender` in `cli/shots/lib.ts`) and fails when any load failed; report.json `missing` is a `frame-missing` error in `shots check` / `validate` (`cli/shots/report.ts`, `tests/shots-missing-frames.test.ts`). | | 90 | 2026-10 | `apple submit --build N --version X --yes` after `apple submit --no-submit --yes` (the documented two-step flow) printed the plan (every item "in it already", then "submit PATCH submitted: true"), logged "not added ...: already in the draft submission" and ended with "nothing to submit" without the PATCH: the submit only ran for new items or an UNRESOLVED_ISSUES submission. | `willSubmit` (cli/lib/review-submit.ts) also submits a READY_FOR_REVIEW draft that already holds the named app version (iap-submit: one of the asked-for items), with the same "not ready yet" retry. `tests/resubmit.test.ts` ("the second step submits the prepared draft"). | | 85 | 2026-10 | `shots record --source web` filmed at the page's CSS size: Chromium's screencast ignores the deviceScaleFactor (maxWidth only shrinks), so a 1080x1920 preview of a 430x932 @2x page was a 2.5x upscale. | `Page.captureScreenshot` with `clip.scale` = dpr (long side capped at 1920), 3 requests in flight, timestamped frames joined by ffmpeg; a warning when the first frame is smaller than asked or the rate is under 20 frames/s. `recordSize` / `captureShortfall` in `cli/shots/devices.ts`, `tests/video.test.ts`. | | 80 | 2026-10 | A game's hero logo and titles sat against the screenshot edges, a game with a macOS version had no Mac screenshots, and the contact sheet timed out at a flat 30 s with 9 sizes. | Templates keep 7 % / 5 % safe margins and `shots check` fails text or logo boxes under 6 % / 4 % (`marginViolations`, report.json `boxes`); `apple.mac` adds the `mac` size (APP_DESKTOP, macOS version); `sheetTimeoutMs` scales with images, sizes and load. `tests/mac-landscape.test.ts`, `tests/browser/landscape.browser.ts`. | diff --git a/skills/store-pitfalls/SKILL.md b/skills/store-pitfalls/SKILL.md index cc5b7da..f6131d3 100644 --- a/skills/store-pitfalls/SKILL.md +++ b/skills/store-pitfalls/SKILL.md @@ -1136,3 +1136,15 @@ that fails without the fix (see CONTRIBUTING.md). Owner-only operations (submit, release, delete, prices, testers, replies, ...) refuse `--confirm`: the owner still types the id in a terminal. MCP and the desktop app still never send writes. - **Guard:** `decide` / `OWNER_NO_CONFIRM` in `cli/api/gate.ts`, `tests/api.test.ts`. + +## 93. Screenshots rendered without their frames (empty store images, check said 0 errors) +- **Symptom:** `shots render` finished and `shots check` reported 0 errors / 0 warnings, but the images showed only + captions and backgrounds: no app or game. The render log had only `404 .../frames/...` warnings. +- **Cause:** the raw frames folder (`shots.frames`) is usually git-ignored; in a fresh checkout it was never captured, + and the 404s were recorded in report.json `missing` but never checked. +- **Do:** run `storebound shots capture` (or restore the frames folder) before rendering. Never ship renders whose + log has 404 lines. +- **Guard:** `shots render` refuses to start while any frame a selected scene uses is missing (`framesForRender` in + `cli/shots/lib.ts`) and exits 1 if any file failed to load while rendering; `shots check` and `validate` raise a + `frame-missing` error for every render whose report entry has failed loads (`cli/shots/report.ts`, + `tests/shots-missing-frames.test.ts`). diff --git a/tests/shots-missing-frames.test.ts b/tests/shots-missing-frames.test.ts new file mode 100644 index 0000000..32ce4e6 --- /dev/null +++ b/tests/shots-missing-frames.test.ts @@ -0,0 +1,37 @@ +// Guardrail from real use: the raw frames folder was git-ignored and never captured, `shots render` produced store +// images with no app or game in them (only 404 lines in the log) and `shots check` still said 0 errors. Now render +// refuses to start without every frame, and report.json `missing` entries are check errors. +import { describe, expect, test } from 'bun:test'; +import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { framesForRender } from '../cli/shots/lib'; +import { allIssues, scanRenders } from '../cli/shots/report'; +import { png } from './fixtures'; + +const scenes = [{ frames: [{ src: 'home.png' }, { src: 'back.png' }] }, { frames: [{ src: 'level.png' }] }]; + +describe('missing frames', () => { + test('framesForRender lists every frame per language and device class', () => { + expect(framesForRender(scenes, ['ios69', 'ipad13', 'feature'], ['en'], {})).toEqual([ + 'en/phone/back.png', 'en/phone/home.png', 'en/phone/level.png', 'en/tablet/back.png', 'en/tablet/home.png', 'en/tablet/level.png']); + expect(framesForRender(scenes, ['mac'], ['en', 'tr'], {})).toEqual(['en/landscape/home.png', 'en/landscape/level.png', 'tr/landscape/home.png', 'tr/landscape/level.png']); + expect(framesForRender(scenes, ['feature'], ['en'], { feature: { frame: 'promo.png' } })).toEqual(['en/phone/promo.png']); + }); + + test('a render recorded with failed loads is a shots check error', () => { + const dir = mkdtempSync(join(tmpdir(), 'sb-missing-')); + try { + mkdirSync(join(dir, 'en', 'ios69'), { recursive: true }); + writeFileSync(join(dir, 'en', 'ios69', '01.png'), png(1320, 2868)); + writeFileSync(join(dir, 'en', 'ios69', '02.png'), png(1320, 2868)); + writeFileSync(join(dir, 'report.json'), JSON.stringify({ + 'en/ios69/01.png': { overflow: [], missing: ['404 http://127.0.0.1:8792/frames/en/phone/home.png'], fonts: { fallback: [], missing: [] } }, + 'en/ios69/02.png': { overflow: [], missing: [], fonts: { fallback: [], missing: [] } }, + })); + const issues = allIssues(scanRenders(dir, { langs: ['en'], sizes: ['ios69'], probe: () => null })).filter(x => x.issue.code === 'frame-missing'); + expect(issues.map(x => `${x.where} ${x.issue.level}`)).toEqual(['en/ios69/01.png error']); + expect(issues[0]!.issue.msg).toContain('/frames/en/phone/home.png'); + } finally { rmSync(dir, { recursive: true, force: true }); } + }); +});