feat(viewer): push-time panes-density readability lint

docs/superpowers/specs/2026-06-08-flow-readability-lint-design.md

@@ -0,0 +1,205 @@
+# Push-time readability lint ("flow validation")
+
+**Date:** 2026-06-08
+**Status:** Implemented 2026-06-15 — **adapted** from the design below (see "Implementation note").
+
+## Implementation note (2026-06-15)
+
+Between approval and implementation the codebase grew a severity-tagged **geometry-findings**
+system (`packages/viewer/src/flow-geometry.ts` → `geometryReport`; `Finding{severity,code,message}`)
+that already provides *everything this spec proposed as new plumbing*: the server returns
+`200 + {warnings, findings}` on a non-clean push (`204` when clean, `422` under
+`x-termchart-strict`), and the CLI already prints findings to stderr. `validateContent` also moved
+to `@ivanmkc/termchart-core`.
+
+So the design's **goal** (a static, push-time, warn-only density lint covering every push path)
+was kept, but its **architecture** was adapted to that reality — which also honors this spec's own
+stated principle ("single source of truth … no duplicated lint logic across packages"):
+
+- **No** new `packages/viewer/src/readability.ts` module — the density checks are folded into
+  `geometryReport` and emitted as `Finding`s.
+- **No** `server.ts` change — it already calls `geometryReport` and returns the findings.
+- **No** `push.ts` change — the CLI already surfaces findings (`printGeometry`).
+- New finding `code`s: `rows-overstuffed`, `columns-too-many`, `grid-too-many`, `panes-nested`,
+  `flow-too-many-nodes` (all `warning` severity).
+
+**Thresholds recalibrated against the real corpus** (the spec called them "starting points, not
+load-bearing constants"). The `rows` rule of "> 4 weight OR ≥ 3 heavy" flagged four *curated*
+gallery examples (c4-architecture: weight 9 / 3 heavy; the three explainers: weight 7 / 3 heavy),
+so it was tightened to **≥ 4 heavy OR weight > 10**, which cleanly separates the over-stuffed
+trigger case (screwbits: weight 10 / 4 heavy → warns) from the densest legit gallery views. The
+other thresholds (columns > 3, grid > 8, nested panes, standalone flow > 30 nodes) were already
+clear of the gallery and kept as specified. A "panes density" calibration guard test asserts no
+shipped example trips a finding.
+
+Files actually touched: `packages/viewer/src/flow-geometry.ts`,
+`packages/viewer/test/flow-geometry.test.ts` (the original "Files touched" list below is the
+pre-adaptation plan).
+
+---
+
+## Problem
+
+A pushed view can render unreadably small. The trigger case is
+`explainer/screwbits` (the [demo workspace](https://termchart-viewer-yhjtx5dusq-uc.a.run.app/w/demo/#explainer%2Fscrewbits)):
+a `rows` panes layout with **5 panes, ~4 of them "heavy"** (markdown → flow →
+component → vegalite → component). Stacked vertically, each pane gets a sliver of
+the viewport, so the flow (10 nodes, TB) and the charts render cramped and tiny.
+
+Nothing today catches this. The two existing checks are orthogonal:
+
+- `packages/viewer/src/validate.ts` — **structural only** at push time: does the
+  JSON parse, does it have the right shape (`{nodes,edges}`, `{layout,panes}`,
+  component tree). It does not look at how much content is crammed into one view.
+- `packages/cli/src/lint.ts` — Mermaid **terminal-subset** lint (unsupported
+  constructs, node-count cliff for the ASCII renderer). Unrelated to the rich
+  viewer.
+
+The diagram-recipes skill *advertises* a "geometry lint" that returns readability
+warnings on flow push — but no such code exists. This spec fills that gap.
+
+## Goal
+
+Add a static, push-time **readability lint** that warns (does not block) when a
+view is over-stuffed and will render unreadably dense, covering **every** push
+path. Warn-only so thresholds can be calibrated against the real corpus before any
+tightening.
+
+## Non-goals (YAGNI)
+
+- **No** screenshot/vision check (true pixel readability) — considered and
+  rejected as too expensive for a per-push gate.
+- **No** rejecting pushes — warn-only.
+- **No** auto-fixing the renderer (pagination / min-scale).
+- **No** in-viewer warning banner — possible later add, out of scope here.
+
+## Architecture
+
+A new pure module **`packages/viewer/src/readability.ts`**, mirroring the shape and
+conventions of `validate.ts`. It never renders; it reads the parsed spec.
+
+```
+server push handler (packages/viewer/src/server.ts):
+  validateContent(type, content)   → 400 on structural error      (unchanged)
+  readabilityLint(type, content)   → string[] warnings            (NEW)
+  store.set + hub.broadcast        → push always succeeds          (unchanged)
+  respond:
+    204                            if no warnings                  (unchanged shape)
+    200 + { warnings: string[] }   if any warnings                (NEW)
+```
+
+Running it **server-side**, right after `validateContent` passes, is what makes it
+universal: it covers the CLI, the ADK listener agent, and raw `POST /push` alike —
+not just the CLI. The server is the single source of truth; the CLI only prints
+what the server returns (no duplicated lint logic across packages).
+
+### Module interface
+
+```ts
+// packages/viewer/src/readability.ts
+
+/** A non-fatal readability concern: the view is renderable but will be cramped. */
+export interface ReadabilityWarning {
+  /** Stable code for testing / future filtering, e.g. "rows-overstuffed". */
+  code: string;
+  /** Human-readable, actionable message. */
+  message: string;
+}
+
+/**
+ * Inspect a parsed push payload for layouts that will render unreadably dense.
+ * Pure: no rendering, no I/O. `content` is the raw JSON string (already known to
+ * parse + be structurally valid, since validateContent runs first).
+ * Returns [] when the view is comfortably sized.
+ */
+export function readabilityLint(type: string, content: string): ReadabilityWarning[];
+```
+
+The server flattens warnings to `string[]` (`message`) for the response body; the
+`code` exists for unit tests and possible future client-side filtering.
+
+## Heuristics — the "density budget"
+
+Estimate each pane's comfortable **weight** (≈ readable blocks of vertical room),
+then flag layouts whose total exceeds what a viewport shows at a readable size.
+
+### Per-pane weight
+
+| Pane type            | Weight |
+|----------------------|--------|
+| `flow`               | 2, **+1 per ~8 nodes beyond 6** (10 nodes ≈ 2–3) |
+| `vegalite`           | 2 (axes + legend need room) |
+| `component`          | 1, or **2** if the tree is large (contains `Table`/`SimpleGrid`, or > ~40 nodes) |
+| `markdown` / `text`  | `ceil(chars / 1200)`, capped at 2 |
+| `mermaid`            | 1 |
+| nested `panes`       | 2 (cumulative shrink) |
+
+A pane with weight ≥ 2 is "heavy".
+
+### Layout rules (starting thresholds)
+
+These are warn-only and **explicitly tunable** against the corpus; they are
+starting points, not load-bearing constants.
+
+- **`rows`**: warn if total weight **> 4** OR **≥ 3 heavy** panes.
+  → screwbits ≈ weight 8–10 with ~4 heavy → warns.
+- **`columns`**: warn if **> 3** panes (each column too narrow to read).
+- **`grid`**: warn if **> 8** tiles (each tile drops below a readable size).
+- **nested panes**: warn whenever a pane's content is itself a `panes` payload.
+- **standalone `flow`** (top-level push, not in panes): warn if node count is very
+  high (**> 30**) — suggest splitting into multiple scopes.
+
+### Warning messages (actionable)
+
+Each warning names the offender and gives the fix. Examples:
+
+- `rows-overstuffed`:
+  > `panes 'rows' has 5 panes incl. 3 heavy (flow, vegalite, component); stacked they'll render cramped. Split across scopes, use a grid, or fold detail into fewer panes.`
+- `columns-too-many`:
+  > `panes 'columns' has 4 panes; columns get too narrow past 3. Drop one or switch to rows/grid.`
+- `grid-too-many`:
+  > `panes 'grid' has 10 tiles; each drops below a readable size past ~8. Split across scopes.`
+- `panes-nested`:
+  > `panes[2] nests another panes layout; nested panes compound the shrink. Flatten into one layout or a separate scope.`
+- `flow-too-many-nodes`:
+  > `flow has 38 nodes; past ~30 it renders too small to read. Split into linked scopes or collapse detail.`
+
+## Surfacing warnings (CLI)
+
+`packages/cli/src/push.ts` currently treats any 2xx as success and ignores the
+body. Change: on a **200** response, parse `{ warnings }`, print each to **stderr**
+(prefixed, e.g. `readability: <message>`), and still **return 0**. A **204**
+(clean, or an old server that predates this change) prints nothing. Fully
+backward-compatible.
+
+## Testing
+
+- **Unit** (`packages/viewer/test/readability.test.ts`, mirroring
+  `validate.test.ts`):
+  - clean single-pane view → no warnings
+  - screwbits-shaped `rows` (5 panes, ~4 heavy) → `rows-overstuffed`
+  - `columns` with 4 panes → `columns-too-many`
+  - `grid` with 10 tiles → `grid-too-many`
+  - nested panes → `panes-nested`
+  - standalone flow with 35 nodes → `flow-too-many-nodes`
+  - light report (one markdown + one small component in rows) → no warnings
+  - non-structured types (`mermaid`/`markdown`/`text` at top level) → no warnings
+- **Server**: push that triggers a warning returns `200 + {warnings}` AND still
+  stores + broadcasts (push not blocked).
+- **CLI** (`packages/cli/test/push.test.ts`): a `200 + {warnings}` response prints
+  each warning to stderr and exits 0; a `204` prints nothing and exits 0.
+
+## Rollout note
+
+The lint runs in the viewer server, so the **deployed Cloud Run viewer must be
+redeployed** for cloud pushes to surface warnings; a local dev viewer picks it up
+immediately. The CLI change is independent and backward-compatible (against an old
+server it simply never sees a 200-with-warnings).
+
+## Files touched
+
+- `packages/viewer/src/readability.ts` — new pure lint module.
+- `packages/viewer/src/server.ts` — call lint after validate; respond 200+warnings.
+- `packages/cli/src/push.ts` — print warnings from a 200 response to stderr.
+- `packages/viewer/test/readability.test.ts` — new unit tests.
+- `packages/cli/test/push.test.ts` — CLI stderr/exit-code test.

packages/viewer/src/flow-geometry.ts

@@ -54,7 +54,10 @@ export type Severity = "error" | "warning";
  *  readable (near-misses, crossings, a better direction). The `message` is the human-readable text. */
 export interface Finding {
   severity: Severity;
-  code: "missing-ref" | "edge-over-node" | "node-overlap" | "edge-near-node" | "crossings" | "direction" | "low-readability";
+  code:
+    | "missing-ref" | "edge-over-node" | "node-overlap" | "edge-near-node" | "crossings" | "direction" | "low-readability"
+    // Panes-density ("readability") codes: a structurally-valid layout packed too dense to read.
+    | "rows-overstuffed" | "columns-too-many" | "grid-too-many" | "panes-nested" | "flow-too-many-nodes";
   message: string;
   count: number;
 }
@@ -453,6 +456,102 @@ export function bestLayout(spec: unknown): ChosenLayout {
   return { ...chosen, attempts };
 }
 
+// ── Panes-density ("readability") lint ─────────────────────────────────────────────────────────
+// A view can be structurally valid yet render unreadably small when too much heavy content is
+// stacked into one layout (the trigger case: `rows`, 5 panes, 4 heavy → each gets a sliver). Estimate
+// each pane's "weight" (≈ readable blocks of vertical room) and flag layouts that exceed what a
+// viewport shows at a readable size. Warn-only. The thresholds are calibrated against the shipped
+// gallery (the "panes density readability" calibration guard in the tests keeps them honest) — the
+// densest legit views (c4-architecture, the explainers: rows, ~3 heavy) stay clean; the over-stuffed
+// case (5 panes / 4 heavy) warns. They are tunable, not load-bearing constants.
+
+const PANES_LAYOUTS = new Set(["rows", "columns", "grid"]);
+/** A standalone (top-level) flow past this node count renders too small to read; suggest splitting. */
+const MAX_STANDALONE_FLOW_NODES = 30;
+
+function flowNodeCount(content: string): number {
+  try {
+    const o = JSON.parse(content) as { nodes?: unknown };
+    return Array.isArray(o.nodes) ? o.nodes.length : 0;
+  } catch {
+    return 0;
+  }
+}
+
+/** A component tree is "heavy" (weight 2) when it holds a Table/SimpleGrid or a large node count. */
+function componentWeight(content: string): number {
+  if (/"type"\s*:\s*"(Table|SimpleGrid)"/.test(content)) return 2;
+  return (content.match(/"type"\s*:/g)?.length ?? 0) > 40 ? 2 : 1;
+}
+
+/** Estimate a pane's vertical "weight"; weight ≥ 2 is "heavy". */
+function paneWeight(type: string, content: string): number {
+  switch (type) {
+    case "flow": {
+      const n = flowNodeCount(content);
+      return 2 + (n > 6 ? Math.ceil((n - 6) / 8) : 0); // +1 per ~8 nodes beyond 6
+    }
+    case "vegalite":
+      return 2; // axes + legend need room
+    case "component":
+      return componentWeight(content);
+    case "markdown":
+    case "text":
+      return Math.min(2, Math.ceil(content.length / 1200));
+    case "panes":
+      return 2; // nested panes compound the shrink
+    default:
+      return 1; // mermaid, ansi, …
+  }
+}
+
+/** Layout-level density findings for a `panes` payload (warn-only). Independent of the per-pane
+ *  geometry checks that geometryReport already recurses for. */
+function panesDensityFindings(panes: unknown[], layoutRaw: unknown): Finding[] {
+  const out: Finding[] = [];
+  const layout = typeof layoutRaw === "string" && PANES_LAYOUTS.has(layoutRaw) ? layoutRaw : "columns";
+  const cells: { type: string; weight: number }[] = [];
+  let nestedIdx = -1;
+  panes.forEach((p, i) => {
+    if (p && typeof p === "object") {
+      const pane = p as { type?: unknown; content?: unknown };
+      if (typeof pane.type === "string" && typeof pane.content === "string") {
+        cells.push({ type: pane.type, weight: paneWeight(pane.type, pane.content) });
+        if (pane.type === "panes" && nestedIdx < 0) nestedIdx = i;
+      }
+    }
+  });
+  const n = cells.length;
+  const heavy = cells.filter((c) => c.weight >= 2);
+  const totalWeight = cells.reduce((a, c) => a + c.weight, 0);
+
+  // Nested panes compound the shrink — flag regardless of the outer layout.
+  if (nestedIdx >= 0)
+    out.push({
+      severity: "warning", code: "panes-nested", count: 1,
+      message: `panes[${nestedIdx}] nests another panes layout; nested panes compound the shrink. Flatten into one layout or a separate scope.`,
+    });
+
+  // A payload has exactly one layout, so the three rules are mutually exclusive.
+  if (layout === "rows" && (heavy.length >= 4 || totalWeight > 10))
+    out.push({
+      severity: "warning", code: "rows-overstuffed", count: n,
+      message: `panes 'rows' has ${n} panes incl. ${heavy.length} heavy (${heavy.map((c) => c.type).join(", ")}); stacked they'll render cramped. Split across scopes, use a grid, or fold detail into fewer panes.`,
+    });
+  else if (layout === "columns" && n > 3)
+    out.push({
+      severity: "warning", code: "columns-too-many", count: n,
+      message: `panes 'columns' has ${n} panes; columns get too narrow past 3. Drop one or switch to rows/grid.`,
+    });
+  else if (layout === "grid" && n > 8)
+    out.push({
+      severity: "warning", code: "grid-too-many", count: n,
+      message: `panes 'grid' has ${n} tiles; each drops below a readable size past ~8. Split across scopes.`,
+    });
+
+  return out;
+}
+
 /**
  * Collect geometry warnings for a pushed payload: a `flow` spec, or any `flow` panes inside a
  * `panes` payload (each pane's content is a JSON string). Returns [] for other types / on parse
@@ -486,12 +585,27 @@ export function geometryReport(type: string, content: string, depth = 0): { warn
         severity: "warning", code: "low-readability", count: 1,
         message: `very large — opened at the readable ${MIN_READABLE_PX}px floor it spans ≈${overflow.toFixed(1)}× the screen, so the reader pans a lot. Reduce nodes or split into \`panes\` (and let the engine pick \`direction\`).`,
       });
+    // Standalone (top-level push) flow only: past ~30 nodes it renders too small regardless of layout.
+    // Inside `panes` (depth > 0) the panes-density rule owns the budget, so don't double-flag here.
+    if (depth === 0) {
+      const n = Array.isArray((v as { nodes?: unknown[] }).nodes) ? (v as { nodes: unknown[] }).nodes.length : 0;
+      if (n > MAX_STANDALONE_FLOW_NODES)
+        findings.push({
+          severity: "warning", code: "flow-too-many-nodes", count: n,
+          message: `flow has ${n} nodes; past ~${MAX_STANDALONE_FLOW_NODES} it renders too small to read. Split into linked scopes or collapse detail.`,
+        });
+    }
     return { warnings: findings.map((f) => `flow: ${f.message}`), findings };
   }
   if (type === "panes" && v && typeof v === "object" && Array.isArray((v as { panes?: unknown[] }).panes)) {
     const warnings: string[] = [];
     const findings: Finding[] = [];
     const panes = (v as { panes: unknown[] }).panes;
+    // Layout-level density (over-stuffed rows / too many columns or tiles / nested panes).
+    for (const f of panesDensityFindings(panes, (v as { layout?: unknown }).layout)) {
+      findings.push(f);
+      warnings.push(f.message);
+    }
     panes.forEach((p, i) => {
       if (p && typeof p === "object") {
         const pane = p as { type?: unknown; content?: unknown; title?: unknown };