fix(subject_imperative): mixed definitions by StaticRocket · Pull Request #329 · commit-check/commit-check · GitHub

Following the documentation the old imperative check was renamed to subject_imperative. Replace all old definitions with the new name to match documented behavior.

Summary by CodeRabbit

Refactor
- Renamed a commit rule identifier from "imperative" to "subject_imperative"; validation behavior, messages, and outcomes remain unchanged.
Tests
- Updated test cases and expectations to reference the renamed rule so validation results stay consistent and passing.
Chores
- Simplified CI checkout steps to rely on default branch/commit selection.

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

Walkthrough

Renamed the commit-check identifier from "imperative" to "subject_imperative" across rule definitions, validator mapping, requested checks, and tests; no validation logic, regexes, or messages were changed.

Changes

Cohort / File(s)	Change Summary
Rule catalog commit_check/rules_catalog.py	Renamed COMMIT_RULES entry key from "imperative" to "subject_imperative" (other fields unchanged).
Validation engine mapping commit_check/engine.py	Updated ValidationEngine.VALIDATOR_MAP to use "subject_imperative" as the key for SubjectImperativeValidator.
Main/requested checks commit_check/main.py	Replaced "imperative" with "subject_imperative" in the list of requested commit-checks when message checks are enabled.
Tests tests/engine_test.py, tests/engine_comprehensive_test.py	Updated tests to use ValidationRule(check="subject_imperative") and to expect VALIDATOR_MAP mapping under "subject_imperative" instead of "imperative".
CI workflow .github/workflows/main.yml	Removed explicit with: ref: ${{ github.head_ref }} from two actions/checkout steps (build and docs jobs).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Verify no remaining references to "imperative" in code, tests, docs.
Run full test suite to confirm validator lookups and CLI behavior.
Inspect changelog/docs for mentions of the old key.

Possibly related PRs

fix: update regex to support breaking changes #305 — modifies SubjectImperativeValidator regex (breaking-change ! handling); related because it changes the same validator's behavior while this PR renames its rule key.
refactor: reduce functions cognitive complexity #276 — refactors imperative-check lookup/printing; related due to overlap in the rule identifier and lookup behavior.

Suggested labels

bug

Poem

🐰 I hopped through lines with a curious twitch,
Swapped a name in the rules with a tiny switch,
"subject_imperative" now hopping in place,
I nibble a carrot and guard the code space,
Soft thumps of tests — a cozy, tidy stitch.

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 accurately reflects the primary change: renaming 'imperative' to 'subject_imperative' across the codebase to resolve inconsistency between documentation and implementation.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches 🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 14e883c and 19280cb.

📒 Files selected for processing (1)

.github/workflows/main.yml (0 hunks)

💤 Files with no reviewable changes (1)

.github/workflows/main.yml

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

GitHub Check: install (3.14, macos-latest)
GitHub Check: install (3.14, ubuntu-24.04)
GitHub Check: install (3.13, windows-latest)
GitHub Check: install (3.11, windows-latest)
GitHub Check: install (3.13, ubuntu-24.04)
GitHub Check: install (3.14, windows-latest)
GitHub Check: install (3.10, macos-latest)
GitHub Check: install (3.11, macos-latest)
GitHub Check: install (3.10, ubuntu-24.04)
GitHub Check: install (3.12, windows-latest)
GitHub Check: install (3.9, ubuntu-24.04)
GitHub Check: install (3.9, macos-latest)
GitHub Check: install (3.12, macos-latest)
GitHub Check: install (3.12, ubuntu-24.04)
GitHub Check: install (3.11, ubuntu-24.04)
GitHub Check: install (3.9, windows-latest)
GitHub Check: install (3.10, windows-latest)
GitHub Check: Run benchmarks

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

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

✅ Deploy Preview for commit-check ready!

Name	Link
🔨 Latest commit	19280cb
🔍 Latest deploy log	https://app.netlify.com/projects/commit-check/deploys/6941f006fef3c30008e23c1c
😎 Deploy Preview	https://deploy-preview-329--commit-check.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c792143 and 98c420a.

📒 Files selected for processing (1)

commit_check/rules_catalog.py (1 hunks)

🧰 Additional context used 🧠 Learnings (4) 📓 Common learnings

Learnt from: CR
Repo: commit-check/commit-check PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-03T10:28:06.793Z
Learning: Applies to .commit-check.yml : Maintain default validation rules for the tool in .commit-check.yml

📚 Learning: 2025-10-03T10:28:06.793Z

Learnt from: CR
Repo: commit-check/commit-check PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-03T10:28:06.793Z
Learning: Applies to .commit-check.yml : Maintain default validation rules for the tool in .commit-check.yml

Applied to files:

commit_check/rules_catalog.py

📚 Learning: 2025-10-03T10:28:06.793Z

Learnt from: CR
Repo: commit-check/commit-check PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-03T10:28:06.793Z
Learning: Applies to commit_check/commit.py : Enforce Conventional Commits format: type(scope): description

Applied to files:

commit_check/rules_catalog.py

📚 Learning: 2025-10-03T10:28:06.793Z

Learnt from: CR
Repo: commit-check/commit-check PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-03T10:28:06.793Z
Learning: Applies to commit_check/commit.py : Support breaking change notation with ! (e.g., feat!: breaking change)

Applied to files:

commit_check/rules_catalog.py

⏰ 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: Run benchmarks

CodSpeed Performance Report

Merging #329 will not alter performance

Comparing StaticRocket:bugfix/subject_imperative (19280cb) with main (22cf07a)

Summary

✅ 167 untouched
⏩ 85 skipped1

85 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

Actionable comments posted: 0

🧹 Nitpick comments (2)

commit_check/main.py (1)
147-166: Consider temporary backward-compat alias for legacy imperative configs.

If users still have configs referencing imperative (pre-migration), this rename can effectively disable the validator (unknown check → skipped). If you want smoother upgrades, consider accepting both keys for a release (e.g., request both in CLI and/or alias in the engine/catalog).
-                    "subject_imperative",
+                    "subject_imperative",
+                    "imperative",  # backward-compat alias (optional)
commit_check/engine.py (1)
522-540: Optional: keep imperative as an alias to avoid silent skips.

Given unknown checks are silently skipped, legacy imperative configs could unintentionally bypass validation. If backward compatibility is desired:
     VALIDATOR_MAP: Dict[str, Type[BaseValidator]] = {
         "message": CommitMessageValidator,
         "subject_capitalized": SubjectCapitalizationValidator,
         "subject_imperative": SubjectImperativeValidator,
+        "imperative": SubjectImperativeValidator,  # backward-compat alias (optional)
         "subject_max_length": SubjectLengthValidator,

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 98c420a and fab3e0a.

📒 Files selected for processing (4)

commit_check/engine.py (1 hunks)
commit_check/main.py (1 hunks)
commit_check/rules_catalog.py (1 hunks)
tests/engine_test.py (5 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

commit_check/rules_catalog.py

🧰 Additional context used 📓 Path-based instructions (2) commit_check/main.py

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

commit_check/main.py: Expose CLI flags: --message, --branch, --help, --version, --config, --dry-run; support combining checks
Exit with code 0 on success and 1 on validation failure; emit colorized ASCII-art rejection on errors

Files:

commit_check/main.py

tests/**

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Ensure tests run via pytest -v and cover commit, branch, author, and CLI behaviors

Files:

tests/engine_test.py

🧠 Learnings (6) 📓 Common learnings

Learnt from: CR
Repo: commit-check/commit-check PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-03T10:28:06.793Z
Learning: Applies to .commit-check.yml : Maintain default validation rules for the tool in .commit-check.yml

📚 Learning: 2025-10-03T10:28:06.793Z

Learnt from: CR
Repo: commit-check/commit-check PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-03T10:28:06.793Z
Learning: Applies to commit_check/commit.py : Support breaking change notation with ! (e.g., feat!: breaking change)

Applied to files:

commit_check/main.py
tests/engine_test.py

📚 Learning: 2025-10-03T10:28:06.793Z

Learnt from: CR
Repo: commit-check/commit-check PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-03T10:28:06.793Z
Learning: Applies to commit_check/main.py : Expose CLI flags: --message, --branch, --help, --version, --config, --dry-run; support combining checks

Applied to files:

commit_check/main.py

📚 Learning: 2025-10-03T10:28:06.793Z

Learnt from: CR
Repo: commit-check/commit-check PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-03T10:28:06.793Z
Learning: Applies to commit_check/commit.py : Enforce Conventional Commits format: type(scope): description

Applied to files:

commit_check/main.py

📚 Learning: 2025-10-03T10:28:06.793Z

Learnt from: CR
Repo: commit-check/commit-check PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-03T10:28:06.793Z
Learning: Applies to commit_check/commit.py : Provide special handling for merge commit messages

Applied to files:

commit_check/main.py

📚 Learning: 2025-10-03T10:28:06.793Z

Learnt from: CR
Repo: commit-check/commit-check PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-10-03T10:28:06.793Z
Learning: Applies to commit_check/commit.py : Support the set of commit types: build, chore, ci, docs, feat, fix, perf, refactor, revert, style, test

Applied to files:

commit_check/main.py

🧬 Code graph analysis (1) tests/engine_test.py (1)

commit_check/rule_builder.py (1)

ValidationRule (14-40)

⏰ 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: Run benchmarks

🔇 Additional comments (3)

commit_check/main.py (1)

147-166: Good alignment: CLI now requests subject_imperative under --message.

This matches the PR objective (catalog/config key consistency), so the check can be disabled via config as expected. As per learnings, CLI exposure/combining-checks behavior remains intact.

commit_check/engine.py (1)

522-540: Validator map rename to subject_imperative is correct and consistent.

This completes the end-to-end rename (rules → engine → CLI/tests), enabling config-based disabling to work.

tests/engine_test.py (1)

811-864: Tests updated to exercise the renamed check key (subject_imperative).

These cases now correctly bind to ValidationEngine.VALIDATOR_MAP["subject_imperative"] and validate the same behavior as before.

You know what, this cannot be toggled because the config logic right now doesn't match the catalog. Seems like everything else is using the old "imperative" definition, but it is a little weird how little the catalog is actually used right now. Most of the test files seem to define their own version of the check locally, which is still using the old name.

Do you want to update the config structure to match current tests or adjust all the existing references to match the current documentation?

Thank you for raising the PR. I think it would be better to rename all existing imperative checks to subject_imperative to align with the documentation. Could you also help fix the CI failures?

Following the documentation the old imperative check was renamed to subject_imperative. Replace all old definitions with the new name to match documented behavior. Fixes: 9f90bbd ("feat: Breaking Changes & Configuration Migration (v2 version) (commit-check#280)") Signed-off-by: Randolph Sapp <rs@ti.com>

Sure, I'll take a look. Not sure how to address that reported performance regression though. Seems like a false positive or something.

Ref pinning will incorrectly try to check out the branch name on the project origin, not the pull request origin. Fixes: 6a7a35c ("feat: inital commit") Signed-off-by: Randolph Sapp <rs@ti.com>

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.75%. Comparing base (22cf07a) to head (19280cb).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #329      +/-   ##
==========================================
+ Coverage   87.46%   87.75%   +0.28%     
==========================================
  Files           8        8              
  Lines         694      694              
==========================================
+ Hits          607      609       +2     
+ Misses         87       85       -2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@shenxianpeng was that all that you needed addressed?

The performance regression does not need to be addressed, as it may be a false positive.

Thank you

StaticRocket requested a review from a team as a code owner December 11, 2025 23:42

StaticRocket requested review from shenxianpeng and removed request for a team December 11, 2025 23:42

StaticRocket mentioned this pull request Dec 11, 2025

docs(readme): simplify the readme and example make TexasInstruments/processor-sdk-doc#578

Merged

coderabbitai Bot reviewed Dec 11, 2025

View reviewed changes

Comment thread commit_check/rules_catalog.py

StaticRocket force-pushed the bugfix/subject_imperative branch from 98c420a to fab3e0a Compare December 12, 2025 00:05

coderabbitai Bot reviewed Dec 12, 2025

View reviewed changes

StaticRocket force-pushed the bugfix/subject_imperative branch from fab3e0a to ee940f6 Compare December 12, 2025 00:23

StaticRocket changed the title fix(subject_imperative): catalog entry fix(subject_imperative): mixed definitions Dec 12, 2025

This was referenced Dec 12, 2025

Update LPM Performance Measurements for 11.02 TexasInstruments/processor-sdk-doc#577

Merged

feat(linux): Switch from Neo-AI-DLR to TVM-RT TexasInstruments/processor-sdk-doc#581

Merged

shenxianpeng force-pushed the bugfix/subject_imperative branch from ee940f6 to bcbe201 Compare December 15, 2025 09:59

StaticRocket force-pushed the bugfix/subject_imperative branch from bcbe201 to 14e883c Compare December 16, 2025 23:40

ci(main): do not ref pin when checking prs

19280cb

Ref pinning will incorrectly try to check out the branch name on the project origin, not the pull request origin. Fixes: 6a7a35c ("feat: inital commit") Signed-off-by: Randolph Sapp <rs@ti.com>

shenxianpeng approved these changes Dec 17, 2025

View reviewed changes

shenxianpeng merged commit d356a6e into commit-check:main Dec 17, 2025
33 checks passed

shenxianpeng added the bug Something isn't working label Dec 17, 2025

coderabbitai Bot mentioned this pull request Aug 8, 2026

feat: check imperative mood by form rather than by vocabulary #540

Merged

fix(subject_imperative): mixed definitions by StaticRocket · Pull Request #329 · commit-check/commit-check · GitHub

Uh oh!

Uh oh!

fix(subject_imperative): mixed definitions - #329

fix(subject_imperative): mixed definitions#329shenxianpeng merged 2 commits intocommit-check:maincommit-check/commit-check:mainfrom StaticRocket:bugfix/subject_imperativeStaticRocket/commit-check:bugfix/subject_imperativeCopy head branch name to clipboard

Conversation

StaticRocket commented Dec 11, 2025 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Poem

Pre-merge checks and finishing touches

Uh oh!

netlify Bot commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for commit-check ready!

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codspeed-hq Bot commented Dec 11, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

CodSpeed Performance Report

Merging #329 will not alter performance

Summary

Footnotes

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

StaticRocket commented Dec 12, 2025

Uh oh!

shenxianpeng commented Dec 14, 2025

Uh oh!

StaticRocket commented Dec 16, 2025

Uh oh!

sonarqubecloud Bot commented Dec 16, 2025

Quality Gate passed

Uh oh!

codecov Bot commented Dec 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

StaticRocket commented Dec 16, 2025

Uh oh!

shenxianpeng commented Dec 17, 2025

Uh oh!

shenxianpeng left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

fix(subject_imperative): mixed definitions#329
shenxianpeng merged 2 commits into
commit-check:mainfrom
StaticRocket:bugfix/subject_imperative

StaticRocket commented Dec 11, 2025 •
edited by coderabbitai Bot
Loading

coderabbitai Bot commented Dec 11, 2025 •
edited
Loading

netlify Bot commented Dec 11, 2025 •
edited
Loading

codspeed-hq Bot commented Dec 11, 2025 •
edited
Loading

codecov Bot commented Dec 16, 2025 •
edited
Loading