M1 Phase 3: attribute validation + the generation workflow - #303
Conversation
Pulls M2's attribute allowlist forward, per operator decision, because Phase 3's skill loop is "self-correct until validate is clean" and clean did not mean the page works. M0 Phase 2 proved a document using lvt-filter, lvt-scroll and a literal lvt-totally-made-up validated with zero errors: unknown lvt-* attributes are emitted as inert HTML. A hallucinated attribute would have survived every iteration of the loop. That document now fails, with a migration hint where a real one exists: unknown attribute "lvt-scroll" (use lvt-fx:scroll) unknown attribute "lvt-sortable" Three design calls. The allowlist is hand-maintained so validate stays fast and dependency-free, but TestKnownAttributesAreReal checks every entry against the vendored bundle and Tinkerdown's own source. Without that it would be an unfalsifiable claim -- exactly what left 8 of 11 documented attributes stale before M0 Phase 2. Namespaces are validated, members are not. lvt-el:bogus:on:success still passes: there is no enumerable member set for lvt-on:, which takes arbitrary DOM events, and hard-coding one for the others would create a second list to rot. Documented as a deliberate limit rather than left to look like an oversight. Hints only where a real migration exists. A guessed suggestion is worse than none -- it sends a self-correcting agent somewhere specific and wrong, which is harder to recover from than "unknown". Also fixes a false positive this created in M0 Phase 2's guard. vocabulary.go's migration table names dead attributes in order to suggest replacements, and a substring scan cannot tell mentioning an attribute from implementing one, so the guard reported six dead attributes as live. Excluding the file would have been a blind spot -- the thing that guard exists to prevent -- so the exclusion is paid for by TestMigrationHintsAgreeWithRemovedLists, which asserts every migrated-away name is recorded dead AND genuinely absent from the shipped client. The second assertion covers a failure mode that did not exist until this commit: a hint pointing away from a live attribute would send an agent away from working code. Verification: GOWORK=off go test ./... green including the root package with all 32 !ci e2e files (835s). Both new guards verified to fail when their conditions are violated. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018M9pJSPmG6i1D8s6rpEV4h
…ure class Extends skills/tinkerdown with the full generation workflow: read the approved surface, write the document, validate and self-correct, check the operation summary if privileged, serve. Two operator decisions are recorded in it. Serving happens via `tinkerdown serve` on a scratch directory, not the playground's ParseString path -- the playground is an HTTP endpoint set requiring a running server, a POST and a session ID, and it is not disk-free anyway (websocket.go:495 still writes each block to /tmp). A directory the operator can re-run and inspect is more useful, and if the UI turns out to be worth keeping it already is a file. The validate loop stops after about five rounds, because a request that still fails likely needs a capability the vocabulary lacks, and saying so beats substituting attributes until something passes. Dogfooding the workflow found the phase's most valuable defect. Following these instructions produced a page that validated clean, summarised as privileged, served without error, and rendered nothing: lvt-* markup only binds inside a ```lvt fence, and in the markdown body it is ordinary HTML. No browser console error, no server-log warning. Every available signal said success. That is the third distinct way a document passes validation and does not work, after unknown attributes and unapproved names -- and the hardest to notice, since nothing anywhere reports it. InertAttributes now catches it at the gate, naming the remedy, and the skill leads step 2 with the fence requirement. Prose prevents the mistake; the gate catches it. Worth recording that I had just closed the first class and believed that made the loop honest. "Clean" is a claim about what was checked, and each layer added revealed another thing that was not. Only dogfooding exercised the whole chain rather than the part I was thinking about. Verification: GOWORK=off go test ./... green including the root package with all 32 !ci e2e files (852s). End-to-end: a console generated by following the workflow serves and renders live sqlite data, verified by screenshot with zero JS exceptions. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018M9pJSPmG6i1D8s6rpEV4h
|
Reviewed the diff (checked out This is a strong PR — the "validates clean but does nothing" framing is right, and the three new gates (vocabulary, placement, policy) close real gaps. Two things below undercut the guarantees the PR is explicitly trying to establish, in the same spirit as the bugs the PR itself is fixing. 1.
|
Review-bot catches on #303. The first undercut a guarantee this PR is built on. TestKnownAttributesAreReal could never fail. It scanned production Go for evidence each allowlist entry was real, but the allowlist lives in vocabulary.go, which is production Go -- so every entry matched its own map key. Adding "lvt-totally-made-up": true passed cleanly. Fixed by excluding vocabulary.go from the scan, exactly as collectGo already excludes it in attribute_docs_test.go. This is the third instance of this self-certifying bug, and the worst: I fixed it in collectGo in this same commit, with a comment saying a substring scan cannot tell mentioning an attribute from implementing one, then did not apply that reasoning to the scanner guarding the new allowlist. Knowing the failure mode was not enough; only running the falsification test was. Verified now by adding a fictional entry and watching the guard fail. data-lvt- was a blanket-allowed prefix, so data-lvt-sortable validated clean -- the same hole this file exists to close, one namespace over. Unlike lvt-on:/lvt-el: (arbitrary events and states), data-lvt-* is a closed enumerable set, so it is now checked exactly against the 22 names in the bundle, with the invented-name test that was missing. The honesty guard covers this set too. Also hoists a double parse: InertAttributes and UnknownAttributes were each called twice per file, and this runs in the self-correct loop the skill describes, so the waste compounds per iteration. Verification: GOWORK=off go test ./... green including the root package with all 32 !ci e2e files (827s). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018M9pJSPmG6i1D8s6rpEV4h
|
All three fixed — the first was a guarantee this PR is built on, and you were right.
The part worth naming: this is the third instance of this self-certifying bug, and I fixed it in
Double parse — hoisted; it compounds inside the self-correct loop. On your nit: confirmed the same, Full suite green at 827s. |
ReviewReviewed Overall this is a solid, well-motivated change — the falsifiability fix for Potential bug:
|
The M1 reference console — approve/deny PII data-export access requests — on existing primitives plus two generic core additions, hardened across five review rounds. Generic core: - Atomic multi-statement SQL actions (Action.Statements + transactional ExecTx on SQLExecutor), so a state change and its audit record commit or roll back together. One shared source.RunSQLAction/SubstituteParams for the runtime and webhook exec paths (no drift; :operator injected in one place). - Policy-lint fix: built-in source affordances (Add/Delete/Toggle/Refresh) are no longer flagged as unapproved actions; exact-match IsBuiltinAction + custom actions dispatch ahead of the datatable-prefix fallback. Reference app (examples/pii-access-approval/): server-authoritative bounded export (row cap read from the row, not the client), durable idempotent audit trail (status='pending' guards), reserved server-set :operator (no audit spoofing), governed-writes-only design (read-only display sources + a non-app-bindable writable store + a governed intake), orders_pii surfaced in the operation summary. Tests: ExecTx commit/rollback/readonly, ValidateActions, refs/policy, a non-browser CI regression over the real manifest SQL (non-matching/replayed ids, audit-spoof, intake-forge), and a four-channel chromedp e2e + the frontmatter-shadowing runtime demo Phase 1 could not build. Also reconstructs the M1 Phase 3 plan tracker + Learn (a #303 drift). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018M9pJSPmG6i1D8s6rpEV4h
1 participant
Third phase of M1 in the ephemeral-UI reframe plan, following #301 and #302.
Makes
validatea gate the generation loop can actually rely on, and gives the skill the workflow to use it.Pulled M2's attribute allowlist forward
The skill's loop is "self-correct until validate is clean." M0 Phase 2 proved clean meant almost nothing: a document using
lvt-filter,lvt-scrolland a literallvt-totally-made-upvalidated with zero errors, because unknownlvt-*attributes are emitted as inert HTML. A hallucinated attribute survived every iteration.Three design calls: the allowlist is hand-maintained so
validatestays fast, butTestKnownAttributesAreRealchecks every entry against the vendored bundle and Tinkerdown's source — without it, it'd be the same unfalsifiable claim that left 8 of 11 documented attributes stale. Namespaces are validated, members aren't (lvt-on:takes arbitrary DOM events; hard-coding member lists would create a second list to rot) — documented as a deliberate limit. Hints only where a real migration exists, because a guessed suggestion sends a self-correcting agent somewhere specific and wrong.Dogfooding found a third silent-failure class
I followed the workflow I'd just written and produced a page that validated clean, summarised as privileged, served without error, and rendered nothing. No JS exception, no server-log warning.
lvt-*markup only binds inside a ```lvt fence. In the markdown body it's ordinary HTML.lvt-*outside a fenceFixed at the gate, not just in prose:
I'd just closed the first class and believed that made the loop honest. "Clean" is a claim about what was checked, and each layer revealed another thing that wasn't. Only dogfooding exercised the whole chain rather than the part I was thinking about.
Two operator decisions recorded
Serve via
tinkerdown serveon a scratch directory, not the playground. The plan preferred the playground'sParseStringpath as "disk-free" — but it's an HTTP endpoint set needing a running server, a POST and a session ID, and it isn't disk-free anyway (websocket.go:495still writes each block to/tmp). A directory the operator can re-run and inspect is more useful, and if the UI proves worth keeping it already is a file.The validate loop stops after ~5 rounds. A request still failing likely needs a capability the vocabulary lacks; saying so beats substituting attributes until something passes.
A guard from M0 caught a regression from this phase
vocabulary.go's migration table names dead attributes to suggest replacements, andTestRemovedAttributesAreReallyGonecan't distinguish mentioning from implementing — it reported six dead attributes as live. Excluding the file would have been a blind spot, so the exclusion is paid for byTestMigrationHintsAgreeWithRemovedLists, which asserts every migrated-away name is recorded dead and genuinely absent from the shipped client. That second assertion guards a failure mode that didn't exist until this PR: a hint pointing at a live attribute would send an agent away from working code.Verification
GOWORK=off go test ./...green including the root package with all 32//go:build !cie2e files (852s)🤖 Generated with Claude Code
https://claude.ai/code/session_018M9pJSPmG6i1D8s6rpEV4h