From 19eec16d884df69e1b67c91f2fe31e85883f5899 Mon Sep 17 00:00:00 2001 From: Cirdes Date: Sat, 19 Sep 2026 11:39:38 -0300 Subject: [PATCH 01/25] [Documentation] RubyUI 2.0: design and execution plan 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) --- design/2026-09-19-rubyui-2-0-design.md | 536 +++++++++++++++++++++++++ 1 file changed, 536 insertions(+) create mode 100644 design/2026-09-19-rubyui-2-0-design.md diff --git a/design/2026-09-19-rubyui-2-0-design.md b/design/2026-09-19-rubyui-2-0-design.md new file mode 100644 index 000000000..60c61f311 --- /dev/null +++ b/design/2026-09-19-rubyui-2-0-design.md @@ -0,0 +1,536 @@ +# RubyUI 2.0 — design + +Date: 2026-09-19 +Status: draft + +RubyUI 2.0 replaces Phlex with plain Ruby classes and ERB sidecar templates, +compiled by Herb. This document is the design and the execution plan. It +supersedes `design/v2/00-charter.md` and `design/v2/03-plan-fase1.md` on the +`v2-herb` branch, which stay as reference. + +--- + +## 1. Summary + +A 2.0 component is a plain Ruby class next to a `.html.erb` template. The class +owns the Ruby — variants, sizes, attribute merging; the template owns the +markup. Nothing inherits from Phlex. + +Callers write either form: + +```erb +<%= render RubyUI::Dialog.new do %>…<% end %> +… +``` + +The second compiles to the first. They are the same thing, so the tag syntax is +a documentation and packaging decision, not an architectural one. + +Distribution is unchanged in spirit: `rails g ruby_ui:component Dialog` copies +the class and the template into the host app, and the user owns and edits them. +What changes is that `ruby_ui` becomes a small runtime dependency instead of a +pure installer. + +The work is sequenced in three phases: freeze 1.6's rendered HTML as an +executable contract, migrate the gem against that contract, then migrate the +documentation site. + +## 2. Why 2.0 + +Phlex writes HTML in Ruby. For a component library whose value proposition is +"compose freely, restyle any part with Tailwind, you own the code", the markup +is the artifact users came for — and they read and edit it in Ruby method calls +rather than in HTML. + +ERB puts the markup back in HTML, and Herb makes that HTML checkable: a +malformed sidecar fails at compile time with the tag, the line and a +suggestion, rather than silently rendering wrong. That matters most for exactly +the code we ask users to edit. + +## 3. Scope + +**In scope.** The 1.6 component surface, ported — 54 component directories, +256 component classes. The 52 doc-page classes the gem ships (`*_docs.rb`). +The generators and installer. The documentation site. + +**Out of scope.** New components. Visual redesign. API redesign without a +written reason. A Phlex compatibility layer. A Phlex-to-ERB codemod (see +§9.3). + +## 4. Architecture + +### 4.1 Anatomy of a component + +``` +app/components/ruby_ui/dialog/ + dialog.rb # plain Ruby: constructor, default_attrs, class tables + dialog.html.erb # one element, rendered with the computed attributes + dialog_controller.js # unchanged from 1.6 +``` + +```ruby +module RubyUI + class DialogContent < Base + SIZES = { xs: "max-w-sm", sm: "max-w-md", md: "max-w-lg", lg: "max-w-2xl" } + + def initialize(size: :md, **attrs) + @size = enum(size, SIZES, default: :md) + super(**attrs) + end + + private + + def default_attrs + { data_ruby_ui__dialog_target: "dialog", class: [BASE, SIZES[@size]] } + end + end +end +``` + +```erb +
><%= component.content %>
+``` + +The Ruby from 1.6 carries over unchanged except for the `enum` coercion +(§5, decision B); `view_template` is deleted and its markup becomes the +sidecar. + +### 4.2 Render path + +``` +app/views/pages/home.html.erb + <%= render RubyUI::Dialog.new do %>…<% end %> + │ + │ ① constant RubyUI::Dialog → Zeitwerk (push_dir + collapse) + │ ② render of an object → ActionView calls #render_in + ▼ +RubyUI::Base#render_in(view_context, **, &block) + │ ③ content = view_context.capture(self, &block) + │ ④ view_context.render(template: …, locals: { component: self }) + ▼ +app/components/ruby_ui/dialog/dialog.html.erb + │ ⑤ compiled by the registered :erb handler + ▼ + Herb (via ReActionView) +``` + +Herb participates only in ⑤, and only to validate: Erubi and Herb produce the +same HTML. The gate proved this — six Dialog snapshots byte-identical on both +lanes (`experiments/v2-gate/HERB_FINDINGS.md` on `v2-herb`). + +Component tags are sugar over ②. `` is rewritten at compile +time into `render RubyUI::Dialog.new`, so nothing below ② changes. + +### 4.3 The layer + +Two files, promoted from `experiments/v2-gate/app/components/ruby_ui/` where +they were written and proved: + +| File | Responsibility | +| --- | --- | +| `base.rb` | `initialize(**attrs)` → `attrs`; `render_in(view_context, **, &block)` → capture into `content`, render the sidecar; `template_path` derived from the class file; `helpers` (view context, render-time only) | +| `attributes.rb` | `mix` (Phlex `Helpers#mix` semantics), `merge_classes` (tailwind_merge), `flat` (Phlex 2.4.1 serialization → flat string-keyed hash for `tag.attributes`) | + +226 lines at the end of the gate, against a 500-line ceiling. The ceiling +stands for 2.0. `base.rb` and `attributes.rb` are copied into the host app by +the installer, so every line is a line the user reads. + +Differences from 1.6's `Base` to carry into the documentation: + +- `attrs` keys are Strings (`attrs["class"]`), the flat form `tag.attributes` + consumes. 1.6 code reading `attrs[:class]` — `PaginationItem` — changes one + character. +- `true` serializes as `""`; Rails then emits `disabled="disabled"` for HTML + boolean attributes and `aria-x=""` for the rest. Both canonicalize like + Phlex's bare attribute. +- Phlex's attribute guards are not ported (unsafe `on*` names, `srcdoc`, + `javascript:` refs, the `:id` key check). Nothing in the snapshots depends on + them. Ported or not is a 2.0 decision, recorded here as *not ported*. +- The development-only `` comment is dropped. + +### 4.4 Distribution and install + +`rails g ruby_ui:component Dialog` copies `dialog.rb` and `dialog.html.erb` +into `app/components/ruby_ui/dialog/`, and the Stimulus controller into +`app/javascript/controllers/ruby_ui/`, as in 1.6. `ruby_ui.gemspec` already +packages `lib/**/*`, so `.erb` ships without a change. + +The initializer the installer writes: + +```ruby +Rails.autoloaders.main.inflector.inflect("ruby_ui" => "RubyUI") +Rails.autoloaders.main.push_dir(Rails.root.join("app/components/ruby_ui"), namespace: RubyUI) +Rails.autoloaders.main.collapse(Rails.root.join("app/components/ruby_ui/*")) + +ActiveSupport.on_load(:action_controller) do + append_view_path Rails.root.join("app/components") +end + +ReActionView.config.intercept_erb = true +ReActionView.config.validation_mode = :raise +``` + +`extend Phlex::Kit` is removed. Zeitwerk ignores `.html.erb`, so the sidecar +living in an autoloaded directory is inert. + +**Known side effect, to be documented.** `app/components` is ViewComponent's +home directory. Making it a view path does not break ViewComponent — it +resolves its own sidecars by compiled method, not by virtual path — but it does +make templates under `app/components` resolvable by `render template:`. +`append_view_path`, not `prepend`, so RubyUI never shadows the host app's own +views. + +**Runtime dependencies.** `ruby_ui` goes from zero runtime dependencies to +`tailwind_merge` and `reactionview` (which brings `herb`). The constraint on +`reactionview` must have no upper bound: 0.4.1 pins `herb >= 0.10.4, < 0.11.0`, +and an upper bound here would block users when herb 0.11 lands. + +## 5. Decisions + +Ten decisions, with the reason each was taken. Deviations from these during +execution go in `design/v2/decisions.md`, one line each, with the reason. + +| # | Decision | Reason | +| --- | --- | --- | +| 1 | Start fresh from `main`; `v2-herb` is reference only | Its three commits have three destinations: the golden suite belongs on `main`, the research is reference, and `experiments/v2-gate` is throwaway by its own design (182 files, a second Rails app). Carrying all three forward means the 2.0 line hauls a throwaway app forever and keeps a charter whose premises this document overrides. | +| 2 | Authoring syntax is `` | Herb's design resolves a tag statically from its name, with no lookup and no registry, and its own docs state that dashed names like `` are left alone. A `rui-` prefix is reachable through a custom resolver, but it would make RubyUI the library that invented its own dialect. Going with the ecosystem costs verbosity and buys the formatter, the linter and the LSP for free. | +| 3 | `ruby_ui` becomes a small runtime dependency; components stay copied | The tag rewriter and the layer run inside the host app at template compile time, so something must load them there. Shipping a compiler as code the user "owns" means nobody updates it and every bug becomes a silent fork. Markup stays copied and editable, which is the product. | +| 4 | Migrate the gem entirely before the site | Maintainer decision. The cost is accepted and mitigated in §6.2: hard components first, so a design error surfaces in the first weeks rather than at component 55. | +| 5 | No codemod in 2.0 | Deferred, not rejected. Revisit once the gem migration has shown how mechanical the transformation actually is. | +| 6 | The golden suite lands on `main` in its own PR, before any 2.0 work | It protects 1.6 today — it catches regressions in ordinary bug-fix PRs. On a v2 branch it would protect nothing, and `main` and the branch would diverge in exactly the file that defines what "identical" means. `f7cbeda` is self-contained and cherry-picks cleanly. | +| 7 | **A.** Test harness is `actionview` + `reactionview`, no controller, no dummy app | Exactly one component touches the view context — `DataTableForm`, for CSRF — and it already falls back to the literal `"csrf-token-placeholder"` that the snapshots recorded. `DataTableSortHead`, the obvious candidate for needing routes, builds its URL with `CGI` from an explicit `path:`. `reactionview` is included because decision D makes Herb what users compile with; testing on Erubi would test something we do not ship, and it puts the Herb validators over every sidecar on every CI run. | +| 8 | **B.** A coercion helper in `Base`; not the `literal` gem | ~14 components index Symbol-keyed hashes with a user-supplied value and 11 already call `.to_sym`. `DialogContent` and `Badge` do not: `SIZES["lg"]` is `nil`, the class is dropped, nothing is raised. This is a 1.6 bug reachable from `params`, independent of any tag syntax. `Literal::Enum#coerce` looks up by member value and never treats `"lg"` and `:lg` as equivalent, so it does not remove the coercion — it would earn its place only as a full object model (`Base < Literal::Object`, `prop` replacing every constructor), which is a second large migration stacked on the first. Recorded as a legitimate 3.0 direction. | +| 9 | **C.** Sidecar next to the class, reached through `append_view_path` | Keeps class, template and Stimulus controller in one directory, as 1.6 already keeps class and controller. Proved in the gate in development with reloading and in production with eager loading. `append` rather than `prepend` so the library never shadows the host app. | +| 10 | **D.** Herb is required from 2.0.0, through ReActionView | Verified on released Rails (§8): `rails 8.1.3.1 + reactionview 0.4.1 + herb 0.10.4` resolves, boots, renders, runs custom transform visitors, and rejects malformed HTML at compile time. Every user gets validation from day one, and when herb ships component tags every user gets the tag syntax through `bundle update` — no reinstall, no migration. The accepted cost: two pre-1.0 gems become required, and `intercept_erb` compiles the whole host app through Herb, so a user with malformed HTML anywhere sees it on install day. | + +## 6. Phases + +### Phase 1 — The ruler + +**Goal.** A golden suite green on `main`, covering all 54 component +directories, validated against today's code. + +The suite renders every component in the 1.6 catalog, reduces each render to a +canonical form and compares it to a committed snapshot. Its output is the thing +everything else depends on: **once recorded, the 186 snapshots are the frozen +contract of 1.6's rendered HTML, and the Phlex source can be deleted.** Phase 2 +compares against the snapshot, not against a running Phlex component. + +**Steps.** + +1. Branch from `main`. Cherry-pick `f7cbeda`: `canonical_html.rb` (the + executable definition of "acceptable difference"), `catalog.rb` (the + `component`/`scenario` DSL), `harness.rb` (pins the two sources of + randomness), `scenarios.rb` (the catalog), `golden_test.rb` (the runner), + the rake task, and `nokogiri` as a development dependency. +2. Record the snapshots **fresh** against current `main`. Do not copy the 186 + from `v2-herb`. +3. Diff the fresh recording against `v2-herb`'s. `main` has moved nine commits + since the branch point and exactly one touches a component — + `10c01f0 [Bug Fix] HoverCard: let the card escape a clipping ancestor (#530)`. + **Acceptance: only HoverCard differs, and every diff is explained by #530.** + Any other diff is investigated before proceeding. +4. Resolve the whitespace question (§9.1) with a measurement, not a guess. +5. PR to `main`. + +**Acceptance.** `cd gem && bundle exec rake` green on Ruby 3.3 and 3.4. Every +directory under `lib/ruby_ui/` has at least one scenario; every `RubyUI::Base` +subclass is reached; no snapshot file is orphaned; the normalizer is idempotent +over all snapshots; every scenario renders identically twice. + +**No decision in this phase depends on anything else in this document.** The +ruler is pure 1.6 — it does not know what the layer, the syntax or Herb are. +That is why it goes first, and it means Phase 1 can start before the Herb +conversation happens. + +### Phase 2 — The gem + +#### 2.0 Foundation + +No component work. Builds the apparatus and closes the four decisions above in +code. + +- Promote `Base` and `Attributes` into `gem/lib/ruby_ui/`, with tests of their + own and the differential test against Phlex 2.4.1 kept. +- Replace the test harness: `actionview` as a development dependency + (`reactionview` is already a runtime one, §4.4); a minimal `ActionView::Base` + with a view path into `gem/lib/ruby_ui`, with ReActionView's handler + registered so tests compile exactly as users will. + `ComponentTest#phlex { }` is replaced by rendering an ERB fixture. +- Add the **ERB lane** to the golden suite: the 186 scenarios become + `.html.erb` fixtures under `gem/test/golden/views/`, rendered through the 2.0 + component and compared against the frozen snapshot. +- Implement the `enum` coercion helper in `Base`. +- Point `docs/Gemfile` at the published `ruby_ui` 1.6 instead of + `path: "../gem"`, so the site keeps building and the CI Docs job stays green + while the gem is mid-migration. Phase 3 reverts it. + +**Acceptance.** The layer is in the gem with its own tests. The ERB lane runs +with at least one component at parity. A component renders in a real Rails 8.1 +application. All three CI jobs are green. + +#### 2.1 The hard components first + +Dialog (9 classes), Select (8), ToggleGroup and Toggle (3), Data Table (32). +About 52 classes, and they are the ones that exercise everything that can go +wrong: a block that receives the component, a generated id, a component that +renders no root element, a `` root, one component reading +another's computed `attrs`, a `style:` hash, merged `data-action` ordering. + +Dialog is already proved. The other three are not. + +**Known limitation.** `ToggleGroup` and `ToastRegion` call `yield(self)`, and +Herb's component-tag visitor emits a bare `do` with no block parameter, so they +cannot be written as tags. They keep the `render X.new do |group|` form, which +stays available and documented. This is item 2 of §9.2. + +**Acceptance.** The 18 snapshots of these four components identical to the +frozen contract; the 1.6 Stimulus controllers unedited. + +#### 2.2 The bulk + +The remaining ~50 components, in batches. + +**Definition of done, per component.** A plain Ruby class with no Phlex; a +sidecar; the snapshot matching; a scenario passing the String form of every +enum attribute; the Stimulus controller untouched. + +#### 2.3 Generators and installer + +`component_generator.rb` copies `.rb` and `.html.erb`. `install_generator.rb` +writes the initializer of §4.4 and copies `base.rb` and `attributes.rb`. +`dependencies.yml` is unchanged — it describes JS packages. The gemspec drops +`phlex` and gains `tailwind_merge` and `reactionview` as runtime dependencies. + +#### 2.4 Release + +Version, CHANGELOG, and the manual migration guide. At this point the +documentation site is still Phlex; the announcement has to say so. + +### Phase 3 — The site + +144 Ruby files under `docs/app`, 10,383 lines in `app/views`, 68 page files +under `app/views/docs` (58 at the top level, 10 in subdirectories), and not one +`.erb`. + +**The component doc pages are a gem artifact, not a site artifact.** This is +easy to miss and it moves work across the phase boundary: + +``` +gem/lib/ruby_ui/button/button_docs.rb 52 files, the source + │ rails g ruby_ui:install:docs (strips _docs, copies) + ▼ +docs/app/views/docs/button.rb the installed copy +``` + +`DocsGenerator` ships the doc pages to any host app, so they are part of the +library surface. The site's copies are copies, not symlinks — unlike the +Stimulus controllers — and **10 of the 52 have already drifted from their +source**. Migrating a doc page is therefore gem work with a site consumer, and +the drift should be resolved in the same pass rather than carried into 2.0. + +Whether the doc pages should keep shipping in the gem at all is an open +question for this phase: they are the only part of the gem that is neither a +component nor a generator. + +#### 3.0 Redesign `VisualCodeExample` + +Today a doc page passes a heredoc of Phlex source to +`Docs::VisualCodeExample`, which `eval`s it in the page's context to render the +live preview and prints the same string as the code sample. In 2.0 the example +is ERB, not Ruby, so `eval` cannot survive. + +**Each example becomes a real `.html.erb` file.** The page renders the file for +the preview and reads the same file from disk for the code block. The `eval` +goes away, the example is genuinely compiled — so the tag syntax works in +examples — and what is on screen is literally what is in the file. Roughly 450 +files of one to five lines each. + +#### 3.1 Move the documentation primitives out of the gem + +`gem/lib/ruby_ui/docs/` holds six Phlex classes — `visual_code_example`, +`header`, `components_table`, `component_setup_tabs`, `sidebar_examples`, +`base`. They are site infrastructure, not library surface, and they are already +excluded from the gem's test autoload. They move to `docs/app/`. Check +`mcp:build` for a dependency on them first. + +#### 3.2 Chrome and layout + +`Views::Base`, layouts, navigation, marketing pages — the ~86 Ruby files that +are not component pages. + +#### 3.3 The pages + +The 52 `_docs.rb` sources in the gem, plus the 16 site-only pages +(installation, theming and the rest). Depends on 3.0 and 3.2. Mechanical and +large. Each of the 10 drifted pages is reconciled against its source as it is +migrated, with the reason for the drift recorded. + +#### 3.4 Close-out + +`docs/Gemfile` points back at `path: "../gem"`; `phlex` and `phlex-rails` come +out; `mcp/data/registry.json` is rebuilt; the CI Docs job is green without the +pin. + +**Acceptance.** No view `.rb` under `docs/app/views`; no `_docs.rb` left in the +gem; no mention of phlex in `docs/Gemfile.lock`; all 68 pages rendering; no +drift between a doc page in the gem and its copy in the site. + +## 7. Testing strategy + +| Question | Answer | +| --- | --- | +| Does a component render the same HTML as 1.6? | The golden suite's ERB lane, against the frozen snapshot | +| Does an attribute reach the element correctly? | The differential test against Phlex 2.4.1 in `Attributes` | +| Is a sidecar valid HTML? | Herb's validators, over every sidecar, on every CI run | +| Does a component behave in a browser? | Not covered during Phase 2 — see §9.4 | + +## 8. Evidence + +Everything below was verified on 2026-09-19 unless stated. + +**Herb's release state.** The latest release of the `herb` gem is v0.10.4 +(2026-09-10). It is **not** an ancestor of `main`: it was cut from a release +branch that forked at v0.10.3 (2026-08-01), and it changes `Gemfile.lock`, docs +and JS package versions only. `main` is **653 commits ahead of v0.10.4**. +PR [#2032](https://github.com/marcoroth/herb/pull/2032), "Engine: Implement +`ComponentVisitor`" (merged 2026-08-06, `66e4d1a50`), is in `main` and **not** +in v0.10.4. Its supporting PRs are +[#2055](https://github.com/marcoroth/herb/pull/2055) (`build` factories for AST +nodes), [#2317](https://github.com/marcoroth/herb/pull/2317) +(`track_locations`) and [#1436](https://github.com/marcoroth/herb/pull/1436) +(`dot_notation_tags`). The slot system — +[#2564](https://github.com/marcoroth/herb/pull/2564), +[#2577](https://github.com/marcoroth/herb/pull/2577), +[#2578](https://github.com/marcoroth/herb/pull/2578), +[#2651](https://github.com/marcoroth/herb/pull/2651) — is also unreleased. + +**Herb 0.10.4, probed directly.** `Herb::AST::ERBContentNode.build` and +`Herb::Token.from` do not exist, so the AST cannot be rewritten with the +released API. `Herb.parse("hi")` parses cleanly. +`Herb::Engine.new(source, visitors: [...])` is available. + +**Herb's component-tag design.** From `docs/docs/projects/engine.md` on `main`: +a tag is transformed only when its name is CamelCase in every segment; +`` is left alone; resolution is decided entirely from the tag +name with no lookup at compile time or render time. The built-in components +generated from `config/slots/components.yml` are `Fragment`, `Fallback`, +`Async`, `Lazy`. `ComponentTags::Visitor` carries an explicit experimental +warning. Attribute values are String literals unless written with the `:attr` +directive, and block parameters are not expressible. + +**ReActionView 0.4.1.** Depends on `actionview >= 7.0` and +`herb >= 0.10.4, < 0.11.0`. `ReActionView.config.transform_visitors` is public +configuration and the handler passes it through to `Herb::Engine`. + +**Boot test on released Rails.** `rails 8.1.3.1` + `reactionview 0.4.1` + +`herb 0.10.4` resolves without conflict. A minimal Rails application with +`intercept_erb = true` and `validation_mode = :raise` boots and renders; a +custom `Herb::Visitor` registered through `transform_visitors` is invoked +during compilation; `
` is rejected at compile time and +surfaces as `ActionView::SyntaxErrorInTemplate` carrying Herb's annotated +message (missing closing tag, line, suggestion). This is the `Gemfile.stable` +lane the gate planned and never ran. + +**RubyUI 1.6.** `ruby_ui.gemspec` declares **no** runtime dependencies; `phlex` +and `tailwind_merge` are development dependencies. `s.files` is +`Dir["README.md", "LICENSE.txt", "lib/**/*"]`, which already packages `.erb`. +Exactly one component reaches for the view context: `DataTableForm`, for the +CSRF token, with a fallback to `"csrf-token-placeholder"`. Eleven components +call `.to_sym`; `DialogContent` and `Badge` index Symbol-keyed hashes without +coercing. + +**Counts.** 54 component directories under `gem/lib/ruby_ui`, 256 component +classes (excluding `docs/` and `*_docs.rb`), 52 `*_docs.rb` doc-page classes, +186 golden snapshots. `docs/app` holds 144 Ruby files and 10,383 lines under +`app/views`, with 68 page files under `app/views/docs` and no `.erb` anywhere. +42 of the 52 doc pages are byte-identical to their copy in the site; 10 have +drifted. + +**`main` versus `v2-herb`.** `main` is nine commits ahead of the branch point +and exactly one touches a component: `10c01f0`, the HoverCard fix (#530). The +branch's three commits are `f7cbeda` (the golden suite, 196 files, self +contained), `67231db` (three markdown documents) and `96aa866` (182 files, all +under `experiments/v2-gate`). + +**The gate (2026-09-07, `v2-herb`).** Dialog's six snapshots byte-identical on +the herb and erubi lanes. The layer at 226 lines against a 500-line ceiling. +No template adjustment needed — every markup shape Dialog and Button use +compiled unchanged. Twenty-three classified findings in +`experiments/v2-gate/HERB_FINDINGS.md`. + +## 9. Open questions + +### 9.1 The ruler's whitespace blind spot — resolve in Phase 1 + +`CanonicalHtml` collapses runs of whitespace to a single space and drops text +nodes that collapse to empty, preserving whitespace only inside `pre` and +`textarea`. So it treats `ab` and +`a\nb` as identical — and a browser does not, in an +inline formatting context. + +ERB emits newlines where Phlex emitted nothing. Most of RubyUI lays out with +flex and `gap-*` and is immune; `Typography`, `InlineCode`, `InlineLink`, +`ShortcutKey` and inline badges are not. + +**Resolution:** during Phase 1, count how many components actually place inline +siblings next to each other. If the number is small, write those sidecars +whitespace-tight and assert it. If it is not, the canonical form needs a second +mode that records whether inter-element whitespace was present. Deciding before +the count is deciding without the number. + +### 9.2 Three questions for upstream + +To settle with Herb's maintainer: + +1. **Attribute typing.** A plain tag attribute is a String. Can a component + declare an attribute's type so `size="lg"` arrives as `:lg`, rather than + users writing `:size=":lg"`? Decision B works around this in RubyUI; an + upstream answer would remove the workaround. +2. **Block parameters.** `` does not exist, so `ToggleGroup` and + `ToastRegion` have no tag form. Is one planned? +3. **Release timing.** herb 0.11 with component tags, and the ReActionView + release that accepts it — 0.4.1 pins `herb < 0.11.0`, so both are needed. + Decision D is designed so the answer changes the announcement, not the + architecture. + +### 9.3 Codemod — revisit after Phase 2 + +Users of 1.6 have Phlex components copied into their apps. 2.0 ships a manual +migration guide. Whether a Phlex-to-ERB codemod is worth building should be +decided once the gem migration has shown how mechanical the transformation is — +the gem's own 256 classes are the sample. + +### 9.4 No browser coverage during Phase 2 + +System tests live in `docs/`, and `docs/` is pinned to 1.6 for the duration of +Phase 2, so no 2.0 component is exercised in a browser until Phase 3. + +The mitigation is the argument that byte-identical HTML plus unchanged +JavaScript implies unchanged behaviour: the Stimulus controllers see the same +DOM and the same `data-*` attributes. That argument is strong but not total, +and its gap is precisely §9.1 — the canonical form cannot see the whitespace a +browser renders. Accepted knowingly; revisit if §9.1's count comes back large. + +## 10. Risks + +| Risk | Exposure | Mitigation | +| --- | --- | --- | +| Two pre-1.0 gems become required for every user | Decision D | Verified working on released Rails (§8). ReActionView is a handler over ActionView, not a framework. | +| `intercept_erb` validates the host app's own templates | A user with malformed HTML anywhere sees errors on install day | Documented prominently in the install guide, with `validation_mode` as the escape hatch | +| herb 0.11 requires a new ReActionView release too | The tag syntax waits on two projects, not one | Decision D ships 2.0.0 without depending on either date | +| The documentation site runs Phlex while the library no longer does | Between 2.4 and Phase 3 | Acknowledged in the release announcement rather than discovered by readers | +| `app/components` becomes a view path | ViewComponent users | `append_view_path`, and documented | +| Single-maintainer upstream | Herb and ReActionView | The `render X.new` form has no Herb dependency at all, so the library still functions if upstream stalls | + +## 11. What this document replaces + +`design/v2/00-charter.md` and `design/v2/03-plan-fase1.md` on `v2-herb` are +superseded. Their premises that no longer hold: ReActionView as an open +question, a pinned unreleased Rails commit as the baseline, and the authoring +syntax left undecided. + +Still true and worth reading from that branch: +`experiments/v2-gate/HERB_FINDINGS.md` (23 classified findings) and +`experiments/v2-gate/LAYER.md` (what the layer does, with line counts). Both +are promoted into `design/v2/` when the 2.0 branch is cut. From 3e80293ec519ad715f4da04a66791e22474fd394 Mon Sep 17 00:00:00 2001 From: Cirdes Date: Sat, 19 Sep 2026 11:50:05 -0300 Subject: [PATCH 02/25] [Documentation] RubyUI 2.0: Phase 1 implementation plan 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) --- design/2026-09-19-rubyui-2-0-design.md | 29 +- .../2026-09-19-golden-suite-implementation.md | 703 ++++++++++++++++++ 2 files changed, 726 insertions(+), 6 deletions(-) create mode 100644 design/plans/2026-09-19-golden-suite-implementation.md diff --git a/design/2026-09-19-rubyui-2-0-design.md b/design/2026-09-19-rubyui-2-0-design.md index 60c61f311..bd51cc716 100644 --- a/design/2026-09-19-rubyui-2-0-design.md +++ b/design/2026-09-19-rubyui-2-0-design.md @@ -473,11 +473,23 @@ ERB emits newlines where Phlex emitted nothing. Most of RubyUI lays out with flex and `gap-*` and is immune; `Typography`, `InlineCode`, `InlineLink`, `ShortcutKey` and inline badges are not. -**Resolution:** during Phase 1, count how many components actually place inline -siblings next to each other. If the number is small, write those sidecars -whitespace-tight and assert it. If it is not, the canonical form needs a second -mode that records whether inter-element whitespace was present. Deciding before -the count is deciding without the number. +**Measured 2026-09-19, while planning Phase 1: 8 components, 50 adjacent inline +pairs** — `badge` (27, an artefact of the `all_variants` scenario), `dialog` +(6), `codeblock` (5), `sheet` (5), `sidebar` (3), `carousel` (2), `command` (1), +`context_menu` (1). The count excludes pairs whose parent is a flex or grid +container, which ignores the whitespace; including them inflates it to 21 +components and 86 pairs. + +**Resolution:** leave the normalizer alone and write those eight components' +sidecars whitespace-tight in Phase 2, named explicitly in each component's +task. A second comparison mode would have to be threaded through the canonical +form, the fixed-point assertion and all 186 snapshots for eight components, +most of them benign — the `sr-only` label beside a close icon renders the same +either way, and `codeblock`'s tokens sit inside `pre`, which the normalizer +already preserves. + +Phase 1 reproduces the measurement and records it in `design/v2/decisions.md`; +the script is `gem/test/golden/tools/inline_adjacency.rb`. ### 9.2 Three questions for upstream @@ -510,7 +522,12 @@ The mitigation is the argument that byte-identical HTML plus unchanged JavaScript implies unchanged behaviour: the Stimulus controllers see the same DOM and the same `data-*` attributes. That argument is strong but not total, and its gap is precisely §9.1 — the canonical form cannot see the whitespace a -browser renders. Accepted knowingly; revisit if §9.1's count comes back large. +browser renders. + +With §9.1 now measured at eight components, the gap is bounded and named rather +than unknown. Accepted knowingly: those eight are the only places where Phase 2 +could ship a visible difference the suite reports as parity, and their tasks +carry the instruction that closes it. ## 10. Risks diff --git a/design/plans/2026-09-19-golden-suite-implementation.md b/design/plans/2026-09-19-golden-suite-implementation.md new file mode 100644 index 000000000..e595cc321 --- /dev/null +++ b/design/plans/2026-09-19-golden-suite-implementation.md @@ -0,0 +1,703 @@ +# Golden Suite (RubyUI 2.0, Phase 1) Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Land the golden HTML suite on `main`, validated against today's components, so that RubyUI 1.6's rendered HTML becomes a frozen, executable contract that the 2.0 migration can be measured against. + +**Architecture:** The suite renders every component in the 1.6 catalog, reduces each render to a canonical form with an HTML5-spec parser, and compares it byte-for-byte against a committed snapshot. The code already exists and was proved on the `v2-herb` branch; this plan ports it to `main`, re-records the one component that changed since, resolves the one known blind spot in the normalizer, and closes the one bug that prevents the catalog from being complete. + +**Tech Stack:** Ruby 3.3 and 3.4, Minitest, Nokogiri 1.18 (development dependency, golden suite only), Phlex 2.4.1, tailwind_merge. + +**Spec:** `design/2026-09-19-rubyui-2-0-design.md` — this plan implements its Phase 1 (§6). Read §1, §6 "Phase 1" and §9.1 before starting. + +## Global Constraints + +- Work in `gem/`. Run every command from `gem/`, never from the repo root. +- Ruby 3.2+ syntax, 2-space indent, `snake_case` files, `CamelCase` classes. StandardRB is enforced and `bundle exec rake` runs it. +- Do not touch `docs/` or `mcp/` in any task of this plan. `git status --porcelain docs mcp` must be empty at the end of every task. +- Do not modify any file under `gem/lib/ruby_ui/` except in Task 3, which changes exactly one line of one file. +- `nokogiri` is a **development** dependency. Nothing in this plan may add a runtime dependency; `ruby_ui.gemspec` has none and Phase 1 does not change that. +- Never hand-edit a file under `gem/test/golden/snapshots/`. Snapshots are produced by `bundle exec rake golden:update` and reviewed as a diff. +- Never commit with `bundle exec rake` failing. +- Branch from `main` and open a PR. Do not push to `main`. + +--- + +## File Structure + +| File | Responsibility | +| --- | --- | +| `gem/test/golden_test.rb` | The runner. One Minitest test per scenario, plus three coverage tests that stop the ruler from silently shrinking. | +| `gem/test/golden/scenarios.rb` | The catalog — what gets rendered. The file to read first; ~1136 lines of `component`/`scenario` blocks. | +| `gem/test/golden/canonical_html.rb` | Parse, normalize, serialize. The executable definition of "acceptable difference". | +| `gem/test/golden/catalog.rb` | The `component`/`scenario` DSL and the coverage queries (`component_directories`, `component_classes`). | +| `gem/test/golden/harness.rb` | Pins the two sources of randomness; records which classes a render touched. | +| `gem/test/golden/snapshots/**/*.html` | 186 recorded snapshots, one file per pinned scenario. | +| `gem/test/golden/tools/inline_adjacency.rb` | Task 2 only. A standalone report that counts where the canonical form is blind to whitespace. Not loaded by the suite. | +| `gem/Rakefile` | Adds the `golden` and `golden:update` tasks. `golden` is also reached by `rake test`, so CI covers it with no workflow change. | +| `gem/ruby_ui.gemspec` | Adds `nokogiri` as a development dependency. | +| `design/v2/01-research/golden-suite.md` | What the suite covers, what it deliberately does not, and what its normalization treats as acceptable. `CLAUDE.md` links to it. | +| `design/v2/decisions.md` | The living decision log. Created in Task 2 with its first entry. | + +--- + +## Task 1: Port the golden suite onto `main` + +The suite exists as a single self-contained commit on `v2-herb` (`f7cbeda`). It touches nothing that `main` has changed since, so it cherry-picks cleanly. After porting it, exactly two snapshots are stale — HoverCard's — because `10c01f0` (#530, "let the card escape a clipping ancestor") landed on `main` after the branch point. Re-recording those two and reviewing the diff is what validates the ruler: if anything else differs, the ruler is wrong and the task stops. + +**Files:** +- Create (via cherry-pick): `gem/test/golden_test.rb`, `gem/test/golden/canonical_html.rb`, `gem/test/golden/catalog.rb`, `gem/test/golden/harness.rb`, `gem/test/golden/scenarios.rb`, 186 files under `gem/test/golden/snapshots/` +- Create (separately): `design/v2/01-research/golden-suite.md` +- Modify (via cherry-pick): `gem/Rakefile`, `gem/ruby_ui.gemspec`, `gem/Gemfile.lock`, `CLAUDE.md`, `gem/AGENTS.md` +- Modify (by re-recording): `gem/test/golden/snapshots/hover_card/default.html`, `gem/test/golden/snapshots/hover_card/with_options.html` + +**Interfaces:** +- Consumes: nothing. This is the first task. +- Produces, for Tasks 2 and 3 and for all of Phase 2: + - `Golden::Catalog.scenarios` → `Array`; each responds to `component` (String), `name` (String), `slug` (`"component/name"`), `block` (Proc), `pending` (String or nil), `pinned?` (Boolean), `snapshot_path` (String), `test_name` (Symbol) + - `Golden::Catalog.component(name) { ... }` and `Golden::Catalog.scenario(name, pending: nil) { ... }` — the catalog DSL + - `Golden::Catalog.component_directories` → `Array`, every directory under `lib/ruby_ui/` except `docs` + - `Golden::Catalog.component_classes` → `Array`, the names of every `RubyUI::Base` subclass + - `Golden::Catalog::SNAPSHOT_ROOT` → absolute path to `gem/test/golden/snapshots` + - `Golden::CanonicalHtml.call(html)` → `String`, the canonical form; idempotent + - `Golden::Harness.render(&block)` → `String`, raw HTML, with randomness pinned + - `Golden::Harness.classes_rendered` → `Hash`, keys are class names touched by renders so far + - Commands: `bundle exec rake golden` (verify), `bundle exec rake golden:update` (re-record) + +- [ ] **Step 1: Branch from an up-to-date `main`** + +```bash +cd /Users/cirdes/Workspaces/ruby_ui +git checkout main +git pull --ff-only +git checkout -b feat/golden-suite +``` + +- [ ] **Step 2: Cherry-pick the suite without committing** + +`-n` stages the change without creating a commit, so the re-record in Step 8 lands in the same commit as the port. + +```bash +git cherry-pick -n f7cbeda +``` + +Expected: no output, exit 0. + +- [ ] **Step 3: Verify the cherry-pick is clean** + +```bash +git diff --name-only --diff-filter=U +git status --porcelain | wc -l +``` + +Expected: the first command prints nothing (zero conflicted files). The second prints `196`. + +If there are conflicts, STOP. `main` has changed in a way this plan did not anticipate; report it rather than resolving by hand. + +- [ ] **Step 4: Bring the suite's reference document** + +The cherry-pick edits `CLAUDE.md` to point at `design/v2/01-research/golden-suite.md`. That file lives on `v2-herb` in a different commit, so without this step the link dangles. + +```bash +git checkout 67231db -- design/v2/01-research/golden-suite.md +git status --porcelain design/ +``` + +Expected: `A design/v2/01-research/golden-suite.md` + +- [ ] **Step 5: Install the new development dependency** + +```bash +cd gem +bundle install +``` + +Expected: resolves and installs `nokogiri` 1.18.x. `gem/Gemfile.lock` was already updated by the cherry-pick, so `bundle install` should not modify it further. + +- [ ] **Step 6: Run the suite and confirm it fails in exactly the expected way** + +This is the failing-test step. The suite is the test; the expected failure is the HoverCard change that landed on `main` after the snapshots were recorded. + +```bash +cd gem +bundle exec rake golden +``` + +Expected: `191 runs, 752 assertions, 2 failures, 0 errors, 2 skips` + +The two failures must be, and only be: +- `GoldenSuiteTest#test_hover_card__default` +- `GoldenSuiteTest#test_hover_card__with_options` + +The two skips are `context_menu/label_*`, declared `pending:` in `scenarios.rb` because of a 1.6 bug that Task 3 fixes. + +**If any other scenario fails, STOP.** The port is not a port any more — something about the ruler or about `main` is not what this plan assumes. Report the failing scenarios and their diffs. + +- [ ] **Step 7: Read the two diffs and confirm they are #530** + +The failure output prints the diff inline. Confirm both changes are the HoverCard fix and nothing else: + +- `hover_card/default` and `hover_card/with_options`: the root `
` gains `class="group/hover-card is-absolute"` +- `hover_card/default`: the content `
` changes `absolute` to `group-[.is-absolute]/hover-card:absolute group-[.is-fixed]/hover-card:fixed` + +Cross-check against the commit that made the change: + +```bash +cd /Users/cirdes/Workspaces/ruby_ui +git show 10c01f0 -- gem/lib/ruby_ui/hover_card/ +``` + +Expected: the classes in the diff appear in that commit. If they do not, STOP — a component changed for a reason nobody has accounted for. + +- [ ] **Step 8: Re-record** + +```bash +cd gem +bundle exec rake golden:update +``` + +Expected: the task runs the suite with `UPDATE_GOLDEN_SNAPSHOTS=1` and exits 0. + +- [ ] **Step 9: Confirm the re-record touched exactly two files** + +```bash +cd /Users/cirdes/Workspaces/ruby_ui +git status --porcelain gem/test/golden/snapshots | grep -v '^A ' +``` + +Expected, exactly these two lines and no others: + +``` +AM gem/test/golden/snapshots/hover_card/default.html +AM gem/test/golden/snapshots/hover_card/with_options.html +``` + +`AM` means the file was added by the cherry-pick and then modified by the re-record. If a third file appears, STOP — the re-record overwrote a snapshot that should not have changed. + +- [ ] **Step 10: Run the full default task** + +```bash +cd gem +bundle exec rake +``` + +Expected: the unit suite and the golden suite pass, then `407 files inspected, no offenses detected`. Skipped tests are reported (the two `context_menu` pendings) and that is not a failure. + +- [ ] **Step 11: Commit** + +```bash +cd /Users/cirdes/Workspaces/ruby_ui +git add -A +git commit -m "$(cat <<'MSG' +[Feature] Golden HTML suite: the 1.6 parity ruler + +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 Opus 5 (1M context) +MSG +)" +``` + +--- + +## Task 2: Measure the normalizer's whitespace blind spot + +`Golden::CanonicalHtml` collapses runs of whitespace to a single space and drops text nodes that collapse to empty, preserving whitespace only inside `pre` and `textarea`. So it treats `ab` and `a\nb` as identical — and a browser does not, when the parent lays its children out in an inline formatting context. + +This matters for Phase 2, not for Phase 1: ERB emits a newline where Phlex emitted nothing, so an inline-adjacent pair could render differently in a browser while the ruler reports parity. `scenarios.rb` already acknowledges the blindness in its header comment; this task counts how much of the catalog it touches and records the decision that follows. + +It changes no component and no snapshot. + +**Files:** +- Create: `gem/test/golden/tools/inline_adjacency.rb` +- Create: `design/v2/decisions.md` + +**Interfaces:** +- Consumes: the snapshots recorded in Task 1, read from disk at `gem/test/golden/snapshots`. +- Produces: `design/v2/decisions.md`, the living decision log that every later phase appends to. One entry per decision or deviation, newest last, each with a reason. + +- [ ] **Step 1: Write the report script** + +Create `gem/test/golden/tools/inline_adjacency.rb`: + +```ruby +# frozen_string_literal: true + +# Counts where the golden suite's canonical form is blind to whitespace. +# +# cd gem && bundle exec ruby test/golden/tools/inline_adjacency.rb +# +# The canonical form collapses whitespace between elements, so two adjacent +# inline-level elements compare equal whether or not a space separated them. +# A browser renders those two cases differently. This report finds every such +# adjacency in the recorded snapshots, so Phase 2 knows which sidecars have to +# be written whitespace-tight. +# +# Two judgements, both approximations, both deliberate: +# +# * Inline-level is an intrinsically inline tag, or any element whose class +# list contains `inline`, `inline-block` or `inline-flex` — because +# Tailwind overrides display and the tag name alone is not enough. +# * Whitespace between siblings only renders as a space when the parent +# establishes an inline formatting context. A flex or grid parent ignores +# it, so those parents are skipped. Without this filter the count is +# inflated roughly threefold by icons sitting next to labels inside +# flex buttons. + +require "nokogiri" + +SNAPSHOT_ROOT = File.expand_path("../snapshots", __dir__) + +INLINE_TAGS = %w[ + a abbr b bdi bdo br cite code data dfn em i kbd mark q rp rt ruby s samp + small span strong sub sup time u var wbr img svg +].freeze + +INLINE_CLASSES = /(?:\A|\s)(?:inline|inline-block|inline-flex)(?:\s|\z)/ +FLOW_CLASSES = /(?:\A|\s)(?:flex|grid|inline-flex|inline-grid)(?:\s|\z)/ + +def inline?(node) + return false unless node.element? + return true if INLINE_TAGS.include?(node.name) + + node["class"].to_s.match?(INLINE_CLASSES) +end + +def inline_formatting_context?(node) + return true if node.fragment? + + !node["class"].to_s.match?(FLOW_CLASSES) +end + +findings = Hash.new { |hash, key| hash[key] = [] } + +Dir.glob(File.join(SNAPSHOT_ROOT, "**", "*.html")).sort.each do |path| + slug = path.delete_prefix("#{SNAPSHOT_ROOT}/").delete_suffix(".html") + component = slug.split("/").first + + Nokogiri::HTML5.fragment(File.read(path)).traverse do |node| + next unless node.element? || node.fragment? + next unless inline_formatting_context?(node) + + children = node.children.reject { |child| child.text? && child.text.strip.empty? } + + children.each_cons(2) do |left, right| + next unless inline?(left) && inline?(right) + + findings[component] << "#{slug}: <#{left.name}> + <#{right.name}>" + end + end +end + +puts "components affected: #{findings.keys.size}" +puts "adjacent inline pairs: #{findings.values.sum(&:size)}" +puts + +findings.keys.sort.each do |component| + puts "#{component} (#{findings[component].size})" + findings[component].first(3).each { |line| puts " #{line}" } + puts " …" if findings[component].size > 3 +end +``` + +- [ ] **Step 2: Run it and confirm the count** + +```bash +cd gem +bundle exec ruby test/golden/tools/inline_adjacency.rb +``` + +Expected, exactly: + +``` +components affected: 8 +adjacent inline pairs: 50 +``` + +and these eight components, with these counts: + +| Component | Pairs | +| --- | --- | +| `badge` | 27 | +| `codeblock` | 5 | +| `dialog` | 6 | +| `sheet` | 5 | +| `sidebar` | 3 | +| `carousel` | 2 | +| `command` | 1 | +| `context_menu` | 1 | + +If the numbers differ, the snapshots on disk are not the ones Task 1 recorded, or Nokogiri's HTML5 parser behaves differently on this machine. Investigate before writing the decision — the decision is only worth what the number is worth. + +- [ ] **Step 3: Write the decision log** + +Create `design/v2/decisions.md`: + +```markdown +# RubyUI 2.0 — decisions + +One entry per decision or deviation from `design/2026-09-19-rubyui-2-0-design.md`, +newest last, each with the reason. The ten decisions taken before execution +started are in §5 of that document; this file records what happens after. + +## 1. The normalizer's whitespace blind spot (§9.1) — measured 2026-09-19 + +`Golden::CanonicalHtml` is blind to whitespace between adjacent inline-level +elements whose parent establishes an inline formatting context. Measured with +`gem/test/golden/tools/inline_adjacency.rb` over the recorded snapshots: +**8 components, 50 adjacent inline pairs**. + +| Component | Pairs | What they are | +| --- | --- | --- | +| `badge` | 27 | the `all_variants` scenario, 28 badges in a row — an artefact of how the scenario is written, not of a composition users write | +| `dialog` | 6 | the close button's `` next to its `sr-only` label | +| `codeblock` | 5 | highlighted token spans | +| `sheet` | 5 | the close button, as in `dialog` | +| `sidebar` | 3 | icon next to label | +| `carousel` | 2 | the previous and next buttons | +| `command` | 1 | adjacent `` items | +| `context_menu` | 1 | adjacent `` items | + +**Decision: leave the normalizer alone; write these eight components' sidecars +whitespace-tight in Phase 2.** Each of the eight gets an explicit line in its +Phase 2 task saying so, and the sidecar must not put a newline between the +inline siblings named above. + +**Why not extend the normalizer.** A second comparison mode that records +inter-element whitespace would have to be threaded through the canonical form, +the fixed-point assertion and all 186 snapshots, for eight components — most of +which are benign anyway: the `sr-only` label next to a close icon renders the +same either way, and `codeblock`'s tokens sit inside `pre`, which the +normalizer already preserves. The cost is not proportional to the risk. + +**What would reverse this.** A Phase 2 component showing a visible spacing +difference that the suite reported as parity. That is the failure this decision +accepts, and §9.4 is the reason it cannot be caught automatically before +Phase 3. +``` + +- [ ] **Step 4: Verify nothing else changed** + +```bash +cd gem +bundle exec rake +``` + +Expected: green, `407 files inspected, no offenses detected`. The report script lives under `test/` and is not loaded by the suite, but StandardRB does inspect it — fix any offense with `bundle exec standardrb --fix` and re-run. + +```bash +cd /Users/cirdes/Workspaces/ruby_ui +git status --porcelain gem/test/golden/snapshots gem/lib docs mcp +``` + +Expected: no output. This task changes no snapshot, no component, and nothing outside `gem/test/golden/tools/` and `design/`. + +- [ ] **Step 5: Commit** + +```bash +cd /Users/cirdes/Workspaces/ruby_ui +git add gem/test/golden/tools/inline_adjacency.rb design/v2/decisions.md +git commit -m "$(cat <<'MSG' +[Documentation] Measure the golden suite's whitespace blind spot + +The canonical form collapses whitespace between elements, so adjacent +inline-level elements compare equal whether or not a space separated +them — and a browser renders those two cases differently when the +parent lays out inline. ERB emits a newline where Phlex emitted +nothing, so Phase 2 needs to know how much of the catalog this touches. + +8 components, 50 pairs. Adds the report that counts it and records the +decision: write those eight whitespace-tight rather than grow a second +comparison mode. + +Co-Authored-By: Claude Opus 5 (1M context) +MSG +)" +``` + +--- + +## Task 3: Fix `ContextMenuLabel` and pin its two scenarios + +Two scenarios in the catalog are declared `pending:` and carry no snapshot, so two of the catalog's renders are not pinned. The reason is a one-line bug in `ContextMenuLabel`: + +```ruby +class: ["px-2 py-1.5 text-sm font-semibold text-foreground", inset?: "pl-8"] +``` + +Ruby parses that as `["...", {inset?: "pl-8"}]` — an Array whose second element is a Hash. The Hash is serialized into the `class` attribute, so every `ContextMenuLabel` renders: + +```html +
+``` + +Three consequences: `pl-8` is never applied, so `inset:` does nothing; a garbage class token ships in the HTML; and the serialization differs between Ruby 3.3 (`{:inset?=>"pl-8"}`) and 3.4 (`{inset?: "pl-8"}`), which is why the scenarios could not be pinned across the CI matrix. + +This is the last hole in the contract Phase 2 freezes. It is a 1.6 bug fix in its own right. `grep` confirms it is the only occurrence of the pattern in the gem. + +> **Scope note.** This task goes beyond the spec's Phase 1, which asks only for the ruler. It is here because Phase 2 freezes the contract, and a scenario with no snapshot is a render nobody is measuring. It is independently reviewable: Tasks 1, 2 and 4 stand without it. Drop it and Phase 1 still succeeds, with two unpinned renders and a known bug shipping in 1.6. + +**Files:** +- Modify: `gem/lib/ruby_ui/context_menu/context_menu_label.rb:20` +- Modify: `gem/test/ruby_ui/context_menu_test.rb` +- Modify: `gem/test/golden/scenarios.rb:447-449` +- Create (by re-recording): `gem/test/golden/snapshots/context_menu/label_default.html`, `gem/test/golden/snapshots/context_menu/label_inset.html` + +**Interfaces:** +- Consumes: `bundle exec rake golden:update` and `Golden::Catalog.scenario(name, pending: nil)` from Task 1. +- Produces: a catalog with no `pending:` scenarios — `Golden::Catalog.scenarios.all?(&:pinned?)` is true. + +- [ ] **Step 1: Read the two pending scenarios** + +```bash +cd gem +sed -n '440,460p' test/golden/scenarios.rb +``` + +Note the exact scenario names and the loop that generates them. The next steps refer to them. + +- [ ] **Step 2: Write the failing test** + +Add to `gem/test/ruby_ui/context_menu_test.rb`: + +```ruby +def test_context_menu_label_does_not_leak_a_hash_into_the_class_attribute + output = phlex { RubyUI.ContextMenuLabel { "Label" } } + + refute_includes output, "inset?", + "ContextMenuLabel serialized its conditional-class Hash into the class attribute" +end + +def test_context_menu_label_applies_the_inset_class_only_when_inset + inset = phlex { RubyUI.ContextMenuLabel(inset: true) { "Label" } } + plain = phlex { RubyUI.ContextMenuLabel(inset: false) { "Label" } } + + assert_includes inset, "pl-8" + refute_includes plain, "pl-8" +end +``` + +- [ ] **Step 3: Run the tests to verify they fail** + +```bash +cd gem +bundle exec rake test N=/context_menu_label/ +``` + +Expected: both new tests FAIL. +- The first fails because the output contains `{inset?: "pl-8"}`. +- The second fails because `pl-8` is absent from the `inset: true` render and the literal `"pl-8"` inside the Hash makes it present in both, depending on which assertion runs first. + +- [ ] **Step 4: Fix the one line** + +In `gem/lib/ruby_ui/context_menu/context_menu_label.rb`, replace `default_attrs`: + +```ruby + def default_attrs + { + class: ["px-2 py-1.5 text-sm font-semibold text-foreground", (inset? ? "pl-8" : nil)] + } + end +``` + +- [ ] **Step 5: Run the tests to verify they pass** + +```bash +cd gem +bundle exec rake test N=/context_menu_label/ +``` + +Expected: both PASS. + +- [ ] **Step 6: Remove the `pending:` marker** + +In `gem/test/golden/scenarios.rb` around line 449, drop the `pending:` argument and the comment above it that explains why it was there. The scenario declaration becomes: + +```ruby + scenario "label_#{style}" do +``` + +- [ ] **Step 7: Record the two new snapshots** + +```bash +cd gem +bundle exec rake golden:update +``` + +- [ ] **Step 8: Confirm exactly two snapshots were created and none changed** + +```bash +cd /Users/cirdes/Workspaces/ruby_ui +git status --porcelain gem/test/golden/snapshots +``` + +Expected, exactly these two lines: + +``` +?? gem/test/golden/snapshots/context_menu/label_default.html +?? gem/test/golden/snapshots/context_menu/label_inset.html +``` + +(The exact file names come from the scenario names read in Step 1.) + +**If any existing snapshot shows as modified, STOP.** The fix changed a component other than `ContextMenuLabel`, which it must not. + +- [ ] **Step 9: Read the two new snapshots** + +```bash +cd gem +cat test/golden/snapshots/context_menu/label_*.html +``` + +Expected: no `inset?` anywhere; `pl-8` present in the inset snapshot and absent from the other. + +- [ ] **Step 10: Run the full default task** + +```bash +cd gem +bundle exec rake +``` + +Expected: green, and the skip count is now **0** — `rake golden` no longer reports "You have skipped tests". + +- [ ] **Step 11: Commit** + +```bash +cd /Users/cirdes/Workspaces/ruby_ui +git add -A +git commit -m "$(cat <<'MSG' +[Bug Fix] ContextMenuLabel: stop serializing a Hash into the class attribute + +`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. + +Co-Authored-By: Claude Opus 5 (1M context) +MSG +)" +``` + +--- + +## Task 4: Open the pull request + +**Files:** none. + +**Interfaces:** +- Consumes: the three commits from Tasks 1–3. +- Produces: a PR against `main`. Phase 2 branches from `main` after it merges, so that the 2.0 line inherits the ruler rather than forking it. + +- [ ] **Step 1: Confirm the branch is clean and complete** + +```bash +cd /Users/cirdes/Workspaces/ruby_ui +git status --porcelain +git log --oneline main..HEAD +``` + +Expected: no output from the first command; three commits from the second. + +- [ ] **Step 2: Run everything one more time from a clean state** + +```bash +cd gem +bundle exec rake +``` + +Expected: green, `407 files inspected, no offenses detected`, zero skips. + +- [ ] **Step 3: Ask the user before pushing** + +Pushing and opening a PR are outward-facing. Do not run Step 4 until the user has said to go ahead. + +- [ ] **Step 4: Push and open the PR** + +```bash +cd /Users/cirdes/Workspaces/ruby_ui +git push -u origin feat/golden-suite +gh pr create --base main --title "[Feature] Golden HTML suite: the 1.6 parity ruler" --body "$(cat <<'MSG' +## 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. 186 snapshots over 54 component directories. + +Three coverage tests stop the ruler from quietly shrinking: every component +directory must have a scenario, every `RubyUI::Base` subclass must be reached by +one, and no snapshot may exist without a scenario. The normalizer is asserted to +be idempotent over every snapshot, 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 golden` is reached by `rake test`, so CI covers it on Ruby 3.3 and 3.4 +with no workflow change. `nokogiri` is added as a development dependency; the +gem still ships with no runtime dependencies. + +## Why now + +This is Phase 1 of `design/2026-09-19-rubyui-2-0-design.md`. 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 `main` on its own: it catches markup regressions in +ordinary bug-fix PRs today. + +## Also in this PR + +- **HoverCard snapshots re-recorded.** #530 changed its markup after the + snapshots were first taken. Those two files are the only difference between + the recording on `v2-herb` and the recording against `main` — which is what + validates the ruler. +- **`ContextMenuLabel` bug fix.** `class: [..., inset?: "pl-8"]` is an Array + whose second element is a Hash, so every label shipped a literal + `{inset?: "pl-8"}` class token and `inset:` never applied `pl-8`. Fixing it + pins the last two unpinned scenarios. +- **The whitespace report.** The canonical form is blind to whitespace between + adjacent inline elements. `gem/test/golden/tools/inline_adjacency.rb` counts + where that matters; the finding and the decision are in + `design/v2/decisions.md`. + +## Test steps + +```bash +cd gem +bundle exec rake golden # the suite alone +bundle exec rake # unit tests + golden + standardrb +``` + +Both green, zero skips. To see the ruler work, change a class in any component +and re-run `rake golden`. + +🤖 Generated with [Claude Code](https://claude.com/claude-code) +MSG +)" +``` + +--- + +## Definition of done for Phase 1 + +- `bundle exec rake` is green on `main` on Ruby 3.3 and 3.4. +- 188 snapshots exist; no scenario is `pending:`; the suite reports zero skips. +- Every one of the 54 component directories has at least one scenario; every `RubyUI::Base` subclass is reached; no orphan snapshot files. +- The only snapshot difference between the `v2-herb` recording and the `main` recording is HoverCard, explained by #530. +- `design/v2/decisions.md` exists and records the whitespace finding with a real number. +- `git status --porcelain docs mcp` is empty. + +Phase 2 branches from `main` after this merges. From cce429930736ae0a7a7abe6c969c1517a0e54100 Mon Sep 17 00:00:00 2001 From: Cirdes Date: Sat, 19 Sep 2026 13:31:26 -0300 Subject: [PATCH 03/25] [Documentation] RubyUI 2.0: fold the adversarial review into spec and plan MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- design/2026-09-19-rubyui-2-0-design.md | 240 ++++++-- .../2026-09-19-golden-suite-implementation.md | 553 ++++++++++++++---- 2 files changed, 613 insertions(+), 180 deletions(-) diff --git a/design/2026-09-19-rubyui-2-0-design.md b/design/2026-09-19-rubyui-2-0-design.md index bd51cc716..f763a6cb8 100644 --- a/design/2026-09-19-rubyui-2-0-design.md +++ b/design/2026-09-19-rubyui-2-0-design.md @@ -50,8 +50,9 @@ the code we ask users to edit. ## 3. Scope **In scope.** The 1.6 component surface, ported — 54 component directories, -256 component classes. The 52 doc-page classes the gem ships (`*_docs.rb`). -The generators and installer. The documentation site. +256 component classes. The 52 doc-page classes the gem ships (`*_docs.rb`), +which leave the gem (decision 11, Phase 2.3). The generators and installer. +The documentation site. **Out of scope.** New components. Visual redesign. API redesign without a written reason. A Phlex compatibility layer. A Phlex-to-ERB codemod (see @@ -143,9 +144,24 @@ Differences from 1.6's `Base` to carry into the documentation: - `true` serializes as `""`; Rails then emits `disabled="disabled"` for HTML boolean attributes and `aria-x=""` for the rest. Both canonicalize like Phlex's bare attribute. -- Phlex's attribute guards are not ported (unsafe `on*` names, `srcdoc`, - `javascript:` refs, the `:id` key check). Nothing in the snapshots depends on - them. Ported or not is a 2.0 decision, recorded here as *not ported*. +- **Phlex's attribute guards are ported.** Phlex 2.4.1 raises on an unsafe + attribute name (`onclick`, `srcdoc`, any `on*`) and drops a `javascript:` + href; the gate's `Attributes` did neither, and review showed + `href="javascript:alert(1)" onclick="alert(1)"` reaching the page through + `tag.attributes`. An application passing untrusted values to a component + would lose a protection it has today. The guards get their own unit tests + in `Attributes`; the golden suite cannot see them and is not evidence about + them. The `:id` key check is not ported — it guards a Phlex-specific + convention with no equivalent here. +- `render_in` assigns `content` on every call — `nil` when there is no block. + The gate's version assigned it only with a block, so an instance rendered + twice repeated its first content. +- The sidecar is found through a lookup **scoped to the directory that holds + `ruby_ui/`**, not through the application's view-path chain, and two + candidates for one component is an error. Review showed that with the + chain, `append` lets a host `ruby_ui/…` template silently replace the + sidecar and `prepend` lets the sidecar silently shadow the host — the + ordering only picks which side loses quietly. - The development-only `` comment is dropped. ### 4.4 Distribution and install @@ -162,23 +178,29 @@ Rails.autoloaders.main.inflector.inflect("ruby_ui" => "RubyUI") Rails.autoloaders.main.push_dir(Rails.root.join("app/components/ruby_ui"), namespace: RubyUI) Rails.autoloaders.main.collapse(Rails.root.join("app/components/ruby_ui/*")) -ActiveSupport.on_load(:action_controller) do - append_view_path Rails.root.join("app/components") -end +RubyUI.component_root = Rails.root.join("app/components") ReActionView.config.intercept_erb = true ReActionView.config.validation_mode = :raise ``` `extend Phlex::Kit` is removed. Zeitwerk ignores `.html.erb`, so the sidecar -living in an autoloaded directory is inert. - -**Known side effect, to be documented.** `app/components` is ViewComponent's -home directory. Making it a view path does not break ViewComponent — it -resolves its own sidecars by compiled method, not by virtual path — but it does -make templates under `app/components` resolvable by `render template:`. -`append_view_path`, not `prepend`, so RubyUI never shadows the host app's own -views. +living in an autoloaded directory is inert. `component_root` is the one +directory the sidecar lookup searches (§4.3); `app/components` is **not** +added to the application's view paths, so nothing about how the host resolves +its own templates changes, and ViewComponent — whose home directory this is — +is untouched. + +**Installing is not the same as enabling.** `intercept_erb = true` routes +every ERB template in the host application through Herb, and a template Herb +rejects does not degrade to its old output: with `validation_mode: :raise` it +raises, with `:none` its output is **empty**, with `:overlay` it is replaced by +the error overlay (verified against herb 0.10.4, §8). The installer therefore +runs a preflight — compile every template under `app/views` through +`Herb::Engine` and list the rejections — before it writes the interception +line, and the install guide says what to do with the list. The only opt-out is +`intercept_erb = false`, which also turns off the tag syntax; `validation_mode` +is not an escape hatch. **Runtime dependencies.** `ruby_ui` goes from zero runtime dependencies to `tailwind_merge` and `reactionview` (which brings `herb`). The constraint on @@ -200,8 +222,8 @@ execution go in `design/v2/decisions.md`, one line each, with the reason. | 6 | The golden suite lands on `main` in its own PR, before any 2.0 work | It protects 1.6 today — it catches regressions in ordinary bug-fix PRs. On a v2 branch it would protect nothing, and `main` and the branch would diverge in exactly the file that defines what "identical" means. `f7cbeda` is self-contained and cherry-picks cleanly. | | 7 | **A.** Test harness is `actionview` + `reactionview`, no controller, no dummy app | Exactly one component touches the view context — `DataTableForm`, for CSRF — and it already falls back to the literal `"csrf-token-placeholder"` that the snapshots recorded. `DataTableSortHead`, the obvious candidate for needing routes, builds its URL with `CGI` from an explicit `path:`. `reactionview` is included because decision D makes Herb what users compile with; testing on Erubi would test something we do not ship, and it puts the Herb validators over every sidecar on every CI run. | | 8 | **B.** A coercion helper in `Base`; not the `literal` gem | ~14 components index Symbol-keyed hashes with a user-supplied value and 11 already call `.to_sym`. `DialogContent` and `Badge` do not: `SIZES["lg"]` is `nil`, the class is dropped, nothing is raised. This is a 1.6 bug reachable from `params`, independent of any tag syntax. `Literal::Enum#coerce` looks up by member value and never treats `"lg"` and `:lg` as equivalent, so it does not remove the coercion — it would earn its place only as a full object model (`Base < Literal::Object`, `prop` replacing every constructor), which is a second large migration stacked on the first. Recorded as a legitimate 3.0 direction. | -| 9 | **C.** Sidecar next to the class, reached through `append_view_path` | Keeps class, template and Stimulus controller in one directory, as 1.6 already keeps class and controller. Proved in the gate in development with reloading and in production with eager loading. `append` rather than `prepend` so the library never shadows the host app. | -| 10 | **D.** Herb is required from 2.0.0, through ReActionView | Verified on released Rails (§8): `rails 8.1.3.1 + reactionview 0.4.1 + herb 0.10.4` resolves, boots, renders, runs custom transform visitors, and rejects malformed HTML at compile time. Every user gets validation from day one, and when herb ships component tags every user gets the tag syntax through `bundle update` — no reinstall, no migration. The accepted cost: two pre-1.0 gems become required, and `intercept_erb` compiles the whole host app through Herb, so a user with malformed HTML anywhere sees it on install day. | +| 9 | **C.** Sidecar next to the class, found by a lookup scoped to the component root | Keeps class, template and Stimulus controller in one directory, as 1.6 already keeps class and controller. The gate proved the sidecar-next-to-class shape in development with reloading and in production with eager loading, but resolved it through the application's view-path chain; review showed that both `prepend` and `append` merely choose which side of a name collision loses silently. A lookup scoped to one root has no other side: a collision with the host is impossible, and two sidecars for one class is an error. | +| 10 | **D.** Herb is required from 2.0.0, through ReActionView | Verified on released Rails (§8): `rails 8.1.3.1 + reactionview 0.4.1 + herb 0.10.4` resolves, boots, renders, runs custom transform visitors, and rejects malformed HTML at compile time. Every user gets validation from day one, and when herb ships component tags every user gets the tag syntax through `bundle update` — no reinstall, no migration. The accepted cost: two pre-1.0 gems become required, and `intercept_erb` compiles the whole host app through Herb, so a user with malformed HTML anywhere sees it on install day. `validation_mode` does not soften this — `:none` empties the template and `:overlay` replaces it — so the installer preflights the host's templates (§4.4), and the only opt-out is `intercept_erb = false`, which also disables the tag syntax. | ## 6. Phases @@ -230,8 +252,13 @@ compares against the snapshot, not against a running Phlex component. `10c01f0 [Bug Fix] HoverCard: let the card escape a clipping ancestor (#530)`. **Acceptance: only HoverCard differs, and every diff is explained by #530.** Any other diff is investigated before proceeding. -4. Resolve the whitespace question (§9.1) with a measurement, not a guess. -5. PR to `main`. +4. Harden the canonical form (§9.1): refuse a wrapper escape, distinguish an + empty element from a whitespace-only one, use HTML's whitespace — each + with a test that fails first — and move the coverage guard from + instantiation to render. No recorded snapshot changes. +5. Inventory what the canonical form still cannot see, and record the + decision in `design/v2/decisions.md`. +6. PR to `main`. **Acceptance.** `cd gem && bundle exec rake` green on Ruby 3.3 and 3.4. Every directory under `lib/ruby_ui/` has at least one scenario; every `RubyUI::Base` @@ -261,13 +288,40 @@ code. `.html.erb` fixtures under `gem/test/golden/views/`, rendered through the 2.0 component and compared against the frozen snapshot. - Implement the `enum` coercion helper in `Base`. +- Port Phlex's attribute guards into `Attributes` (§4.3), with unit tests + that assert an unsafe name raises and a `javascript:` reference is dropped. +- `render_in` assigns `content` on every call; test that an instance rendered + twice, the second time without a block, renders empty content. +- Implement the scoped sidecar lookup (§4.3, §4.4) and test it against a + host template at the same virtual path, against two overlapping roots, and + across two view contexts. The gate's `@template_path ||=` cache is keyed per + class and ignored the view context; the scoped lookup must not. +- Define the **strict lane**. The canonical form is, by design, blind to + whitespace at a text–element boundary and between inline siblings (§9.1). + Sidecars are therefore written in ERB trim mode (`<%-` / `-%>`) so they emit + no whitespace Phlex did not, and the components that carry text — + Typography, InlineCode, InlineLink, ShortcutKey, Badge, FormFieldError and + the others the Phase 1 inventory names — are additionally compared **raw**, + with only attribute order normalized. A strict-lane failure is a real + difference, not noise. +- **MCP.** `mcp/data/registry.json` embeds the source of every component file + and CI rebuilds it and fails on any diff. Every Phase 2 PR that touches + `gem/lib/ruby_ui` rebuilds it (`cd mcp && bundle exec exe/ruby-ui-mcp-build`) + and commits the result. `RegistryBuilder` also extracts examples from + `*_docs.rb`; decision 11 (Phase 2.3) says what happens to those. +- **Fresh-app install test.** A script, run in CI, that does `rails new`, adds + the gem, runs the installer and the preflight, generates one component and + renders it through a request. The golden suite renders without Rails and + cannot see installation, reloading, CSRF (`DataTableForm` falls back to a + placeholder outside a request) or assets. - Point `docs/Gemfile` at the published `ruby_ui` 1.6 instead of `path: "../gem"`, so the site keeps building and the CI Docs job stays green while the gem is mid-migration. Phase 3 reverts it. -**Acceptance.** The layer is in the gem with its own tests. The ERB lane runs -with at least one component at parity. A component renders in a real Rails 8.1 -application. All three CI jobs are green. +**Acceptance.** The layer is in the gem with its own tests, guards included. +The ERB lane runs with at least one component at parity, and the strict lane +with at least one text-bearing component. The fresh-app script passes. All +three CI jobs are green with the registry rebuilt. #### 2.1 The hard components first @@ -292,15 +346,32 @@ frozen contract; the 1.6 Stimulus controllers unedited. The remaining ~50 components, in batches. **Definition of done, per component.** A plain Ruby class with no Phlex; a -sidecar; the snapshot matching; a scenario passing the String form of every -enum attribute; the Stimulus controller untouched. +sidecar in trim mode; the snapshot matching; the strict lane matching if the +component carries text; a scenario passing the String form of every enum +attribute; **its existing unit tests in `gem/test/ruby_ui/` ported to the new +harness, none deleted** — they are the inventory of what the component promises +beyond its markup; the Stimulus controller untouched; the MCP registry rebuilt. #### 2.3 Generators and installer `component_generator.rb` copies `.rb` and `.html.erb`. `install_generator.rb` -writes the initializer of §4.4 and copies `base.rb` and `attributes.rb`. -`dependencies.yml` is unchanged — it describes JS packages. The gemspec drops -`phlex` and gains `tailwind_merge` and `reactionview` as runtime dependencies. +writes the initializer of §4.4, runs the Herb preflight, and copies `base.rb` +and `attributes.rb`. `dependencies.yml` is unchanged — it describes JS +packages. The gemspec drops `phlex` and gains `tailwind_merge` and +`reactionview` as runtime dependencies. + +**Decision 11 — `ruby_ui:install:docs` is removed in 2.0.** `DocsGenerator` +copies the gem's 52 `*_docs.rb` pages into a host application's +`app/views/docs/`. Those pages are Phlex — `Views::Base`, `view_template`, +`Docs::VisualCodeExample` with `eval`'d Phlex examples — and Phase 3.0 is what +redesigns them. Shipping 2.4 with the generator intact would ship an installer +that generates code requiring the renderer the gem just removed. The +alternative, migrating the 52 pages before 2.4, pulls the whole +`VisualCodeExample` redesign into Phase 2. The feature is documented nowhere — +not on the site, not in the README — so it is removed with a CHANGELOG line, +and the pages leave the gem in Phase 3.1 with the other documentation +primitives. `RegistryBuilder` reads the same files for MCP examples and is +updated in that same Phase 3.1 change. #### 2.4 Release @@ -329,9 +400,9 @@ Stimulus controllers — and **10 of the 52 have already drifted from their source**. Migrating a doc page is therefore gem work with a site consumer, and the drift should be resolved in the same pass rather than carried into 2.0. -Whether the doc pages should keep shipping in the gem at all is an open -question for this phase: they are the only part of the gem that is neither a -component nor a generator. +Decision 11 (Phase 2.3) removes `ruby_ui:install:docs` in 2.0, so by the time +this phase starts the pages are no longer a shipped feature — they are site +content that happens to live in the gem, and 3.1 moves them out. #### 3.0 Redesign `VisualCodeExample` @@ -351,8 +422,10 @@ files of one to five lines each. `gem/lib/ruby_ui/docs/` holds six Phlex classes — `visual_code_example`, `header`, `components_table`, `component_setup_tabs`, `sidebar_examples`, `base`. They are site infrastructure, not library surface, and they are already -excluded from the gem's test autoload. They move to `docs/app/`. Check -`mcp:build` for a dependency on them first. +excluded from the gem's test autoload. They move to `docs/app/`, and so do +the 52 `*_docs.rb` pages (decision 11). `RegistryBuilder` extracts MCP examples +from those pages (`mcp/lib/ruby_ui/mcp/builders/registry_builder.rb`); it is +pointed at their new home in the same change. #### 3.2 Chrome and layout @@ -381,8 +454,12 @@ drift between a doc page in the gem and its copy in the site. | Question | Answer | | --- | --- | | Does a component render the same HTML as 1.6? | The golden suite's ERB lane, against the frozen snapshot | +| Is an element that was empty still empty, not whitespace-only? | The canonical form, after Phase 1 hardens it (§9.1) | +| Did whitespace at a text boundary change? | The strict lane, raw output, for text-bearing components (Phase 2.0) | | Does an attribute reach the element correctly? | The differential test against Phlex 2.4.1 in `Attributes` | +| Is an unsafe attribute still refused? | Unit tests on `Attributes`' guards, not the golden suite | | Is a sidecar valid HTML? | Herb's validators, over every sidecar, on every CI run | +| Does the component keep its non-markup promises (lifecycle, caller API, CSRF)? | The ported unit tests, and the fresh-app install script | | Does a component behave in a browser? | Not covered during Phase 2 — see §9.4 | ## 8. Evidence @@ -432,6 +509,25 @@ surfaces as `ActionView::SyntaxErrorInTemplate` carrying Herb's annotated message (missing closing tag, line, suggestion). This is the `Gemfile.stable` lane the gate planned and never ran. +**Review findings, verified 2026-09-19.** With herb 0.10.4, +`Herb::Engine.new("
", validation_mode: mode)` gives: +`:raise` → `Herb::Engine::CompilationError`; `:none` → 31 bytes of source with +no `
`; `:overlay` → the error overlay. `Golden::CanonicalHtml` on +`v2-herb` returns equal canonical forms for `
safe
` and +`
safe
`; for `
` and +`
\n
`; for `

Hello w

` and `

Hellow

`; +and for `class="a\vb"` and `class="a b"`. Phlex 2.4.1 raises +`Phlex::ArgumentError` on an `onclick` attribute and drops a `javascript:` +href; the gate's `Attributes` passes both through. The gate's `render_in` +assigns `@content` only when a block is given. `Golden::Harness` records a +class on `initialize`, not on render. Raw Phlex output of all 188 scenarios +contains zero elements with whitespace-only content and zero text–element +boundaries carrying a space. `mcp/lib/ruby_ui/mcp/builders/registry_builder.rb` +embeds each component file's content, and `.github/workflows/ci.yml` rebuilds +the registry and fails on a diff. `ruby_ui:install:docs` is referenced nowhere +in `docs/app`, `gem/README.md` or the generators other than its own file. The +Task 1 outputs also reproduce on Ruby 3.3.5. + **RubyUI 1.6.** `ruby_ui.gemspec` declares **no** runtime dependencies; `phlex` and `tailwind_merge` are development dependencies. `s.files` is `Dir["README.md", "LICENSE.txt", "lib/**/*"]`, which already packages `.erb`. @@ -473,23 +569,45 @@ ERB emits newlines where Phlex emitted nothing. Most of RubyUI lays out with flex and `gap-*` and is immune; `Typography`, `InlineCode`, `InlineLink`, `ShortcutKey` and inline badges are not. -**Measured 2026-09-19, while planning Phase 1: 8 components, 50 adjacent inline -pairs** — `badge` (27, an artefact of the `all_variants` scenario), `dialog` -(6), `codeblock` (5), `sheet` (5), `sidebar` (3), `carousel` (2), `command` (1), -`context_menu` (1). The count excludes pairs whose parent is a flex or grid -container, which ignores the whitespace; including them inflates it to 21 -components and 86 pairs. - -**Resolution:** leave the normalizer alone and write those eight components' -sidecars whitespace-tight in Phase 2, named explicitly in each component's -task. A second comparison mode would have to be threaded through the canonical -form, the fixed-point assertion and all 186 snapshots for eight components, -most of them benign — the `sr-only` label beside a close icon renders the same -either way, and `codeblock`'s tokens sit inside `pre`, which the normalizer -already preserves. - -Phase 1 reproduces the measurement and records it in `design/v2/decisions.md`; -the script is `gem/test/golden/tools/inline_adjacency.rb`. +**Measured 2026-09-19, then reviewed.** A heuristic inventory +(`gem/test/golden/tools/inline_adjacency.rb`) reported 8 components and 50 +adjacent inline pairs. Review of the pairs showed the count is neither +accurate nor an upper bound: Carousel's are absolutely positioned buttons, +Command's and ContextMenu's anchors are `flex` and so block-level, Dialog's, +Sheet's and Sidebar's are `sr-only` labels, Codeblock's sit inside `pre`, which +the normalizer already preserves — leaving Badge's 27, an artefact of one +scenario. The script also misclassifies `span.block`, `span.hidden` and +`span.absolute` as inline, misses `inline-grid` and responsive variants, and +does not look at text–element boundaries at all. + +The text–element case is the one that matters, and it is not cosmetic. +`FormField`'s controller enables validation when `errorTarget.textContent` is +truthy and hides the target otherwise; `FormFieldError` styles itself with +`empty:hidden`. Phlex emits `
`; an ERB sidecar naturally emits +`
\n
`. The canonical form on `v2-herb` called those identical. +They are not: `""` is falsy and `"\n"` is truthy, and `:empty` matches only the +first. Behaviour flips on whitespace alone, in a component the inline-pair +count never named. + +**Resolution, in three parts, none of which is the count.** + +1. **Phase 1 hardens the canonical form** so that an empty element and a + whitespace-only element canonicalize differently, a fragment that escapes + the `` or U+000B — but every one of them is a difference the ERB lane in Phase 2 could introduce and the ruler would miss. The one that matters most is behavioural: `FormField`'s controller (`gem/lib/ruby_ui/form/form_field_controller.js:9`) enables validation when `errorTarget.textContent` is truthy, and `FormFieldError` uses `empty:hidden`, so `
` and `
\n
` are different components to a browser. The ruler on `v2-herb` calls them identical. -This matters for Phase 2, not for Phase 1: ERB emits a newline where Phlex emitted nothing, so an inline-adjacent pair could render differently in a browser while the ruler reports parity. `scenarios.rb` already acknowledges the blindness in its header comment; this task counts how much of the catalog it touches and records the decision that follows. +Separately, the coverage guard records a class when it is **instantiated**, not when it renders. A component that is only ever `new`ed for its `attrs` — `PaginationItem` does this with `Button` — would count as covered without a single byte of its markup being measured. Today every recorded class does reach `view_template` (review checked), so moving the hook changes nothing now and closes the loophole for later. + +This task is TDD in the plain sense: each defect gets a test that fails on the ported code, then the fix. + +**Files:** +- Create: `gem/test/golden/canonical_html_test.rb` +- Create: `gem/test/golden/harness_test.rb` +- Modify: `gem/test/golden/canonical_html.rb` (`parse`, `emit_element`, `significant_children`'s comment, `collapse`, `canonical_attribute`, one new constant, one new predicate) +- Modify: `gem/test/golden/harness.rb` (`RecordsRenderedClass`, and the comment above `classes_rendered`) + +**Interfaces:** +- Consumes: `Golden::CanonicalHtml.call(html)`, `Golden::Harness.render(&block)`, `Golden::Harness.classes_rendered` from Task 1. +- Produces: the same names with tightened semantics. `CanonicalHtml.call` raises `ArgumentError` on a fragment that escapes the wrapper; canonicalizes an element with only whitespace children as ` `; splits token lists and collapses text on `[\t\n\f\r ]` only. `Harness.classes_rendered` contains a class only after that class's `before_template` ran. + +- [ ] **Step 1: Write the failing canonicalizer tests** + +Create `gem/test/golden/canonical_html_test.rb`: + +```ruby +# frozen_string_literal: true + +require "test_helper" +require "golden/canonical_html" + +# Adversarial tests for the normalizer: inputs a browser distinguishes that the +# canonical form must not conflate, and input it must refuse rather than +# silently truncate. Each of these compared equal on the ported code. +class GoldenCanonicalHtmlTest < Minitest::Test + def canonical(html) + Golden::CanonicalHtml.call(html) + end + + def test_refuses_a_fragment_that_escapes_the_template_wrapper + error = assert_raises(ArgumentError) do + canonical("
safe
") + end + + assert_match(/escaped the ` in the input closes the wrapper early, and + # whatever follows lands beside it — outside what gets compared. + # Refuse rather than silently drop it. + unless wrapped.children.size == 1 + raise ArgumentError, + "fragment escaped the ?): #{html[0, 120].inspect}" + end + + wrapped.children.first.children + end +``` + +**(c)** In `emit_element`, replace the `elsif children.empty?` branch, and add the predicate below `significant_children`. Replace the comment on `significant_children` too — its claim that `
` and `
\n
` are identical to a browser is the defect. + +```ruby + elsif children.empty? + # `
` and `
\n
` are not the same element to a + # browser: `:empty` matches only the first, and `textContent` is + # truthy only on the second. Keep a single space to tell them apart. + filler = whitespace_only_content?(node) ? " " : "" + out << (INDENT * depth) << open << filler << close << "\n" +``` + +```ruby + # Comments and formatting whitespace are not children for layout + # purposes: `
\n \n
` and `
` build the + # same tree. Whether an element had *only* such children is a separate + # question, answered by whitespace_only_content? — see emit_element. + def significant_children(node, mode) + return node.children.to_a unless mode == :normal + node.children.reject { |child| child.comment? || (child.text? && collapse(child.text).empty?) } + end + + def whitespace_only_content?(node) + node.children.any? { |child| child.text? && !child.text.empty? && collapse(child.text).empty? } + end +``` + +**(d)** Use HTML whitespace in `collapse` and in the token-list branch of `canonical_attribute`: + +```ruby + def collapse(text) + text.gsub(HTML_WHITESPACE, " ").delete_prefix(" ").delete_suffix(" ") + end +``` + +```ruby + elsif TOKEN_LISTS.include?(name) + [name, value.split(HTML_WHITESPACE).reject(&:empty?).join(" ")] +``` + +- [ ] **Step 4: Run the canonicalizer tests and confirm all five pass** + +```bash +cd gem +bundle exec rake test N=/GoldenCanonicalHtmlTest/ +``` + +Expected: `5 runs, ... 0 failures, 0 errors`. + +- [ ] **Step 5: Write the failing harness test** + +Create `gem/test/golden/harness_test.rb`: + +```ruby +# frozen_string_literal: true + +require "test_helper" +require "golden/harness" + +class GoldenHarnessTest < Minitest::Test + def test_records_a_class_when_it_renders_not_when_it_is_instantiated + with_fresh_recording do + Golden::Harness.render { RubyUI::Button.new(variant: :outline).attrs } + + refute_includes Golden::Harness.classes_rendered.keys, "RubyUI::Button", + "an instantiated-but-unrendered component must not count as covered" + + Golden::Harness.render { RubyUI.Button { "x" } } + + assert_includes Golden::Harness.classes_rendered.keys, "RubyUI::Button" + end + end + + private + + # The recording hash is process-wide and the golden scenarios fill it; swap + # it out so this test sees only its own renders. + def with_fresh_recording + saved = Golden::Harness.classes_rendered + Golden::Harness.instance_variable_set(:@classes_rendered, {}) + yield + ensure + Golden::Harness.instance_variable_set(:@classes_rendered, saved) + end +end +``` + +- [ ] **Step 6: Run it and confirm it fails** + +```bash +cd gem +bundle exec rake test N=/GoldenHarnessTest/ +``` + +Expected: `1 runs, ... 1 failures` — the `refute_includes`, because `Button.new` records on `initialize`. + +- [ ] **Step 7: Patch `harness.rb`** + +Replace the `RecordsRenderedClass` module at the bottom of `gem/test/golden/harness.rb`: + +```ruby + # Recording on render rather than on instantiation: a component that is only + # `new`ed for its computed attributes (PaginationItem does this with Button) + # has not been measured, and the coverage guard must not count it. + # `before_template` is the hook Phlex calls on every render and no component + # overrides, so prepending it on Base reaches every subclass. + module RecordsRenderedClass + def before_template + Golden::Harness.record(self.class) + super + end + end +``` + +And replace the comment above `classes_rendered`: + +```ruby + # Coverage bookkeeping: which classes have rendered while a scenario was + # active. See RecordsRenderedClass for why render, not instantiation. + def classes_rendered + @classes_rendered ||= {} + end +``` + +- [ ] **Step 8: Run the harness test and confirm it passes** + +```bash +cd gem +bundle exec rake test N=/GoldenHarnessTest/ +``` + +Expected: `1 runs, ... 0 failures`. + +- [ ] **Step 9: Run the golden suite and confirm no snapshot changed** + +The hardening tightens what the ruler distinguishes; it must not move what it already measures. + +```bash +cd gem +bundle exec rake golden +``` + +Expected: `191 runs, ... 0 failures, 0 errors, 2 skips`. The coverage test `test_every_component_class_is_rendered_by_a_scenario` still passes — if it fails naming a class, that class is only instantiated and never rendered by any scenario, which is a real gap in the catalog; add a scenario that renders it rather than reverting the hook. + +```bash +cd /Users/cirdes/Workspaces/ruby_ui +git status --porcelain gem/test/golden/snapshots +``` + +Expected: no output. **If any snapshot shows as modified, STOP.** Either the measurement that justified this task is wrong or a patch changed more than it should; report the file and its diff. + +- [ ] **Step 10: Run the full default task** + +```bash +cd gem +bundle exec rake +``` + +Expected: green, `409 files inspected, no offenses detected` — 407 plus the two test files this task adds. StandardRB counts one per Ruby file; fix any offense with `bundle exec standardrb --fix`. + +- [ ] **Step 11: Commit** + +```bash +cd /Users/cirdes/Workspaces/ruby_ui +git add gem/test/golden/ +git commit -m "$(cat <<'MSG' +[Feature] Golden suite: harden the canonical form and the coverage guard + +Four false equivalences the ruler accepted, each now a failing test +before its fix: a stray 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 Fable 5.1 +MSG +)" +``` + +--- + +## Task 3: Inventory the normalizer's remaining blind spot + +Task 2 closed the cases the canonical form *can* close without changing what it measures. What remains, by design, is whitespace between two element siblings and at a text–element boundary in `:normal` mode: `ab` and `a\nb` still canonicalize the same, as do `Hello ` and `Hello`. In an inline formatting context a browser renders those differently, and ERB emits newlines where Phlex emitted nothing. + +This task ships a script that lists candidate spots, and records the decision. **The script is an inventory, not a criterion.** Review of its output showed it over- and under-counts — it classifies by tag name and a few Tailwind classes, ignores `absolute`, `hidden`, `sr-only` and responsive variants, and never looks at text nodes — and the decision below does not depend on its number being right. It depends on Task 2 (the case that flips behaviour is now caught for every component) and on Phase 2's strict lane. It changes no component and no snapshot. @@ -224,36 +521,30 @@ It changes no component and no snapshot. - Create: `design/v2/decisions.md` **Interfaces:** -- Consumes: the snapshots recorded in Task 1, read from disk at `gem/test/golden/snapshots`. +- Consumes: the snapshots on disk at `gem/test/golden/snapshots`. - Produces: `design/v2/decisions.md`, the living decision log that every later phase appends to. One entry per decision or deviation, newest last, each with a reason. -- [ ] **Step 1: Write the report script** +- [ ] **Step 1: Write the inventory script** Create `gem/test/golden/tools/inline_adjacency.rb`: ```ruby # frozen_string_literal: true -# Counts where the golden suite's canonical form is blind to whitespace. +# Lists candidate spots where the golden suite's canonical form is blind to +# whitespace: two adjacent inline-level elements under a parent that lays out +# inline. An inventory to read, not a number to rely on. # # cd gem && bundle exec ruby test/golden/tools/inline_adjacency.rb # -# The canonical form collapses whitespace between elements, so two adjacent -# inline-level elements compare equal whether or not a space separated them. -# A browser renders those two cases differently. This report finds every such -# adjacency in the recorded snapshots, so Phase 2 knows which sidecars have to -# be written whitespace-tight. -# -# Two judgements, both approximations, both deliberate: -# -# * Inline-level is an intrinsically inline tag, or any element whose class -# list contains `inline`, `inline-block` or `inline-flex` — because -# Tailwind overrides display and the tag name alone is not enough. -# * Whitespace between siblings only renders as a space when the parent -# establishes an inline formatting context. A flex or grid parent ignores -# it, so those parents are skipped. Without this filter the count is -# inflated roughly threefold by icons sitting next to labels inside -# flex buttons. +# Known limits, all deliberate: inline-level is judged from the tag name plus +# `inline`, `inline-block` and `inline-flex` in the class list; a parent is +# skipped if its class list has `flex`, `grid`, `inline-flex` or `inline-grid`. +# That is enough to find candidates and not enough to prove absence — it does +# not see `absolute`, `hidden`, `sr-only`, `block` on an inline tag, +# responsive or state variants, or whitespace at a text–element boundary. +# Task 2 of the Phase 1 plan closes the empty-versus-whitespace-only case for +# every component; Phase 2's strict lane covers text-bearing components raw. require "nokogiri" @@ -300,8 +591,8 @@ Dir.glob(File.join(SNAPSHOT_ROOT, "**", "*.html")).sort.each do |path| end end -puts "components affected: #{findings.keys.size}" -puts "adjacent inline pairs: #{findings.values.sum(&:size)}" +puts "components with candidates: #{findings.keys.size}" +puts "candidate pairs: #{findings.values.sum(&:size)}" puts findings.keys.sort.each do |component| @@ -311,7 +602,7 @@ findings.keys.sort.each do |component| end ``` -- [ ] **Step 2: Run it and confirm the count** +- [ ] **Step 2: Run it and confirm the output** ```bash cd gem @@ -321,24 +612,11 @@ bundle exec ruby test/golden/tools/inline_adjacency.rb Expected, exactly: ``` -components affected: 8 -adjacent inline pairs: 50 +components with candidates: 8 +candidate pairs: 50 ``` -and these eight components, with these counts: - -| Component | Pairs | -| --- | --- | -| `badge` | 27 | -| `codeblock` | 5 | -| `dialog` | 6 | -| `sheet` | 5 | -| `sidebar` | 3 | -| `carousel` | 2 | -| `command` | 1 | -| `context_menu` | 1 | - -If the numbers differ, the snapshots on disk are not the ones Task 1 recorded, or Nokogiri's HTML5 parser behaves differently on this machine. Investigate before writing the decision — the decision is only worth what the number is worth. +with `badge` 27, `dialog` 6, `codeblock` 5, `sheet` 5, `sidebar` 3, `carousel` 2, `command` 1, `context_menu` 1. If the numbers differ, the snapshots on disk are not the ones Task 1 recorded; investigate before continuing. - [ ] **Step 3: Write the decision log** @@ -348,43 +626,43 @@ Create `design/v2/decisions.md`: # RubyUI 2.0 — decisions One entry per decision or deviation from `design/2026-09-19-rubyui-2-0-design.md`, -newest last, each with the reason. The ten decisions taken before execution -started are in §5 of that document; this file records what happens after. - -## 1. The normalizer's whitespace blind spot (§9.1) — measured 2026-09-19 - -`Golden::CanonicalHtml` is blind to whitespace between adjacent inline-level -elements whose parent establishes an inline formatting context. Measured with -`gem/test/golden/tools/inline_adjacency.rb` over the recorded snapshots: -**8 components, 50 adjacent inline pairs**. - -| Component | Pairs | What they are | -| --- | --- | --- | -| `badge` | 27 | the `all_variants` scenario, 28 badges in a row — an artefact of how the scenario is written, not of a composition users write | -| `dialog` | 6 | the close button's `` next to its `sr-only` label | -| `codeblock` | 5 | highlighted token spans | -| `sheet` | 5 | the close button, as in `dialog` | -| `sidebar` | 3 | icon next to label | -| `carousel` | 2 | the previous and next buttons | -| `command` | 1 | adjacent `` items | -| `context_menu` | 1 | adjacent `` items | - -**Decision: leave the normalizer alone; write these eight components' sidecars -whitespace-tight in Phase 2.** Each of the eight gets an explicit line in its -Phase 2 task saying so, and the sidecar must not put a newline between the -inline siblings named above. - -**Why not extend the normalizer.** A second comparison mode that records -inter-element whitespace would have to be threaded through the canonical form, -the fixed-point assertion and all 186 snapshots, for eight components — most of -which are benign anyway: the `sr-only` label next to a close icon renders the -same either way, and `codeblock`'s tokens sit inside `pre`, which the -normalizer already preserves. The cost is not proportional to the risk. +newest last, each with the reason. The eleven decisions taken before execution +started are in §5 and §6 of that document; this file records what happens after. + +## 1. The normalizer's whitespace blind spot (§9.1) — 2026-09-19 + +`Golden::CanonicalHtml` does not see whitespace between two element siblings, +or at a text–element boundary, in `:normal` mode. It cannot without giving up +the fixed-point property that makes the byte comparison a structural one. + +`gem/test/golden/tools/inline_adjacency.rb` lists 8 components and 50 candidate +pairs. Reading them: Badge's 27 are the `all_variants` scenario laying 28 +badges side by side — an artefact of the scenario, not a composition users +write; Dialog's, Sheet's and Sidebar's 14 are the close button's icon beside +its `sr-only` label, invisible either way; Codeblock's 5 sit inside `pre`, +which the normalizer preserves verbatim; Carousel's 2 are absolutely +positioned; Command's and ContextMenu's anchors are `flex` and so block-level. +The script also cannot see the text–element case at all, which is the one that +carries behaviour — `FormField` flips on `
` versus +`
\n
`, and that is now caught by the hardened canonical form (Phase 1 +plan, Task 2), not by this inventory. + +**Decision.** The number is not the criterion and does not need to be +accurate. Three things are: + +1. The canonical form distinguishes an empty element from a whitespace-only + one, for every component, as of Phase 1 Task 2. +2. Phase 2 sidecars are written in ERB trim mode (`<%-` / `-%>`), so the ERB + lane emits no whitespace Phlex did not. This is a rule for all 256 + classes, not for eight. +3. Phase 2.0 defines a strict lane — raw output, attribute order normalized, + nothing else — and every component that carries text runs through it. + Badge, Typography, InlineCode, InlineLink, ShortcutKey and FormFieldError + are the first entries on that list; this inventory is one way to find more. **What would reverse this.** A Phase 2 component showing a visible spacing -difference that the suite reported as parity. That is the failure this decision -accepts, and §9.4 is the reason it cannot be caught automatically before -Phase 3. +difference in a browser that both lanes reported as parity. §9.4 of the design +is the reason that cannot be caught automatically before Phase 3. ``` - [ ] **Step 4: Verify nothing else changed** @@ -394,14 +672,14 @@ cd gem bundle exec rake ``` -Expected: green, `407 files inspected, no offenses detected`. The report script lives under `test/` and is not loaded by the suite, but StandardRB does inspect it — fix any offense with `bundle exec standardrb --fix` and re-run. +Expected: green, `410 files inspected, no offenses detected` — one more than Task 2 for the script. Fix any offense with `bundle exec standardrb --fix`. ```bash cd /Users/cirdes/Workspaces/ruby_ui git status --porcelain gem/test/golden/snapshots gem/lib docs mcp ``` -Expected: no output. This task changes no snapshot, no component, and nothing outside `gem/test/golden/tools/` and `design/`. +Expected: no output. - [ ] **Step 5: Commit** @@ -409,26 +687,23 @@ Expected: no output. This task changes no snapshot, no component, and nothing ou cd /Users/cirdes/Workspaces/ruby_ui git add gem/test/golden/tools/inline_adjacency.rb design/v2/decisions.md git commit -m "$(cat <<'MSG' -[Documentation] Measure the golden suite's whitespace blind spot - -The canonical form collapses whitespace between elements, so adjacent -inline-level elements compare equal whether or not a space separated -them — and a browser renders those two cases differently when the -parent lays out inline. ERB emits a newline where Phlex emitted -nothing, so Phase 2 needs to know how much of the catalog this touches. +[Documentation] Inventory the golden suite's remaining whitespace blind spot -8 components, 50 pairs. Adds the report that counts it and records the -decision: write those eight whitespace-tight rather than grow a second -comparison mode. +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 Opus 5 (1M context) +Co-Authored-By: Claude Fable 5.1 MSG )" ``` --- -## Task 3: Fix `ContextMenuLabel` and pin its two scenarios +## Task 4: Fix `ContextMenuLabel` and pin its two scenarios Two scenarios in the catalog are declared `pending:` and carry no snapshot, so two of the catalog's renders are not pinned. The reason is a one-line bug in `ContextMenuLabel`: @@ -446,17 +721,18 @@ Three consequences: `pl-8` is never applied, so `inset:` does nothing; a garbage This is the last hole in the contract Phase 2 freezes. It is a 1.6 bug fix in its own right. `grep` confirms it is the only occurrence of the pattern in the gem. -> **Scope note.** This task goes beyond the spec's Phase 1, which asks only for the ruler. It is here because Phase 2 freezes the contract, and a scenario with no snapshot is a render nobody is measuring. It is independently reviewable: Tasks 1, 2 and 4 stand without it. Drop it and Phase 1 still succeeds, with two unpinned renders and a known bug shipping in 1.6. +> **Scope note.** This task goes beyond the spec's Phase 1, which asks only for the ruler. It is here because Phase 2 freezes the contract, and a scenario with no snapshot is a render nobody is measuring. It is independently reviewable: Tasks 1, 2, 3 and 5 stand without it. Drop it and Phase 1 still succeeds, with two unpinned renders and a known bug shipping in 1.6. **Files:** - Modify: `gem/lib/ruby_ui/context_menu/context_menu_label.rb:20` - Modify: `gem/test/ruby_ui/context_menu_test.rb` - Modify: `gem/test/golden/scenarios.rb:447-449` +- Modify (by rebuilding): `mcp/data/registry.json` — it embeds the source of `context_menu_label.rb`, and CI fails on a stale copy - Create (by re-recording): `gem/test/golden/snapshots/context_menu/label_default.html`, `gem/test/golden/snapshots/context_menu/label_inset.html` **Interfaces:** - Consumes: `bundle exec rake golden:update` and `Golden::Catalog.scenario(name, pending: nil)` from Task 1. -- Produces: a catalog with no `pending:` scenarios — `Golden::Catalog.scenarios.all?(&:pinned?)` is true. +- Produces: a catalog with no `pending:` scenarios — `Golden::Catalog.scenarios.all?(&:pinned?)` is true — and a registry that matches the gem. - [ ] **Step 1: Read the two pending scenarios** @@ -495,9 +771,7 @@ cd gem bundle exec rake test N=/context_menu_label/ ``` -Expected: both new tests FAIL. -- The first fails because the output contains `{inset?: "pl-8"}`. -- The second fails because `pl-8` is absent from the `inset: true` render and the literal `"pl-8"` inside the Hash makes it present in both, depending on which assertion runs first. +Expected: both new tests FAIL. The first because the output contains `{inset?: "pl-8"}`; the second because the literal `"pl-8"` inside the serialized Hash is present in both renders, so `refute_includes plain, "pl-8"` fails. - [ ] **Step 4: Fix the one line** @@ -562,20 +836,41 @@ cat test/golden/snapshots/context_menu/label_*.html Expected: no `inset?` anywhere; `pl-8` present in the inset snapshot and absent from the other. -- [ ] **Step 10: Run the full default task** +- [ ] **Step 10: Rebuild the MCP registry** + +`mcp/data/registry.json` embeds the full source of every component file; CI rebuilds it and fails on any diff. + +```bash +cd /Users/cirdes/Workspaces/ruby_ui/mcp +bundle install +bundle exec exe/ruby-ui-mcp-build +cd .. +git status --porcelain mcp +``` + +Expected: `Wrote /Users/cirdes/Workspaces/ruby_ui/mcp/data/registry.json`, then exactly ` M mcp/data/registry.json`. Confirm the diff is only the `context_menu_label.rb` content: + +```bash +git diff --stat mcp/data/registry.json +git diff mcp/data/registry.json | grep '^[-+]' | grep -v '^[-+][-+]' | grep -c 'inset' +``` + +Expected: one file changed; the second command prints a small positive number (the changed line appears in both `-` and `+` forms). If the diff touches any other component, STOP — the registry on `main` was already stale and that is a separate finding. + +- [ ] **Step 11: Run the full default task** ```bash cd gem bundle exec rake ``` -Expected: green, and the skip count is now **0** — `rake golden` no longer reports "You have skipped tests". +Expected: green, `410 files inspected, no offenses detected`, and the skip count is now **0** — `rake golden` no longer reports "You have skipped tests". -- [ ] **Step 11: Commit** +- [ ] **Step 12: Commit** ```bash cd /Users/cirdes/Workspaces/ruby_ui -git add -A +git add gem/lib/ruby_ui/context_menu/context_menu_label.rb gem/test/ruby_ui/context_menu_test.rb gem/test/golden/scenarios.rb gem/test/golden/snapshots/context_menu/ mcp/data/registry.json git commit -m "$(cat <<'MSG' [Bug Fix] ContextMenuLabel: stop serializing a Hash into the class attribute @@ -585,21 +880,22 @@ 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. +catalog now has no unpinned renders. Rebuilds the MCP registry, which +embeds the component's source. -Co-Authored-By: Claude Opus 5 (1M context) +Co-Authored-By: Claude Fable 5.1 MSG )" ``` --- -## Task 4: Open the pull request +## Task 5: Open the pull request **Files:** none. **Interfaces:** -- Consumes: the three commits from Tasks 1–3. +- Consumes: the four commits from Tasks 1–4. - Produces: a PR against `main`. Phase 2 branches from `main` after it merges, so that the 2.0 line inherits the ruler rather than forking it. - [ ] **Step 1: Confirm the branch is clean and complete** @@ -610,16 +906,18 @@ git status --porcelain git log --oneline main..HEAD ``` -Expected: no output from the first command; three commits from the second. +Expected: no output from the first command; four commits from the second. - [ ] **Step 2: Run everything one more time from a clean state** ```bash cd gem bundle exec rake +cd ../mcp +bundle exec exe/ruby-ui-mcp-build && git diff --exit-code data/registry.json ``` -Expected: green, `407 files inspected, no offenses detected`, zero skips. +Expected: gem green, `410 files inspected, no offenses detected`, zero skips; the registry rebuild produces no diff. - [ ] **Step 3: Ask the user before pushing** @@ -635,14 +933,14 @@ gh pr create --base main --title "[Feature] Golden HTML suite: the 1.6 parity ru 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. 186 snapshots over 54 component directories. +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::Base` subclass must be reached by -one, and no snapshot may exist without a scenario. The normalizer is asserted to -be idempotent over every snapshot, 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. +directory must have a scenario, every `RubyUI::Base` subclass must actually +render in one, and no snapshot may exist without a scenario. The normalizer is +asserted to be idempotent over every snapshot, 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 golden` is reached by `rake test`, so CI covers it on Ruby 3.3 and 3.4 with no workflow change. `nokogiri` is added as a development dependency; the @@ -660,6 +958,13 @@ ordinary bug-fix PRs today. ## Also in this PR +- **The ruler is hardened against four false equivalences** found in review, + each with a test that failed before the fix: a stray `` silently + truncated the input; an empty element and a whitespace-only element compared + equal (they are different to `FormField`'s controller and to `empty:hidden`); + class tokens and text collapsed on Ruby's `\s`, which includes U+000B, rather + than HTML's whitespace. The coverage guard now records a class when it + renders, not when it is instantiated. No recorded snapshot changed. - **HoverCard snapshots re-recorded.** #530 changed its markup after the snapshots were first taken. Those two files are the only difference between the recording on `v2-herb` and the recording against `main` — which is what @@ -667,11 +972,11 @@ ordinary bug-fix PRs today. - **`ContextMenuLabel` bug fix.** `class: [..., inset?: "pl-8"]` is an Array whose second element is a Hash, so every label shipped a literal `{inset?: "pl-8"}` class token and `inset:` never applied `pl-8`. Fixing it - pins the last two unpinned scenarios. -- **The whitespace report.** The canonical form is blind to whitespace between - adjacent inline elements. `gem/test/golden/tools/inline_adjacency.rb` counts - where that matters; the finding and the decision are in - `design/v2/decisions.md`. + pins the last two unpinned scenarios. The MCP registry is rebuilt to match. +- **A whitespace inventory.** The canonical form is, by design, blind to + whitespace between element siblings. `gem/test/golden/tools/inline_adjacency.rb` + lists candidate spots; the decision it informs — an inventory, not a bound — + is in `design/v2/decisions.md`. ## Test steps @@ -682,7 +987,8 @@ bundle exec rake # unit tests + golden + standardrb ``` Both green, zero skips. To see the ruler work, change a class in any component -and re-run `rake golden`. +and re-run `rake golden`. To see the hardening, feed +`Golden::CanonicalHtml.call` a `
\n
` and a `
`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) MSG @@ -693,11 +999,12 @@ MSG ## Definition of done for Phase 1 -- `bundle exec rake` is green on `main` on Ruby 3.3 and 3.4. +- `bundle exec rake` is green on `main` on Ruby 3.3 and 3.4, and `mcp/data/registry.json` is current. - 188 snapshots exist; no scenario is `pending:`; the suite reports zero skips. -- Every one of the 54 component directories has at least one scenario; every `RubyUI::Base` subclass is reached; no orphan snapshot files. +- Every one of the 54 component directories has at least one scenario; every `RubyUI::Base` subclass **renders** in one (recorded at `before_template`, not at `initialize`); no orphan snapshot files. +- The canonical form refuses a fragment that escapes its `` in the input closes the wrapper early, and + # whatever follows lands beside it — outside what gets compared. + # Refuse rather than silently drop it. + unless wrapped.children.size == 1 + raise ArgumentError, + "fragment escaped the ?): #{html[0, 120].inspect}" + end + + wrapped.children.first.children end private @@ -109,7 +124,11 @@ def emit_element(node, depth, out, mode) children.each { |child| emit(child, depth, out, inner_mode) } out << close << "\n" elsif children.empty? - out << (INDENT * depth) << open << close << "\n" + # `
` and `
\n
` are not the same element to a + # browser: `:empty` matches only the first, and `textContent` is + # truthy only on the second. Keep a single space to tell them apart. + filler = whitespace_only_content?(node) ? " " : "" + out << (INDENT * depth) << open << filler << close << "\n" else out << (INDENT * depth) << open << "\n" children.each { |child| emit(child, depth + 1, out, :normal) } @@ -134,15 +153,19 @@ def child_mode(node, mode) :normal end - # An element whose only children are comments and formatting whitespace - # has to canonicalise the same way as an element with no children at all, - # otherwise `
` and `
\n
` — identical to a browser — - # would compare unequal. + # Comments and formatting whitespace are not children for layout + # purposes: `
\n \n
` and `
` build the + # same tree. Whether an element had *only* such children is a separate + # question, answered by whitespace_only_content? — see emit_element. def significant_children(node, mode) return node.children.to_a unless mode == :normal node.children.reject { |child| child.comment? || (child.text? && collapse(child.text).empty?) } end + def whitespace_only_content?(node) + node.children.any? { |child| child.text? && !child.text.empty? && collapse(child.text).empty? } + end + def open_tag(node) attributes = node.attribute_nodes.map { |attribute| canonical_attribute(attribute) }.sort_by(&:first) return "<#{node.name}>" if attributes.empty? @@ -160,7 +183,7 @@ def canonical_attribute(attribute) if BOOLEAN.include?(name) && (value.empty? || value.downcase == name) [name, nil] elsif TOKEN_LISTS.include?(name) - [name, value.split(/\s+/).reject(&:empty?).join(" ")] + [name, value.split(HTML_WHITESPACE).reject(&:empty?).join(" ")] else [name, value] end @@ -172,7 +195,7 @@ def attribute_name(attribute) end def collapse(text) - text.gsub(/\s+/, " ").strip + text.gsub(HTML_WHITESPACE, " ").delete_prefix(" ").delete_suffix(" ") end def escape_text(text) diff --git a/gem/test/golden/canonical_html_test.rb b/gem/test/golden/canonical_html_test.rb new file mode 100644 index 000000000..35625512f --- /dev/null +++ b/gem/test/golden/canonical_html_test.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require "test_helper" +require "golden/canonical_html" + +# Adversarial tests for the normalizer: inputs a browser distinguishes that the +# canonical form must not conflate, and input it must refuse rather than +# silently truncate. Each of these compared equal on the ported code. +class GoldenCanonicalHtmlTest < Minitest::Test + def canonical(html) + Golden::CanonicalHtml.call(html) + end + + def test_refuses_a_fragment_that_escapes_the_template_wrapper + error = assert_raises(ArgumentError) do + canonical("
safe
") + end + + assert_match(/escaped the