Ensure CI runs on uncategorized changes

[justin808]

Summary

• treat uncategorized changes as non-docs so CI always runs on master
• default to running the full suite when detection misses and log it

Pull Request checklist

• Add/update test to cover these changes
• Update documentation
• Update CHANGELOG file

Other Information

None.

Summary by CodeRabbit

• Chores
  • Improved CI pipeline robustness by detecting uncategorized file changes and automatically running all CI jobs (lint, tests, generators) when such changes are encountered.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

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 51 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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.

📥 Commits

Reviewing files that changed from the base of the PR and between 90f6310 and 5caded5.

📒 Files selected for processing (1)

• script/ci-changes-detector (5 hunks)

Walkthrough

The script/ci-changes-detector script now tracks uncategorized file changes via a new UNCATEGORIZED_CHANGED flag. When enabled, it triggers all CI jobs (lint, tests, generators, and pro variants), ensuring comprehensive testing for changes that don't match existing file categories.

Changes

Cohort / File(s)	Change Summary
CI Change Detection Logic script/ci-changes-detector	Added UNCATEGORIZED_CHANGED flag initialization, logic to set flag when changed files don't match any category, output flag in "Changed file categories" section, and conditional logic to enable all CI jobs when uncategorized changes are detected.

Sequence Diagram

sequenceDiagram
    participant Script as ci-changes-detector
    participant Logic as Category Matcher
    participant Decision as CI Decision Engine
    participant Output as RUN_* Flags

    Script->>Logic: Process changed files
    activate Logic
    Logic->>Logic: Match against categories
    alt File matches category
        Logic->>Script: Set corresponding flag
    else File doesn't match
        Logic->>Script: Set UNCATEGORIZED_CHANGED=true
    end
    deactivate Logic

    Script->>Decision: Check all flags including UNCATEGORIZED_CHANGED
    activate Decision
    alt UNCATEGORIZED_CHANGED is true
        Decision->>Output: Enable all RUN_* flags<br/>(lint, ruby tests, js tests,<br/>dummy tests, generators,<br/>pro-lint, pro tests, pro-dummy)
    else Only specific flags set
        Decision->>Output: Enable corresponding<br/>RUN_* flags
    end
    deactivate Decision

    Output->>Output: Export via JSON &<br/>GITHUB_OUTPUT

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• The change spans multiple logical sections of the script (flag initialization, category matching, output, and decision logic)
• New behavior affects all CI jobs, warranting careful verification of triggering conditions
• Consider verifying that UNCATEGORIZED_CHANGED correctly identifies truly uncategorized files vs. intentionally excluded categories

Suggested labels

review-needed, full-ci

Suggested reviewers

• AbanoubGhadban
• alexeyr-ci2

Poem

🐰 Files unmatched now have their say,
UNCATEGORIZED_CHANGED leads the way!
All CI jobs will run so true,
When mystery changes hop on through! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Ensure CI runs on uncategorized changes' directly aligns with the main change: adding UNCATEGORIZED_CHANGED flag logic to trigger all CI jobs when unmatched file changes are detected.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

script/ci-changes-detector (1)

142-146: Excellent safety net for unmatched file patterns.

The default case correctly implements the fail-safe behavior: any file not explicitly categorized will trigger the full CI suite. This ensures no changes slip through without proper testing, which is especially important for:

• Changes to CI infrastructure files (like this script)
• New file types added to the repository
• Configuration files not yet categorized

The comment clearly documents the intent, and setting both DOCS_ONLY=false and UNCATEGORIZED_CHANGED=true is the right approach.

Optional consideration: If certain files genuinely need no CI (e.g., .gitignore, .gitattributes, LICENSE, editor configs like .vscode/*), you could add an explicit "skip" category before the default case. However, the current conservative approach is safer and the performance impact is likely negligible.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 3eb3961 and 90f6310.

📒 Files selected for processing (1)

• script/ci-changes-detector (4 hunks)

⏰ 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). (8)

• GitHub Check: rspec-package-tests (3.4, latest)
• GitHub Check: examples (3.4, latest)
• GitHub Check: build-dummy-app-webpack-test-bundles (3.4, 22, latest)
• GitHub Check: pro-lint-js-and-ruby
• GitHub Check: build-dummy-app-webpack-test-bundles
• GitHub Check: build-dummy-app-webpack-test-bundles
• GitHub Check: build
• GitHub Check: claude-review

🔇 Additional comments (3)

script/ci-changes-detector (3)

67-67: LGTM: Flag initialization follows established patterns.

The UNCATEGORIZED_CHANGED flag is properly initialized alongside other change detection flags, maintaining consistency with the existing codebase structure.

---

215-215: Clear and informative output message.

The output line maintains consistency with other category messages and clearly communicates both what happened (uncategorized changes detected) and what action will be taken (running full suite). This transparency is valuable for debugging CI behavior.

---

262-272: Robust fail-safe logic that enables comprehensive testing.

This conditional block properly implements the core requirement: when changes don't match any explicit category, run all CI jobs to ensure nothing is missed. The approach is:

• Comprehensive: All 8 CI job flags are enabled, covering both main and Pro variants
• Well-placed: Executes after individual category logic, acting as an override
• Well-documented: The comment clearly explains the defensive strategy

This guarantees CI reliability on the master branch even when the change detection encounters unexpected files.

[claude]

Code Review - PR #2079: Ensure CI runs on uncategorized changes

Summary

This PR adds a critical safety net to the CI changes detector script by introducing an UNCATEGORIZED_CHANGED flag that catches any files not explicitly handled by the case statement and ensures full CI runs for them. This is a defensive approach that prevents silent failures from unrecognized file types.

---

✅ Strengths

1. Excellent defensive programming:

• The *) catch-all case prevents the script from silently categorizing unknown files as "docs-only"
• This directly addresses the risk mentioned in the master health monitoring docs about silent failures

2. Appropriate scope:

• Running the full test suite for uncategorized files is the correct conservative choice
• The explicit logging message (Uncategorized changes (running full suite for safety)) makes the behavior transparent

3. Clean implementation:

• Minimal code changes (19 additions)
• Follows the existing pattern for other category flags
• No performance impact - just one additional condition check

4. Consistent with project's defensive stance:

• Aligns with CLAUDE.md guidance on "NEVER wait for CI to verify fixes"
• Addresses the circular dependency problem mentioned in master-health-monitoring.md

---

🔍 Potential Improvements

1. Missing test coverage consideration:

While the PR checklist marks "Add/update test to cover these changes" as not applicable, consider whether this script should have tests. Given its critical role in CI orchestration, a test suite could prevent regressions:

# Example test cases:
# - Verify known file types are correctly categorized
# - Verify uncategorized files trigger UNCATEGORIZED_CHANGED=true
# - Verify UNCATEGORIZED_CHANGED sets all RUN_* flags to true

Recommendation: Not required for this PR, but consider adding integration tests in a follow-up PR.

2. Logging for troubleshooting:

When UNCATEGORIZED_CHANGED=true is set, it would be helpful to log WHICH specific files triggered it, especially for debugging:

# In the *) case:
DOCS_ONLY=false
UNCATEGORIZED_CHANGED=true
echo "[DEBUG] Uncategorized file: $file" >&2  # Could be opt-in via DEBUG env var
;;

This would help developers understand why full CI was triggered and potentially identify files that should be explicitly categorized.

3. Documentation gap:

The change isn't mentioned in CHANGELOG.md, but per the guidelines, this is likely correct since it's an infrastructure improvement rather than a user-facing change. However, it might be worth a brief mention in the commit message or PR description about why this was needed (e.g., "Prevents edge cases where new file types could be misclassified as docs-only").

---

🔐 Security Considerations

✅ No security concerns. The script:

• Doesn't execute user input
• Only reads git metadata
• Properly handles empty/zero-SHA cases (lines 12-14)
• Uses set -euo pipefail for safe error handling

---

🚀 Performance Considerations

✅ No performance impact. The changes:

• Add one boolean flag check (O(1))
• Add one additional case branch (executes once per file)
• Don't introduce any loops or expensive operations

---

🐛 Potential Bugs

None identified. The implementation is straightforward and follows bash best practices:

• Proper boolean flag initialization (line 67)
• Consistent conditional structure (lines 263-272)
• Proper output formatting (line 215)

---

📋 Testing Recommendations

Manual testing to verify:

1. Test with an uncategorized file:

# Create a file that isn't matched by existing patterns
touch .obscure-config-file
git add .obscure-config-file
script/ci-changes-detector origin/master HEAD
# Should show: "• Uncategorized changes (running full suite for safety)"
# Should set all RUN_* flags to true

2. Verify JSON output:

CI_JSON_OUTPUT=1 script/ci-changes-detector origin/master HEAD | jq .
# All flags should be true when UNCATEGORIZED_CHANGED is triggered

3. Test in actual CI workflow:

• Verify that GitHub Actions receives the correct outputs
• Confirm all CI jobs run when uncategorized files are present

---

🎯 Recommendation

✅ APPROVE with suggestions for follow-up improvements.

This PR successfully addresses a critical gap in the CI detection logic. The implementation is clean, safe, and follows existing patterns. The suggestions above are nice-to-haves that could be addressed in future PRs:

1. Optional: Add integration tests for the script
2. Optional: Add debug logging to show which files triggered uncategorized state
3. Consider: Brief mention in commit message about the motivation

The defensive approach of running full CI for uncategorized changes is exactly the right call, especially given the project's emphasis on not letting master stay broken.

---

Great work on catching this edge case! 🎉