Add Shakapacker 9.0+ private_output_path integration for server bundles #2028
Conversation
|
Warning Rate limit exceeded@justin808 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 1 minutes and 53 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (12)
WalkthroughImplements automatic detection and integration of Shakapacker 9.0's private_output_path for server bundle configuration. Adds version-aware template generation, configuration auto-detection logic, diagnostic tooling enhancements, and path normalization utilities to align React on Rails with Shakapacker conventions. Changes
Sequence DiagramsequenceDiagram
participant Gen as Generator
participant Conf as Configuration
participant Utils as Utils
participant PackerUtils as PackerUtils
participant Doctor as Doctor
Gen->>Gen: generate config (template-driven)
Gen->>+PackerUtils: shakapacker_version_9_or_higher?
PackerUtils-->>-Gen: version ≥ 9?
rect rgb(200, 220, 240)
Note over Conf,Utils: Configuration Setup
Conf->>+PackerUtils: shakapacker_version_requirement_met?("9.0.0")
PackerUtils-->>-Conf: meets requirement?
Conf->>Conf: auto_detect_server_bundle_path_from_shakapacker
Conf->>Utils: normalize_to_relative_path(detected_path)
Utils-->>Conf: relative_path
Conf->>Conf: update server_bundle_output_path
end
rect rgb(240, 220, 200)
Note over Doctor: Doctor Diagnostics
Doctor->>+PackerUtils: shakapacker_version_requirement_met?("9.0.0")
PackerUtils-->>-Doctor: version info
Doctor->>Doctor: check_shakapacker_private_output_path
Doctor->>Doctor: compare paths & emit recommendations
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
Code Review - PR #2028: Shakapacker 9.0+ private_output_path IntegrationSummaryThis PR adds automatic detection and integration of Shakapacker 9.0+'s ✅ Strengths1. Excellent Backward Compatibility
2. Comprehensive Test Coverage
3. Strong Documentation
4. Developer Experience
🔍 Potential Issues & Suggestions1. Critical: Auto-Detection Timing Risk
|
Code Review - PR #2028: Shakapacker 9.0+ private_output_path IntegrationOverall AssessmentStatus: ✅ Excellent Quality - This is a well-designed, thoroughly tested feature that follows best practices throughout. 🎯 Strengths1. Excellent Backward Compatibility DesignThe auto-detection logic in
2. Comprehensive Test CoverageThe test suite is thorough:
3. Version-Aware Generator TemplatesThe generator helper (
4. Excellent Documentation
🔍 Code Quality ObservationsSecurity✅ Good: The PR includes Performance✅ No concerns:
Error Handling✅ Robust:
💡 Minor Suggestions1. Path Normalization Edge CaseIn # Current code
path_str.sub(%r{^#{Regexp.escape(rails_root_str)}/?}, "")
# Edge case: Rails.root = "/", path = "/app"
# Would incorrectly match and return "app" instead of "/app"Recommendation: Add a boundary check or document this assumption. Though extremely rare, you could add: return path_str if rails_root_str == "/"2. Generator Default AssumptionIn return true unless defined?(ReactOnRails::PackerUtils)This is pragmatic, but could generate incorrect config if someone installs an older Shakapacker version. Consider whether the comment should emphasize this assumption more strongly. Current risk: Low - the code works correctly, but users might not realize they got v9+ config with older Shakapacker. 3. Doctor Method ComplexityThe
Not blocking: The current implementation is readable and well-tested. 🔒 Security Review✅ No security concerns identified
🐛 Potential IssuesNone IdentifiedThe code is solid. The only theoretical concern is: Edge Case: If a user has Shakapacker 9.0+ but ✅ Best Practices CompliancePer the project's
📋 Pre-Merge ChecklistBefore merging, verify:
🎉 ConclusionRecommendation: APPROVE ✅ This is exemplary code that:
The implementation demonstrates careful consideration of edge cases and provides a smooth migration path for users. Great work! 📝 Nit: Documentation TypoIn Note: This review was conducted according to the repository's CLAUDE.md guidelines, focusing on code quality, security, performance, and best practices. |
Code Review - PR #2028OverviewThis PR adds excellent Shakapacker 9.0+ integration for automatic server bundle path detection. The implementation is well-thought-out with strong backward compatibility and comprehensive testing. ✅ Strengths1. Excellent Test Coverage
2. Strong Backward Compatibility
3. Security Best Practices
4. Developer Experience
5. Code Quality
🔍 Potential Issues & Suggestions1. Missing Test Coverage for Auto-Detection Logic
|
CI Failure - Not Related to This PRThe failing CI job is due to a pre-existing bug in master's CI configuration ( lines 73-75). The issue: Matrix entries with conditional expressions that evaluate to are passed as literal values, causing "Unknown engine false" error. This is not caused by the Shakapacker integration changes in this PR. The actual Shakapacker integration code:
I'll monitor master for a fix or add a workaround commit if needed. |
Code Review - PR #2028: Shakapacker 9.0+ private_output_path Integration🎯 Overall AssessmentThis is an excellent PR that delivers a significant DX improvement by eliminating configuration duplication between Shakapacker and React on Rails. The implementation is well-designed, thoroughly tested, and backward compatible. ✅ Strengths1. Excellent Architecture & Design
2. Outstanding Test Coverage
3. Code Quality
4. Documentation
5. Developer Experience
💡 Suggestions for Consideration1. Path Normalization Edge Case (Minor)Location: The path normalization logic should handle these edge cases:
Example test case to add: # In spec/react_on_rails/utils_spec.rb
it "handles multiple consecutive slashes" do
expect(normalize_to_relative_path("//ssr-generated//")).to eq("ssr-generated")
end2. Configuration Priority Documentation (Minor)Location: The template comments are excellent, but could benefit from explicitly stating the priority order: # Configuration Priority (highest to lowest):
# 1. Explicit config.server_bundle_output_path setting
# 2. Auto-detected from shakapacker.yml private_output_path (Shakapacker 9.0+)
# 3. Default: "ssr-generated"This makes the behavior crystal clear for users. 3. Doctor Check Performance (Very Minor)Location: The
4. Generator Helper Error Context (Minor)Location: The rescue StandardError
# If we can't determine version, assume latest
true
endConsider logging the error in development mode to help debug version detection failures: rescue StandardError => e
Rails.logger&.debug("Could not determine Shakapacker version: #{e.message}") if defined?(Rails)
true
end5. Test Coverage Gap (Minor)The tests for
Based on the diff, these might already be covered in the path normalization tests. 🔒 SecurityNo security concerns. The feature actually improves security by:
⚡ PerformanceNo performance concerns:
🧪 Testing RecommendationsBefore merge, verify:
Based on the commit messages, it looks like these have been tested extensively. 📝 Minor Nitpicks
🎉 ConclusionThis PR is production-ready and represents a thoughtful, well-executed enhancement to React on Rails. The implementation demonstrates:
Recommendation: ✅ APPROVE with minor suggestions above for future consideration. The Shakapacker 9.0+ integration will significantly improve the developer experience by eliminating a common source of configuration errors. Great work! This is exactly the kind of thoughtful, incremental improvement that makes frameworks better over time. 🤖 Generated with Claude Code |
There was a problem hiding this comment.
Pull Request Overview
This PR integrates Shakapacker 9.0+ private_output_path configuration with React on Rails for server bundles, providing automatic synchronization between webpack and Rails configurations. The implementation gracefully handles version detection and provides clear diagnostic feedback through the doctor command.
Key Changes:
- Automatic detection of
private_output_pathfromshakapacker.ymlwhen using Shakapacker 9.0+ - Version-aware generator templates that configure appropriately based on Shakapacker version
- Enhanced doctor diagnostics with recommendations for optimal configuration
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
spec/react_on_rails/utils_spec.rb |
Comprehensive tests for path normalization covering edge cases including Rails.root handling, special characters, and nil values |
spec/lib/react_on_rails/doctor_spec.rb |
Tests for Shakapacker integration diagnostics across version scenarios including success, mismatch, and error cases |
lib/react_on_rails/utils.rb |
Adds normalize_to_relative_path utility to convert absolute paths to Rails.root-relative paths with proper documentation |
lib/react_on_rails/doctor.rb |
Implements check_shakapacker_private_output_path to validate configuration and provide version-specific recommendations |
lib/react_on_rails/configuration.rb |
Adds auto-detection logic for server_bundle_output_path from Shakapacker's private_output_path with graceful error handling |
lib/generators/react_on_rails/generator_helper.rb |
Adds version detection helper for Shakapacker 9.0+ with proper fallback logic |
lib/generators/react_on_rails/base_generator.rb |
Updates config generation to use templates for version-aware configuration |
lib/generators/react_on_rails/templates/base/base/config/webpack/serverWebpackConfig.js.tt |
Template uses config.privateOutputPath for Shakapacker 9.0+, hardcoded path for older versions |
lib/generators/react_on_rails/templates/base/base/config/shakapacker.yml.tt |
Conditionally enables private_output_path configuration based on Shakapacker version |
lib/generators/react_on_rails/templates/base/base/config/initializers/react_on_rails.rb.tt |
Adds comprehensive documentation about Shakapacker 9.0+ integration with clear migration guidance |
docs/core-concepts/webpack-configuration.md |
Documents the Shakapacker 9.0+ integration with configuration examples and benefits |
docs/api-reference/configuration.md |
Extensive documentation updates covering server bundle security, organization, and Shakapacker 9.0+ integration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
justin808
force-pushed
the
feature/server-bundle-path-validation-rebased
branch
from
34e4785 to
acdc849
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
lib/generators/react_on_rails/generator_helper.rb (1)
99-114: Consider the implications of defaulting totrueon version check failures.The method defaults to
truein two scenarios:
- When Shakapacker is not yet installed (line 107)
- When version detection fails (line 112)
While optimistic defaults work for fresh installs, silently defaulting to
trueon errors could generate incorrect configuration if version detection genuinely fails in an existing project.Consider logging a warning when the rescue block is triggered:
def shakapacker_version_9_or_higher? return @shakapacker_version_9_or_higher if defined?(@shakapacker_version_9_or_higher) @shakapacker_version_9_or_higher = begin # If Shakapacker is not available yet (fresh install), default to true # since we're likely installing the latest version return true unless defined?(ReactOnRails::PackerUtils) ReactOnRails::PackerUtils.shakapacker_version_requirement_met?("9.0.0") rescue StandardError - # If we can't determine version, assume latest + puts "Warning: Could not determine Shakapacker version, assuming 9.0+" true end endlib/react_on_rails/configuration.rb (1)
261-290: Consider validating empty string from private_output_path.The auto-detection logic checks for
nilat line 276 but doesn't handle the case whereprivate_output_pathreturns an empty string. If Shakapacker's config returns"", the code would proceed to normalize and set an empty path.Apply this diff to handle empty strings:
begin private_path = ::Shakapacker.config.private_output_path - return unless private_path + return if private_path.nil? || private_path.to_s.empty? # Convert from Pathname to relative string pathlib/react_on_rails/doctor.rb (1)
1408-1470: Consider normalizing both paths before comparison.At line 1439, the method compares
relative_path(normalized from Shakapacker) withrails_bundle_path(extracted directly from the config file at line 684). Ifrails_bundle_pathcontains an absolute path or inconsistent formatting, the comparison might fail even when the paths logically match.Apply this diff to normalize both paths for comparison:
# Shakapacker 9.0+ is available begin private_path = ::Shakapacker.config.private_output_path if private_path relative_path = ReactOnRails::Utils.normalize_to_relative_path(private_path) + normalized_rails_path = ReactOnRails::Utils.normalize_to_relative_path(rails_bundle_path) - if relative_path == rails_bundle_path + if relative_path == normalized_rails_path checker.add_success("\n ✅ Using Shakapacker 9.0+ private_output_path: '#{relative_path}'") checker.add_info(" Auto-detected from shakapacker.yml - no manual config needed") else checker.add_warning(<<~MSG.strip) \n ⚠️ Configuration mismatch detected! Shakapacker private_output_path: '#{relative_path}' - React on Rails server_bundle_output_path: '#{rails_bundle_path}' + React on Rails server_bundle_output_path: '#{normalized_rails_path}'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
docs/api-reference/configuration.md(1 hunks)docs/core-concepts/webpack-configuration.md(1 hunks)lib/generators/react_on_rails/base_generator.rb(1 hunks)lib/generators/react_on_rails/generator_helper.rb(1 hunks)lib/generators/react_on_rails/templates/base/base/config/initializers/react_on_rails.rb.tt(1 hunks)lib/generators/react_on_rails/templates/base/base/config/shakapacker.yml.tt(1 hunks)lib/generators/react_on_rails/templates/base/base/config/webpack/serverWebpackConfig.js.tt(2 hunks)lib/react_on_rails/configuration.rb(2 hunks)lib/react_on_rails/doctor.rb(4 hunks)lib/react_on_rails/utils.rb(1 hunks)spec/lib/react_on_rails/doctor_spec.rb(1 hunks)spec/react_on_rails/utils_spec.rb(1 hunks)
🧰 Additional context used
🧠 Learnings (15)
📓 Common learnings
Learnt from: alexeyr-ci2
Repo: shakacode/react_on_rails PR: 1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
Learnt from: justin808
Repo: shakacode/react_on_rails PR: 1770
File: lib/generators/react_on_rails/templates/base/base/app/javascript/src/HelloWorld/ror_components/HelloWorld.client.jsx:2-2
Timestamp: 2025-09-16T08:01:11.146Z
Learning: React on Rails uses webpack CSS Modules configuration with namedExports: true, which requires the import syntax `import * as style from './file.module.css'` rather than the default export pattern. This configuration enables better tree shaking and bundle size optimization for CSS modules.
Learnt from: Romex91
Repo: shakacode/react_on_rails PR: 1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.
📚 Learning: 2025-09-16T08:01:11.146Z
Learnt from: justin808
Repo: shakacode/react_on_rails PR: 1770
File: lib/generators/react_on_rails/templates/base/base/app/javascript/src/HelloWorld/ror_components/HelloWorld.client.jsx:2-2
Timestamp: 2025-09-16T08:01:11.146Z
Learning: React on Rails uses webpack CSS Modules configuration with namedExports: true, which requires the import syntax `import * as style from './file.module.css'` rather than the default export pattern. This configuration enables better tree shaking and bundle size optimization for CSS modules.
Applied to files:
docs/core-concepts/webpack-configuration.mdlib/generators/react_on_rails/templates/base/base/config/initializers/react_on_rails.rb.ttlib/generators/react_on_rails/templates/base/base/config/webpack/serverWebpackConfig.js.ttdocs/api-reference/configuration.mdlib/generators/react_on_rails/templates/base/base/config/shakapacker.yml.ttlib/react_on_rails/configuration.rb
📚 Learning: 2025-02-12T16:38:06.537Z
Learnt from: Romex91
Repo: shakacode/react_on_rails PR: 1697
File: package-scripts.yml:28-28
Timestamp: 2025-02-12T16:38:06.537Z
Learning: The file `node_package/lib/ReactOnRails.full.js` is autogenerated during the build process and should not be present in the repository.
Applied to files:
docs/core-concepts/webpack-configuration.mdlib/generators/react_on_rails/templates/base/base/config/initializers/react_on_rails.rb.ttlib/generators/react_on_rails/templates/base/base/config/webpack/serverWebpackConfig.js.ttdocs/api-reference/configuration.md
📚 Learning: 2025-04-26T21:55:55.874Z
Learnt from: alexeyr-ci2
Repo: shakacode/react_on_rails PR: 1732
File: spec/dummy/client/app-react16/startup/ReduxSharedStoreApp.client.jsx:40-44
Timestamp: 2025-04-26T21:55:55.874Z
Learning: In the react_on_rails project, files under `app-react16` directories are copied/moved to corresponding `/app` directories during the conversion process (removing the `-react16` suffix), which affects their relative import paths at runtime.
Applied to files:
docs/core-concepts/webpack-configuration.mdlib/generators/react_on_rails/templates/base/base/config/webpack/serverWebpackConfig.js.ttdocs/api-reference/configuration.mdlib/react_on_rails/configuration.rblib/react_on_rails/utils.rbspec/react_on_rails/utils_spec.rb
📚 Learning: 2025-10-23T17:22:01.074Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1875
File: lib/react_on_rails/utils.rb:112-124
Timestamp: 2025-10-23T17:22:01.074Z
Learning: In React on Rails, when Pro is installed but not licensed, the intended behavior is to raise an error on boot. The `react_on_rails_pro?` method validates licenses and should raise errors early (including during path resolution in methods like `server_bundle?`) to enforce licensing requirements rather than failing later with obscure errors.
Applied to files:
lib/generators/react_on_rails/templates/base/base/config/initializers/react_on_rails.rb.ttlib/generators/react_on_rails/templates/base/base/config/webpack/serverWebpackConfig.js.ttdocs/api-reference/configuration.mdlib/react_on_rails/configuration.rbspec/lib/react_on_rails/doctor_spec.rbspec/react_on_rails/utils_spec.rblib/react_on_rails/doctor.rb
📚 Learning: 2025-02-13T16:50:47.848Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/clientStartup.ts:18-21
Timestamp: 2025-02-13T16:50:47.848Z
Learning: In the react_on_rails module, the `reactOnRailsPageUnloaded` function in clientStartup.ts is intentionally kept private as it's only used internally as a callback for `onPageUnloaded`.
Applied to files:
lib/generators/react_on_rails/templates/base/base/config/initializers/react_on_rails.rb.ttlib/generators/react_on_rails/templates/base/base/config/webpack/serverWebpackConfig.js.tt
📚 Learning: 2025-07-08T05:57:29.630Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1745
File: node_package/src/RSCRequestTracker.ts:8-14
Timestamp: 2025-07-08T05:57:29.630Z
Learning: The global `generateRSCPayload` function in React on Rails Pro (RORP) is provided by the framework during rendering requests, not implemented in application code. The `declare global` statements are used to document the expected interface that RORP will inject at runtime.
Applied to files:
lib/generators/react_on_rails/templates/base/base/config/initializers/react_on_rails.rb.ttlib/generators/react_on_rails/templates/base/base/config/webpack/serverWebpackConfig.js.ttdocs/api-reference/configuration.md
📚 Learning: 2025-09-15T21:24:48.207Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1781
File: node_package/src/ClientSideRenderer.ts:82-95
Timestamp: 2025-09-15T21:24:48.207Z
Learning: In React on Rails, the force_load feature includes both explicit `data-force-load="true"` usage and the ability to hydrate components during the page loading state (`document.readyState === 'loading'`). Both capabilities require a Pro license, so the condition `!railsContext.rorPro && (isComponentForceLoaded || document.readyState === 'loading')` correctly gates both scenarios.
Applied to files:
lib/generators/react_on_rails/templates/base/base/config/initializers/react_on_rails.rb.ttdocs/api-reference/configuration.mdlib/generators/react_on_rails/generator_helper.rbspec/react_on_rails/utils_spec.rb
📚 Learning: 2025-02-18T13:08:01.477Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: lib/react_on_rails/helper.rb:190-197
Timestamp: 2025-02-18T13:08:01.477Z
Learning: RSC support validation in React on Rails Pro is handled through a chain of validations:
1. Pro version check in `run_stream_inside_fiber`
2. RSC support check during pack generation via `ReactOnRailsPro.configuration.enable_rsc_support`
3. RSC support validation during component registration
This makes additional validation in the helper methods unnecessary.
Applied to files:
lib/generators/react_on_rails/templates/base/base/config/initializers/react_on_rails.rb.ttdocs/api-reference/configuration.mdlib/generators/react_on_rails/generator_helper.rbspec/react_on_rails/utils_spec.rblib/react_on_rails/doctor.rb
📚 Learning: 2025-02-13T19:09:15.991Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/RSCWebpackLoader.ts:0-0
Timestamp: 2025-02-13T19:09:15.991Z
Learning: In React Server Components webpack loader, using `new Function('return import("react-server-dom-webpack/node-loader")')()` is necessary as a workaround to bypass TypeScript compilation issues with direct dynamic imports.
Applied to files:
lib/generators/react_on_rails/templates/base/base/config/webpack/serverWebpackConfig.js.tt
📚 Learning: 2024-10-08T20:53:47.076Z
Learnt from: theforestvn88
Repo: shakacode/react_on_rails PR: 1620
File: spec/dummy/client/app/startup/HelloTurboStream.jsx:3-3
Timestamp: 2024-10-08T20:53:47.076Z
Learning: The `RailsContext` import in `spec/dummy/client/app/startup/HelloTurboStream.jsx` is used later in the project, as clarified by the user theforestvn88.
Applied to files:
docs/api-reference/configuration.md
📚 Learning: 2025-02-13T16:50:26.861Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/turbolinksUtils.ts:34-36
Timestamp: 2025-02-13T16:50:26.861Z
Learning: In React on Rails, when checking for Turbolinks version 5 using `turbolinksVersion5()`, always ensure `Turbolinks` exists first by checking `turbolinksInstalled()` to prevent TypeError when accessing properties.
Applied to files:
lib/generators/react_on_rails/generator_helper.rb
📚 Learning: 2024-12-12T13:07:09.929Z
Learnt from: alexeyr-ci
Repo: shakacode/react_on_rails PR: 1644
File: node_package/src/ReactOnRailsRSC.ts:87-87
Timestamp: 2024-12-12T13:07:09.929Z
Learning: When handling errors in 'node_package/src/ReactOnRailsRSC.ts', include the error stack in error messages in development and test environments to aid debugging.
Applied to files:
spec/lib/react_on_rails/doctor_spec.rb
📚 Learning: 2025-02-13T14:29:49.267Z
Learnt from: AbanoubGhadban
Repo: shakacode/react_on_rails PR: 1644
File: spec/react_on_rails/utils_spec.rb:218-218
Timestamp: 2025-02-13T14:29:49.267Z
Learning: In RSpec tests, prefer using local variables over constants within test blocks to avoid constant redefinition warnings and maintain better test isolation.
Applied to files:
spec/react_on_rails/utils_spec.rb
📚 Learning: 2024-10-08T20:53:47.076Z
Learnt from: justin808
Repo: shakacode/react_on_rails PR: 1622
File: spec/dummy/spec/rake/assets_precompile_rake_spec.rb:12-12
Timestamp: 2024-10-08T20:53:47.076Z
Learning: When stubbing environment variables in RSpec tests, use `before` and `after` hooks to ensure that the original values are restored after the tests, preventing any side effects on other tests. Example provided by justin808:
```ruby
describe "My test" do
before do
original_value = ENV["VARIABLE_NAME"]
allow(ENV).to receive(:[]).with("VARIABLE_NAME").and_return("stubbed_value")
end
after do
allow(ENV).to receive(:[]).with("VARIABLE_NAME").and_call_original
ENV["VARIABLE_NAME"] = original_value
end
it "tests something" do
# Your test code here
end
end
```
This practice ensures test isolation and reliability.
Applied to files:
spec/react_on_rails/utils_spec.rb
🧬 Code graph analysis (4)
lib/generators/react_on_rails/generator_helper.rb (1)
lib/react_on_rails/packer_utils.rb (1)
shakapacker_version_requirement_met?(30-33)
lib/react_on_rails/configuration.rb (1)
lib/react_on_rails/utils.rb (1)
normalize_to_relative_path(455-469)
spec/react_on_rails/utils_spec.rb (1)
lib/react_on_rails/utils.rb (1)
normalize_to_relative_path(455-469)
lib/react_on_rails/doctor.rb (2)
lib/react_on_rails/system_checker.rb (3)
add_info(28-30)add_success(24-26)add_warning(20-22)lib/react_on_rails/utils.rb (1)
normalize_to_relative_path(455-469)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: build
- GitHub Check: claude-review
- GitHub Check: markdown-link-check
🔇 Additional comments (11)
lib/generators/react_on_rails/templates/base/base/config/shakapacker.yml.tt (1)
32-37: LGTM! Version-aware configuration template is well-designed.The conditional generation based on Shakapacker version is clean and maintainable. The commented fallback for older versions provides good documentation without breaking existing setups.
lib/generators/react_on_rails/base_generator.rb (1)
102-103: LGTM! Template-driven generation enables version-aware configuration.The switch from
copy_filetotemplateis necessary to process the ERB conditionals inshakapacker.yml.tt. This change is minimal and focused.lib/generators/react_on_rails/templates/base/base/config/webpack/serverWebpackConfig.js.tt (1)
47-71: LGTM! Version-aware server output path configuration is well-implemented.The conditional logic correctly uses
config.privateOutputPathfor Shakapacker 9.0+ and falls back to a hardcoded path for older versions. The comments provide clear migration guidance.lib/react_on_rails/utils.rb (1)
446-469: LGTM! Path normalization utility is robust and well-documented.The implementation correctly handles nil, Pathname, and String inputs. The use of
Regexp.escapefor the Rails.root prefix prevents regex injection issues. Documentation and examples are clear.docs/core-concepts/webpack-configuration.md (1)
81-111: LGTM! Documentation clearly explains the Shakapacker 9.0+ integration.The new section provides clear examples and explains the benefits of using
private_output_path. The fallback guidance for older versions is helpful for users who haven't upgraded yet.docs/api-reference/configuration.md (1)
111-163: LGTM! Comprehensive documentation of server bundle security and organization.The documentation clearly explains the Shakapacker 9.0+ integration, the benefits of
private_output_path, and the security implications ofenforce_private_server_bundles. The directory structure example is particularly helpful for understanding the separation between client and server bundles.lib/generators/react_on_rails/templates/base/base/config/initializers/react_on_rails.rb.tt (1)
29-32: No breaking change concerns identified after verification.The asymmetry between the template default (
enforce_private_server_bundles = true) and the library default (line 50 inlib/react_on_rails/configuration.rb:false) is intentional. The default isfalsefor backward compatibility, while the generated initializer is secure-by-default for new projects.The implementation includes appropriate safeguards:
- Validation logic (
lib/react_on_rails/configuration.rblines 237-259) raises errors ifenforce_private_server_bundles=truewithout properserver_bundle_output_pathconfiguration- Template guidance directs users to use Shakapacker 9.0+ auto-detection or manually configure the path
- Doctor task (
rails react_on_rails:doctor) helps verify configurationExisting projects are unaffected unless they explicitly regenerate their initializer, and any misconfiguration triggers clear error messages guiding users to proper setup.
lib/react_on_rails/configuration.rb (1)
187-187: LGTM! Auto-detection positioned correctly in the configuration flow.The auto-detection method is appropriately invoked at the end of
setup_config_values, ensuring all other configuration validations complete first before attempting auto-detection.spec/react_on_rails/utils_spec.rb (1)
902-999: Excellent test coverage for path normalization utility.The test suite comprehensively covers the
normalize_to_relative_pathmethod, including:
- Standard absolute → relative conversions
- Pathname object handling
- Edge cases (nil, already relative, Rails.root as substring)
- Complex path scenarios (special characters, spaces, dots)
The test structure is clear and follows RSpec best practices.
spec/lib/react_on_rails/doctor_spec.rb (1)
526-636: Well-structured tests for Shakapacker integration diagnostics.The test suite thoroughly validates the
check_shakapacker_private_output_pathhelper across multiple scenarios:
- Absent Shakapacker gem
- Pre-9.0 versions without
private_output_pathsupport- 9.0+ scenarios (matching paths, mismatches, unset config, error handling)
The tests use proper test doubles and verify specific diagnostic messages, ensuring the doctor feature provides accurate guidance to users.
lib/react_on_rails/doctor.rb (1)
670-718: LGTM! Enhanced server rendering configuration analysis.The updated
analyze_server_rendering_configmethod now includes:
- Display of
server_bundle_output_pathandenforce_private_server_bundlesconfiguration- Integration with the new Shakapacker diagnostics helper
The additions are well-integrated and provide users with comprehensive server bundle configuration information.
CI Fix AppliedAdded the The Problem: The workflow's conditional matrix include logic evaluates to - ruby-version: ${{ (condition) && '3.2'}} # Evaluates to false when condition is falseThe Fix: The This ensures the full test matrix runs (both latest and minimum dependency versions) and avoids the "Unknown engine false" error. CI should now pass! ✅ |
Review Feedback Addressed ✅Added validation and error messaging for hardcoded server bundle paths (addressing review comment on serverWebpackConfig.js.tt:61). Changes in commit 42da62c: 1. Runtime Validation for Older ShakapackerFor Shakapacker < 9.0 using hardcoded paths, added validation that displays helpful warnings: ⚠️ Server bundle output directory does not exist: /path/to/ssr-generated
It will be created during build, but ensure React on Rails is configured:
config.server_bundle_output_path = "ssr-generated" in config/initializers/react_on_rails.rb
Run: rails react_on_rails:doctor to validate your configuration2. Code Quality Improvements
Benefits
|
justin808
force-pushed
the
feature/server-bundle-path-validation-rebased
branch
from
42da62c to
ee74d03
Compare
|
✅ Rebased onto latest master (276507d) Picked up important CI fixes from master:
Removed the All RuboCop checks passing ✅ |
CI Failure Fixed ✅Root Cause: The webpack config template was using without a fallback, causing it to be when wasn't set in . Fix Applied (commit 78a2ff7): // Before (broken):
path: config.privateOutputPath
// After (fixed):
path: config.privateOutputPath || require('path').resolve(__dirname, '../../ssr-generated')This ensures the server bundle is always built to a valid location, even when:
The fallback maintains full backward compatibility while still supporting the new auto-detection feature when properly configured. CI should now pass! 🚀 |
justin808
force-pushed
the
feature/server-bundle-path-validation-rebased
branch
from
78a2ff7 to
adb0aa9
Compare
|
✅ Rebased onto latest master (ad73088) Picked up important fixes:
The Knip fix should resolve the build failures. CI running now! 🚀 |
Adds clear documentation and automated validation to prevent configuration mismatches between webpack and React on Rails configurations. **Documentation Improvements:** - Add prominent warnings in both webpack config and Rails initializer templates - Clarify that both server_bundle_output_path settings must match - Explain the security rationale (private directories prevent code exposure) - Update generator templates with cross-reference comments **Doctor Validation:** - Add automated check that compares webpack serverWebpackConfig.js output.path with React on Rails server_bundle_output_path configuration - Display success message when configs are in sync - Provide detailed warning with fix instructions when mismatch detected - Gracefully handles missing config files or parsing errors **Why This Matters:** Webpack (build-time) and React on Rails (runtime) need to agree on where server bundles are located. Misconfiguration causes SSR failures that can be hard to debug. This change makes the requirement explicit and adds automated validation via the doctor command. **Technical Notes:** - Validation runs as part of `rails react_on_rails:doctor` analysis - Parses webpack config to extract output.path value - Compares paths relative to Rails.root - No breaking changes - pure documentation and tooling improvement Related to closed PR #1808 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…sive tests
Addresses critical issues identified in code review:
**Issue 1: Regex Pattern Too Restrictive**
- Original regex only matched hardcoded path.resolve() pattern
- Real dummy app uses config.outputPath, which wasn't matched at all
- Validation would silently skip without detecting actual configs
**Fix:**
- Support hardcoded path: require('path').resolve(__dirname, '../../path')
- Detect config.outputPath usage (can't validate, inform user)
- Detect variable usage (can't validate, inform user)
- Clear messaging for each pattern type
**Issue 2: Missing Path Normalization**
- String equality comparison failed on equivalent paths
- ./ssr-generated vs ssr-generated would be false mismatch
- Trailing slashes caused false positives
**Fix:**
- normalize_path() method strips leading ./ and /
- Removes trailing slashes
- Handles whitespace
- Graceful handling of nil/non-string values
**Issue 3: No Test Coverage**
- New validation methods had zero tests
- High risk of regressions
**Fix:**
- Added 20+ comprehensive test cases covering:
- File not found scenarios
- Hardcoded paths (matching and mismatched)
- config.outputPath detection
- Variable detection
- Error handling
- Path normalization edge cases
- All return nil vs success vs warning paths
**Testing Results:**
- All 20 new tests passing
- 30/31 total specs passing (1 pre-existing failure unrelated)
- Covers real-world patterns including dummy app config
**Technical Details:**
- extract_webpack_output_path() now pattern-aware
- Better user messaging for unvalidatable configs
- Maintains backward compatibility
- No breaking changes
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
…improve docs
Addresses feedback from detailed code review:
**1. Critical: Fix Asymmetric RuboCop Directives**
- Issue: Line 667 only disabled Metrics/CyclomaticComplexity
- But line 702 enabled BOTH Metrics/AbcSize and Metrics/CyclomaticComplexity
- This asymmetry could cause violations
- Fix: Changed line 667 to disable both cops symmetrically
- Result: RuboCop reports method doesn't actually need AbcSize disabled, auto-fixed
**2. Simplify Regex Pattern for Clarity**
- Issue: Pattern `['"]\.\.[\/\.\.]+` was hard to read
- Fix: Changed to explicit `['"]\.\.\/\.\.\/` to clearly match ../../path
- Makes intent obvious: we match exactly two parent directory traversals
- RuboCop auto-removed redundant escapes inside %r{}
**3. Improve Documentation Discoverability**
- Issue: Users might not know about the validation feature
- Fix: Added "Run 'rails react_on_rails:doctor' to verify these configs are in sync"
- Placement: In generator template right next to server_bundle_output_path config
- Helps users discover the validation tool when they need it most
**Testing:**
- ✅ All 20 validation tests still passing
- ✅ RuboCop passes with zero offenses
- ✅ Regex pattern matches test cases correctly
- ✅ 30/31 total specs passing (1 pre-existing failure unrelated)
**Other Suggestions Considered:**
- Reordering validation (decided current flow is good - validates immediately after config display)
- Magic string constant (not needed - single use in method, clear context)
- Trailing newline check (git hooks already enforce this automatically)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
Addresses code review feedback about fragile regex matching by implementing a proper integration with Shakapacker 9.0+ private_output_path. **Key Changes:** **1. Auto-Detection from Shakapacker (configuration.rb)** - New `auto_detect_server_bundle_path_from_shakapacker` method - Automatically reads `private_output_path` from Shakapacker 9.0+ config - Only applies if user hasn't explicitly set `server_bundle_output_path` - Gracefully falls back to default if detection fails - Logs info message when auto-detection succeeds **2. Removed Fragile Regex Validation (doctor.rb)** - Removed `validate_server_bundle_path_sync` method (regex matching) - Removed `extract_webpack_output_path` method (pattern detection) - Removed `normalize_path` method (no longer needed) - Replaced with `check_shakapacker_private_output_path` method **3. Recommendation-Based Doctor Checks (doctor.rb)** - Detects Shakapacker version and capabilities - Pre-9.0: Recommends upgrading for better DX - 9.0+ without config: Shows how to configure private_output_path - 9.0+ with config matching: Success message - 9.0+ with config mismatch: Warning with fix instructions - No Shakapacker: Informs about manual configuration **4. Updated Generator Templates** - **React on Rails initializer**: Documents Shakapacker 9.0+ approach first - **Webpack config**: Shows config.privateOutputPath pattern - Both templates emphasize single source of truth in shakapacker.yml - Clear migration path for older Shakapacker versions **5. Comprehensive Test Coverage (8 new tests)** - Shakapacker not defined scenario - Pre-9.0 Shakapacker (no private_output_path support) - 9.0+ with matching config - 9.0+ with mismatched config - 9.0+ without config - Error handling - All tests passing **Benefits:** - No fragile regex parsing of webpack configs - Single source of truth in shakapacker.yml - Automatic configuration for Shakapacker 9.0+ users - Backward compatible with older Shakapacker versions - Clear upgrade path and recommendations - Robust error handling **Breaking Changes:** None - Existing configurations continue to work - Auto-detection only applies to default values - Explicit user configuration always takes precedence Addresses: @justin's feedback on PR #1967 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…tion Documents the recommended approach of using Shakapacker 9.0+ private_output_path for server bundle configuration, providing a single source of truth. **Documentation Updates:** **1. Configuration API Reference (docs/api-reference/configuration.md)** - Added prominent recommendation for Shakapacker 9.0+ approach - Documents shakapacker.yml private_output_path configuration - Explains auto-detection behavior - Preserves documentation for older versions **2. Webpack Configuration Guide (docs/core-concepts/webpack-configuration.md)** - New section: "Server Bundle Configuration (Shakapacker 9.0+)" - Complete example with shakapacker.yml and webpack config - Lists benefits of the new approach: - Single source of truth - Automatic synchronization - No configuration duplication - Better maintainability - Notes compatibility with older versions **Key Points:** - Shakapacker 9.0+ users get automatic configuration - Backward compatible with manual configuration - Generator templates already show both approaches - Doctor command guides users to upgrade **Related Changes:** - Generator templates already updated in previous commit - Auto-detection implemented in configuration.rb - Doctor provides version-aware recommendations 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Adds commented-out private_output_path configuration to the generated shakapacker.yml, making it easy for users to enable the Shakapacker 9.0+ integration feature. **Changes:** - Added private_output_path: ssr-generated (commented out) - Clear documentation that it's a Shakapacker 9.0+ feature - Notes that React on Rails automatically detects it - Positioned logically after public_output_path configuration **User Experience:** Users can now simply uncomment one line in shakapacker.yml to enable automatic configuration instead of needing to manually add it. **Complete Integration:** - ✅ Auto-detection in configuration.rb - ✅ Doctor recommendations - ✅ Generator templates (webpack + initializer) - ✅ Shakapacker.yml template (NEW) - ✅ Documentation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
…ration
This commit addresses code review feedback and enhances the Shakapacker 9.0+
integration with several key improvements:
1. **Extract path normalization to shared helper**
- Added Utils.normalize_to_relative_path method
- Eliminates code duplication between configuration.rb and doctor.rb
- Handles edge cases: Pathname objects, special characters, trailing slashes
- Uses proper regex escaping with Regexp.escape
2. **Add comprehensive test coverage**
- Added 17 new test cases for path normalization
- Covers absolute/relative paths, nil handling, special characters
- Tests paths with spaces, dots, hyphens, and substring matching
- All 81 utils tests passing
3. **Make generator version-aware**
- Added shakapacker_version_9_or_higher? helper method
- serverWebpackConfig.js.tt now generates optimal code based on version:
* Shakapacker 9.0+: Uses config.privateOutputPath
* Shakapacker < 9.0: Uses hardcoded path with upgrade notes
- shakapacker.yml.tt automatically enables private_output_path for 9.0+
- Provides clear upgrade path for older versions
4. **Improve error handling**
- Added safe navigation operator for Rails.logger calls
- Graceful fallbacks throughout auto-detection code
- Proper exception handling in generator helper
5. **Code quality improvements**
- All RuboCop violations fixed (0 offenses)
- Added complexity disable directive for well-structured method
- Clear documentation and comments throughout
- New installations get optimal configuration automatically
- No manual config needed for Shakapacker 9.0+ users
- Single source of truth in shakapacker.yml
- Better maintainability through DRY principles
- Comprehensive test coverage ensures reliability
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Addresses review feedback to provide clear error messages when using hardcoded paths with older Shakapacker versions. Changes: - Add runtime validation in serverWebpackConfig.js template for Shakapacker < 9.0 - Display helpful warning with configuration instructions if path doesn't exist - Extract hardcoded "ssr-generated" to DEFAULT_SERVER_BUNDLE_OUTPUT_PATH constant - Improve logging: use debug level for auto-detection (not user-initiated) - Add documentation about absolute paths in normalize_to_relative_path The validation helps users quickly identify configuration mismatches and provides actionable guidance to fix them using rails react_on_rails:doctor. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
The generated serverWebpackConfig.js was failing when Shakapacker 9+ was detected but private_output_path wasn't configured in shakapacker.yml. This caused config.privateOutputPath to be undefined, resulting in webpack building to an incorrect location. Changes: - Add fallback to hardcoded path when config.privateOutputPath is undefined - Ensures server bundle is always built to a valid location - Maintains backward compatibility with all Shakapacker versions This fixes the "server-bundle.js cannot be read" error in generator examples. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
justin808
force-pushed
the
feature/server-bundle-path-validation-rebased
branch
from
adb0aa9 to
4fd1204
Compare
Knip Warnings - Pre-existing Issue ℹ️The build job shows Knip warnings for spec/dummy dependencies:
These are false positives - both dependencies ARE used but in ways Knip can't detect:
This is a pre-existing issue in master, not introduced by this PR. These same dependencies exist in Options:
The Shakapacker integration feature itself is complete and working. All other CI checks should pass. ✅ |
|
@claude review this PR |
|
Claude finished @justin808's task —— View job Code Review for PR #2028Task: Review Shakapacker 9.0+ private_output_path integration Executive SummaryThis is an excellent PR that delivers a high-quality feature with thoughtful design, comprehensive testing, and clear documentation. The implementation provides automatic detection of Shakapacker 9.0+ Recommendation: ✅ APPROVE - Ready to merge with minor observations noted below. 🎯 Strengths1. Outstanding Architecture & DesignThe auto-detection logic is exemplary:
lib/react_on_rails/configuration.rb:265-290 def auto_detect_server_bundle_path_from_shakapacker
return if server_bundle_output_path != ReactOnRails::DEFAULT_SERVER_BUNDLE_OUTPUT_PATH
return unless defined?(::Shakapacker)
return unless ::Shakapacker.config.respond_to?(:private_output_path)
# ... graceful error handling
endThis pattern is excellent - each 2. Comprehensive Test Coveragespec/react_on_rails/utils_spec.rb:902-999 - Path normalization tests are thorough:
spec/lib/react_on_rails/doctor_spec.rb:536-636 - Doctor diagnostics fully tested:
3. Excellent Developer ExperienceThe doctor command provides actionable guidance:
lib/react_on_rails/doctor.rb:1411-1470 - All scenarios handled with appropriate messaging. 4. Security Best Practiceslib/react_on_rails/utils.rb:446-473: path_str.sub(%r{^#{Regexp.escape(rails_root_str)}/?}, "")✅ Uses 5. Code Quality
6. Documentation Excellencedocs/api-reference/configuration.md:
Generator templates:
🔍 Observations & Minor Suggestions1. Webpack Config Fallback ✅ Already Fixedlib/generators/react_on_rails/templates/base/base/config/webpack/serverWebpackConfig.js.tt:61 The latest commit (4fd1204) properly adds a fallback: path: config.privateOutputPath || require('path').resolve(__dirname, '../../ssr-generated')This is excellent - ensures the server bundle always has a valid output path even when 2. Auto-Detection Logging Level ✅ Well-Chosenlib/react_on_rails/configuration.rb:283-284 Using
3. Generator Version Detectionlib/generators/react_on_rails/generator_helper.rb:104-112 The optimistic default ( return true unless defined?(ReactOnRails::PackerUtils)Reasoning: This assumes fresh installs will use latest Shakapacker, which is reasonable. The fallback in the webpack config ensures it still works even if this assumption is wrong. Minor concern: If someone generates with Shakapacker not installed, then installs an older version, they'd get the wrong template. However:
Verdict: Current approach is acceptable. 4. Path Normalization Edge Caselib/react_on_rails/utils.rb:459-473 Consider what happens if # Rails.root = "/", path = "/app"
# Would incorrectly match and return "app" instead of "/app"Mitigation: The Optional enhancement: Add a guard clause: return path_str if rails_root_str == "/"But this is not blocking - just a theoretical edge case. 5. Error Handling Specificitylib/react_on_rails/configuration.rb:285-289 rescue StandardError => e
Rails.logger&.debug("ReactOnRails: Could not auto-detect...")
endCatching
Could be improved: Catch more specific exceptions ( 🧪 TestingTest Coverage Assessment: Excellent
Tests NOT present (and why that's OK):
📋 Pre-Merge ChecklistBased on CLAUDE.md requirements, verify:
Note: The author has already addressed CI failures through multiple commits and rebases. The latest commit (4fd1204) adds the critical fallback for webpack config. 🔒 Security Review✅ No security concerns identified
⚡ Performance✅ Minimal performance impact
📝 Code Quality Issues Found: NoneAll code follows project standards:
💡 Notable Implementation Details1. Constant ExtractionThe addition of
2. Webpack ValidationserverWebpackConfig.js.tt:67-79 - For Shakapacker < 9.0, adds helpful validation warnings when the output directory doesn't exist. This guides users to proper configuration. 3. Doctor Integrationlib/react_on_rails/doctor.rb:670-718 - The doctor command now displays:
This provides a complete diagnostic picture. 🎉 ConclusionThis PR demonstrates excellent software engineering:
The implementation shows careful consideration of:
Minor Observations SummaryThe observations noted above are not blocking issues - they're minor points for future consideration:
Final Recommendation✅ APPROVE AND MERGE This is production-quality code that significantly improves the React on Rails developer experience. Great work, @justin808! Reviewed commit: 4fd1204 (Fix webpack config to fallback when privateOutputPath is undefined) |
Summary
This PR integrates Shakapacker 9.0+
private_output_pathconfiguration with React on Rails, eliminating the need for manualserver_bundle_output_pathconfiguration and providing automatic synchronization between webpack and Rails configs.Key Changes
Auto-Detection
private_output_pathfromshakapacker.yml(Shakapacker 9.0+)"ssr-generated"if not configuredGenerator Updates
private_output_pathfor Shakapacker 9.0+Doctor Enhancements
private_output_pathsupportDocumentation
Benefits
shakapacker.ymlTesting
Migration Path
For users on Shakapacker 9.0+:
private_output_path: ssr-generatedtoconfig/shakapacker.ymlconfig.server_bundle_output_pathfrom React on Rails initializer (optional - auto-detected)rails react_on_rails:doctorto verifyFor users on older Shakapacker:
Related
Supersedes #1967 (rebased for cleaner history)
🤖 Generated with Claude Code
This change is
Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests