Detect unnoticed changes across generated webpack/Rspack configs - #4839
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (83)
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
WalkthroughThe PR adds generated-tree approval utilities and expands generator golden-output coverage from one file to complete configuration trees across Base, Pro, RSC, and Tailwind variants. It also removes an extra space from documentation references. ChangesDocumentation reference formatting
Generated configuration tree coverage
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant GeneratorGoldenOutput
participant BaseProRscGenerator
participant GeneratedTreeApproval
participant ApprovedConfigTree
GeneratorGoldenOutput->>BaseProRscGenerator: Generate and transform configuration
BaseProRscGenerator-->>GeneratorGoldenOutput: Return complete configuration tree
GeneratorGoldenOutput->>GeneratedTreeApproval: Compare actual and approved trees
GeneratedTreeApproval->>ApprovedConfigTree: Read approved files
GeneratedTreeApproval-->>GeneratorGoldenOutput: Return tree differences
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR expands generator golden-output coverage from individual server configuration files to complete Webpack and Rspack configuration trees across eight representative variants.
Confidence Score: 5/5The PR appears safe to merge; no actionable correctness, security, or test-coverage defect remains in the changed paths. The approval helper compares complete generated file sets and byte contents deterministically, while the expanded spec follows the production base-to-Pro-to-RSC transformation order for each represented configuration variant. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart LR
V["Variant matrix"] --> B["BaseGenerator.copy_webpack_config"]
B --> P{"Pro or RSC?"}
P -->|Yes| PT["Apply Pro config transforms"]
P -->|No| R{"RSC?"}
PT --> R
R -->|Yes| RT["Create and transform RSC configs"]
R -->|No| C["Snapshot generated config tree"]
RT --> C
C --> A["Compare with checked-in approved tree"]
A --> D["Report added, removed, and changed files"]
Reviews (1): Last reviewed commit: "Approve complete generated config trees" | Re-trigger Greptile |
![github-code-quality[bot]](https://avatars.github.com/u/9919?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
react_on_rails/spec/react_on_rails/generators/generator_golden_output_spec.rb (1)
405-418: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a named RSpec subject for the tree comparison.
The per-variant
itblocks (lines 405-418) callGeneratedTreeApproval.compareand assert on the result inline, without a named subject. Define the comparison as a named subject instead.♻️ Proposed refactor using a named subject
GeneratorGoldenOutput::VARIANTS.each do |variant| it "matches the approved tree for the #{variant[:name]} variant" do - GeneratorGoldenOutput.generate(variant) do |actual_root| - comparison = GeneratedTreeApproval.compare( - actual_root, - GeneratorGoldenOutput.approved_root(variant) - ) - - expect(comparison.match?).to be(true), - -> { GeneratorGoldenOutput.mismatch_message(variant, comparison) } - end + GeneratorGoldenOutput.generate(variant) do |actual_root| + comparison_result = GeneratedTreeApproval.compare( + actual_root, + GeneratorGoldenOutput.approved_root(variant) + ) + + expect(comparison_result.match?).to be(true), + -> { GeneratorGoldenOutput.mismatch_message(variant, comparison_result) } + end end endBased on coding guidelines "use named RSpec subjects such as
subject(:method_result)".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@react_on_rails/spec/react_on_rails/generators/generator_golden_output_spec.rb` around lines 405 - 418, In the generated configuration trees spec, define a named RSpec subject for the result of GeneratedTreeApproval.compare using the current actual and approved roots, then update the expectation to assert against that subject while preserving the existing mismatch message and per-variant behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@react_on_rails/spec/react_on_rails/generators/generator_golden_output_spec.rb`:
- Around line 405-418: In the generated configuration trees spec, define a named
RSpec subject for the result of GeneratedTreeApproval.compare using the current
actual and approved roots, then update the expectation to assert against that
subject while preserving the existing mismatch message and per-variant behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: de2bccfa-0065-48c1-ba7b-1b7671d5e216
⛔ Files ignored due to path filters (67)
react_on_rails/spec/react_on_rails/fixtures/generated/README.mdis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/rspack_base/config/rspack/ServerClientOrBoth.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/rspack_base/config/rspack/clientWebpackConfig.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/rspack_base/config/rspack/commonWebpackConfig.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/rspack_base/config/rspack/development.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/rspack_base/config/rspack/production.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/rspack_base/config/rspack/rspack.config.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/rspack_base/config/rspack/serverWebpackConfig.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/rspack_base/config/rspack/test.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/rspack_pro/config/rspack/ServerClientOrBoth.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/rspack_pro/config/rspack/clientWebpackConfig.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/rspack_pro/config/rspack/commonWebpackConfig.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/rspack_pro/config/rspack/development.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/rspack_pro/config/rspack/production.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/rspack_pro/config/rspack/rspack.config.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/rspack_pro/config/rspack/serverWebpackConfig.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/rspack_pro/config/rspack/test.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/rspack_rsc/config/rspack/ServerClientOrBoth.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/rspack_rsc/config/rspack/clientWebpackConfig.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/rspack_rsc/config/rspack/commonWebpackConfig.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/rspack_rsc/config/rspack/development.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/rspack_rsc/config/rspack/production.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/rspack_rsc/config/rspack/rscWebpackConfig.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/rspack_rsc/config/rspack/rspack.config.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/rspack_rsc/config/rspack/serverWebpackConfig.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/rspack_rsc/config/rspack/test.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_base/config/webpack/ServerClientOrBoth.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_base/config/webpack/clientWebpackConfig.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_base/config/webpack/commonWebpackConfig.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_base/config/webpack/development.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_base/config/webpack/production.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_base/config/webpack/serverWebpackConfig.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_base/config/webpack/test.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_base/config/webpack/webpack.config.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_base_shakapacker8/config/webpack/ServerClientOrBoth.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_base_shakapacker8/config/webpack/clientWebpackConfig.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_base_shakapacker8/config/webpack/commonWebpackConfig.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_base_shakapacker8/config/webpack/development.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_base_shakapacker8/config/webpack/production.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_base_shakapacker8/config/webpack/serverWebpackConfig.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_base_shakapacker8/config/webpack/test.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_base_shakapacker8/config/webpack/webpack.config.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_pro/config/webpack/ServerClientOrBoth.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_pro/config/webpack/clientWebpackConfig.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_pro/config/webpack/commonWebpackConfig.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_pro/config/webpack/development.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_pro/config/webpack/production.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_pro/config/webpack/serverWebpackConfig.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_pro/config/webpack/test.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_pro/config/webpack/webpack.config.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_rsc/config/webpack/ServerClientOrBoth.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_rsc/config/webpack/clientWebpackConfig.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_rsc/config/webpack/commonWebpackConfig.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_rsc/config/webpack/development.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_rsc/config/webpack/production.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_rsc/config/webpack/rscWebpackConfig.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_rsc/config/webpack/serverWebpackConfig.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_rsc/config/webpack/test.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_rsc/config/webpack/webpack.config.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_tailwind/config/webpack/ServerClientOrBoth.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_tailwind/config/webpack/clientWebpackConfig.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_tailwind/config/webpack/commonWebpackConfig.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_tailwind/config/webpack/development.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_tailwind/config/webpack/production.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_tailwind/config/webpack/serverWebpackConfig.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_tailwind/config/webpack/test.jsis excluded by!**/generated/**react_on_rails/spec/react_on_rails/fixtures/generated/webpack_tailwind/config/webpack/webpack.config.jsis excluded by!**/generated/**
📒 Files selected for processing (3)
react_on_rails/lib/generators/react_on_rails/generator_helper.rbreact_on_rails/spec/react_on_rails/generators/generator_golden_output_spec.rbreact_on_rails/spec/react_on_rails/support/generated_tree_approval.rb
![claude[bot]](https://avatars.github.com/in/1236702?s=60&v=4){kind=small}
ihabadham
changed the title
Contributor
Review summaryThis PR adds a generic What I checked
Minor (non-blocking)
Not verified
Overall this is a well-scoped, well-documented test-infrastructure change with no functional/runtime risk (the one production code change is a whitespace fix). No blocking issues found. |
Collaborator
Author
|
+ci-status |
Collaborator
Author
|
+ci-run-hosted |
Contributor
CI StatusHead SHA: Only the required gate is active unless hosted CI is requested. |
Contributor
Hosted CI RequestedTriggered 9 workflow(s) for View progress in the Actions tab. |
github-actions
Bot
added
the
ready-for-hosted-ci
Collaborator
Author
|
No description provided. |
Collaborator
Author
|
Review follow-up: I’m skipping the suggestion to use a named RSpec subject. |
Collaborator
Author
|
No description provided. |
Contributor
Review: Detect unnoticed changes across generated webpack/Rspack configsReviewed the PR diff ( OverviewAdds a Strengths
Minor / non-blocking
VerdictNo correctness, security, or performance issues found in the test infrastructure itself. The design (generic tree-diff helper + variant matrix run through the real generator lifecycle) is a solid improvement over the single-file golden test, and the reasoning in the PR description for scope decisions (non-goals, variant selection) is sound and matches what's implemented. |
Collaborator
Author
|
No description provided. |
ihabadham
force-pushed
the
ihabadham/test/approve-generated-config-tree
branch
from
August 4, 2026 18:42
b1c6116 to
b4eb3bb
Compare
Collaborator
Author
|
+ci-status |
Collaborator
Author
|
+ci-run-hosted |
Contributor
CI StatusHead SHA: Optimized hosted CI is enabled for this PR. |
Contributor
Hosted CI RequestedTriggered 9 workflow(s) for View progress in the Actions tab. |
Contributor
Review: Detect unnoticed changes across generated webpack/Rspack configsOverviewThis PR generalizes the existing single-file golden-output spec ( Code quality
Potential concerns (low severity / worth a second look, not blocking)
Security / performance
Test coverage
Overall this is a solid, well-scoped test-infrastructure PR with good internal consistency between its stated claims and the actual diff. |
Collaborator
Author
|
Final refresh against current
No local test suite was run, following the global React on Rails PR rule; test evidence is from hosted CI. |
justin808
added a commit
that referenced
this pull request
Aug 6, 2026
…out-vm-pool * origin/main: Docs: move agent coordination to the HTTP backend (#4764) Detect unnoticed changes across generated webpack/Rspack configs (#4839) Fix durable ShakaPerf release evidence reuse (#4833) Docs: add missing content — release notes, upgrade guide, config, API references (#4843) (#4844) Fix incorrect docs: helper names, defaults, requirements, runtime refs (#4836) Scroll-priority streaming: candidate architecture evaluation (#4835) (#4841) Fix generated server config lint cleanup (#4840) Document serialized release backport policy (#4592) Package version-matched agent skills and docs (#4809) Fix ci-required base ref for PR merge-ref checkouts (#4819) Honor response charset and reject non-2xx HTTP-served SSR bundles (#4817) [Pro] Redact RSC render-error metadata on the fetched (client-navigation) payload path (#4821) Forward-port the 17.0.1 changelog section to main (#4814) # Conflicts: # CHANGELOG.md
justin808
added a commit
that referenced
this pull request
Aug 8, 2026
…ential-broker * origin/main: (75 commits) Silence routine startup diagnostics for Rails commands (#4849) Docs: move agent coordination to the HTTP backend (#4764) Detect unnoticed changes across generated webpack/Rspack configs (#4839) Fix durable ShakaPerf release evidence reuse (#4833) Docs: add missing content — release notes, upgrade guide, config, API references (#4843) (#4844) Fix incorrect docs: helper names, defaults, requirements, runtime refs (#4836) Scroll-priority streaming: candidate architecture evaluation (#4835) (#4841) Fix generated server config lint cleanup (#4840) Document serialized release backport policy (#4592) Package version-matched agent skills and docs (#4809) Fix ci-required base ref for PR merge-ref checkouts (#4819) Honor response charset and reject non-2xx HTTP-served SSR bundles (#4817) [Pro] Redact RSC render-error metadata on the fetched (client-navigation) payload path (#4821) Forward-port the 17.0.1 changelog section to main (#4814) Handle selector metacharacters in renderComponent DOM IDs (#4808) [Pro] Prevent caching RSC renders with errors (#4804) Agents: trust Copilot review identities (#4807) Agents: bind fleet closeout to generated pack (#4805) Docs: ADR 0002 — Skills-in-package over MCP for agent-native DX (#4735) Scope GitHub release commands to the origin repository (#4803) ...
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
1 participant
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.
Fixes #4838
Why
#4790 made unexpected changes to generated
serverWebpackConfig.jsvisible, but a React on Rails installation produces a coordinated configuration tree. The remaining generated files had focused behavior and transform coverage without one review point for additions, removals, renames, or textual drift across the final installation output.That left coverage inconsistent within the same generated entity. It also meant Pro/RSC post-render transformations could change client, server/client orchestration, or RSC config output without appearing in the existing server-template approval.
This PR keeps the existing semantic, structural-anchor, and historical/customized transform specs. It adds exact-output coverage as another layer rather than replacing them.
Approach selected
After assessing the alternatives in #4838, this PR approves each meaningful generated configuration tree rather than maintaining a second filename registry or adding unrelated assertions file by file.
A reusable
GeneratedTreeApprovalhelper receives an actual root and an approved root. It discovers files recursively and reports:The helper has no knowledge of React on Rails filenames or generator variants. The generator spec owns only the option matrix and the lifecycle required to produce each tree.
Generated lifecycle covered
For each variant, the spec:
BaseGenerator#copy_webpack_configpath into a temporary destination;config/subtree with its checked-in approval.This exposed that the previous RSC server golden represented intermediate base-template output. The new RSC trees contain the final client-reference discovery transformation a real RSC installation receives.
Variant scope
The matrix covers ten meaningful branches without taking the Cartesian product:
webpack_basewebpack_prowebpack_rscwebpack_tailwindwebpack_typescriptrspack_baserspack_prorspack_rscrspack_typescriptwebpack_base_shakapacker8Tailwind and Shakapacker-version branches are each represented once because they are independent of the product/bundler combinations they do not alter. TypeScript is represented once per bundler because it changes the bundler-specific main entry.
Generated-output cleanup
Expanding byte-exact coverage exposed an existing trailing space emitted by
add_documentation_reference. The helper now emits the same comment without trailing whitespace, and every approval is regenerated from that production output rather than edited manually.Relationship to existing tests
This PR deliberately retains:
The tree approvals answer a separate question: did any final generated configuration output change? Reviewers must then decide whether that change is intentional.
Non-goals
mergeimport. Its intended behavior remains a separate semantic decision; the approvals faithfully record the current output.Validation
247 files inspected, no offenses detected.webpack.config.ts/rspack.config.ts.git diff --checkpassed.Changelog
No changelog entry: this adds test infrastructure and removes trailing whitespace from a generated documentation comment without changing runtime behavior.
Churn note
Most of the diff is checked-in generator output: 82 JavaScript/TypeScript approvals across ten variants. The executable test infrastructure is the generic 70-line tree helper plus the existing golden spec's lifecycle expansion.
Related
Summary by CodeRabbit
Bug Fixes
Tests