Skip to content

Close the push→display validation gap: reject anything the viewer can't render#219

Merged
ivanmkc merged 3 commits into
masterfrom
fix/server-push-validation
Jul 4, 2026
Merged

Close the push→display validation gap: reject anything the viewer can't render#219
ivanmkc merged 3 commits into
masterfrom
fix/server-push-validation

Conversation

@ivanmkc

@ivanmkc ivanmkc commented Jul 3, 2026

Copy link
Copy Markdown
Owner

Problem

The server accepted (204) payloads the viewer has no renderer for. The pusher only discovered the breakage later, as visible error blocks in the web UI — with no feedback loop to fix it. Confirmed on a live workspace (189 boards):

  • Unknown pane types invented by agents — Panel, Image, Table, Text, pane — render as [unsupported type: X] blocks (3 boards affected, plus 3 top-level datatable boards pushed before Add Masonry component with optional swimlanes #214 shipped the renderer).
  • Unknown component namesButton, MapLink, ListItem (for List.Item), component — render as [unknown component: X] markers (6 boards affected).
  • Flow edges referencing missing nodes are silently dropped by React Flow — graph looks subtly wrong, no signal.
  • Vega-Lite specs with nothing to draw fail only at vega-embed time in the browser.

Fix

All checks live in @ivanmkc/termchart-core's validateContent — the single source of truth already shared by the server (authoritative 400 with a path-pointed reason) and the CLI (offline fast-fail with the same message) — so the pushing agent gets immediate, actionable feedback and can fix + re-push:

  • unknown top-level and nested pane types → rejected, response lists the supported set
  • component whitelist enforcement for the children tree and node-valued props, mirroring the client resolver's isComponentNode heuristic exactly (data-shaped props like Vega encodings / Table data pass through untouched), with a did-you-mean hint: unknown component "ListItem" (did you mean "List.Item"?)
  • flow: unique non-empty string node ids; source/target/parentId must reference existing nodes
  • vegalite: must have a view-defining key (mark/layer/facet/*concat/repeat/spec)

New exports KNOWN_PUSH_TYPES / KNOWN_COMPONENTS are locked to the client registry + component resolver by a new parity test (known-parity.test.ts), so adding a renderer/component on one side without the other fails the build.

Verification

  • 14 new unit/wire tests; full suites green (viewer 524, cli 173)
  • Regression sweep with the real validator over all 192 live boards + 66 diagram-recipes examples: zero false positives — the only rejects are boards already visibly broken in the UI today
  • E2E against the built dist/server.js: each broken class → descriptive 400; valid component/datatable/flow pushes → 204

…wer can't render

The server 204'd payloads it had no renderer for; the pusher only discovered the
breakage as '[unsupported type: X]' / '[unknown component: X]' blocks or silently
dropped edges in the browser, with no feedback loop to fix them. Observed live:
boards with invented types (pane types "Panel"/"Image"/"pane"), unknown
components ("Button", "MapLink", "ListItem"), all stored as successes.

validateContent (shared: server 400 + CLI offline fast-fail) now rejects, with
path-pointed messages:
- unknown top-level/pane types, listing the supported set
- component nodes (children AND node-valued props, mirroring the client
  resolver's isComponentNode heuristic exactly) not in the whitelist, with a
  did-you-mean hint (ListItem → "List.Item")
- flow nodes without unique string ids, edges/parentId referencing missing
  nodes (React Flow silently drops those edges)
- vegalite specs with no view-defining key (nothing to draw)

New KNOWN_PUSH_TYPES / KNOWN_COMPONENTS exports are kept in lockstep with the
client registry + component resolver by a new parity test.

Verified against all 192 live boards and 66 diagram-recipes examples: zero
false positives — the only rejects are boards already visibly broken today.
…harness

Validator stress tests (hot-path bounds): a 10k-node/20k-edge flow and a wide
~1 MB component tree validate in <2s with errors still caught at the end of the
payload; a near-cap (~600 KB content) push round-trips through the real server
in <5s (204) and a buried unknown component still returns the path-pointed 400.

The stress e2e suite (34 browser cases) had rotted independently of this
branch — every sidebar click timed out because it matched on the button's
inner text ('stress/<name>') which the scope-proj/scope-agent span split
removed (rich.e2e was already migrated to the title selector), and it treated
HTTP 200 (stored with geometry findings) as a rejected push. Fixed both, and
updated two fixture expectations to the new contract: flow_dangling and
component_unknown are now rejected at push time instead of stored-and-broken.

Suites: viewer 528, cli 173, rich e2e 62/62, board-sort 12/12, template 17/17,
responsive 20/20, overlap 31/31, stress 34/34.
@ivanmkc

ivanmkc commented Jul 3, 2026

Copy link
Copy Markdown
Owner Author

Added stress + integration coverage (second commit):

Validator stress tests (validate.test.ts, server.test.ts): 10k-node/20k-edge flow and ~1 MB-wide component tree validate in <2 s with errors still caught at the end of the payload; a near-cap push round-trips the real HTTP server in <5 s, and a buried unknown component still comes back as a path-pointed 400.

Full e2e/integration matrix run (real server + headless Chromium): rich 62/62 · board-sort 12/12 · template 17/17 · responsive 20/20 · overlap 31/31 · stress 34/34.

The stress e2e harness turned out to be broken on master independently of this branch (stale sidebar locator from the scope-proj/scope-agent span split; treated HTTP 200 stored-with-findings as a reject) — fixed both, and updated flow_dangling / component_unknown fixture expectations to the new push-time-reject contract.

@ivanmkc ivanmkc marked this pull request as ready for review July 4, 2026 00:18
…whitelists

/simplify pass over the branch:

- isComponentNodeLike now lives ONLY in core and the client resolver imports it
  (its isComponentNode was a hand-kept copy the parity test couldn't guard);
  RESOLVE_MAX_DEPTH likewise imports core's MAX_DEPTH instead of mirroring it.
- KNOWN_PUSH_TYPES / KNOWN_COMPONENTS are as-const tuples exporting
  KnownPushType / KnownComponent; the client LOADERS map is keyed by
  KnownPushType and REGISTRY satisfies Record<KnownComponent, ...> — list
  drift is now a compile error at the offending line, with the runtime parity
  test kept as a JS-level backstop.
- Deleted registerComponent (no callers; post-validation any runtime-registered
  name would be rejected at push before it could render — a dead escape hatch
  contradicting the closed-world contract).
- Hot-path trims: supported-list strings joined once at module scope; the
  flow parentId second pass skips when no node carries one; edge endpoint
  checks unrolled via a helper instead of an array-literal loop per edge;
  node-valued array props validated through the tree walker's own array
  handling instead of a duplicated index loop.
- Test dedup: dropped two spot-check list tests subsumed by known-parity, and
  the server stress test's timing bound (owned by the unit stress test).

Suites: viewer 526, cli 173, rich e2e 62/62 after the refactor.
@ivanmkc ivanmkc merged commit 8c28b4f into master Jul 4, 2026
5 checks passed
@ivanmkc ivanmkc deleted the fix/server-push-validation branch July 4, 2026 01:44
ivanmkc pushed a commit that referenced this pull request Jul 4, 2026
… works, journey selection still the gap

#219 (push --type validation) merged into master. Re-ran journeys: agents now
get an actionable error on an invalid --type and retry with a VALID type every
time (no more silently-stored unrenderable boards — finding #2 fixed). Scores
stay ~1/5 because agents retry to a valid-but-wrong type (Claude falls back to
mermaid); picking the RIGHT journey is a recipe-activation gap, not validation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants