Skip to content

M2: deterministic validation (upstream Validate() + tinkerdown semantic checks) - #307

Merged
adnaan merged 4 commits into
mainfrom
m2/kickoff-design
Jul 24, 2026
Merged

M2: deterministic validation (upstream Validate() + tinkerdown semantic checks)#307
adnaan merged 4 commits into
mainfrom
m2/kickoff-design

Conversation

@adnaan

@adnaan adnaan commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Milestone M2 — deterministic validation, executed per the plan's convention 9 (design expanded at kickoff, then implemented). Four commits: the design expansion, an upstream-release record, and two tinkerdown phases.

What M2 delivers

tinkerdown validate now catches the whole "passes validate, silently breaks at serve" class the M1 Learn kept hitting — and, crucially, the generation context (llm-system-prompt.md/SKILL.md/reference.md) already teaches what the validator now enforces, so the two agree: this is first-pass generation reliability, not just more diagnostics.

Phase 1 — upstream livetemplate.Validate() (released v0.21.0)

The structured, line-numbered parse/reactive-AST diagnostics live in livetemplate's internal/parse (unreachable downstream) and its render path swallows them. livetemplate#519 added Validate(templateText) → []Diagnostic built on the internal primitives, shipped as v0.21.0. Recorded here (commit 984d3b9); no tinkerdown code in this phase.

Phase 2 — tinkerdown consumes Validate() per block (addda7e)

  • validate runs each lvt block through livetemplate.Validate() — an unclosed {{range}} now fails with line N: message instead of surfacing only at serve as a silently-dropped block.
  • Dropped the /tmp/lvt-*.tmpl temp-file dance for an in-memory WithParseFS (same parseSources path, disk-free).
  • The check immediately found a real bug (markdown-data-bookmarks used {{split}}, registered nowhere) → added split as tinkerdown's first base block helper, wired to reach both parse and tree-gen.

Phase 3 — tinkerdown-owned semantic checks (b185ed0)

  • Bound-refs — a used lvt-source must resolve to a declared source (per-app config).
  • Action-param completeness — every :param a kind:sql action references must be supplied by a form field or data-* (:operator exempt); over-inclusive to bias toward a safe miss.
  • State-ref diagnostic now teaches lvt-source, not the undefined state="block-id".
  • lvt-persist reports a migration hint.
  • The :name scanner is extracted into one scanParams shared by SubstituteParams (runtime) and ReferencedParams (validate) — single source of truth.

Verification

examples/ corpus 55/55, skill corpus 10/10, full suite green (incl. the PII approve→ExecTx→audit e2e through the refactored SubstituteParams). Every new guard has a verified-to-fail test. Each phase's tracker + Learn is in docs/plans/2026-07-09-ephemeral-ui-reframe.md.

Follow-ups noted in the Learn (out of scope)

  • Docs llms.txt/playground.html still teach lvt-persist (not corpus-validated); the client-bundle getAttribute("lvt-persist") vestige rides with a client release.
  • Field-name/schema validation (.RowCap casing) re-sequenced to M3 — it needs source-schema introspection.

🤖 Generated with Claude Code

https://claude.ai/code/session_018M9pJSPmG6i1D8s6rpEV4h

adnaan and others added 4 commits July 23, 2026 14:29
Expands the outline-only M2 into full phase blocks, informed by M1's
verified Learn feed-forwards and a direct read of ../livetemplate @ v0.20.1.

Milestone shape (operator decision 2026-07-23): keep the upstream
`Validate()` — cross-repo, upstream-first — over a tinkerdown-only lint.
Three phases:

1. (upstream ../livetemplate) `livetemplate.Validate(templateText)` +
   tagged release. Earns its place because the structured, line-numbered
   parse/reactive-AST diagnostics live in `internal/parse` (unreachable
   downstream) and the public render path silently swallows them — a
   public API built on the internal primitives is the only way tinkerdown
   gets them. Render-determinism is an optional `WithSampleData` mode
   (needs typed state); data-free parse+AST checks are the core.
2. (tinkerdown) consume `Validate()` per lvt block for structured
   diagnostics; drop the /tmp temp-file dance at websocket.go:495.
3. (tinkerdown) the tinkerdown-owned semantic gaps M1 verified: bound-refs
   (a used lvt-source must resolve to a declared source — proven-open
   this session), action-param completeness, the state-ref diagnostic
   teaching the real lvt-source fix, lvt-persist→removed-with-migration.

Design corrections vs the outline, recorded in-plan:
- The attribute allowlist stays in tinkerdown: livetemplate-Go never
  references the client-only lvt-mod:/lvt-nav:/lvt-ignore namespaces, so
  an upstream KnownAttributes export would drift from the client;
  vocabulary.go + TestKnownAttributesAreReal (guarded vs the vendored
  bundle) is already the best source-of-truth.
- Field-name/schema validation (Phase 5 feed-forward #2) re-sequenced to
  M3, where its introspection dependency already lives.

Roadmap, gap-analysis row #1, and section headers updated in step; M3–M5
remain outline-only.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018M9pJSPmG6i1D8s6rpEV4h
Ticks the Phase 1 tracker and writes the Learn. livetemplate v0.21.0 ships
Validate(templateText string, opts ...ValidateOption) ([]Diagnostic, error).

Key drift from the plan, recorded in the Learn:
- Built on the serve-faithful ParseFS/(*Template).Parse path, NOT raw
  internal/parse: .Parse runs the full serve pipeline and returns parse errors
  directly (unlike the swallowing Execute), and the ParseFS path clones the
  component-bearing template so components resolve (the .Parse method drops them).
- Diagnostic shipped as {Line, Severity, Message}: html/template reports no
  column (verified) and no check had a hint, so Col/Hint were dropped pre-release
  after PR review. They can be appended back non-breakingly with the deferred
  sample-data mode.
- WithSampleData deferred (advisor): no consumer this cycle, determinism already
  shipped engine-side in v0.20.0. SeverityWarning reserved as its home.
- The justification sharpened to an empirical gap: tinkerdown validate passes an
  unclosed {{range}} in a block as clean (never runs livetemplate's parser on
  block content) — Validate closes exactly this, unreachable downstream.

Feed-forward to Phase 2: consume via WithValidateComponents(getComponentTemplates()...),
map {Line, Message} onto fileValidationError, pin v0.21.0 (provenance = No,
ClientVersion still 0.20.0), drop the /tmp temp-file dance at websocket.go:495.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018M9pJSPmG6i1D8s6rpEV4h
…block

Pins livetemplate v0.21.0 and runs each lvt block's template through the new
Validate(), so a template-syntax or composition error in a block — an unclosed
{{range}}, an unknown function, an unresolved component — is reported with a
line and message instead of surfacing only at serve as a silently-dropped block.

- validate.go: validateBlockTemplates runs livetemplate.Validate(block.Content,
  WithValidateComponents(ComponentTemplates()...)) per interactive block, in
  sorted-ID order, and gates validFiles. Diagnostics render as "line N: message".
- websocket.go: exported getComponentTemplates -> ComponentTemplates so validate
  and serve resolve components against one set; dropped the /tmp/lvt-*.tmpl
  temp-file dance for an in-memory WithParseFS (same parseSources path, disk-free,
  behavior-identical to serve's old WithParseFiles).

The new check immediately found a real pre-existing bug: examples/markdown-data-
bookmarks used {{split ...}}, registered nowhere, so that block had always
failed to render at serve (invisible before this phase). Fixed by adding
capability (operator decision): split is Tinkerdown's first base block helper
(blockHelperFuncs), merged into ComponentTemplates' set funcs so it reaches both
a block's parse (set funcs reach the main parse) and its tree generation
(getComponentFuncs merges the set funcs). A test pins split in both funcMaps.

Verified: examples corpus 55/55 green (datatable/table blocks resolve, no false
positives); PII e2e passes through the disk-free path (29.9s); full suite green.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018M9pJSPmG6i1D8s6rpEV4h
Closes four "passes validate, breaks at serve" gaps M1's Learn verified:

- Bound refs (unresolvedSourceDiags): a source bound via lvt-source must resolve
  to a declared source (frontmatter ∪ the tinkerdown.yaml governing the file,
  resolved per-app via sourceConfigForFile so a multi-app `validate examples/`
  checks each app against its own config). A typo is reported with the declared
  list instead of erroring only at serve as "source not found".
- Action-param completeness (unsuppliedActionParams): every :param a kind:sql
  action references must be supplied by a form field or data-* attribute
  (:operator is server-set, excluded). The supplied set is deliberately
  over-inclusive (every data-* key + form field anywhere), so it fires only when
  a param is supplied nowhere — biasing to a safe miss over a false positive.
- State-ref diagnostic teaches the real fix (add lvt-source), not the undefined
  state="block-id".
- lvt-persist reports a migration hint (removed from knownAttributes + added to
  suggestAttribute) instead of a confusing "no state reference".

The :name scanner is extracted into a single scanParams shared by SubstituteParams
(runtime SQL) and the new ReferencedParams (validate) — one source of truth, so
what substitution consumes and what validate requires cannot drift. The
SubstituteParams refactor is behavior-equivalent (its existing suites + the PII
approve→ExecTx→audit e2e stay green).

The bound-refs check caught two skill examples (02-dashboard, 10-status-page)
that declared their source in a documentation yaml block rather than frontmatter,
unlike the other 8 — moved to frontmatter so the few-shot corpus uniformly teaches
"declare the sources you bind" (M2's generation-reliability goal).

Verified: examples corpus 55/55, skill corpus 10/10, full suite green (incl. the
PII e2e through the refactored SubstituteParams). Each new guard has a
verified-to-fail test.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018M9pJSPmG6i1D8s6rpEV4h
@claude

claude Bot commented Jul 24, 2026

Copy link
Copy Markdown

Review: M2 deterministic validation

Nicely scoped PR — the phase split (upstream `Validate()` → per-block consumption → tinkerdown-owned semantics) is clean, the `scanParams`/`ReferencedParams` extraction removes real duplication, and the corpus-as-false-positive-gate discipline shows in the Learn notes (the per-app `sourceConfigForFile` fix is a good catch of exactly the kind of bug this milestone exists to prevent).

Bug: `unsuppliedActionParams` misses frontmatter-declared actions

`cmd/tinkerdown/commands/validate.go:654-684` (`unsuppliedActionParams`) only looks up actions in `fileConfig.Actions` (the `tinkerdown.yaml` map):

```go
func unsuppliedActionParams(page *tinkerdown.Page, fileConfig *config.Config) []string {
if page == nil || fileConfig == nil {
return nil
}
...
for _, actionName := range page.Refs().Actions {
action, ok := fileConfig.Actions[actionName]
if !ok || action.Kind != "sql" {
continue
}
```

But `kind: sql` actions can also be declared in page frontmatter (`PageConfig.Actions`, populated at `page.go:257-268`), and those are real, executable actions — `internal/server/websocket.go:1287` (`getPageActions`) merges `h.page.Config.Actions` into the actions a page can invoke at runtime, and they go through the same `SubstituteParams` call that this check exists to guard. Since `unsuppliedActionParams` never consults `page.Config.Actions`:

  1. A frontmatter action referenced by name in `page.Refs().Actions` but absent from `fileConfig.Actions` just `continue`s — silently skipped, not just under-checked.
  2. Worse, the early return `if page == nil || fileConfig == nil { return nil }` means a document with no governing `tinkerdown.yaml` at all (frontmatter-only actions) skips the entire check — exactly the "passes validate, breaks at serve" scenario this milestone is meant to close.

Compare with `unresolvedSourceDiags` a few lines up (`validate.go:576-599`), which correctly unions `page.Config.Sources` (frontmatter) with `fileConfig.Sources` before checking — the source check got this right, but the action-param check wasn't given the same treatment. Worth mirroring: build the "declared sql actions" set from `page.Config.Actions ∪ fileConfig.Actions` (dropping the `fileConfig == nil` short-circuit) so frontmatter-only actions are covered too. `TestUnsuppliedActionParams`/`_OperatorExcluded` currently only exercise the `tinkerdown.yaml`-declared path, so this gap isn't caught by the new tests either.

Minor / non-blocking

  • `suppliedParamNames` (`validate.go:688`) scans `ServerBlocks`, `InteractiveBlocks`, and `StaticHTML`, but not `WasmBlocks`. Probably fine given actions are server-executed and wasm blocks are unlikely to host action-invoking forms, but worth a one-line confirmation/comment if that's intentional.
  • `hasConfigFile`'s filename list (`tinkerdown.yaml`/`lmt.yaml`/`livemdtools.yaml`) duplicates the list `config.LoadFromDir` presumably already knows — you flagged this yourself in the Phase 3 Learn as a DRY risk; a shared exported list would remove the drift risk entirely rather than just documenting it.
  • Solid test coverage and Learn documentation otherwise — the fidelity guards (`component_funcs_test.go` pinning `split` in both the parse and tree-gen funcMaps) are exactly the right shape to prevent the split-brain regression you called out.

No security or performance concerns beyond the above — the added checks are O(document size) and run once per file during `validate`, not on the hot serve path.

@adnaan
adnaan merged commit f8167fc into main Jul 24, 2026
4 checks passed
@adnaan
adnaan deleted the m2/kickoff-design branch July 24, 2026 01:18
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.

1 participant