Skip to content

chore: expand imperative verb whitelist and improve validation logic - #527

Merged
shenxianpeng merged 3 commits into
mainfrom
claude/refresh-sample-output-602anc
Aug 5, 2026
Merged

chore: expand imperative verb whitelist and improve validation logic#527
shenxianpeng merged 3 commits into
mainfrom
claude/refresh-sample-output-602anc

Conversation

@shenxianpeng

@shenxianpeng shenxianpeng commented Aug 5, 2026

Copy link
Copy Markdown
Member

Summary

Extended the imperative verb whitelist in commit_check/imperatives.py with 100+ additional verbs and added comprehensive test coverage to ensure correct imperative forms are not rejected while maintaining validation of incorrect verb forms.

Key Changes

  • Expanded IMPERATIVES set: Added 100+ new imperative verbs including:

    • Common verbs previously missing: be, do, die, let, work, teach, treat
    • British English spellings (e.g., normalise, prioritise, serialise) alongside American variants
    • Technical/domain-specific verbs: deduplicate, hoist, inline, paginate, shard, stub, subclass
    • Alphabetically sorted the complete list for maintainability
  • Updated documentation: Clarified the philosophy in comments:

    • Explained why both -ize and -ise spellings are included
    • Noted that gaps in the whitelist are bugs in the file, not author mistakes
    • Emphasized that false negatives (rejecting correct subjects) are worse than false positives
  • Added test coverage:

    • New test_correct_imperative_subjects_are_not_rejected() test with 12 real-world examples that were previously rejected
    • New test_wrong_verb_forms_still_fail() test to ensure the validation still catches actual errors (past tense, gerunds, etc.)
    • Tests document the expected behavior and prevent regression

Implementation Details

The changes maintain backward compatibility while significantly improving the validator's accuracy. The expanded list now covers most common imperative verbs encountered in commit messages, reducing false rejections while the new tests ensure the rule still catches genuinely incorrect verb forms.

https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn

Summary by CodeRabbit

  • Enhancements
    • Expanded accepted imperative verbs in commit subject validation.
    • Added support for both British “-ise” and American “-ize” spellings.
    • Improved handling of valid imperative subjects while continuing to reject incorrect verb forms.
  • Tests
    • Added coverage for new verbs, spelling variants, and regression cases.

CC003 matches the subject's first word against a fixed list, so a verb missing
from it rejects a subject that was written correctly. The contributor then has
to reword something that was never wrong, and the only way to learn which words
are acceptable is trial and error.

Measured against 40,000 git.git subjects — a project that writes strictly
imperative subjects, with its area prefixes stripped so only the verb is
compared — the list rejected 17.9% of them. This brings that to 10.5%.

The additions come from that corpus, filtered by hand to unambiguous verbs:
avoid, clarify, teach, factor, free, mention, plug, inline, restore, retire,
treat, tighten and the rest. Adverb-led subjects ("always quote the path") and
non-verbs were left out — the first are correct English the list cannot
represent, and the second would weaken the check.

Both spellings of every -ize/-ise verb are now present. The file had sixteen
-ize verbs and four -ise ones, so a project writing British English was being
told "normalise" is not imperative.

Wrong forms still fail, which is the property that matters: "updated the
parser", "adding a new flag", "fixes the crash" and "removed the dead code" are
covered by a test alongside the recovered words.

The remaining 10.5% is the point of the follow-up issue: a list can only ever
approximate "is this an English imperative verb".

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn
@shenxianpeng
shenxianpeng requested a review from a team as a code owner August 5, 2026 09:04
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Changes

Imperative subject validation

Layer / File(s) Summary
Expand imperative vocabulary
commit_check/imperatives.py
The IMPERATIVES constant adds many imperative verbs and British and American spelling variants.
Validate imperative subjects
tests/engine_test.py
Tests accept the added forms and reject past-tense, gerund, and third-person forms.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related issues

Possibly related PRs

Suggested labels: enhancement, tests

Suggested reviewers: lntutor

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the whitelist expansion and related validation coverage in the pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch claude/refresh-sample-output-602anc

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.

They assert which subjects are accepted, not how fast the check runs, and
marking them adds twelve identical-cost cases to what CodSpeed measures. The
companion test for wrong verb forms was already unmarked, so the pair was
inconsistent as well.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
tests/engine_test.py (1)

1356-1371: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Exercise a newly added spelling pair in this test.

serialise already exists in commit_check/imperatives.py at Lines 439-440. This case does not verify the vocabulary expansion. The test also does not cover both spellings of a newly added -ise/-ize pair. Replace it with customise and customize, added at Lines 98-99.

Suggested test update
-            "refactor: serialise the payload",
+            "refactor: customise the payload",
+            "refactor: customize the payload",
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/engine_test.py` around lines 1356 - 1371, Update the imperative-message
test cases by replacing the existing “serialise” entry with both “customise” and
“customize,” using the corresponding imperative phrases. Keep the cases in the
rejected-message list and preserve the existing spelling-pair coverage intent.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@tests/engine_test.py`:
- Around line 1356-1371: Update the imperative-message test cases by replacing
the existing “serialise” entry with both “customise” and “customize,” using the
corresponding imperative phrases. Keep the cases in the rejected-message list
and preserve the existing spelling-pair coverage intent.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 34687327-1676-4f7e-9650-f2418781a3f6

📥 Commits

Reviewing files that changed from the base of the PR and between 3ba4f08 and 7239ac8.

📒 Files selected for processing (2)
  • commit_check/imperatives.py
  • tests/engine_test.py

@codecov

codecov Bot commented Aug 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.68%. Comparing base (5c39e5f) to head (5289553).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #527      +/-   ##
==========================================
+ Coverage   97.40%   97.68%   +0.28%     
==========================================
  Files          12       12              
  Lines        1231     1254      +23     
==========================================
+ Hits         1199     1225      +26     
+ Misses         32       29       -3     

☔ View full report in Codecov by Harness.
📢 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.

The British-spelling case used serialise, which the file already contained
before this change, so it proved nothing about the addition. customise is the
useful one: customize was listed and customise was not, which is exactly the
half-a-pair bug the sweep was meant to close. Both spellings are now asserted.

Reported by CodeRabbit on #527, and correct — verified against the merge base
that serialise predates the change while customise does not.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn
@sonarqubecloud

sonarqubecloud Bot commented Aug 5, 2026

Copy link
Copy Markdown

@shenxianpeng shenxianpeng changed the title Expand imperative verb whitelist and improve validation logic chore: expand imperative verb whitelist and improve validation logic Aug 5, 2026
@shenxianpeng
shenxianpeng merged commit 1072b87 into main Aug 5, 2026
27 checks passed
@shenxianpeng
shenxianpeng deleted the claude/refresh-sample-output-602anc branch August 5, 2026 09:20
@codspeed-hq

codspeed-hq Bot commented Aug 5, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by ×2.9

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 1 improved benchmark
✅ 437 untouched benchmarks
⏩ 121 skipped benchmarks1

Performance Changes

Benchmark BASE HEAD Efficiency
test_empty_message_passes 6.7 ms 2.3 ms ×2.9

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.


Comparing claude/refresh-sample-output-602anc (5289553) with main (a0d3f77)2

Open in CodSpeed

Footnotes

  1. 121 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.

  2. No successful run was found on main (3ba4f08) during the generation of this report, so a0d3f77 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

shenxianpeng added a commit that referenced this pull request Aug 7, 2026
The pin was v2.13.0, whose requirements.txt installs commit-check 2.13.1.
That engine cannot resolve a rebase target existing only as origin/main in
a pull request checkout, so it reported "not rebased" for a branch that
was -- the false CC202 this workflow has carried since its first run.

v2.13.1 of the action installs commit-check 2.13.4, which carries the
merge-base fix from #532. Confirmed by reading requirements.txt at the tag
rather than assuming the action version tracks the engine version:

    v2.13.0 (124de73) -> commit-check==2.13.1
    v2.13.1 (562a184) -> commit-check==2.13.4

The branch was already rebased before this change -- git merge-base
--is-ancestor origin/main HEAD returned true while CC202 still failed --
so the failure was the engine, not the branch, and rebasing again could
never have fixed it.

Two things ride along on the newer engine. Skipped checks now report as
skipped rather than as passes (#537), so a run bypassed by ignore_authors
says so instead of showing green ticks over nothing. And the imperative
whitelist goes from 396 verbs to 529, retiring a class of false CC003 --
2.13.1 rejected "treat", which #527 had added three releases earlier.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01U9zFxq8V4qxG4aMzJhGBFn
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant