Add PR Testing Agent documentation #2109
Conversation
Creates a specialized agent definition for comprehensive PR testing validation that is skeptical of testing claims unless verified, assumes automated tests have gaps, and requires clear communication about what was tested vs what needs manual testing. Key sections: - Agent behavior guidelines (skeptical stance, clear communication) - Testing requirements by change type (Ruby, JS/TS, builds, webpack, CI, generators, engines) - Environment-specific limitations (Conductor workspace, Ruby 3.4 SSL issues) - Pre-merge testing checklist template - Real-world scenario examples with good/bad examples This guide pulls all testing information from existing documentation (testing-build-scripts.md, SWITCHING_CI_CONFIGS.md, TESTING_LOCALLY.md, CLAUDE.md) and integrates project-specific tools (bin/ci-rerun-failures, bin/ci-switch-config, etc.). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
|
Warning Rate limit exceeded@justin808 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 11 minutes and 41 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 (2)
WalkthroughIntroduces a new documentation file at Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 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 Testing Agent DocumentationSummaryThis PR adds comprehensive documentation for a specialized PR testing agent. The documentation is well-structured, thorough, and provides clear guidance on testing requirements across different change types. ✅ Strengths
📝 Suggestions for Improvement1. Minor Markdown Formatting IssuesLine 431-434 has malformed code fence in nested markdown - the inner code block should use triple backticks not single backticks. Same issue on line 436-440. Lines 498, 529, 569 have quadruple backticks when they should have triple backticks. These formatting issues may cause the code blocks to render incorrectly in markdown viewers. 2. Add Reference from CLAUDE.mdConsider adding a reference to this new document in the main CLAUDE.md file, similar to how other docs are referenced. This would make it more discoverable for Claude Code users and maintain consistency. 3. Consider Adding a Table of ContentsGiven the document's length (621 lines), a table of contents at the top would improve navigation. 🔒 Security Considerations✅ No security concerns - this is documentation only ⚡ Performance Considerations✅ N/A - documentation file 🧪 Test Coverage✅ Appropriately marked as "N/A, documentation only" in the PR checklist 📚 Documentation QualityExcellent overall! The documentation:
The skeptical stance and emphasis on verification aligns well with the project's quality standards outlined in CLAUDE.md, particularly the "CRITICAL - LOCAL TESTING REQUIREMENTS" section. 🎯 RecommendationApprove with minor fixes - The markdown formatting issues should be corrected before merge, but the content is excellent and will be very valuable for maintaining testing quality. Suggested next steps:
📋 Checklist ValidationPer CLAUDE.md requirements:
Great work on creating this comprehensive testing guide! This will significantly improve PR quality and reduce the chance of untested changes making it to master. The emphasis on distinguishing between "verified" and "untested" fixes is particularly valuable and addresses a real pain point in the development process. |
PR Review: Add PR Testing Agent documentationThis is an excellent addition to the project documentation infrastructure. The PR Testing Agent doc provides comprehensive, practical guidance for ensuring thorough testing before merging PRs. Strengths1. Outstanding Structure and Organization
2. Excellent Integration with Existing Docs
3. Strong Skeptical Stance Philosophy 4. Practical, Copy-Paste Ready Content 5. Excellent Real-World Scenarios Suggestions for Improvement1. Minor Formatting Issue (Line 380-381) 2. Consider Adding Quick Reference Section 3. Optional: Add Section Numbers Code Quality Assessment
Final RecommendationAPPROVE with minor suggestions. This is high-quality documentation that will significantly improve PR testing rigor. Once the minor formatting issue is addressed, this is ready to merge. Great work on this! |
Code Review - PR Testing Agent Documentation✅ Overall AssessmentThis is excellent documentation that fills a critical gap in the development workflow. The content is comprehensive, well-structured, and highly practical. All CI checks are passing, and this is a documentation-only change with minimal risk. 🎯 Strengths
📋 Minor Suggestions1. File Size ConsiderationAt 621 lines, this is significantly larger than other docs in
Consider: Could this be split into:
However, keeping it together is also valid for discoverability. Not a blocker. 2. Cross-Reference ValidationThe document references several files - all verified to exist ✅:
3. Markdown FormattingAll code blocks are properly fenced with triple backticks ✅ 4. Potential AdditionsConsider adding a section on:
🔍 Content ReviewTesting Requirements CoverageExcellent coverage of all major scenarios:
Communication GuidanceThe document excels at providing clear communication templates:
Real-World ExamplesThe scenarios are realistic and helpful:
🚀 RecommendationAPPROVE - This is ready to merge. The documentation is comprehensive, well-written, and aligns with project standards. The file size is larger than others but justified by the comprehensive coverage. All referenced files exist and CI checks pass. 📝 Post-Merge SuggestionsAfter merging, consider:
CI Status: ✅ All checks passing |
Adds two new sections to PR Testing Agent documentation: 1. When to Use This Agent - Automatic invocation triggers (PR creation, CI failures, "ready to merge" checks) - Manual invocation scenarios (validating testing claims, reviewing PRs) - Usage pattern examples 2. Integration with Existing Workflows - Relationship to code review process and code-reviewer agent - Integration at each workflow stage (pre-commit, pre-push, during CI, pre-merge) - Complete workflow example from code change to merge These sections clarify when and how to invoke the agent, and how it fits into the existing development workflow alongside git hooks, CI, and code review. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Code Review: PR Testing Agent DocumentationOverall AssessmentExcellent addition! This provides comprehensive, actionable guidance for testing PRs. Strengths
Issues Found1. Markdown Formatting (Lines 541-549, 637-639)Nested bash code blocks use single backticks instead of triple backticks. Won't render correctly. 2. Build Checklist - yarn.lock Issue (Line 219)Command Problem: Removing yarn.lock defeats --frozen-lockfile which requires existing lockfile. Fix:
3. Documentation Overlap (Lines 217-244)Build config checklist overlaps with testing-build-scripts.md. Add cross-reference to prevent divergence. Questions
RecommendationApprove with minor fixes. Content is excellent. Formatting issues are trivial. Next Steps
Great work! |
PR Review: PR Testing Agent DocumentationSummaryThis PR adds comprehensive documentation for a specialized PR testing agent. The documentation is exceptionally thorough and well-structured, providing clear guidance on testing validation before merging PRs. ✅ Strengths1. Excellent Structure & Organization
2. Comprehensive Coverage
3. Practical Examples
4. Strong Core Philosophy
5. Well-Integrated with Existing Docs
🔍 Suggestions for Improvement1. Minor InconsistenciesIssue: Formatting inconsistency in code blocks
Recommendation: Standardize on triple backticks with language for all code blocks: # command hereLocations to fix:
2. Potential RedundancyIssue: The "Manual Testing Required" checklists appear in multiple places:
Recommendation: Consider adding a note at the beginning of "Pre-Merge Testing Checklist" that says:
This helps readers understand the relationship between sections. 3. Missing Cross-ReferenceIssue: The document mentions Recommendation: Add a section or note about when to split PRs vs. comprehensive testing: ### When PRs Are Too Large for Comprehensive Testing
If you find yourself with 10+ files changed across multiple categories (Ruby + JS + build + CI), consider:
- Splitting into smaller, focused PRs (see `.claude/docs/pr-splitting-strategy.md`)
- Testing incrementally with each sub-PR
- Reducing testing surface area per PR4. Clarification Needed: Agent vs. HumanIssue: It's not entirely clear whether this document:
Recommendation: Add a clarifying paragraph at the top: ## Document Purpose
This document serves two purposes:
1. **For AI Agents:** Defines the behavior and decision-making process for the PR Testing Agent
2. **For Human Developers:** Provides a testing validation checklist and communication templates
When invoked, the PR Testing Agent follows these guidelines to validate testing claims and generate testing checklists.5. Potential Addition: Quick ReferenceSuggestion: Consider adding a "Quick Reference" section at the top or bottom with the most common commands: ## Quick Reference: Essential Testing Commands
```bash
# Before every commit
bundle exec rubocop # MANDATORY
# Before creating PR
rm -rf node_modules && yarn install --frozen-lockfile # If build configs changed
bundle exec rake run_rspec:gem # Ruby changes
yarn run test && yarn run build # JS/TS changes
cd spec/dummy && bin/dev # Browser testing
# Debugging CI failures
gh run list --branch master --limit 5 # Check for pre-existing failures
bin/ci-rerun-failures # Rerun failed jobs
bin/ci-switch-config minimum # Switch to minimum config |
PR Review: PR Testing Agent DocumentationSummaryThis is an excellent addition to the project documentation. The PR Testing Agent guide provides comprehensive, actionable guidance for ensuring thorough testing before merging PRs. The skeptical stance and emphasis on verification over claims is exactly what's needed to prevent regressions. Strengths1. Comprehensive Coverage
2. Clear Communication Guidelines
3. Tool Integration
4. Environment Awareness
5. Workflow Integration (Second Commit)
Suggestions for Improvement1. Minor Formatting Inconsistency 2. Add Cross-References Section 3. Clarify Agent Invocation 4. Add Success Metrics 5. Playwright E2E Testing Security, Testing, Performance
RecommendationsBefore Merge:
After Merge:
Overall AssessmentScore: 9/10 - Excellent work! This fills a critical gap in testing practices. The skeptical stance, clear templates, and tool integration make this immediately actionable. Recommendation: Approve and merge Great job pulling together information from multiple sources into a cohesive, actionable guide. The real-world examples and skeptical mindset will help prevent undertested PRs. Thank you for creating this! |
…ormatting Improvements based on feedback: 1. Quick Reference section - Links to all related documentation at top - Jump-to-section navigation - Makes document easier to navigate 2. Clarified agent invocation - Explains this is documentation, not automated tool - Shows how to use with AI assistants - Provides example prompts 3. Added Playwright E2E Testing section (Section 7) - When E2E tests are required (user-facing changes) - How to run Playwright tests - What to verify in browser - Added to pre-merge checklist 4. Success Criteria section - Defines what "well-tested PR" means - 7 concrete criteria with checkboxes - Example testing documentation format - Clear standards for reviewers and contributors 5. Fixed code block formatting - PR comment template now uses indented code blocks - Avoids nested code block rendering issues - Cleaner markdown output 6. Renumbered sections - Playwright E2E Testing is now section 7 - Rails Engine Changes moved to section 8 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
PR Review: PR Testing Agent DocumentationOverall Assessment✅ Excellent documentation addition! This is a comprehensive and valuable resource for ensuring thorough testing before merging PRs. The document is well-structured, practical, and addresses real pain points in the development workflow. Strengths
Code Quality & Best Practices✅ Documentation Structure: Well-organized with clear sections, jump links, and a logical flow from introduction → requirements → checklists → examples. ✅ Cross-References: All file references are valid and use correct relative paths:
✅ Markdown Formatting: Consistent use of code blocks, checklists, emphasis markers, and section headers. ✅ Command Examples: All commands are accurate and copy-pasteable (verified against CLAUDE.md). Potential Issues1. Path Reference Clarity (Minor)Lines 11-15: The relative paths in the Quick Reference section use different formats:
While technically correct, this inconsistency might confuse readers. Consider using a consistent format (e.g., all relative to repo root or all relative to current file location). Suggestion: Add a note explaining the path convention, or normalize to one style. 2. "Agent" Terminology (Documentation Clarity)Lines 47-62: The "How to Invoke" section clarifies that this is documentation, not an automated tool, which is good. However, the title "PR Testing Agent" and frequent use of "agent" throughout might still create confusion. Observation: The section correctly sets expectations that this is a reference guide, but some readers might still expect a command like Suggestion: Consider adding a brief note at the very top (line 3-4) like: **Note**: This is a reference guide and checklist, not an automated CLI tool.3. Section Duplication (Content Organization)Lines 233-268 vs 509-596: The "Build Configuration Changes" checklist appears twice:
While intentional for different use cases, this creates maintenance overhead. If the checklist changes, both places need updating. Suggestion: Consider referencing the detailed section from the condensed checklist: #### If build configs changed:
- [ ] See [Build Configuration Changes](#3-build-configuration-changes) for full mandatory checklistSecurity Concerns✅ No security issues identified. The document provides security-conscious guidance:
Performance Considerations✅ N/A - This is documentation only, no runtime performance impact. Test Coverage✅ Appropriate for documentation-only PR. As stated in the PR checklist:
Verification: This PR correctly follows the guideline from CLAUDE.md:
RecommendationsHigh PriorityNone - the documentation is production-ready as-is. Medium Priority
Low Priority
ConclusionRecommendation: Approve and merge ✅ This PR adds significant value to the project's development workflow. The documentation is thorough, practical, and addresses real pain points. The minor suggestions above are optional improvements, not blockers. Impact: This will help prevent untested changes from breaking master, reduce CI iteration cycles, and improve communication about testing limitations. Great work! This is exactly the kind of comprehensive testing guidance that complex monorepos need. Meta: Following PR Testing Agent GuidelinesApplying the PR Testing Agent criteria to this PR itself: ✅ Automated Tests: N/A (documentation only) This PR passes its own testing criteria. 🎯 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.claude/docs/pr-testing-agent.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
.claude/docs/pr-testing-agent.md
[style] ~32-~32: Try using a synonym here to strengthen your wording.
Context: ...h pr create`) - Responding to PR review comments about testing - CI failures occur and n...
(COMMENT_REMARK)
[grammar] ~582-~582: Ensure spelling is correct
Context: ...tion tests: rake run_rspec:dummy ### Environment Limitations **Mark any testing blocked...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
.claude/docs/pr-testing-agent.md
56-56: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
231-231: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
278-278: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
477-477: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
728-728: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
772-772: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
801-801: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
840-840: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ 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). (1)
- GitHub Check: claude-review
🔇 Additional comments (10)
.claude/docs/pr-testing-agent.md (10)
243-268: Specifybashlanguage for build checklist code block (line 243).-```bash +```bash # Step 1: Clean install (MOST CRITICAL)This block is already correct. However, verify line 278 below also has proper language spec.
755-772: Add language specification to markdown code block in testing scenario example (line 755).-````markdown +````markdown Fixed RSpec failures in helper_spec.rb Changes: - Updated matcher syntax for RSpec 3.12 compatibility Verified locally: ```bash $ bundle exec rspec spec/react_on_rails/helper_spec.rb 42 examples, 0 failures-
+Note: This block already has markdown language spec. Verify the surrounding context and ensure consistency with other blocks. --- `780-800`: **Add language specification to markdown code block in Scenario 2 (line 780).** ```diff -```markdown +```markdown Update React to 19.0.0 Verified locally: - [x] Clean install: `rm -rf node_modules && yarn install --frozen-lockfile` ✅ - [x] Build: `yarn run build` ✅ - [x] Artifacts: `ls -la packages/react-on-rails/lib/ReactOnRails.full.js` ✅ - [x] yalc publish: `yarn run yalc:publish` ✅ - [x] Tests: `yarn run test` (42 passed) ✅ - [x] Browser: Dummy app renders correctly ✅ Tested in dummy app: ```bash cd spec/dummy bin/dev # Visited http://localhost:3000/hello_world # ✅ Component renders # ✅ No console errors # ✅ React DevTools shows React 19.0.0-
+Note: This block already has markdown spec on line 780. Verify line 801 below. --- `109-139`: **Approve workflow example for clarity and structure.** The workflow example in lines 109–139 effectively demonstrates the integration of the PR Testing Agent into a development cycle. It shows clear decision points and references to the agent, making it a strong pedagogical element. --- `170-196`: **Approve Ruby code changes section for comprehensiveness.** The Ruby code changes section (lines 172–196) clearly distinguishes between automated testing (CI-verified) and manual verification required, with concrete bash commands for local verification. The section appropriately flags the Conductor workspace limitation. --- `509-581`: **Approve Pre-Merge Testing Checklist for thoroughness and actionability.** The checklist (lines 509–581) is well-structured with clear sections for different change types, uses checkboxes effectively, and provides specific commands for each category. This is a strong reference for developers. --- `598-682`: **Approve Success Criteria section for clarity and completeness.** The success criteria (lines 598–682) comprehensively covers seven dimensions of a well-tested PR, with clear expectations for each. This provides concrete guidance on what "ready to merge" means from a testing perspective. --- `745-840`: **Approve Real-World Scenarios section for practical guidance.** The scenarios (lines 745–840) effectively illustrate the contrast between insufficient and required documentation with concrete examples. Scenario 1 shows test output verification, Scenario 2 demonstrates comprehensive build testing, and Scenario 3 addresses CI investigation with environment limitations. These examples reinforce the core principles of the agent. --- `343-355`: **All project-specific tools are present and accessible.** ✅ Verification confirms the CI management tools referenced in the documentation exist and are executable: - `bin/ci-switch-config` ✅ - `bin/ci-rerun-failures` ✅ - `bin/ci-run-failed-specs` ✅ The documentation references are valid. --- `1-25`: **All referenced documentation files exist in the repository.** Verification confirms that all five files referenced in the PR testing agent document are present and accessible via their documented paths. The relative path references correctly resolve from the document's location. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| **CRITICAL: These changes REQUIRE extensive manual testing** | ||
|
|
||
| Changes to any of these files trigger **MANDATORY manual testing checklist:** | ||
|
|
||
| - `package.json` (workspace config, dependencies, scripts) | ||
| - `package-scripts.yml` (prepack, prepare, yalc scripts) | ||
| - `webpack.config.js` / `config/webpack/*` | ||
| - `Gemfile` / `Gemfile.development_dependencies` | ||
| - `.github/workflows/*` (CI configuration) | ||
|
|
||
| **MANDATORY Manual Testing Checklist:** | ||
|
|
||
| ```bash | ||
| # Step 1: Clean install (MOST CRITICAL) | ||
| rm -rf node_modules yarn.lock | ||
| yarn install --frozen-lockfile | ||
| # ❌ STOP if this fails - nothing else matters | ||
|
|
||
| # Step 2: Test build scripts | ||
| yarn run build | ||
| ls -la packages/react-on-rails/lib/ReactOnRails.full.js | ||
| # ❌ STOP if artifact missing | ||
|
|
||
| # Step 3: Test prepack | ||
| yarn nps build.prepack | ||
| # ❌ STOP if this fails | ||
|
|
||
| # Step 4: Test yalc publish (critical for local dev) | ||
| yarn run yalc:publish | ||
| # ❌ STOP if this fails | ||
|
|
||
| # Step 5: Test package structure | ||
| yarn workspaces info | ||
| # Verify workspace linking | ||
|
|
||
| # Step 6: Run test suite | ||
| bundle exec rake | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Convert CRITICAL emphasis to proper markdown heading.
Line 231 uses bold text instead of a markdown heading. Replace with ### to properly structure the section:
-**CRITICAL: These changes REQUIRE extensive manual testing**
-
-Changes to any of these files trigger **MANDATORY manual testing checklist:**
+### CRITICAL: These changes require extensive manual testing
+Changes to any of these files trigger **MANDATORY manual testing checklist:**🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
231-231: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🤖 Prompt for AI Agents
.claude/docs/pr-testing-agent.md around lines 231 to 268: the section currently
uses bold text "**CRITICAL: These changes REQUIRE extensive manual testing**"
but should be a proper markdown heading; replace the bold line with a level-3
heading (### CRITICAL: These changes REQUIRE extensive manual testing) so the
document structure is correct and the section appears in TOC/outline consumers,
ensuring the rest of the checklist remains unchanged underneath.
| ``` | ||
| ❌ CANNOT test build scripts in isolated workspace | ||
| ⚠️ MANUAL TESTING REQUIRED before merge: | ||
| 1. Clone PR branch in full repo environment | ||
| 2. Run complete checklist above | ||
| 3. Document results in PR comment | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add language specification to code block showing environment limitation.
Line 278's code block lacks a language identifier:
-```
+```text
❌ CANNOT test build scripts in isolated workspace
⚠️ MANUAL TESTING REQUIRED before merge:
1. Clone PR branch in full repo environment
2. Run complete checklist above
3. Document results in PR comment
-```
+```🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
278-278: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In .claude/docs/pr-testing-agent.md around lines 278 to 284, the fenced code
block is missing a language identifier; update the opening fence to include the
language tag "text" (i.e., change "```" to "```text") so the block is explicitly
marked as plain text, leaving the closing fence unchanged and ensuring no other
content is modified.
| ``` | ||
| ⚠️ UNTESTED - Requires environment not available in Conductor workspace | ||
|
|
||
| MANUAL TESTING REQUIRED before merge: | ||
| 1. Clone PR in full repo: git clone https://github.com/shakacode/react_on_rails.git | ||
| 2. Checkout PR branch: gh pr checkout <PR-NUMBER> | ||
| 3. Run: [exact commands needed] | ||
| 4. Document results in PR comment | ||
|
|
||
| Cannot proceed without manual verification. | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add language specification to environment limitation code block (line 477).
-```
+```text
⚠️ UNTESTED - Requires environment not available in Conductor workspace
MANUAL TESTING REQUIRED before merge:
1. Clone PR in full repo: git clone https://github.com/shakacode/react_on_rails.git
2. Checkout PR branch: gh pr checkout <PR-NUMBER>
3. Run: [exact commands needed]
4. Document results in PR comment
Cannot proceed without manual verification.
-```
+```🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
477-477: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
.claude/docs/pr-testing-agent.md around lines 477-487: the fenced code block
indicating the environment limitation should specify the language so renderers
treat it as plain text; update the opening fence from ``` to ```text and ensure
the closing fence is a plain ``` (remove any stray diff markers like +/-),
preserving the block content and spacing exactly.
.claude/docs/pr-testing-agent.md
Outdated
| ``` | ||
| Fix webpack CSS modules configuration for Shakapacker 9 | ||
|
|
||
| - Override namedExport: false to maintain compatibility | ||
| - Set exportLocalsConvention to camelCase | ||
|
|
||
| Tested locally: | ||
| - Created debug script to verify loader options | ||
| - Confirmed CSS modules import as default export | ||
| - Verified in browser: styles apply correctly | ||
| - No console errors | ||
|
|
||
| Requires manual verification: | ||
| - Full webpack build in production mode | ||
| - Test in actual app with Shakapacker 9.3.0 | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add language specification to markdown code block in commit message example (line 728).
-```
+```markdown
Fix webpack CSS modules configuration for Shakapacker 9
- Override namedExport: false to maintain compatibility
- Set exportLocalsConvention to camelCase
Tested locally:
- Created debug script to verify loader options
- Confirmed CSS modules import as default export
- Verified in browser: styles apply correctly
- No console errors
Requires manual verification:
- Full webpack build in production mode
- Test in actual app with Shakapacker 9.3.0
-```
+```🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
728-728: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In .claude/docs/pr-testing-agent.md around lines 728 to 743 the fenced code
block for the commit message example is missing a language specifier; change the
opening fence from ``` to ```markdown so the block is marked as Markdown (and
ensure the closing ``` is present and aligned), preserving the exact content
inside the block.
Adds detailed guidance on how to use the PR Testing Agent with Claude Code: Quick Start: - Shows basic prompt patterns for referencing the agent - Examples of asking for specific sections Common Workflows (5 scenarios): 1. Before creating a PR - get testing checklist 2. Validating your testing - check against criteria 3. Generating PR documentation - use templates 4. Investigating CI failures - reproduction steps 5. Code review - identify testing gaps Referencing Specific Sections: - By section number - By change type - By checklist name - By success criteria Complete Example Workflow: - Full end-to-end example from code change to PR creation - Shows actual prompts and expected Claude Code responses - Demonstrates validation and documentation generation Tips for Effective Use: - Be specific vs vague prompts (good/bad examples) - Reference by change type - Ask for validation - Request templates What Claude Code Can/Cannot Do: - Clear list of capabilities and limitations - Sets realistic expectations Automatic Context: - Explains when guidelines are available - No special setup needed This makes the agent immediately actionable for developers using Claude Code. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Splits the 1,172-line combined file into two focused documents: **pr-testing-agent.md** (493 lines) - The "what" and "why": - Agent behavior and principles (skeptical stance, communication style) - Testing requirements by change type (8 sections: Ruby, JS/TS, Build, Webpack, CI, Generators, Playwright E2E, Rails Engine) - Environment-specific testing limitations - Success criteria for well-tested PRs - Agent mindset summary **pr-testing-guide.md** (693 lines) - The "how": - When to use and how to invoke - Using with Claude Code (complete workflow examples) - Integration with existing workflows - Pre-merge testing checklist - Communication templates (PR comments, commit messages) - Real-world testing scenarios (good vs bad examples) - Integration with project tools Benefits: - Each file has a clear, focused purpose - Easier to reference specific parts - Better for both human reading and AI assistant usage - Agent file stays focused on behavior/requirements - Guide file focuses on practical usage Cross-references updated between files for easy navigation. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Summary
Creates a specialized agent definition for comprehensive PR testing validation. The agent is deeply skeptical of testing claims unless verified, assumes automated tests have gaps, and requires clear communication about what was tested versus what needs manual testing.
Pulls all testing information from existing documentation (testing-build-scripts.md, SWITCHING_CI_CONFIGS.md, TESTING_LOCALLY.md, CLAUDE.md) and integrates project-specific tools (bin/ci-rerun-failures, bin/ci-switch-config, etc.).
Key Features
Pull Request checklist
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.