fix: close behavior gaps with cpp-linter v1

[shenxianpeng]

Before: compared with the Python cpp-linter (v1), v2 shows clang-tidy annotations on the whole file instead of the line, posts an approving PR review even with --no-lgtm (the default), posts a new review when every suggestion was already posted, lists files in a random order, lets one failed check pass under PRE_COMMIT=1, and rejects or mishandles a few inputs v1 accepts (-e ".cpp, .h", --extra-arg with double spaces, -v 10, CPP_LINTER_PR_REVIEW_SUMMARY_ONLY=True, meson-style relative paths in clang-tidy output).

After: v2 behaves like v1 in those places. Intentional v2 changes (--pr-review, parallel by default, SARIF, the fix patch, #488's header fix) are untouched.

I compared both CLIs, defaults, env vars, file filtering and every feedback path (annotations, step summary, thread comment, PR review, outputs, exit code). This PR only fixes the clear gaps. Differences that look deliberate or need a maintainer's call (for example lines-changed-only defaulting to true, the swapped -t/-g short flags, no --jobs/--fix) are left out and listed in a separate report for @2bndy5 to weigh in on.

Changes

• Annotations: clang-tidy annotations set start_line/start_column. Before, only end_line/end_column were set, and git-bot-feedback only writes line= from start_line, so the annotation had no line. The title also names the check, as in v1.
• --no-lgtm for PR reviews: restored the check lost in the git-bot-feedback migration (d1e2136). Outdated reviews are still culled; only the approving review is skipped.
• Reused review comments: when every review comment was already posted in a previous review, no new review is posted (v1.15 rest_api.py L468-473).
• Stable file order: the file lists from git-bot-feedback (HashMap/HashSet) are sorted, as v1 does.
• PRE_COMMIT=1: checks_failed > 1 → > 0.
• --extensions: trims whitespace and a leading .; empty values are still an error.
• --extra-arg: splits on any whitespace and drops empty values, so clang-tidy never gets --extra-arg=.
• --verbosity: accepts 10 (debug) and 20 (info), like v1.
• CPP_LINTER_PR_REVIEW_SUMMARY_ONLY: compared case-insensitively.
• Compilation database paths: a clang-tidy path that equals a unit's file (relative to its directory, as meson writes it) now resolves through that unit, like v1.
• Review diagnostics: unfixed clang-tidy diagnostics about another file (eg. a header) are no longer posted on the analyzed file's lines (v1 has this bug too).
• Local runs: warn when --pr-review is set outside CI, like v1.
• Thread comment: clang-tidy (vX) reports: <strong>N concern(s)</strong>, matching the clang-format line.
• --style help: no longer says the style is passed to clang-tidy (it isn't since fix(clang-tidy): never let clang-tidy modify the worktree #488).

Nothing here touches clang_format.rs (#491) or the fix-application code planned for #489/#490.

How

Each fix has a test. The no_lgtm review test now asserts no review is POSTed, and it and summary_only (now using True) fail on main and pass here. The new all_suggestions_reused test runs a review once, feeds its comments back as existing review threads, and asserts the second run posts nothing and keeps the old review; it fails without the fix. cargo clippy --all-features --all-targets is clean and cargo nextest run --all-features -p cpp-linter passes locally (124 tests, clang 18).

Summary by CodeRabbit

• Bug Fixes
  • Improved clang-tidy diagnostic matching for compilation database paths and excluded diagnostics for other files from suggestions.
  • Tidy review annotations now point to the diagnostic’s location and include clearer titles and summaries.
  • Pre-commit runs now fail when any check fails, and file feedback appears in a consistent order.
• Review Improvements
  • Avoided posting duplicate review comments and LGTM reviews when LGTM is disabled.
  • Added a warning when pull-request reviews are requested outside a supported CI environment.
• CLI Improvements
  • Normalized extension and extra-argument input, and accepted additional verbosity and mixed-case summary-only values.
  • Clarified that the clang-format style also applies to clang-tidy suggestions.

[coderabbitai]

📝 Walkthrough

The changes update CLI input parsing, clang-tidy diagnostic matching and suggestions, file processing, pre-commit failure handling, and pull-request review generation.

Changes: Lint and review behavior

Layer / File(s)	Summary
CLI input parsing cpp-linter/src/cli/mod.rs	Extension values are trimmed and have leading dots removed. Numeric verbosity aliases are accepted. Extra arguments are split on whitespace; multiple values have surrounding quotes and whitespace removed, and blank values are omitted.
Diagnostic file matching cpp-linter/src/clang_tools/clang_tidy/mod.rs, cpp-linter/src/clang_tools/clang_tidy/test.rs, cpp-linter/src/common_fs.rs	clang-tidy output matches either the directory-joined compilation-unit path or the database path. Suggestion generation excludes diagnostics whose filename does not match the current file. Tests cover both cases.
Run processing cpp-linter/src/run.rs	Selected files are sorted before processing. The program warns when a local client requests pull-request review outside supported CI platforms. Pre-commit mode fails when any check fails.
Review decisions and annotations cpp-linter/src/rest_client.rs, cpp-linter/tests/reviews.rs	Summary-only configuration matching is case-insensitive. Review posting skips LGTM reviews when no_lgtm is enabled and skips posting when all generated comments were already posted. Tidy annotations include the diagnostic in the title and use the reported line and starting column. Tests cover review reuse and annotation formatting.

Priority: ⬇️ Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: 🔵 Low · up to aee75

Application behavior is not shown to be broken, but environment-dependent tests can fail or test the wrong client. Isolate those settings before merging for a reliable test run.

Pre-merge checks: 5

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly identifies the main change: closing behavior gaps between this implementation and cpp-linter v1. It is concise and related to the full changeset.
Docstring Coverage		Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 28 functions across 7 files.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.25%. Comparing base (aca88b5) to head (aee7581).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #492      +/-   ##
==========================================
- Coverage   99.51%   99.25%   -0.27%     
==========================================
  Files          25       26       +1     
  Lines        5554     5754     +200     
==========================================
+ Hits         5527     5711     +184     
- Misses         27       43      +16     

☔ 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.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[2bndy5]

looks pretty good. I noticed 1 thing that was missed

[2bndy5]

Interesting approach used in the tests/reviews.rs module.