Conversation
Supersedes the charter and Phase 1 plan on v2-herb, which stay as reference. Records the ten decisions taken with the reason for each, and sequences the work in three phases: freeze 1.6's rendered HTML as an executable contract, migrate the gem against it, then the site. Every external claim is dated and verified: herb's release state (main is 653 commits ahead of v0.10.4; component tags are not in any release), and a boot test of rails 8.1.3.1 + reactionview 0.4.1 + herb 0.10.4 that the earlier gate planned and never ran. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four tasks with bite-sized steps: port the golden suite onto main and re-record HoverCard, measure the normalizer's whitespace blind spot, fix the ContextMenuLabel bug that keeps two scenarios unpinned, and open the PR. Every expected output in the plan was produced by running it: the cherry-pick applies with zero conflicts, the suite fails on exactly the two HoverCard scenarios and no others, `golden:update` rewrites exactly those two files, and `bundle exec rake` then passes with 407 files and no offenses. Also resolves the design document's open question 9.1 with the measurement rather than leaving it for the executor: 8 components and 50 adjacent inline pairs, small enough to write the sidecars whitespace-tight instead of growing a second comparison mode. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… plan Ten findings from an independent review, each reproduced before being accepted. The canonical form conflated four things a browser distinguishes (wrapper escape, empty vs whitespace-only element, U+000B in class tokens and text) — one of them flips FormField's controller. validation_mode is not an escape hatch: :none empties a rejected template. append_view_path only reverses which side of a name collision loses. Phlex's attribute guards were being dropped on the strength of snapshot absence. The MCP registry embeds component source and CI fails on a stale copy, so the "never touch mcp/" constraint made the ContextMenuLabel fix fail CI. DocsGenerator would ship Phlex pages from a gem that no longer renders Phlex. Spec: guards ported; sidecar lookup scoped to one root; installer preflight and intercept_erb=false as the only opt-out; strict raw lane and trim mode for Phase 2; MCP rebuild policy; fresh-app install test; existing unit tests ported not deleted; decision 11 removes ruby_ui:install:docs in 2.0; the eight-component completeness claim withdrawn. Plan: new Task 2 hardens the canonical form and the coverage guard, each defect a failing test first, with zero snapshot changes measured against raw Phlex output; the inventory is reframed as an inventory; the ContextMenuLabel task rebuilds the registry; lint counts corrected. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The Global Constraints still pointed the single permitted component edit at Task 3 after the ContextMenuLabel task became Task 4, and Task 1 carried a stale attribution line. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Renders every component in the catalog, reduces each render to a canonical form and compares it against a committed snapshot. 186 snapshots over 54 component directories, plus three coverage tests that fail if a component, a class or a snapshot falls out of the catalog. Ported from the v2-herb branch and re-recorded against main. The only difference is HoverCard, whose markup changed in #530 after the snapshots were first taken. `rake golden` is reached by `rake test`, so CI covers it on Ruby 3.3 and 3.4 with no workflow change. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Four false equivalences the ruler accepted, each now a failing test before its fix: a stray </template> escaped the parsing wrapper and silently dropped everything after it; an empty element and a whitespace-only element canonicalized the same (FormField's controller and `empty:hidden` behave differently on them); class tokens and text collapsed on Ruby's \s, which includes U+000B, rather than HTML's whitespace. The coverage guard recorded a class on instantiation; it now records on render, so a component that is only ever `new`ed for its attrs cannot count as measured. No snapshot changes: the raw Phlex output of all 188 scenarios has no whitespace-only element and no U+000B. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…nd spot The canonical form cannot see whitespace between element siblings or at a text boundary without losing its fixed-point property. Adds a script that lists candidate spots — an inventory, explicitly not a bound — and records the decision: the protection is the hardened canonical form, trim mode in every Phase 2 sidecar, and a strict raw lane for text-bearing components, not a count. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
…tribute
`class: [..., inset?: "pl-8"]` is an Array whose second element is a
Hash, so every ContextMenuLabel shipped a literal `{inset?: "pl-8"}`
class token, `inset:` never applied `pl-8`, and the output differed
between Ruby 3.3 and 3.4.
Pins the two golden scenarios that were pending on this bug, so the
catalog now has no unpinned renders. Rebuilds the MCP registry, which
embeds the component's source.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…the catalog does The catalog generates label_inset and label_flush; the plan guessed label_default. The task already deferred to the real names. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…nd <textarea> The HTML5 parser drops exactly one LF right after a preserved-content start tag (<pre>, <textarea>, <listing>). The serializer never put it back, so <pre>\n\nfoo</pre> canonicalized to <pre>\nfoo</pre>\n, whose second pass drops another line — not a fixed point, so any future snapshot with such content would fail golden_test.rb's fixed-point assertion on every run and could never be pinned. Add a probe for the fixed-point hole and a probe for the sound case a browser also drops (<pre>\nfoo</pre> ≡ <pre>foo</pre>), confirm the first fails and the second passes, then re-emit a leading "\n" in emit_element's preserve branch when the first child is text starting with "\n". Zero snapshot changes: the current catalog has no preserved content starting with LF. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
`rake golden` only ran test/golden_test.rb, so canonical_html_test.rb and harness_test.rb — the tests for the normalizer itself — only ran under the full `rake test`/`rake`. Someone iterating on component markup with the fast `rake golden` loop could break the ruler and not notice until CI. Widen the task's test_globs to test/golden/*_test.rb so the ruler's own 8 tests run alongside it (191 -> 199 runs); `rake test`/`rake` is unaffected since it already requires every test file. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
PLATFORMS only listed arm64-darwin-24 and ruby, and nokogiri is the gem's first native dependency, so ubuntu CI would compile it from source on every matrix job instead of using the platform gem. Add x86_64-linux via `bundle lock --add-platform x86_64-linux`; only the PLATFORMS entry and a platform-specific nokogiri (1.19.4-x86_64-linux-gnu, same version already locked) are new — no resolved version changed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…he hardened normalizer golden-suite.md is the prose canonical_html.rb points to, and it still stated the false equivalence de2b23f removed: item 9 claimed <div></div> and <div>\n</div> canonicalize identically. Flip it to the actual behaviour (they don't — a whitespace-only body serializes as <tag> </tag>, and FormField's controller and empty:hidden read the difference), and add the two other enforced/acceptable-difference behaviours the hardening added: a stray </template> is refused with ArgumentError, and only HTML's ASCII whitespace (not Ruby's \s) splits class tokens or collapses text. Extend "not covered" item 3 to name both §9.1 exclusions (whitespace between element siblings, and at a text-element boundary including a parent's own tags) and point readers at spec §9.1 and decisions.md for the resolution. Drop "not covered" item 10 and "Findings from the first run" #1: both said ContextMenuLabel was unpinned, which f471668 fixed. Keep the TooltipTrigger variant="outline" finding; it is still true. Correct stale counts: 186 -> 188 snapshots (all three mentions), and the context_menu coverage row from "4 (2 pending)" to "4" now that both scenarios are pinned -- the scenario column across the whole table sums to 188, confirming 4 (not the review draft's suggested 6) is the value consistent with the rest of the document. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…er observations design/plans/2026-09-19-golden-suite-implementation.md and design/2026-09-19-rubyui-2-0-design.md still said 186 snapshots throughout; both ContextMenuLabel scenarios are pinned now (f471668), so the suite records 188. Every "186" in both files is now 188 except §8's dated evidence line in the design doc, which describes a specific point-in-time measurement and stays as recorded. Fix two misnumbered task references in the plan: inline_adjacency.rb and decisions.md are created in Task 3, not Task 2 (the File Structure table said Task 2); the ContextMenuLabel bug is fixed in Task 4, not Task 3 (a Task 1 sentence said Task 3). Retitle the "Task 2 of the Phase 1 plan" citation in gem/test/golden/tools/inline_adjacency.rb's header and design/v2/decisions.md to name the actual test (`test_distinguishes_an_empty_element_from_a_whitespace_only_one`) instead of a task number, so the reference survives any future renumbering. Add entry 2 to design/v2/decisions.md: three more normalizer gaps the whole-branch review surfaced (F4 comment-separated text nodes break the fixed point; F5 two more §9.1/strict-lane candidates; F9 U+00A0 is invisible in a diff) as candidates for Phase 2.0's strict lane to weigh -- no code change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Two of 407e084's 186->188 edits described historical facts, not current state, and F8's "search for any other 186" catch-all was over-broad in catching them: the v2-herb source branch holds exactly 186 snapshots (design/2026-09-19-rubyui-2-0-design.md, Phase 1 Step 2), and Task 1's quoted commit message documents the already-made commit a772a49, which committed 186 (design/plans/2026-09-19-golden-suite-implementation.md, Task 1 Step 11). Revert both to 186; every other 186->188 change from 407e084 (current-state counts) stands. Note: the Step 2 "v2-herb" line actually lives in design/2026-09-19-rubyui-2-0-design.md, not in the plan file -- fixed in its real location. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
1 issue found across 208 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="gem/AGENTS.md">
<violation number="1" location="gem/AGENTS.md:37">
P3: `bundle exec rake test N=/button/` does not do what this comment claims. Rake turns `N=/button/` into an unused environment variable; `Minitest::TestTask` (used in gem/Rakefile) consumes neither `N=` nor `TEST=`, so the `-n` filter is silently ignored and the whole suite runs. Either pass the filter to the runner's command line (`bundle exec rake test TESTOPTS="-n /button/"` is minitest's documented rake path) or point to a ruby invocation like `bundle exec ruby -Ilib:test test/ruby_ui/button_test.rb -n /button/`. Please verify the documented command narrows the run before merging, since the parenthetical promises single-test behavior.</violation>
</file>
Note: This PR contains a large number of files. cubic selects up to 200 of the highest-priority eligible files for this review, so some files may not have been reviewed.
Re-trigger cubic
…r does 47 snapshots carried `xmlns:xmlns="http://www.w3.org/2000/svg"`. No component emits that: libxml2 files a plain `xmlns="…"` declaration under the xmlns namespace with prefix "xmlns" and local name "xmlns", and `attribute_name` rebuilt that as `prefix:name`. A prefixed declaration (`xmlns:xlink`) has a different local name and correctly keeps its prefix. Both lanes produced the same artifact, so parity was never wrong — this is a cosmetic fix so the ruler's output reads like HTML. Re-recording touches exactly the 47 snapshots that held the artifact, and every changed line differs only by the `xmlns:xmlns` -> `xmlns` token. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…g it `next_rand` called `argument.to_a` before checking `values.size > MAX_PINNED_RANGE`, so the guard the comment describes could not fire before the allocation it exists to prevent. `Range#size` is O(1) for Integer ranges and nil for non-numeric ones, so the guard now checks `size` first and only materialises the range once it is known to be small enough to pin. 1.6's only shape, `rand(50..89)`, is unaffected. No snapshot changes: 1.6 never calls `next_rand` with a range this guard would have rejected. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e process `Rake::Task[:golden].invoke` is a no-op once `golden` has already run in the same rake process, so `rake golden golden:update` silently skipped the re-record. `golden:update` now reenables the task before invoking it. Verified with `bundle exec rake golden golden:update 2>&1 | grep -c "runs,"`, which now prints 2. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…addendum, follow-up issues design/v2/decisions.md: entry 2 now opens with an explicit decision (none of the three normalizer observations is acted on in Phase 1) in place of the stale "no code changes here" line, keeping its three observation paragraphs unchanged. design/plans/2026-09-19-golden-suite-implementation.md: Task 1's Files line now says 186, not 188 — the cherry-pick brings 186; Task 4 adds the other two. design/plans/2026-09-19-golden-suite-implementation.md: appends an addendum recording what happened after Task 4 — the whole-branch review fix wave and the four PR #536 review follow-up commits — since the task text above is left as the record of what was executed. design/v2/follow-up-issues.md: new file listing the seven 1.6 defects the golden suite pinned faithfully, most raised by the automated review of PR #536, each to be fixed in its own PR with a re-record. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This was referenced Sep 19, 2026
There was a problem hiding this comment.
All reported issues were addressed across 55 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…ead of the upstream conversation main stays as is: the seven 1.6 defects are 2.0-line items. Records the path taken for each of the three §9.2 questions so Phase 2 does not wait on the meeting, and that Phase 2 stacks on feat/golden-suite. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… stays ruby_ui:install:docs keeps working in 2.0.0, so the 52 packaged doc pages, the six docs primitives and the VisualCodeExample redesign move into Phase 2 as sub-phase 2.3. Phase 3 shrinks to the site's own chrome, its 16 pages, and replacing its copies with the gem's pages. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…ted preserved and raw-text elements
Three valid inputs broke the fixed-point property golden_test.rb asserts
over every snapshot, none emitted by any component in the catalog:
<pre><script>if (a < b) {}</script></pre> first pass != second pass
<pre><pre>\n\nx</pre></pre> first pass != second pass
<pre><textarea>\n\nx</textarea></pre> first pass != second pass
Two causes in canonical_html.rb:
1. child_mode returned the outer mode before checking RAW_TEXT, so a
<script>/<style> inside a preserved element went through escape_text
instead of raw; the re-parse then read the escaped text as literal
script content and escaped it again.
2. The newline the HTML parser drops right after a <pre>/<textarea> start
tag was restored only in the branch where the *outer* mode is :normal;
a preserved element nested inside another preserved one took the first
branch and lost it.
Fix: check RAW_TEXT ahead of the :normal guard in child_mode, and compute
the restored newline once in emit_element so it applies regardless of
which branch handles the node.
No component emits these shapes, so all 188 golden snapshots are
unchanged (rake golden: 207 runs, 0 failures; full rake: 503 runs, 0
failures, 0 skips, 410 files inspected, no offenses). The fixed-point
assertion in golden_test.rb is now true for these inputs.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…in range test_rejects_an_unbounded_range_before_materialising_it proves the guard runs before `to_a`: an endless range would raise Ruby's RangeError, not our ArgumentError, if the bound check moved after materialisation. test_pins_rand_to_the_same_value_on_every_render proves the actual snapshot dependency — rand(50..89) returns the same value across renders, not merely a value inside the range. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ius, and two Phase 2/3 references From review of the follow-up commits: the parser drops the nested search form's start tag and its controller and closes the outer form early (the controller calls this.element.requestSubmit and never connects); only the five components that pass a boolean under the aria hash serialize an empty string, not every aria-hidden svg; DocsGenerator discovers pages by *_docs.rb and must be rewritten when those go; the 2.0 note on the nested forms no longer implies a fix on 1.6; Phase 3.0 names the site's Views::Base file. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
cirdes
added this pull request to stack #549
September 20, 2026 17:47
…lete Stimulus descriptor `ctrl+p` alone makes Stimulus listen for an event literally named "ctrl+p", which never fires; the component interpolates the string verbatim and the docs pass `keydown.ctrl+j@window`. The scenario now passes `keydown.ctrl+p@window`, so the snapshot pins a keybinding that works. Raised by the review of #548. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds the golden HTML suite: every component in the catalog is rendered, reduced
to a canonical form by an HTML5-spec parser, and compared byte-for-byte against
a committed snapshot. 188 snapshots over 54 component directories.
Three coverage tests stop the ruler from quietly shrinking: every component
directory must have a scenario, every
RubyUI::Basesubclass must actuallyrender in one (recorded at
before_template, not atinitialize), and nosnapshot may exist without a scenario. The normalizer is asserted to be
idempotent over every snapshot on every run, which is what makes the final byte
comparison a structural comparison rather than a string one, and every scenario
is rendered twice to catch unpinned randomness.
rake goldenis reached byrake test, so CI covers it on Ruby 3.3 and 3.4with no workflow change.
nokogiriis added as a development dependency; thegem still ships with no runtime dependencies.
Why now
This is Phase 1 of
design/2026-09-19-rubyui-2-0-design.md, which this PRalso carries along with its implementation plan. The 2.0 migration replaces
Phlex with plain Ruby classes and ERB templates, and it needs an executable
definition of "the HTML did not change" before the first component moves. A
ruler written after the thing it measures is not a ruler.
It also earns its place on
mainon its own: it catches markup regressions inordinary bug-fix PRs today.
Also in this PR
each with a test that failed before its fix: a stray
</template>silentlytruncated the input; an empty element and a whitespace-only element compared
equal (they are different to
FormField's controller and toempty:hidden);class tokens and text collapsed on Ruby's
\s, which includes U+000B, ratherthan HTML's whitespace; and the LF the parser drops after
<pre>/<textarea>was never re-emitted, so a preserved block starting with a blankline could not be pinned. No recorded snapshot changed.
snapshots were first taken. Those two files are the only difference between
the recording on
v2-herband the recording againstmain— which is whatvalidates the ruler.
ContextMenuLabelbug fix.class: [..., inset?: "pl-8"]is an Arraywhose second element is a Hash, so every label shipped a literal
{inset?: "pl-8"}class token,inset:never appliedpl-8, and theoutput differed between Ruby 3.3 and 3.4. Fixing it pins the last two
unpinned scenarios. The MCP registry is rebuilt to match.
whitespace between element siblings and at text boundaries.
gem/test/golden/tools/inline_adjacency.rblists candidate spots; thedecision it informs — an inventory, not a bound — and the observations from
the whole-branch review are in
design/v2/decisions.md.gem/Gemfile.lockgains thex86_64-linuxplatform so CI installs theprebuilt nokogiri instead of compiling it per job.
Test steps
Both green, zero skips. To see the ruler work, change a class in any component
and re-run
rake golden. To see the hardening, feedGolden::CanonicalHtml.calla<div>\n</div>and a<div></div>.🤖 Generated with Claude Code
Summary by cubic
Adds the golden HTML suite that renders every component, normalizes the output with an HTML5 parser, and compares it byte-for-byte against committed snapshots, giving the RubyUI 2.0 migration an executable definition of "the HTML did not change."
rake goldenruns underrake test, so CI covers all 188 snapshots on Ruby 3.3 and 3.4 with no workflow change. The PR also carries the RubyUI 2.0 design doc and Phase 1 plan, defers follow-up issues #537–#543 to the 2.0 line, and keeps the docs generator in 2.0.New Features
RubyUI::Basesubclass, and every snapshot to have a matching scenario; scenarios render twice to catch unpinned randomness, and the normalizer is checked for idempotence on every snapshot on every run.rake golden:updatere-records snapshots;nokogiriis a development dependency only.rake goldenalso runs the ruler's own tests.Bug Fixes
ContextMenuLabelsoinset:appliespl-8; it previously emitted a literal{inset?: "pl-8"}class token.</template>truncating output, empty vs whitespace-only elements, non-HTML whitespace, and lost newlines after<pre>/<textarea>.xmlns:xmlns; the harness bounds pinned ranges before materialising them, proves the same value returns on every render, and letsgolden:updaterun aftergoldenin the same rake process.keydown.ctrl+p@windowso the snapshot pins a keybinding that actually fires.Written for commit ca928cb. Summary will update on new commits.