From 3f9ccb37d7dab1f5e5c71e645ba01b36b7a0525d Mon Sep 17 00:00:00 2001 From: ivanmkc Date: Sat, 1 Aug 2026 01:47:25 +0000 Subject: [PATCH] viewer/framing-lint: add places-no-image rule Warn on push when PlacesExplorer place cards lack a photo (ImageCarousel / Image / item.img). Correct-by-construction guard so place boards always ship image carousels. Non-blocking warning, same {warnings,findings} contract. --- packages/viewer/src/flow-geometry.ts | 1 + packages/viewer/src/framing-lint.ts | 70 +++++++++++++++++++++++ packages/viewer/test/framing-lint.test.ts | 31 ++++++++++ 3 files changed, 102 insertions(+) diff --git a/packages/viewer/src/flow-geometry.ts b/packages/viewer/src/flow-geometry.ts index 82fe800..7b7b967 100644 --- a/packages/viewer/src/flow-geometry.ts +++ b/packages/viewer/src/flow-geometry.ts @@ -65,6 +65,7 @@ export interface Finding { // Framing codes (framing-lint.ts): does the board tell a first-time reader WHAT it is and HOW to use it? | "framing-no-title" | "framing-no-lede" | "framing-interactive-unexplained" | "framing-data-no-legend" | "framing-skeletal" + | "places-no-image" // Density codes (element-density.ts): UI cells rendered below the readable-width floor. | "narrow-nested-grid" | "narrow-table-cols" | "narrow-standalone-grid"; message: string; diff --git a/packages/viewer/src/framing-lint.ts b/packages/viewer/src/framing-lint.ts index 898cc63..0533526 100644 --- a/packages/viewer/src/framing-lint.ts +++ b/packages/viewer/src/framing-lint.ts @@ -189,6 +189,63 @@ function isPlaceholderTitle(t: string): boolean { return PLACEHOLDER_TITLES.has(t.trim().toLowerCase()); } +/** True if a subtree contains a rendered image component with real content — + * an ImageCarousel with a non-empty `images` array, or an Image with a `src`. + * Bounded (shares MAX_TEXT_NODES cap) so a huge item.node can't stall the lint. */ +function subtreeHasImage(node: unknown, cap = { n: 0 }): boolean { + if (cap.n > MAX_TEXT_NODES) return false; + cap.n++; + if (!node || typeof node !== "object") return false; + const n = node as { type?: unknown; props?: Record; children?: unknown }; + if (n.type === "ImageCarousel") { + const imgs = (n.props as { images?: unknown } | undefined)?.images; + if (Array.isArray(imgs) && imgs.length > 0) return true; + } + if (n.type === "Image") { + const src = (n.props as { src?: unknown } | undefined)?.src; + if (typeof src === "string" && src.trim()) return true; + } + const ch = n.children; + if (Array.isArray(ch)) { for (const c of ch) if (subtreeHasImage(c, cap)) return true; } + else if (ch && typeof ch === "object") { if (subtreeHasImage(ch, cap)) return true; } + return false; +} + +/** A PlacesExplorer item "has an image" if it carries an item-level `img` (map-pin thumbnail), + * a non-empty `images` array, or its detail `node` renders an ImageCarousel/Image. */ +function placesItemHasImage(item: unknown): boolean { + if (!item || typeof item !== "object") return false; + const it = item as { img?: unknown; images?: unknown; node?: unknown }; + if (typeof it.img === "string" && it.img.trim()) return true; + if (Array.isArray(it.images) && it.images.length > 0) return true; + if (it.node && subtreeHasImage(it.node)) return true; + return false; +} + +/** Walk the tree; for every PlacesExplorer with items, report how many lack a photo. + * Correct-by-construction guard: place cards are expected to carry image carousels. */ +function placesImageGaps(node: unknown, out: { group: string; missing: number; total: number }[] = [], cap = { n: 0 }): { group: string; missing: number; total: number }[] { + if (cap.n > MAX_TEXT_NODES) return out; + cap.n++; + if (!node || typeof node !== "object") return out; + const n = node as { type?: unknown; props?: Record; children?: unknown }; + if (n.type === "PlacesExplorer") { + const items = Array.isArray((n.props as { items?: unknown } | undefined)?.items) + ? ((n.props as { items: unknown[] }).items) : []; + if (items.length > 0) { + const missing = items.filter((it) => !placesItemHasImage(it)).length; + const group = typeof (n.props as { group?: unknown } | undefined)?.group === "string" + ? (n.props as { group: string }).group + : (typeof (n.props as { id?: unknown } | undefined)?.id === "string" ? (n.props as { id: string }).id : "?"); + out.push({ group, missing, total: items.length }); + } + } + const ch = n.children; + if (Array.isArray(ch)) for (const c of ch) placesImageGaps(c, out, cap); + else if (ch && typeof ch === "object") placesImageGaps(ch, out, cap); + return out; +} + /** Deterministic framing lint. Same {warnings, findings} shape as geometryReport() so * the push handler can concatenate them and x-termchart-strict gates uniformly. */ export function framingReport(type: string, content: string): { warnings: string[]; findings: Finding[] } { @@ -253,6 +310,19 @@ export function framingReport(type: string, content: string): { warnings: string const allTypes = collectTypes(root); for (const t of allTypes) if (INTERACTIVE_TYPES.has(t)) interactiveTypes.add(t); for (const t of allTypes) if (ENCODED_DATA_TYPES.has(t)) { hasEncodedData = true; break; } + // Rule 6 (correct-by-construction): PlacesExplorer place cards must carry an image + // (ImageCarousel / Image / item.img). Text-only place cards read as unfinished; the + // expectation is a photo per card. Non-blocking warning (same as other framing rules). + for (const gap of placesImageGaps(root)) { + if (gap.missing > 0) { + findings.push({ + severity: "warning", code: "places-no-image", count: gap.missing, + message: `PlacesExplorer "${gap.group}": ${gap.missing}/${gap.total} place card(s) have no photo. ` + + `Every place card should carry an ImageCarousel (fetch Google Places photos and add ` + + `{type:"ImageCarousel",props:{images:[{src,alt},…]}} as the card's first child, and set item.img for the map pin).`, + }); + } + } allText = textOf(root); nodeCount = totalNodeCount(root); // BoardHeader's structured legend/howToUse satisfy the interactive-hint and legend rules — diff --git a/packages/viewer/test/framing-lint.test.ts b/packages/viewer/test/framing-lint.test.ts index 31247a7..4191cff 100644 --- a/packages/viewer/test/framing-lint.test.ts +++ b/packages/viewer/test/framing-lint.test.ts @@ -325,3 +325,34 @@ describe("framingReport — {warnings, findings} shape", () => { expect(bad.warnings[0]).toMatch(/^framing:/); }); }); + +describe("framingReport — PlacesExplorer image carousels (places-no-image)", () => { + const item = (id: string, withImg: boolean) => ({ + id, meta: { name: id }, lat: 33, lng: 130, label: id, + node: { type: "Card", children: [ + ...(withImg ? [{ type: "ImageCarousel", props: { images: [{ src: "https://x/p.jpg", alt: id }] } }] : []), + { type: "Text", children: id }, + ] }, + }); + const board = (items: unknown[]) => j({ + type: "Stack", children: [ + { type: "Title", children: "Yanagawa shops" }, + { type: "Text", children: lede20 + " Tap a pin for photos, details and a map link." }, + { type: "PlacesExplorer", props: { group: "yshop", items } }, + ], + }); + it("flags place cards missing an image carousel", () => { + const r = framingReport("component", board([item("a", true), item("b", false), item("c", false)])); + const f = r.findings.find((x) => x.code === "places-no-image"); + expect(f).toBeTruthy(); + expect(f!.count).toBe(2); + }); + it("passes when every place card has an image (carousel or item.img)", () => { + const items = [ + item("a", true), + { id: "b", meta: { name: "b" }, img: "https://x/pin.jpg", node: { type: "Card", children: [{ type: "Text", children: "b" }] } }, + ]; + const r = framingReport("component", board(items)); + expect(r.findings.map((f) => f.code)).not.toContain("places-no-image"); + }); +});