Conversation
Eight tasks with bite-sized steps: the inline Rails harness, the attribute layer with Phlex's guards, RubyUI::Component with a scoped sidecar lookup, the enum helper, the golden suite's ERB lane proved on Button's 15 fixtures, the strict lane with 36 recorded snapshots, the decision log, and the PR. Every mechanism was spiked before being planned: the scoped lookup ignores a host template at the same virtual path, phlex-rails renders a still-Phlex component from ERB with a captured block, Herb rejects a malformed sidecar, DetailsKey.clear picks up an edit, and the existing suite is unchanged under the new harness with RAILS_ENV=test. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Nine findings, all reproduced or read against the code before being accepted. Blocking: test_helper called component_roots= before the file that defines it was loaded; the attribute guards checked the raw value where Phlex checks the serialized one (href: 1 renders in Phlex, raised here) and lacked Phlex's rescue around an out-of-range character reference; snapshot recording depended on Minitest's test order; strict mode lost the newline the parser drops after <pre> and <textarea>; a scenario with neither block nor fixture defined no test; the hand-picked strict list missed Breadcrumb. Fix-level: four StandardRB offenses in supplied code, golden-only run counts that ignored the tests the PR review added, two dangling spec references. The strict lane now covers every scenario (maintainer decision): an audit found text inside inline elements in 38 of 54 components, so no list survives review. Recording happens once per scenario before either lane compares. Guards serialize first, as Phlex does, with the three shapes added to the differential suite. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…uler's PR The restoration of the newline after <pre>/<textarea> in every mode and raw-text precedence in child_mode closed three normal-mode fixed-point holes, so they now ship on feat/golden-suite (64cf273). Task 6 adds only strict mode; every run count shifts by the three tests that came with the fix. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ss tests fccef75 on feat/golden-suite turned two harness tests into three, so every run count shifts by one. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…les through Herb ReActionView's handler reads Rails.root, and a template outside it is recompiled with Erubi when Herb rejects it. The gem's tests now boot the smallest Rails::Application — no app directory, no routes — with the gem as its root, so every ERB template compiles exactly as it will in a host app, and a malformed one is refused at compile time. RAILS_ENV is pinned to test: in development, 1.6's Base emits a comment before every component and six unit tests fail on it. phlex-rails is a development dependency for the migration only, so the ERB fixtures can render components that are still Phlex. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…uards mix, merge_classes and flat come from the v2 gate, where they were checked against Phlex 2.4.1 for 15 hash shapes and 8 mix shapes; that differential test comes with them. Added on top: Phlex's attribute guards — an unsafe name raises, a javascript: URL in a URL attribute is dropped — so a component given untrusted values keeps the protection it has in 1.6. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…orm, as Phlex does
flat silently omitted href: [] and src: {} instead of raising; Phlex
2.4.1 raises for both. Added refuse_url_value! so a URL attribute
without a String form fails loud, matching the Interfaces contract.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A plain Ruby class ActionView renders through render_in. Its sidecar is found under the RubyUI.component_roots entry that holds the class file, through a LookupContext of its own — the application's view paths are never consulted, so a host template at the same virtual path cannot shadow the sidecar and the sidecar cannot shadow the host. content is set on every render, nil without a block. Named Component while the Phlex RubyUI::Base still exists; it takes the name Base when the last Phlex component is gone. Probe components under test/probes exercise the layer without touching a shipped component. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ibutes size: "lg" from a tag or params and size: :lg from Ruby both select SIZES[:lg]; nil takes the default; anything else raises naming the allowed values, instead of indexing a Symbol-keyed hash with a String and silently dropping the class, as DialogContent and Badge do in 1.6. Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
A scenario may now have an ERB fixture under test/golden/views next to its Phlex block; the suite renders the fixture through the harness and compares it against the same frozen snapshot, as its own test. With phlex-rails loaded a fixture can render a component that is still Phlex, so every fixture can be written and made green before a single component migrates — after which a migration changes only the implementation, never the ruler. Button's 15 fixtures are the proof: byte-identical in canonical form to the snapshots the Phlex lane recorded. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The canonical form is blind to whitespace between siblings and at text boundaries by design. Every scenario now also keeps the preserve-mode form: text verbatim, attributes sorted, the fragment's own edges trimmed. 188 strict snapshots recorded from the Phlex lane while Phlex still renders; a fixture or a migrated component that adds a newline where Phlex emitted none fails here even though the canonical form cannot see it. The fixed-point fix already on the branch (64cf273) is what makes the strict form a fixed point too. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The spec deferred five mechanisms to Phase 2.0. Decisions 5-9 record them: the inline Rails application the tests boot, Component as the layer's name until the last Phlex component is gone, every ERB fixture written before any migration, the strict lane as the canonical form in preserve mode, and a list of component roots. The spec's §4.4 and §6 Phase 2.0 now say what was built. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… the reload registration, name a bad enum default
Attributes.flat diverged from Phlex 2.4.1 on four shapes, all re-measured
against Phlex at runtime before fixing: `class: [true]` and `style:
{display: true}` silently serialized instead of raising (Phlex has no
`case` branch for a boolean token or style value); `data: {at:
Date.today}` silently nested an ISO date instead of raising (Phlex's
nested-attributes case has no branch for Date/Time, only its top-level
case does); and a String `"style"` key was read as CSS instead of
nesting, because `flat` matched on the dasherized name instead of on
the literal Symbol `:style` Phlex keys on. Each is now pinned in
attributes_differential_test.rb, executed against Phlex at runtime: a
FLAT_CASES entry for the String-key shape, a small RAISE_CASES table
for the three that raise.
RubyUI.lookup_for registers its resolver through
ActionView::PathRegistry.cast_file_system_resolvers so the reloader's
DetailsKey.clear and CacheExpiry can see component roots — nothing
tested that. The added test calls RubyUI.lookup_for on every configured
root directly rather than relying on some other test to have exercised
it first: Phase 2.0a has no real gem component under the `lib` root
yet (every component that exists is a probe under test/probes), so
nothing else in the suite ever resolves a sidecar there.
enum's nil path blamed the caller ("nil is not one of ...") when the
class's own `default:` wasn't a table key; it now names the default. A
value the caller actually gave still fails on its own terms.
RAILS_ENV was `||=`; a gem test suite has no legitimate environment but
"test", and an exported RAILS_ENV=development would turn on 1.6's dev
comment and fail six unit tests.
bundle exec rake: 596 runs, 0 failures, 0 skips; 423 files inspected,
no offenses.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…r; decision 5's premise corrected
§4.3 and §5's decision 9 row promised "two sidecars for one class is an
error" and "collision is an error" — the implementation never raises
on a name collision; it simply never consults the host's view paths,
so nothing is there to collide with. Reworded both, the §4.3 `base.rb`
table row (`template_path` doesn't exist; `template` resolved from
`source_file` under `component_root` does), and the §10 risk-table
cell to match. §6 said three CI jobs; `.github/workflows/ci.yml` has
five (gem, docs, mcp, mcp-registry-check, docker-build). Recorded two
carry-forward items the review raised for later phases: subclass
sidecar inheritance (2.1) and component_root/attributes.rb path and
guard questions (2.4).
decisions.md entry 5's stated reason — "a template outside Rails.root
falls back to Erubi" — doesn't hold: without an application
`Rails.root` is nil, `nil.to_s` is `""`, and every identifier
`start_with?("")`, so ReActionView's local_template? is true for
everything, application or not (reactionview/template/local_template.rb).
The reasons that do hold, verified against reactionview/railtie.rb and
railties' Rails.env: the Railtie registers the ERB handler only from
an application-boot initializer, so without one Erubi never yields to
Herb at all; and Rails.env defaults to development unless RAILS_ENV is
set, turning on 1.6's dev comment. Entry 9 gets the paragraph Important
2 earned: the scoped lookup's resolver registration through
ActionView::PathRegistry.cast_file_system_resolvers, why it matters for
the reloader, and the Rails version floor it implies.
Plan's Definition of done recounted after the A1/A2 test additions:
the differential test is 30 cases (19 FLAT_CASES + 8 MIX_CASES + 3
RAISE_CASES) and the guard suite is 11 (`def test_` in
attributes_test.rb), both counted directly rather than assumed.
bundle exec rake: 596 runs, 0 failures, 0 skips; 423 files inspected,
no offenses (docs-only change, numbers unchanged from the prior
commit).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Decision 5 retracted the "template outside Rails.root falls back to Erubi" premise; the comment above TestApp and the spec's §6 harness bullet still stated it, citing the decision by name. Both now give the reason that holds: ReActionView registers its ERB handler only from an application-boot initializer. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
cirdes
added this pull request to stack #549
September 20, 2026 17:47
There was a problem hiding this comment.
All reported issues were addressed across 241 files
Note: This PR contains a large number of files. cubic selects up to 200 of the highest-priority eligible files for this review, so some files may not have been reviewed.
Re-trigger cubic
…s value, as Phlex does
The name guard ran inside emit, which only the scalar and token paths
reached: a Hash under an unsafe name slipped through nested via the `_`
root key, so `onclick: {_: "x"}` and `srcdoc: {_: "x"}` were emitted where
Phlex raises. Phlex checks the name before it looks at the value; now so
does flat. A nested name keeps the character check only, as in Phlex
(`foo-onclick` is allowed on both sides) — pinned by a differential case.
Two raise pairs and a guard test cover the hole (review of #548, P0).
Also `require "date"`: `scalar` names Date, and nothing guarantees a host
loaded it before this file. (`Set` autoloads on Ruby 3.2+, the gem's floor.)
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… the edges Ruby's strip also takes U+000B and NUL, which HTML_WHITESPACE deliberately excludes — a text-bearing component whose text started or ended with one of them would have lost it silently in the strict form. Trim the HTML set only (review of #548). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… hook, §9.4, follow-ups 8–11 - ErbHarnessTest keeps its `ActionView::Base` reference and says why: the ERB handler is registered from `config.after_initialize` inside the `:action_view` load hook, so it needs the constant loaded — without the line the test fails under seeds that run it first (37227). Decision 5 and the TestApp comment name the hook. - Golden::Catalog.component_classes' docstring names both base classes. - Spec §9.4: the strict lane covers every scenario, so the accepted gap is what no HTML comparison sees, not uncovered components. - Plan definition of done: post-review counts, with the as-planned ones. - follow-up-issues.md 8–11: Switch a11y, ComboboxInputTrigger roles, ClipboardPopover initial state, Link type="button" — 1.6 defects the strict snapshots made visible; fixed in their own PRs on the 2.0 line. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
cirdes
added a commit
that referenced
this pull request
Sep 20, 2026
…lete Stimulus descriptor `ctrl+p` alone makes Stimulus listen for an event literally named "ctrl+p", which never fires; the component interpolates the string verbatim and the docs pass `keydown.ctrl+j@window`. The scenario now passes `keydown.ctrl+p@window`, so the snapshot pins a keybinding that works. Raised by the review of #548. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…s scenario fix Re-recorded after merging ca928cb from feat/golden-suite; the canonical and strict forms of command/trigger_with_custom_keybindings now both pin `keydown.ctrl+p@window`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Stacked on #536. Puts the 2.0 foundation into the gem with every component
still Phlex:
RubyUI::ComponentandRubyUI::Attributes— the 2.0 layer from the v2gate, promoted: plain Ruby, ERB sidecar found under
RubyUI.component_rootsthrough a lookup scoped to the root that holds the class file (a host
template at the same virtual path is not picked — tested),
contentreseton every render, Phlex 2.4.1's attribute guards ported (an unsafe name
raises, a
javascript:URL is dropped — tested, and a runtime differentialtest pins the serialization against Phlex itself), and an
enumhelper thatcoerces
"lg"/:lgand names the allowed values on anything else.Rails::ApplicationsoReActionView compiles every template through Herb exactly as a host app
will; a malformed template is refused at compile time.
rendered and compared against the same frozen snapshot. With
phlex-railsloaded a fixture renders a still-Phlex component, so all 188 fixtures can be
green before any migration. Button's 15 are here as the proof.
per scenario (188), recorded from Phlex.
No component migrates. Nothing under
gem/lib/ruby_ui/<component>/changes;the 188 golden snapshots are byte-identical to #536. No runtime dependency is
added — every new gem is development-only, and
phlex-railsleaves with thelast Phlex component.
Why
Phase 2.0 of
design/2026-09-19-rubyui-2-0-design.md. The five mechanisms thespec deferred to this phase are decided in
design/v2/decisions.mdentries 5–9.Test steps
To see the ERB lane work, edit
test/golden/views/button/size_md.html.erbtorender
size: :lgand re-runrake golden.🤖 Generated with Claude Code
Summary by cubic
Puts the RubyUI 2.0 foundation into the gem —
RubyUI::ComponentandRubyUI::Attributes— while every shipped component still renders through Phlex, so the 188 golden snapshots stay byte-identical to the previous phase. No runtime dependency is added; every new gem is development-only.Component layer
RubyUI::Componentis a plain Ruby object ActionView renders viarender_in, looking its ERB sidecar up only under theRubyUI.component_rootsentry that holds the class file; host view paths are never consulted.RubyUI::Attributesports Phlex 2.4.1's serialization and guards — unsafe names raise before their value is read, so a nested hash underonclickorsrcdocraises too, andjavascript:URLs are dropped — pinned by a runtime differential test against Phlex.enumhelper coerces"lg"/:lgto the same table key and raises naming the allowed values otherwise.Harness and golden suite
Rails::ApplicationwithRAILS_ENVpinned totest, so ReActionView compiles every ERB template through Herb as a host app will and refuses malformed templates at compile time.test/golden/views, rendered through the same harness and compared against the same snapshot; withphlex-railsloaded a fixture renders a still-Phlex component, so all 188 fixtures can be green before any migration.keydown.ctrl+p@window.Written for commit f855423. Summary will update on new commits.