Skip to content

fix: print the display name in compact output, not the config key - #529

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

fix: print the display name in compact output, not the config key#529
shenxianpeng merged 2 commits into
mainfrom
claude/refresh-sample-output-602anc

Conversation

@shenxianpeng

@shenxianpeng shenxianpeng commented Aug 5, 2026

Copy link
Copy Markdown
Member

Fixes #528.

What

The same failing check is named two ways depending on the format:

$ echo "docs: revamped the profile" | commit-check -m --subject-imperative=true --no-banner
CC003 subject-imperative check failed ==> docs: revamped the profile

$ echo "docs: revamped the profile" | commit-check -m --subject-imperative=true --compact
[FAIL] CC003 subject_imperative: docs: revamped the profile

Both are text written for a person, so they should agree.

Why kebab, and why JSON stays as it is

The two conventions each have a job, and only one surface was on the wrong side of the line:

Surface Form Converts?
Default text subject-imperative yes, in print_error_message
commit-check-action step log, job summary, PR comment subject-imperative yes, in its own _rule_label
--format json, check field subject_imperative no — and correctly so
--compact subject_imperative no

Kebab is the form the rules reference titles each rule with (### subject-imperative (CC003)), so a name printed to a terminal can be pasted into the docs and found. That is already why the default output converts.

The JSON check field is deliberately untouched: it carries the snake_case key so a consumer can map a failure back to the cchk.toml option that controls it. commit-check-action depends on exactly that — it takes the raw key and converts it itself for display, which is the clearest evidence that the display form is meant to be kebab throughout.

The change

Both call sites now go through one display_name() helper instead of one of them open-coding .replace("_", "-"). That open-coding is how the two drifted apart in the first place, so a shared helper is the part that stops it recurring.

Compatibility

--compact landed in 2.6.0 and is described as being for CI logs. The format of the line is unchanged — only the check name's spelling. The machine-readable path, --format json, is untouched.

The one risk worth naming: anyone grepping subject_imperative out of --compact output would need to grep subject-imperative instead. Whether that is worth a minor or a major is your call — I have no way to see how the flag is used in the wild.

Test plan

A new test asserts the form both text outputs print. The suite had no such assertion, which is how this shipped: test_compact_shows_one_line_per_failure only checks that each line starts with [FAIL].

Verified by reverting the fix and confirming the test fails:

E  assert 'CC003 subject-imperative:' in '[FAIL] CC003 subject_imperative: docs: revamped the profile\n'

Worth noting, because my first attempt at this test passed against the broken code: it asserted on the default -m run, whose failing check is message — a name with no underscore, so the two forms are identical and the assertion could not discriminate. The test now drives --subject-imperative=true specifically to get a name that differs between the forms.

Full suite: 506 passed. Two failures are pre-existing on unmodified main in this environment and unrelated to this change — test_load_config_file_permission_error (does not raise when the suite runs as root) and TestAiAttributionValidator::test_empty_message_passes.

Note on the branch name

AGENTS.md asks for a Conventional Branch type, and claude/ is not in the list it gives — my session is pinned to this branch name. The repository's own cchk.toml sets conventional_branch = false, so CI does not enforce it, and claude/ is in the tool's own default allow-list. Happy to move it if you would rather it read fix/.


Generated by Claude Code

Summary by CodeRabbit

  • Bug Fixes

    • Check names are now displayed consistently in kebab-case across compact failure output and standard error messages.
    • Corrected output for checks configured with underscore-based names.
  • Tests

    • Added regression coverage to verify compact output uses the expected check names.

The same failing check is named two ways depending on the format:

    $ commit-check -m --subject-imperative=true --no-banner
    CC003 subject-imperative check failed ==> docs: revamped the profile

    $ commit-check -m --subject-imperative=true --compact
    [FAIL] CC003 subject_imperative: docs: revamped the profile

Both are text written for a person, so they should agree. The kebab-case
form is the one that agrees with the rules reference, whose headings read
`subject-imperative (CC003)` — so a name printed to a terminal can be
searched for there verbatim, which is why the default output already
converts.

Every other human-facing surface in the ecosystem does the same:
commit-check-action converts the JSON `check` field before showing it in
step logs, job summaries and pull request comments. Compact was the only
one left printing the raw key.

The JSON output is deliberately unchanged. Its `check` field carries the
snake_case key so a consumer can map a failure back to the `cchk.toml`
option that controls it, which is what the action relies on.

Both call sites now go through one helper rather than one of them
open-coding the conversion, which is how they drifted apart to begin
with.

Refs #528
@shenxianpeng
shenxianpeng requested a review from a team as a code owner August 5, 2026 23:27
@github-actions github-actions Bot added the bug Something isn't working label Aug 5, 2026
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@shenxianpeng, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 51 minutes

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 954db714-d578-4db4-9fb2-befe8c67323e

📥 Commits

Reviewing files that changed from the base of the PR and between 99a7d71 and edd9cb6.

📒 Files selected for processing (2)
  • commit_check/rules_catalog.py
  • commit_check/util.py
📝 Walkthrough

Walkthrough

The change adds a shared display_name helper that converts check names from snake_case to kebab-case. Compact failure labels and standard error messages use the helper. A regression test verifies compact output for subject-imperative.

Changes

Check name display

Layer / File(s) Summary
Shared display-name formatter
commit_check/util.py
Adds the public display_name helper for converting underscores to hyphens.
Output integration and regression coverage
commit_check/util.py, tests/main_test.py
Uses the helper for compact failure labels and standard error messages. Tests verify kebab-case compact output.

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

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: using display names instead of configuration keys in compact output.
Linked Issues check ✅ Passed The changes satisfy issue #528 by using kebab-case names for compact output while preserving JSON configuration keys.
Out of Scope Changes check ✅ Passed The helper refactor and regression test directly support the requested compact-output formatting change.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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.

@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.69%. Comparing base (3ba4f08) to head (edd9cb6).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #529      +/-   ##
==========================================
+ Coverage   97.60%   97.69%   +0.08%     
==========================================
  Files          12       12              
  Lines        1254     1258       +4     
==========================================
+ Hits         1224     1229       +5     
+ Misses         30       29       -1     

☔ 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.

@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)
commit_check/util.py (1)

25-29: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use the shared formatter for the catalog name.

Lines 36-38 in commit_check/rules_catalog.py still return self.check.replace("_", "-"). This leaves a second implementation of the same display contract, so the statement that every text surface uses display_name is not true. Reuse display_name there, or move the helper to a dependency-neutral module if the import direction would create a cycle.

🤖 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 `@commit_check/util.py` around lines 25 - 29, Update the catalog-name
formatting in the rules catalog method that currently calls
self.check.replace("_", "-") to reuse the shared display_name formatter from
commit_check/util.py. Preserve the existing display output and ensure all text
surfaces use the single shared implementation; if importing it creates a
dependency cycle, relocate the helper to a dependency-neutral module and update
both callers.
🤖 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 `@commit_check/util.py`:
- Around line 25-29: Update the catalog-name formatting in the rules catalog
method that currently calls self.check.replace("_", "-") to reuse the shared
display_name formatter from commit_check/util.py. Preserve the existing display
output and ensure all text surfaces use the single shared implementation; if
importing it creates a dependency cycle, relocate the helper to a
dependency-neutral module and update both callers.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 892aeb54-f488-4b9a-80d7-4b5b9c9bd453

📥 Commits

Reviewing files that changed from the base of the PR and between 1072b87 and 99a7d71.

📒 Files selected for processing (2)
  • commit_check/util.py
  • tests/main_test.py

Review pointed out that RuleCatalogEntry.name already did the same
snake-to-kebab conversion, so the helper added in the previous commit was
a second copy and its docstring claim that every text surface goes
through it was false. Two copies of the rule this change exists to
enforce is the wrong number.

The helper moves to rules_catalog rather than the catalog importing it
from util. rules_catalog is a pure data module — dataclasses and nothing
else — and commit-check-mcp imports it directly, so it should not gain a
dependency on a module that pulls in os, sys and subprocess. Pointing the
arrow the other way keeps the catalog light and puts the formatter beside
the entry whose name property defines the concept.

Verified there is now one implementation, that both call sites reach it,
and that importing util, rules_catalog, engine and main together raises
no cycle.
@sonarqubecloud

sonarqubecloud Bot commented Aug 5, 2026

Copy link
Copy Markdown

@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
🆕 1 new benchmark
⏩ 121 skipped benchmarks1

Performance Changes

Benchmark BASE HEAD Efficiency
test_empty_message_passes 6.7 ms 2.3 ms ×2.9
🆕 test_compact_names_checks_the_way_the_default_output_does N/A 20.2 ms N/A

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 (edd9cb6) 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 (1072b87) 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
shenxianpeng merged commit 7315edf into main Aug 6, 2026
28 checks passed
@shenxianpeng
shenxianpeng deleted the claude/refresh-sample-output-602anc branch August 6, 2026 05:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

--compact prints the config-key form of a check name, the default output prints the docs form

1 participant