M0 Phase 2: reconcile the attribute docs with what the client implements

[adnaan]

Final phase of M0 in the ephemeral-UI reframe plan, following #296 and #299.

Roughly a quarter of this repo's own attribute documentation was untrue. Of 35 documented lvt-* tokens across the four surfaces that teach the vocabulary, ~10 were wrong.

The category the plan didn't anticipate

The phase was scoped around renames — find what moved upstream, update the name. It found something worse: attributes that were never implemented at all.

• lvt-filter — taught in skills/tinkerdown/reference.md, the exact file M1 plans to hand a generating agent
• lvt-value-* ("extract values from elements") — missed by Phase 1's 11-attribute sample; only the mechanical check surfaced it

Renames are self-announcing: something changed upstream, so there's an event to notice. Fabrications are invisible — no upstream event, no rename, just an attribute that quietly does nothing. That's why these survived longest.

The docs were actively broken, not stale-but-working

The audit posed this as an open question; the answer is the harsher branch. The client carries exactly one back-compat shim (utils/legacy-attr.ts, for lvt-no-intercept). Every other superseded name resolves to nothing — silently, with no console warning.

Corrections

Was	Now
lvt-scroll / lvt-highlight / lvt-animate	lvt-fx:*
lvt-throttle / lvt-debounce	lvt-mod:*
lvt-disable-with	lvt-form:disable-with
lvt-{action}-on:{event}	lvt-el:{method}:on:{state}
lvt-click-away, lvt-window-{event}, lvt-focus-trap, lvt-modal-open/close	removed — callouts name the replacement (native <dialog> for modals)
lvt-filter, lvt-value-*	removed — never implemented; alternatives named

The lifecycle tables were wrong beyond the name: the documented loading state and the disable/enable/focus/blur methods never existed (real states are pending/success/error/done per dom/reactive-attributes.ts:44). Added a § Namespace migration table, lvt-form:preserve as distinct from lvt-ignore, and lvt-datatable — absent despite being the opt-in that actually provides the sorting and pagination reference.md attributed to plain lvt-columns.

No CHANGELOG entry: no example, template, or fixture used a dead attribute, so nothing user-facing regressed. The rot was confined to what we teach.

The durable fix

TestDocumentedAttributesExist checks every documented lvt-* against the vendored bundle (internal/assets/client/tinkerdown-client.browser.js) plus production Go — deliberately not a sibling ../client checkout, so it runs in CI and tests the client that actually ships.

Its first version was self-certifying and passed for the wrong reason. It scanned all .go files for evidence an attribute exists — including itself, and its own doc comment names lvt-filter while explaining the bug. Excluding _test.go fixes it, and is right independently: a fixture using a made-up attribute must not vouch for the docs that invented it. Caught only by deliberately reintroducing the rot and watching the guard fail — a passing guard proves nothing until you've seen it fail.

The finding that outlives this PR

tinkerdown validate does not validate attribute names. Proved empirically: a document using lvt-filter, lvt-scroll, and a literal lvt-totally-made-up validates clean, zero errors. Unknown lvt-* attributes emit as inert HTML.

This guard closes the docs → implementation direction. The direction M1 depends on is the reverse — generated app → implementation — which is entirely unguarded. M1 Phase 3's design has the skill "self-correct on validate diagnostics until clean," and a clean pass demonstrably does not mean the attributes exist: an agent hallucinating lvt-sortable gets green validate and a silently dead page.

The § Risks entry is rewritten as M1-critical-path (it was scoped as an M2 improvement) with an explicit fork: M1 Phase 3 either accepts the gap in writing, or pulls forward the attribute-allowlist portion of M2.

Verification

• GOWORK=off go test ./... green including the root package with all 32 //go:build !ci e2e files (840s)
• skill_examples_test.go + TestLLMSTxtExists 5/5; every skill example still validates
• Guard verified in both directions: fails on reintroduced rot, passes clean
• Manual browser verification is N/A: no rendered output changed and no example markdown was touched, since none used a dead attribute

🤖 Generated with Claude Code

https://claude.ai/code/session_018M9pJSPmG6i1D8s6rpEV4h

[claude]

Review

Solid, well-substantiated docs fix — I independently verified the corrected attribute names against the vendored client bundle (internal/assets/client/tinkerdown-client.browser.js) and production Go, and the claims hold up: lvt-fx:scroll/highlight/animate, lvt-mod:throttle/debounce, lvt-form:disable-with/preserve, and the lvt-el: namespace are all present exactly as the corrected docs describe; lvt-filter and lvt-value-* are indeed absent from both the bundle and non-test Go; lvt-datatable is real (page.go, component_library_e2e_test.go); and lvt-value (bare) is real too — implemented server-side in page.go, not the client. The disable-with gap and the CLAUDE.md-mandated "check the client before documenting" instruction were both followed here in spirit.

The new attribute_docs_test.go guard is a genuinely good idea (docs → implementation is exactly the direction that silently rots), but I found a couple of latent issues in its own logic worth a look:

1. normalizeAttribute's wildcard-family handling for lvt-value-* is dead code — it passes by coincidence, not by design.

The caller does:

attr := normalizeAttribute(strings.TrimRight(m[1], "-:"))

which strips any trailing -/: before normalizeAttribute ever sees the string. That means:

• neverImplemented["lvt-value-"] (with the trailing dash) can never match — normalizeAttribute never receives a string with a trailing dash.
• The strings.HasPrefix(attr, "lvt-value-") branch a few lines later is reachable only for inputs that already survived the map-lookups above, but since the trailing dash is gone by then, the doc's own `lvt-value-*` mention normalizes to "lvt-value" — which happens to collide with the real, distinct lvt-value attribute documented two lines later in the same file (§ Data Binding). So the check passes, but only because an unrelated real attribute of the same trimmed name exists, not because the "fabricated family" logic actually engaged.

Concretely: if a future edit documents another dead wildcard family that doesn't coincidentally share a name with something real (e.g. lvt-widget-* where no bare lvt-widget exists), this guard would silently pass it through as "implemented" once trimmed, rather than flagging it — exactly the class of bug this test exists to catch. Worth either matching against the untrimmed token for the neverImplemented/documentedAsRemoved checks, or trimming inside normalizeAttribute itself so the map keys and the lookup logic agree on which form of the string they're comparing.

2. strings.ContainsAny(attr, "{}") in normalizeAttribute appears unreachable.

attrPattern is `(?:^|[^-\w])(lvt-[a-zA-Z0-9:_-]+)` — the character class can't match { or }, so no captured token can ever contain one (e.g. lvt-{action}-on:{event} gets captured only up through lvt- before the regex stops, or e.g. lvt-el:{method}... captures as lvt-el: and is handled by the namespace/bare-prefix branches instead). Not harmful, just dead code that may give false confidence that placeholder-skipping is happening here when it's actually a side effect of the regex never producing such tokens in the first place.

3. Minor/cosmetic: readGoSources scans ., internal, cmd, pkg — but collectGo(".") already recurses into all of them, since it walks every subdirectory it finds. So internal/cmd/pkg get scanned twice, doubling their bytes in the concatenated buffer. Harmless for substring matching (just wasted work), but the explicit dir list reads as if it's doing something the recursive walk doesn't already cover.

None of these three are blocking — the guard still does its job for every attribute currently in the docs (I confirmed the maps correctly special-case lvt-window-{event} → lvt-window, lvt-value-name, etc.) — but they're worth tightening given the PR's own thesis that "a passing guard proves nothing until you've seen it fail." It might be worth a quick unit test of the test helper (normalizeAttribute/implemented table-driven cases) so these edge cases are pinned down rather than relying on the doc surfaces to keep exercising them correctly by accident.

Other notes:

• Nice catch overall on lvt-columns vs lvt-datatable (sorting/pagination requiring explicit opt-in) — confirmed in page.go/e2e test that lvt-datatable is a real, separate attribute.
• The plan-doc updates (checklist items, Learn section, retired/new risk entries) are thorough and clearly written; no issues there.
• No test coverage concerns beyond the two edge cases above — TestDocumentedAttributesExist itself is a good addition to CI.

[adnaan]

All three taken — the first was a real bug and your diagnosis was exact.

1. lvt-value- was dead code passing by coincidence. Confirmed: normalizeAttribute("lvt-value-") correctly returns "", but the caller trimmed the trailing dash first, so the function never saw it. "lvt-value-" became "lvt-value", which resolves against the real select-binding attribute — the fabricated family wasn't excluded, it was masked by something real. Moved the trim inside normalizeAttribute, after the lookups.

Worth noting that bug was invisible for exactly the reason the original doc rot was: nothing failed.

2. Your suggestion of table-driven tests for the helpers was the most valuable part of the review, and I'd underweighted it. The guard's own correctness was only ever exercised indirectly, through whichever attributes the doc surfaces happened to contain — which is how finding 1 hid. normalizeAttribute and implemented are now pinned directly, including the boundary rule that stops data-lvt-scroll-sticky vouching for the dead lvt-scroll.

Writing them immediately caught a wrong assumption of mine: an unknown family like lvt-widget- normalizes to its trimmed name, not the dashed form I'd asserted. The guard was right; my expectation wasn't. That's the second time on this PR that testing the test found something.

3. readGoSources double-scan — fixed; collectGo(".") already recurses.

One correction to your note on the placeholder branch being dead: strings.HasPrefix(attr, "lvt-value-") is reachable and load-bearing for un-enumerated instances — lvt-value-foo folds onto the family name rather than needing its own map entry. There's now a test case pinning that, so it won't be mistaken for dead again.

All green: guard + helper tests + skill_examples_test.go + TestLLMSTxtExists.

[claude]

Review

This is a genuinely good piece of documentation archaeology, and the meta-story (self-certifying test, _test.go exclusion, the lvt-value- trim-order bug already caught and fixed in the second commit) shows real rigor. attribute_docs_test.go is well-designed: the boundary-safe implemented() matcher, the explicit documentedAsRemoved vs neverImplemented split, and the table-driven TestNormalizeAttribute/TestImplementedMatchesWholeNames pinning tests are exactly the kind of guard this repo needs per CLAUDE.md's "check the client before documenting an attribute" rule.

That said, I checked the PR's core factual claims against the vendored bundle (internal/assets/client/tinkerdown-client.browser.js) directly, and found one real regression:

Bug: lvt-focus-trap is documented as removed, but it's still implemented

Both docs/reference/lvt-attributes.md and the plan doc state lvt-focus-trap is "not implemented by @livetemplate/client" / "absent from the client entirely," and the fix is to delete its docs section and tell users to use native <dialog> instead.

That's not correct. The bundle contains a live, wired-up focus-trap implementation:

s=a=>{if(a.key!=="Tab")return;let l=this.context.getWrapperElement();if(!l)return;
let c=l.querySelectorAll("[lvt-focus-trap]"),d=null; ...};
document[n]=s,document.addEventListener("keydown", ...)

This is a real keydown/Tab handler registered on document that queries [lvt-focus-trap] and cycles focus within matched elements — not dead code, a comment, or a coincidental substring match. lvt-modal-open, lvt-modal-close, lvt-click-away, and lvt-window-* genuinely are absent from the bundle (I checked all of them), so those removals are correct — lvt-focus-trap is the one false positive in that batch.

Why the guard didn't catch this: lvt-focus-trap is listed in documentedAsRemoved (attribute_docs_test.go:133), which makes normalizeAttribute return "" for it — so TestDocumentedAttributesExist never actually checks it against the bundle at all. The removed-list is effectively "trust me" for every entry in it; nothing verifies that a documentedAsRemoved attribute is truly absent from the bundle. Since the whole point of this PR is "don't trust the docs, check the client," it'd be worth either dropping lvt-focus-trap from documentedAsRemoved and restoring its doc section, or — if the goal really is "point users at <dialog> instead of documenting this" — adding a companion assertion that entries in documentedAsRemoved are actually absent from the bundle, so this class of error can't recur silently the same way the original rot did.

Minor / non-blocking

• implemented() in attribute_docs_test.go re-slices and re-converts src[idx:] to a string on every loop iteration (strings.Index(string(src[idx:]), attr)), which is O(n²) in the worst case for a large bundle with many false-boundary hits. Not a real problem given this only runs once in a test over a bounded-size file, but bytes.Index on src[idx:] directly (no string conversion) would be both cheaper and avoid the repeated allocation.
• The neverImplemented map's "lvt-value-" entry documents (via comment) that it's for the fabricated lvt-value-* family, which is good — but it's worth double-checking there isn't a third, un-enumerated lvt-value-*-shaped attribute elsewhere in the docs corpus that isn't lvt-value-name, since the family match is prefix-based (that's fine/intended, just flagging the coupling).

Other spot-checks (all correct)

• lvt-el: methods (reset/addClass/removeClass/toggleClass/setAttr/toggleAttr) and lifecycle states (pending/success/error/done) match the bundle exactly.
• data-lvt-target retargeting and lvt-mod:throttle/lvt-mod:debounce/lvt-fx:* renames all check out.
• lvt-datatable opt-in claim for sortable/paginated tables is confirmed in page.go (lvtDatatableRegex, useDatatable).
• lvt-filter, lvt-value-*, lvt-modal-open/close, lvt-click-away, lvt-window-*, lvt-preserve are all genuinely absent from the shipped bundle — the "never implemented" / "removed" classifications for these are accurate.

Overall: strong PR, well-tested guard, but the lvt-focus-trap removal should be reverted (or the client-absence claim re-verified) before merge, since it's a real, working attribute being told to users as gone.

[adnaan]

Confirmed and fixed — you were right, and re-verifying the whole batch turned up a second false removal you hadn't flagged.

lvt-focus-trap is live exactly as you described. lvt-debounce is too, and distinct from lvt-mod:debounce: it overrides the debounce interval on an auto-wired change binding (state/change-auto-wirer.ts:263) rather than an explicit lvt-on: handler. Both restored, with the two-debounce distinction now documented.

Root cause of the misclassification: I hand-grepped for quoted literals ("lvt-focus-trap"), but the client uses it in a selector — querySelectorAll("[lvt-focus-trap]") — so the literal carries brackets and my pattern missed it. Any attribute used only via selector syntax would have been misclassified identically, so this was systematic rather than one bad entry. That's why I re-checked all 14 rather than just the one you named.

Your structural point was the important half, and I'd underweighted it. documentedAsRemoved/neverImplemented are skip lists, so "this attribute is gone" was the one claim in the guard that nothing verified — an unfalsifiable escape hatch inside a mechanism built to stop trusting unverified assertions. Added TestRemovedAttributesAreReallyGone, which runs the removed-list through the same implemented() matcher. That matcher handles selectors correctly (brackets aren't attribute characters), so the mechanical check catches precisely what my manual search missed. Verified by reintroducing the mistake:

"lvt-focus-trap" is listed as superseded/removed, but it IS implemented in
tinkerdown-client.browser.js — the docs are telling users a working attribute is gone

Also worth naming: the severity here runs opposite to the rot this phase set out to fix. A stale entry misleads someone writing new markup; a wrong deletion tells someone their working code is invalid. I had that backwards while doing the deleting.

On your two minor notes — I've left implemented() on strings.Index for now (bounded input, runs once), and lvt-value-* prefix coupling is intentional and pinned by a test case (lvt-value-foo folds onto the family). Happy to switch to bytes.Index if you'd rather.

Full suite green: 836s including all 32 !ci e2e files.

[claude]

Review

Verified the core claims directly against internal/assets/client/tinkerdown-client.browser.js and the Go sources rather than just reading the diff — everything checked out:

• lvt-fx:*, lvt-mod:*, lvt-form:*, lvt-el:, lvt-on:, lvt-nav: namespaces are all present in the vendored bundle.
• lvt-focus-trap and lvt-debounce (restored in the 3rd commit) are genuinely live — confirmed querySelectorAll("[lvt-focus-trap]")-style usage and a standalone lvt-debounce reference in the bundle.
• lvt-preserve, lvt-click-away (as a standalone attribute), lvt-modal-open/close, and lvt-filter/lvt-value-* are genuinely absent from both the bundle and Go sources — the removals are correct, not just asserted.
• lvt-datatable is real but Tinkerdown-owned (page.go:24,661-662), not client-owned — correctly not expected in the JS bundle.
• docs/llms.txt and docs/llm-system-prompt.md are confirmed clean of the stale names, matching the PR's claim.

Code quality

• attribute_docs_test.go is unusually well-hardened for a docs-guard test: it pins the classification rules directly (TestNormalizeAttribute), pins the boundary-matching rule (TestImplementedMatchesWholeNames), and — notably — closes the "skip list is unfalsifiable" hole by asserting the removed/never-implemented lists are actually absent from the implementation (TestRemovedAttributesAreReallyGone). That last test is the right structural fix: without it, documentedAsRemoved/neverImplemented are the one part of the guard nothing verifies, which is exactly how lvt-focus-trap and lvt-debounce got wrongly deleted in an earlier revision of this same PR.
• Excluding _test.go from readGoSources is correct and non-obvious — the original self-certifying bug (the test's own doc comment naming lvt-filter counted as "implementation evidence") is a good catch, and the fix is justified rather than just applied.
• The documentedAsRemoved/neverImplemented map-based skip-list approach, plus the normalizeAttribute raw-token-before-trim ordering, is well-commented — the comments explain why the ordering matters (the "lvt-value-" → "lvt-value" collision) rather than restating what the code does.

Minor / non-blocking

• implemented() (attribute_docs_test.go:92-112) does strings.Index(string(src[idx:]), attr) in a loop, re-slicing and re-converting src to a string on every iteration. For a multi-hundred-KB minified bundle with a repeated substring this is quadratic in the worst case. It's test-only code that runs a handful of times per go test invocation, so it's not worth optimizing, but worth knowing if the doc surfaces or bundle grow substantially.
• isAttrChar's boundary check does rune(src[end]), a raw byte→rune conversion rather than proper UTF-8 decoding. Fine in practice since attribute names and the bundle are ASCII in the relevant regions, but it's a latent correctness gap if any adjacent byte were part of a multi-byte sequence.
• The namespace check in normalizeAttribute (line 171) treats lvt-on: the same as the Tier-2 namespaces (lvt-fx, lvt-mod, etc.) for the "only check the namespace, not the member" logic. That's a reasonable simplification for this guard, but lvt-on:{event} binds to arbitrary DOM event names rather than a fixed dispatch table the way lvt-el:{method} does — worth a one-line comment noting the difference is intentional, in case a future reader assumes all six namespaces are structurally identical.

Test coverage

Strong — new tests exercise the happy path, the classification edge cases (trailing dash, bare namespace, unknown family), and the specific regression each of the three follow-up commits fixed. The PR's own commit history (three rounds of "review bot found X, verified, fixed") is a good sign that the claims here have already been adversarially checked, and my independent spot-checks against the bundle didn't turn up anything the process missed.

Process note

Per this repo's CLAUDE.md ("check if a lvt-* attribute is already implemented in the livetemplate/client repository before reimplementing it") — this PR doesn't implement any new attributes, only reconciles docs with the already-vendored bundle, so that instruction doesn't add anything beyond what TestDocumentedAttributesExist already enforces here.

No blocking issues found. The two "minor" notes above are optional polish, not correctness concerns.

[adnaan]

Took the lvt-on: note — good catch that the shared case arm reads as though all six namespaces are structurally identical. Added a comment recording that lvt-on:'s member is an arbitrary DOM event name rather than a fixed dispatch table entry, so namespace-only checking is a simplification for the other five and the only option for lvt-on: — there's no enumerable member set to check.

Left implemented() on strings.Index as discussed (bounded input, single run).

Thanks for the three rounds here — the lvt-focus-trap catch in particular. That one would have shipped a regression telling users their working focus-traps were invalid, and re-verifying on the back of it turned up lvt-debounce as a second false removal. The structural fix you pointed at (documentedAsRemoved being unfalsifiable) is now the part of this guard I'd trust most, precisely because it's the part that caught me.

[claude]

Review

Great work on the mechanical guard: TestDocumentedAttributesExist plus the TestNormalizeAttribute/TestImplementedMatchesWholeNames/TestRemovedAttributesAreReallyGone trio is a solid piece of test design (whole-token boundary matching, excluding _test.go from evidence, and pinning the classification rules directly). The 'caught by deliberately reintroducing the rot' verification discipline described in the PR body is exactly right.

That said, I found one correctness bug and one completeness gap in docs/reference/lvt-attributes.md itself, the very file this PR is making authoritative, plus a couple of minor nits.

Bug: self-contradictory Namespace migration table (docs/reference/lvt-attributes.md around line 377)

The new Namespace migration table lists lvt-focus-trap as removed:

| lvt-click-away, lvt-window-{event}, lvt-focus-trap, lvt-modal-open, lvt-modal-close | removed, not implemented by the client |

But lvt-focus-trap is documented right above (around line 203) as a real, current attribute, and I verified it is actually present in internal/assets/client/tinkerdown-client.browser.js. The PR description itself says this was one of the two false removals caught and restored ('Review found lvt-focus-trap documented as removed while a live Tab-cycling handler for it sits in the shipped bundle... Both restored.'). It looks like the restoration updated the dedicated section and the documentedAsRemoved test map, but missed this table.

Same issue one row up (around line 373) for lvt-debounce:

| lvt-throttle / lvt-debounce | lvt-mod:throttle / lvt-mod:debounce |

This implies lvt-debounce is purely a superseded alias for lvt-mod:debounce, but lines 287-295 of the same file correctly document lvt-debounce as a distinct, still-current attribute (overrides the auto-wired change-binding debounce interval). I confirmed the literal lvt-debounce string is present in the bundle too.

This is exactly the failure mode the PR is trying to eliminate, and it is in the table a reader would consult specifically when migrating old markup, so it actively tells them to abandon two attributes that still work. Since normalizeAttribute collapses these bare tokens to themselves and they are not skipped (correctly, since documentedAsRemoved no longer lists them), TestDocumentedAttributesExist won't catch this: the guard only checks whether a token is backed by an implementation somewhere, not whether the surrounding prose correctly characterizes it as removed vs current. Worth a manual fix to the table rows.

Gap: lvt-datatable still missing from the primary reference (docs/reference/lvt-attributes.md)

The PR description says lvt-datatable was added because it was 'absent despite being the opt-in that actually provides the sorting and pagination reference.md attributed to plain lvt-columns.' It was added to skills/tinkerdown/reference.md, but I don't see it anywhere in docs/reference/lvt-attributes.md, not in a dedicated entry, and not in the 'Tinkerdown-Specific Attributes' summary list, even though it is already documented elsewhere (docs/guides/auto-rendering.md, docs/guides/progressive-complexity.md) as a real, implemented attribute. That page's header still claims 'Complete reference for all lvt-* attributes.'

This gap isn't caught by the new test because TestDocumentedAttributesExist only verifies that documented attributes exist; it has no check for the reverse (implemented attributes being documented), so an omission like this is invisible to it.

Minor nits

• Casing inconsistency: the new removed-callout writes lvt-el:removeclass:on:click-away (lowercase removeclass), while the Available methods table and every other example in the same doc use camelCase (addClass, removeClass, ...). Harmless at runtime since HTML lowercases attribute names anyway, but inconsistent for a page meant to be copy-pasted from.
• Guard blind spot for namespace members (by design, just worth surfacing): normalizeAttribute collapses any lvt-fx:/lvt-mod:/lvt-el:/lvt-form:/lvt-nav:/lvt-on: instance down to just checking the namespace prefix exists (e.g. lvt-fx:highlight:on:success becomes lvt-fx:). That's a reasonable trade-off given members are dispatched at runtime, but it does mean a fabricated member of a real namespace (e.g. a hypothetical lvt-fx:pulse) would pass the guard undetected. Might be worth a one-line callout in the test's doc comment so a future reader doesn't assume namespaced attributes get the same fidelity of checking as bare ones.
• documentedAsRemoved["lvt-window"] (attribute_docs_test.go around line 137) looks unreachable in practice now: the only doc surface reference is the placeholder form lvt-window-{event}, which is filtered out earlier by the {} check before the map lookup ever runs. Not harmful, just dead weight.

Everything else

The regex boundary handling, the _test.go-exclusion fix (with the self-referential lvt-filter-in-comments bug it caught), and the TestRemovedAttributesAreReallyGone closing-the-loop-on-the-skip-lists test are all well-reasoned and well-tested. No security or performance concerns; this is a doc/test-only change with no runtime behavior touched, consistent with the 'no CHANGELOG entry' call.

[adnaan]

Both notes addressed — one was right, one wasn't, and my first check of the second was wrong in a way worth recording.

Namespace fidelity — correct, and now documented. lvt-el:bogus:on:success does collapse to lvt-el: and pass. Added a "Known limit" paragraph to the test's doc comment: members dispatch at runtime rather than existing as bundle literals, and lvt-on: takes arbitrary DOM event names with no enumerable set at all, so catching bogus members would mean hard-coding each namespace's member list and keeping it synced with the client — a second invariant to rot. Better stated than silently assumed.

lvt-window is not dead weight — and I confirmed your claim before discovering that. I probed normalizeAttribute("lvt-window-{event}"), got "", and agreed. But that string is never what the pipeline produces: the extraction regex's character class excludes {, so the doc's lvt-window-{event} is captured as lvt-window- and trims to lvt-window — no brace left for the placeholder check. I'd tested an input the pipeline never generates.

Deleting the entry failed TestDocumentedAttributesExist immediately, which is how it surfaced. Restored with a comment explaining why it looks dead, plus a "lvt-window-" case pinning the truncation so the reasoning isn't repeated.

Worth naming: that's the fifth claim in this phase believed because nothing could contradict it — the docs, the self-certifying scan, the unfalsifiable skip list, my own CI monitor's pass/fail condition, and now this. It's also the only one I committed while fixing the others. What caught it wasn't care; it was that a check existed which could fail.

Full suite green: 839s, all 32 !ci e2e files.

[claude]

Review

I independently spot-checked the corrected doc claims against the vendored bundle (internal/assets/client/tinkerdown-client.browser.js) and production Go — lvt-click-away's survival as lvt-el:*:on:click-away (confirmed via SYNTHETIC_TRIGGERS=new Set(["click-away"])), lvt-focus-trap, lvt-debounce vs lvt-mod:debounce, lvt-form:preserve, the lvt-el: method table (reset/addClass/removeClass/toggleClass/setAttr/toggleAttr matches the bundle's dispatch map exactly), lvt-datatable (correctly attributed to Go's page.go, not the client bundle), lvt-ignore-attrs, and data-lvt-force-update/data-lvt-target. All check out. Given the PR description already documents five rounds of self-caught corrections (the lvt-value- masking bug, the two wrongly-removed attributes, the selector-vs-literal grep miss), the remaining content is solid.

Code quality / test coverage — attribute_docs_test.go is well-structured: TestNormalizeAttribute and TestImplementedMatchesWholeNames pin the classification/matching rules directly rather than relying on doc-surface content to exercise them incidentally, and TestRemovedAttributesAreReallyGone closes the "unfalsifiable skip list" hole. The reasoning comments (e.g. the lvt-window- truncation note, the lvt-value- trim-order fix) are exactly the kind that earn their keep — they record non-obvious behavior a future reader would otherwise have to rediscover the hard way.

One gap the guard structurally can't catch, and does exist: I grepped the bundle for all lvt-* literals and found lvt-scroll-away — a live, wired-up attribute (querySelectorAll("[lvt-scroll-away]"), validates its value is top/bottom, console.warns otherwise, requires data-lvt-target) that appears in none of the four doc surfaces this PR audits. That's expected given TestDocumentedAttributesExist only walks docs → implementation, never the reverse, so it's not a bug in this PR's logic — but it's worth naming because the PR's own closing argument is "the reference is now trustworthy as generation context," and an LLM handed reference.md still won't know this attribute exists. The bundle also contains a longer tail of attributes (lvt-spy, lvt-upload, lvt-persist, lvt-redact, lvt-toast-*, lvt-fx:region-select, lvt-fx:auto-click, etc.) that look like distinct authored attributes rather than internal state markers, though I didn't verify each one as carefully as lvt-scroll-away. Documenting all of them is clearly out of this phase's scope (renames/removals/fabrications in already-documented entries), but it might be worth a follow-up note in the plan's risk list alongside the validate-doesn't-check-vocabulary finding, since both point at the same underlying gap: nothing enumerates the client's actual attribute surface and diffs it against the docs.

Minor nit (not blocking): implemented() re-slices and re-string()s the remaining byte range on every failed-boundary iteration (strings.Index(string(src[idx:]), attr)), which is O(n) per retry. Harmless at today's corpus size and it's test-only code, but bytes.Index on src[idx:] directly would avoid the repeated allocation if this ever gets called in a hotter path.

No security or performance concerns for shipped code — this PR only touches docs and a new _test.go file, so nothing here affects runtime behavior.

[adnaan]

Verified and recorded — and it forced an honest correction to my own Learn text.

lvt-scroll-away confirmed live (reads the attribute, validates top/bottom, console.warns otherwise) and documented in none of the four surfaces. Sampling your longer tail: six of seven are undocumented — lvt-spy, lvt-upload, lvt-redact, lvt-fx:region-select, lvt-fx:auto-click; only lvt-persist is documented.

The correction: I'd written that the reference is now "trustworthy as generation context." That overstates it. It's trustworthy in one direction — accurate, not complete — and only the direction that produces incorrect output was closed.

The plan now distinguishes the two tiers rather than lumping them:

• documented-but-absent → the agent emits a page that silently does nothing. Correctness bug. Closed by TestDocumentedAttributesExist.
• undocumented-but-real → the agent never reaches for a capability that exists. Ceiling on quality. Open.

Agreed it's out of scope here — this phase was renames/removals/fabrications in already-documented entries, and a full implementation → docs sweep is its own phase. Recorded as a risk with concrete M1 Phase 3 guidance: treat the reference's coverage as a floor, and check the bundle before concluding a capability doesn't exist.

Left implemented() on strings.Index — test-only, bounded corpus, runs once. Happy to switch to bytes.Index if you'd prefer it on principle.

Also worth noting for the record: the two claude-review failures on this PR were a GitHub outage, not the code. Confirmed by experiment rather than assumed — identical commit, re-run, passes in 4m27s versus 43s failing, with the Actions API 503-ing throughout.