ci: enhance feature version audit

[alandtse]

• Add support for refactor:, perf:, and breaking change (!) commit types
• Implement --apply-bumps flag for automatic version increments in .ini files
• Improve feature change detection to include src/Features/ source files
• Updated GitHub workflows to enforce versioning checks on PRs using --ci
• Fixed redundant logging in release audit workflow

Summary by CodeRabbit

• Enhancements

  • Improved audit detection that recognizes refactors, performance and breaking changes; better attribution across code and feature areas.
  • Smarter per-feature analysis with richer action reporting and an option to automatically apply computed version updates.

• Chores

  • More deterministic CI audit flow: controlled artifact upload/download, clearer exit propagation, guarded PR comment timing tied to audit results, and more reliable timing/error handling.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Reworks CI audit steps for deterministic artifact handling and failure propagation; updates PR comment gating and tolerant artifact download. Expands tools/feature_version_audit.py with additional commit-type detection (refactor/perf/breaking/nonfunctional), broader feature discovery, richer per-feature metadata/actions, and a new apply_version_bump (--apply-bumps) mode.

Changes

Cohort / File(s)	Summary
CI Workflow Configuration ​.github/workflows/build.yaml	Replaced tee usage with explicit redirections and cats; capture PR-audit exit code and write pr-feature-audit.md; upload PR artifact with always(); run PR comment in pull_request_target gated on needs.feature-audit-pr.result (success
Feature Version Audit Tool tools/feature_version_audit.py	Added commit-type regexes (RE_COMMIT_REFACTOR, RE_COMMIT_PERF, RE_COMMIT_BREAKING, RE_COMMIT_NONFUNCTIONAL); get_changed_files gains file_types param and filtering; hardened commit/line parsing; broadened bump detection logic (feat/fix/refactor/perf/breaking and non-functional-only handling); expanded feature discovery to features/ and src/Features (incl. headers/cpp); get_feature_key prefers metadata short_name; enriched per-feature author/actions structures; added apply_version_bump(ini_path, proposed_ver_str) and --apply-bumps CLI flow; adjusted logging/output and PR-check behavior.

Sequence Diagram(s)

sequenceDiagram
    participant GH as GitHub Actions
    participant Audit as feature_version_audit.py
    participant Repo as Repository
    participant PR as PR Commenter

    GH->>Audit: start audit job (--ci / --apply-bumps)
    Audit->>Repo: git list changed files, read feature metadata
    Audit->>Audit: analyze commits (feat/fix/refactor/perf/breaking/nonfunctional)
    alt --apply-bumps
      Audit->>Repo: apply_version_bump(ini_path, new_version)
    end
    Audit->>GH: write audit artifacts (`pr-feature-audit.md`, `feature-version-audit-latest.md`)
    GH->>GH: upload artifacts (always())
    GH->>PR: post comment conditioned on needs.feature-audit-pr.result

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

• ci: create fallback for tj-actions/changed-files #1146: Similar CI workflow edits adding tolerant/fallback behavior for artifact/download steps.
• ci: add feature audit checks #1193: Directly overlaps changes to tools/feature_version_audit.py and CI audit behavior.

Suggested reviewers

• jiayev
• sicsix
• doodlum

Poem

🐰 Hopped through diffs with nimble paws,

Found bumps and regexes, fixed the laws,
Artifacts tucked tidy, CI hums in rhyme,
INIs now bumped in a hop and a time,
🥕 Logs sing softly: "All green this time."

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'ci: enhance feature version audit' directly reflects the main changes—enhancements to the feature version audit tool and workflow, including new commit type support, bump application functionality, and improved change detection.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

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

---

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

[github-actions]

Using provided base ref: 3ecc7df
Using base ref: 3ecc7df
Base commit date: 2026-02-21T17:31:06-08:00 (Saturday, February 21, 2026 05:31 PM)
No actionable suggestions for changed features.

[coderabbitai]

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

.github/workflows/build.yaml (1)

479-488: ⚠️ Potential issue | 🔴 Critical

PR audit failure prevents artifact upload, breaking the downstream comment job.

Two issues here:

1. When --ci detects actionable items the script exits 1. Because the run: block uses the default set -e, cat pr-feature-audit.md on line 483 never executes. More importantly, the "Upload feature audit markdown (PR)" step (lines 484–488) has no if: guard — it defaults to if: success() — so the artifact is never uploaded on failure.

2. The downstream feature-audit-pr-comment job (line 492) explicitly runs on needs.feature-audit-pr.result == 'failure', but the actions/download-artifact step will fail because the artifact doesn't exist.

The file is created (shell redirect opens it before the command runs, and the script writes content before sys.exit(1)), so just add if: always() to the upload step and handle the cat gracefully:

Proposed fix

          - name: Run feature version audit (PR check)
            id: audit
            run: |
-               python tools/feature_version_audit.py --pr-check --ci --base ${{ github.event.pull_request.base.sha }} > pr-feature-audit.md
-               cat pr-feature-audit.md
+               python tools/feature_version_audit.py --pr-check --ci --base ${{ github.event.pull_request.base.sha }} > pr-feature-audit.md || true
+               cat pr-feature-audit.md
          - name: Upload feature audit markdown (PR)
+           if: always()
            uses: actions/upload-artifact@v4
            with:
                name: pr-feature-audit
                path: pr-feature-audit.md

If you still want the job to surface a failure status for the --ci check, you could capture the exit code and re-exit after the upload, or split the --ci exit into a separate step.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/build.yaml around lines 479 - 488, The "Run feature
version audit (PR check)" step (id: audit) can exit non-zero and abort the rest
of the step (so the subsequent cat won't run) and the "Upload feature audit
markdown (PR)" step currently runs only on success; change the upload step to
always run by adding an if: always() condition to the "Upload feature audit
markdown (PR)" step and make the audit step's run block robust: run the python
command but capture its exit code (e.g., run python ... > pr-feature-audit.md ||
echo $? > /tmp/audit_exit; cat pr-feature-audit.md || true) so cat always runs
and the file always exists, then re-exit with the captured exit code at the end
of that step if you still want the job to fail (or split into two steps:
run-and-write then upload then re-exit), referencing the step id audit and the
upload step name "Upload feature audit markdown (PR)".

🧹 Nitpick comments (3)

.github/workflows/build.yaml (1)

492-492: Comment job should also guard the download step with if: always().

Even with the upload fix above, defensively adding if-no-files-found: ignore or if: always() on the download step (line 498–502) would prevent a hard failure if the artifact is unexpectedly missing.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In @.github/workflows/build.yaml at line 492, The download-artifact step in the
comment job should be made defensive so it never hard-fails: update the download
step (the step that calls actions/download-artifact) to include either if:
always() or set if-no-files-found: ignore so missing artifacts don't break the
workflow; locate the download-artifact step in the job that currently has the
job-level if condition (the comment job using always() && github.event_name ==
'pull_request_target' ...) and add the chosen guard to that step.

tools/feature_version_audit.py (2)

672-683: --apply-bumps with --ci always exits 1 even after bumps are applied.

actionable is set during analysis (line 654) and never re-evaluated after bumps are applied. If the intent is "apply the fix, then succeed," the exit check should re-evaluate. If the intent is "signal that files were modified," current behavior is fine — but it may surprise callers. Consider documenting the interaction or adding a note in the --ci help text.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tools/feature_version_audit.py` around lines 672 - 683, The exit check uses
the pre-bump "actionable" value so --apply-bumps with --ci still exits 1; after
successfully applying bumps update the analysis state so actionable reflects the
new files: inside the loop that calls apply_version_bump(fa['ini_path'],
fa['proposed_ver_str']) set fa['needs_bump']=False (or otherwise update fa to
reflect the applied change) and after the loop recompute actionable (e.g.,
actionable = any(fa['needs_bump'] and fa['ini_path'] for fa in
feature_analysis)) before the if actionable and (args.ci or
args.fail_on_actionable) sys.exit(1) check so the script can succeed when bumps
were applied.

---

42-42: RE_COMMIT_BREAKING may false-positive on !( in commit messages.

The character class [:\(] matches both !: and !(. Conventional commits only use ! directly before : (e.g., feat!:, feat(scope)!:), so !( is non-standard and could match unrelated text like fix: avoid !(flag).

Consider tightening:

Proposed fix

-RE_COMMIT_BREAKING = re.compile(r"![:\(]|BREAKING CHANGE:", re.IGNORECASE)
+RE_COMMIT_BREAKING = re.compile(r"^[^:]+!:|BREAKING CHANGE:", re.IGNORECASE)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tools/feature_version_audit.py` at line 42, RE_COMMIT_BREAKING currently uses
a character class that also matches "!(" and can false-positive; update the
regex assigned to RE_COMMIT_BREAKING to only match an exclamation directly
before a colon (e.g., change re.compile(r"![:\(]|BREAKING CHANGE:",
re.IGNORECASE) to re.compile(r"!\s*:|BREAKING CHANGE:", re.IGNORECASE) or
stricter r"!:|BREAKING CHANGE:", re.IGNORECASE) so it no longer matches "!("
while still detecting conventional-breaking "!:” and "BREAKING CHANGE:".

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.github/workflows/build.yaml:
- Around line 452-455: The workflow step that runs
tools/feature_version_audit.py currently redirects stdout into
feature-version-audit-latest.md but the script writes the actual report to a
dated file when --pr-check is not used; update the "Run feature version audit"
step to call the script with an explicit output argument (pass --output
feature-version-audit-latest.md) so the script writes the real report to that
filename instead of relying on shell redirection; locate the invocation of
tools/feature_version_audit.py in the step named "Run feature version audit" and
replace the stdout redirection with the --output option.

In `@tools/feature_version_audit.py`:
- Around line 315-327: The code adds a normalized form of feature names into the
set changed_features (using ''.join(feature_name.lower().split())) but later
checks membership using the raw feature_dir.name, so the normalized entry is
never matched; fix by making the membership check use the same normalization
(compare ''.join(feature_dir.name.lower().split()) against changed_features) or
remove the normalization insertion and only add the raw feature_name to
changed_features so both insertion and lookup use the same form; update the code
paths that use changed_features (the block that references feature_dir.name) to
use the chosen consistent naming scheme.
- Line 128: get_bump_commit currently only checks RE_COMMIT_FEAT, RE_COMMIT_FIX,
RE_COMMIT_REFACTOR, and RE_COMMIT_PERF when deciding to capture the commit
link/author; add RE_COMMIT_BREAKING to that conditional so breaking-change
commits are also detected and attributed. Locate the conditional inside
get_bump_commit and include RE_COMMIT_BREAKING alongside the other RE_COMMIT_*
regexes (the same token used by propose_new_version) so breaking commits produce
the same commit link/author capture.
- Around line 646-648: Change the diagnostic print() calls in
tools/feature_version_audit.py so they go to stderr instead of stdout (e.g.,
replace prints like "Using base ref: {base_ref}" and "Base commit date:
{base_date_iso} ({base_date_human})" and the other diagnostic prints around the
script (previously at the other diagnostic locations) with calls that write to
sys.stderr or use the logging module at INFO/DEBUG), ensuring normal output
redirected to .md files remains clean while diagnostic messages appear on
stderr; update imports to include sys or configure logging as needed and modify
each diagnostic print invocation to 출력 to stderr.
- Around line 350-363: get_changed_files currently filters by SHADER_TYPES so
calls that check src/Features (header_path, cpp_path, feature_src_dir) always
return empty and C++ changes are ignored; fix by updating get_changed_files to
accept an optional file_types parameter (or broaden SHADER_TYPES) and then call
get_changed_files(header_path, base_ref, file_types=(".h", ".hpp", ".cpp",
".c")) and similarly for cpp_path/feature_src_dir (or add C++ extensions to
SHADER_TYPES), ensuring the function signature and any call sites are updated to
handle the new parameter.

---

Outside diff comments:
In @.github/workflows/build.yaml:
- Around line 479-488: The "Run feature version audit (PR check)" step (id:
audit) can exit non-zero and abort the rest of the step (so the subsequent cat
won't run) and the "Upload feature audit markdown (PR)" step currently runs only
on success; change the upload step to always run by adding an if: always()
condition to the "Upload feature audit markdown (PR)" step and make the audit
step's run block robust: run the python command but capture its exit code (e.g.,
run python ... > pr-feature-audit.md || echo $? > /tmp/audit_exit; cat
pr-feature-audit.md || true) so cat always runs and the file always exists, then
re-exit with the captured exit code at the end of that step if you still want
the job to fail (or split into two steps: run-and-write then upload then
re-exit), referencing the step id audit and the upload step name "Upload feature
audit markdown (PR)".

---

Nitpick comments:
In @.github/workflows/build.yaml:
- Line 492: The download-artifact step in the comment job should be made
defensive so it never hard-fails: update the download step (the step that calls
actions/download-artifact) to include either if: always() or set
if-no-files-found: ignore so missing artifacts don't break the workflow; locate
the download-artifact step in the job that currently has the job-level if
condition (the comment job using always() && github.event_name ==
'pull_request_target' ...) and add the chosen guard to that step.

In `@tools/feature_version_audit.py`:
- Around line 672-683: The exit check uses the pre-bump "actionable" value so
--apply-bumps with --ci still exits 1; after successfully applying bumps update
the analysis state so actionable reflects the new files: inside the loop that
calls apply_version_bump(fa['ini_path'], fa['proposed_ver_str']) set
fa['needs_bump']=False (or otherwise update fa to reflect the applied change)
and after the loop recompute actionable (e.g., actionable = any(fa['needs_bump']
and fa['ini_path'] for fa in feature_analysis)) before the if actionable and
(args.ci or args.fail_on_actionable) sys.exit(1) check so the script can succeed
when bumps were applied.
- Line 42: RE_COMMIT_BREAKING currently uses a character class that also matches
"!(" and can false-positive; update the regex assigned to RE_COMMIT_BREAKING to
only match an exclamation directly before a colon (e.g., change
re.compile(r"![:\(]|BREAKING CHANGE:", re.IGNORECASE) to
re.compile(r"!\s*:|BREAKING CHANGE:", re.IGNORECASE) or stricter r"!:|BREAKING
CHANGE:", re.IGNORECASE) so it no longer matches "!(" while still detecting
conventional-breaking "!:” and "BREAKING CHANGE:".

[github-actions]

✅ A pre-release build is available for this PR:
Download

[davo0411]

resolve ai if you deem necessary

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

tools/feature_version_audit.py (2)

256-264: detect_pr_base() is defined but never called.

main() falls back to get_latest_release_tag() when --base is absent, completely bypassing GITHUB_BASE_REF. In a CI --pr-check run without an explicit --base, the tool will use a release tag as the comparison point instead of the PR's target branch.

Either integrate this function into the fallback path, or remove it if the workflow always supplies --base explicitly:

♻️ Option A — integrate into main()

 else:
-    detected = get_latest_release_tag()
-    if detected:
-        base_ref = detected
-    else:
-        print("No valid base ref found.", file=sys.stderr)
-        sys.exit(1)
+    if args.pr_check:
+        base_ref = detect_pr_base()
+    else:
+        detected = get_latest_release_tag()
+        if detected:
+            base_ref = detected
+        else:
+            print("No valid base ref found.", file=sys.stderr)
+            sys.exit(1)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tools/feature_version_audit.py` around lines 256 - 264, detect_pr_base() is
currently unused so when --base is omitted CI pr-checks fall back to
get_latest_release_tag(); modify main() to call detect_pr_base() as the first
fallback when args.base is None (i.e., if base not provided, call
detect_pr_base() and use its return value when non-empty/nonnull), before
falling back to get_latest_release_tag(), or alternatively remove
detect_pr_base() if you guarantee --base is always provided; reference
detect_pr_base(), main(), get_latest_release_tag(), GITHUB_BASE_REF and
DEFAULT_PR_BASE_REF when making the change.

---

675-685: --apply-bumps runs after report generation — bumped files won't be reflected in the output.

When --output and --apply-bumps are used together, the written markdown still shows needs_bump: True with the old version, even though the ini file has already been updated. Not a correctness issue for the audit itself, but can be confusing when reviewing the artifact.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tools/feature_version_audit.py` around lines 675 - 685, The output is
generated before the --apply-bumps loop, so the written report still shows old
versions; fix by applying bumps earlier or updating the in-memory analysis after
applying them: move or invoke the apply_version_bump loop (the block using
args.apply_bumps, feature_analysis, and apply_version_bump) before the report
generation/markdown write, or after each successful apply_version_bump call
update the corresponding feature_analysis entry (fa['prior_ver_str'],
fa['proposed_ver_str'], and fa['needs_bump']) so the later print/write uses the
new values; ensure the final print of applied_count and the sys.exit behavior
remain unchanged.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tools/feature_version_audit.py`:
- Around line 296-298: The unguarded subprocess.check_output call that builds
all_changes in analyze_features should be wrapped in a try/except for
subprocess.CalledProcessError (and optionally OSError); if the git command fails
(bad base_ref or git error) catch the exception, log or warn, and set
all_changes to an empty list (or []), preserving existing behavior of other
calls in this file; update the block that constructs target_dirs, cmd and
all_changes to catch the error and default safely so analyze_features,
FEATURES_DIR and DEFAULT_FEATURE_HEADERS_DIR handling continues without
crashing.

---

Duplicate comments:
In `@tools/feature_version_audit.py`:
- Around line 125-130: get_bump_commit is currently checking commit message
patterns but omits RE_COMMIT_BREAKING so breaking-change commits aren't captured
for attribution; update the loop in get_bump_commit to include
RE_COMMIT_BREAKING alongside
RE_COMMIT_FEAT/RE_COMMIT_FIX/RE_COMMIT_REFACTOR/RE_COMMIT_PERF so the function
records the commit hash and author for breaking changes (the same way other
types are handled) and remains consistent with propose_new_version which uses
RE_COMMIT_BREAKING to determine bumps.
- Around line 317-329: The normalized entry added to changed_features (via
feature_name and the lower/space-stripped variant) never matches because the
membership check uses feature_dir.name raw; update the logic so both sides use
the same normalization: either add the raw feature_dir.name to changed_features
when discovering src/Features entries (in the block that sets
feature_name/changed_features) or, preferably, normalize feature_dir.name before
the membership test (i.e., compare normalized(feature_dir.name) against the
normalized entries in changed_features). Adjust code paths around the variables
feature_name, changed_features, and the membership check that reads
feature_dir.name so normalization is applied consistently.

---

Nitpick comments:
In `@tools/feature_version_audit.py`:
- Around line 256-264: detect_pr_base() is currently unused so when --base is
omitted CI pr-checks fall back to get_latest_release_tag(); modify main() to
call detect_pr_base() as the first fallback when args.base is None (i.e., if
base not provided, call detect_pr_base() and use its return value when
non-empty/nonnull), before falling back to get_latest_release_tag(), or
alternatively remove detect_pr_base() if you guarantee --base is always
provided; reference detect_pr_base(), main(), get_latest_release_tag(),
GITHUB_BASE_REF and DEFAULT_PR_BASE_REF when making the change.
- Around line 675-685: The output is generated before the --apply-bumps loop, so
the written report still shows old versions; fix by applying bumps earlier or
updating the in-memory analysis after applying them: move or invoke the
apply_version_bump loop (the block using args.apply_bumps, feature_analysis, and
apply_version_bump) before the report generation/markdown write, or after each
successful apply_version_bump call update the corresponding feature_analysis
entry (fa['prior_ver_str'], fa['proposed_ver_str'], and fa['needs_bump']) so the
later print/write uses the new values; ensure the final print of applied_count
and the sys.exit behavior remain unchanged.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

tools/feature_version_audit.py (2)

697-701: Nit: --ci and --fail-on-actionable are functionally identical.

Both flags trigger sys.exit(1) on actionable items. Consider documenting that --ci is an alias for --fail-on-actionable (or deprecating one) to avoid confusion about their difference.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tools/feature_version_audit.py` around lines 697 - 701, The flags args.ci and
args.fail_on_actionable are identical in behavior (both cause sys.exit(1) when
actionable is true); either document that "--ci" is an alias of
"--fail-on-actionable" or deprecate one to avoid confusion. Update the CLI
parsing / help text where args.ci and args.fail_on_actionable are defined and
the usage/help string to state that "--ci" is an alias for
"--fail-on-actionable" (or remove one flag and map it to the other), and keep
the actionable check in the end (if actionable and (args.ci or
args.fail_on_actionable): sys.exit(1)) consistent with the chosen approach;
reference the variables args.ci, args.fail_on_actionable and the exit logic
around sys.exit(1) for the change.

---

266-283: The else fallback (line 282) bumps patch for non-matching commit types like chore:, docs:, style:.

This means any commit touching a feature's files will trigger at least a patch version bump suggestion, even for documentation-only or chore commits. If that's intentional (erring on the side of caution for the audit), this is fine — but it may produce noisy bump suggestions for non-functional changes.

Consider filtering out known non-functional commit types (chore, docs, style, ci, test) and returning None (no bump) for those:

Optional: skip bumps for non-functional commits

+RE_COMMIT_NONFUNCTIONAL = re.compile(r"^(chore|docs|style|ci|test|build)(\(|:|\s)", re.IGNORECASE)
+
 def propose_new_version(prior_version, commits):
     if not prior_version:
         return None
     major, minor, patch = prior_version
-    # Any change in functional files should result in at least a patch bump
-    if not commits:
-        return (major, minor, patch + 1)
 
     is_minor = any(RE_COMMIT_FEAT.match(c) or RE_COMMIT_BREAKING.search(c) for c in commits)
     is_patch = any(RE_COMMIT_FIX.match(c) or RE_COMMIT_REFACTOR.match(c) or RE_COMMIT_PERF.match(c) for c in commits)
+    is_nonfunctional_only = all(RE_COMMIT_NONFUNCTIONAL.match(c) for c in commits) if commits else False
 
     if is_minor:
         return (major, minor + 1, 0)
     elif is_patch:
         return (major, minor, patch + 1)
+    elif is_nonfunctional_only:
+        return None
     else:
-        # Fallback to patch bump for any non-chore/docs changes that reached here
         return (major, minor, patch + 1)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tools/feature_version_audit.py` around lines 266 - 283, propose_new_version
currently falls back to a patch bump for any commits that don't match the
existing regexes, which causes docs/chore/style/ci/test commits touching feature
files to produce noisy patch suggestions; update propose_new_version to detect
and ignore known non-functional commit types by adding checks for the
non-functional regexes (e.g. RE_COMMIT_CHORE, RE_COMMIT_DOCS, RE_COMMIT_STYLE,
RE_COMMIT_CI, RE_COMMIT_TEST) and if all commits match only those non-functional
patterns return None (no bump) instead of the current fallback (the function
propose_new_version and its is_minor/is_patch logic should be extended to
include this non-functional-only check).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tools/feature_version_audit.py`:
- Around line 669-686: When applying bumps in the loop that calls
apply_version_bump, don't overwrite fa['prior_ver_str'] (leave the original
prior version intact); only clear fa['needs_bump'] after success. Also update
the feature_actions mapping for that feature (feature_actions[fa['name']]) to
remove or replace any "Needs version bump to X" suggestion so
print_actionable_suggestions no longer reports stale bump requests — e.g.,
filter out entries that start with "Needs version bump" or regenerate that
feature's actions after bump. Finally recompute the actionable flag from the
updated feature_actions (or by checking any non-empty action lists) and
new_features so PR-check logic reflects the applied bumps.

---

Nitpick comments:
In `@tools/feature_version_audit.py`:
- Around line 697-701: The flags args.ci and args.fail_on_actionable are
identical in behavior (both cause sys.exit(1) when actionable is true); either
document that "--ci" is an alias of "--fail-on-actionable" or deprecate one to
avoid confusion. Update the CLI parsing / help text where args.ci and
args.fail_on_actionable are defined and the usage/help string to state that
"--ci" is an alias for "--fail-on-actionable" (or remove one flag and map it to
the other), and keep the actionable check in the end (if actionable and (args.ci
or args.fail_on_actionable): sys.exit(1)) consistent with the chosen approach;
reference the variables args.ci, args.fail_on_actionable and the exit logic
around sys.exit(1) for the change.
- Around line 266-283: propose_new_version currently falls back to a patch bump
for any commits that don't match the existing regexes, which causes
docs/chore/style/ci/test commits touching feature files to produce noisy patch
suggestions; update propose_new_version to detect and ignore known
non-functional commit types by adding checks for the non-functional regexes
(e.g. RE_COMMIT_CHORE, RE_COMMIT_DOCS, RE_COMMIT_STYLE, RE_COMMIT_CI,
RE_COMMIT_TEST) and if all commits match only those non-functional patterns
return None (no bump) instead of the current fallback (the function
propose_new_version and its is_minor/is_patch logic should be extended to
include this non-functional-only check).

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (2)

tools/feature_version_audit.py (2)

291-292: author_stats returned from analyze_features is never consumed.

analyze_features returns author_stats (and initializes the meta key in it), but main() never uses this value — the full-report path rebuilds author stats via build_author_stats(feature_actions) inside generate_audit_report, and the --pr-check path doesn't render author stats at all. Additionally, meta in author_stats is always 0 because analyze_features never increments it, making the returned dict partially inaccurate regardless.

Consider dropping author_stats from the analyze_features return value and constructing it solely through build_author_stats where it is actually needed, keeping analyze_features focused on feature analysis and actions.

Also applies to: 664-664, 666-666

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tools/feature_version_audit.py` around lines 291 - 292, analyze_features
currently returns author_stats (with a meta key set to 0) that main never uses;
remove author_stats from analyze_features' return and its initialization
(author_stats = {} / meta handling), and instead always derive author stats from
feature_actions by calling build_author_stats(feature_actions) where needed
(e.g. inside generate_audit_report/full-report path); update function signatures
and any call sites (analyze_features, main, generate_audit_report) to stop
expecting/propagating author_stats so author stats are constructed only via
build_author_stats and meta is no longer misleading.

---

387-388: prior_ver is None in needs_bump is unreachable dead code.

propose_new_version returns None whenever prior_version is falsy (line 268), so when prior_ver is None, proposed_ver is also None, making proposed_ver is not None evaluate to False before the prior_ver is None clause is ever reached.

♻️ Simplify the condition

-        needs_bump = (proposed_ver is not None and new_ver is not None and (prior_ver is None or proposed_ver > new_ver))
+        needs_bump = (proposed_ver is not None and new_ver is not None and proposed_ver > new_ver)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tools/feature_version_audit.py` around lines 387 - 388, The conditional for
needs_bump contains an unreachable check (prior_ver is None) because
propose_new_version(prior_ver, ...) returns None when prior_ver is falsy; remove
that dead branch and simplify the boolean expression so it only checks that
proposed_ver and new_ver are present and that proposed_ver > new_ver. Update the
assignment of needs_bump (which uses proposed_ver, propose_new_version,
prior_ver, and new_ver) to something like: needs_bump = (proposed_ver is not
None and new_ver is not None and proposed_ver > new_ver).

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tools/feature_version_audit.py`:
- Around line 147-161: The apply_version_bump function's call to RE_VERSION.sub
replaces all Version occurrences; update the substitution call in
apply_version_bump to pass count=1 so only the first match is replaced (locate
the RE_VERSION.sub invocation inside apply_version_bump and add the count=1
argument to limit substitution to a single occurrence).
- Around line 671-700: After applying bumps in the apply_bumps block, remove or
update entries in bump_suggestions to reflect the applied changes so
generate_audit_report no longer rebuilds actions for already-bumped features;
specifically, for each fa in feature_analysis where fa['needs_bump'] was True
and apply_version_bump succeeded (the same logic that sets
fa['needs_bump']=False), remove the corresponding bump_suggestions entry (or
update it) so feature_actions built from bump_suggestions matches the updated
feature_analysis; perform this filtering before calling generate_audit_report
(functions/vars: apply_version_bump, feature_analysis, bump_suggestions,
generate_audit_report, feature_actions).

---

Nitpick comments:
In `@tools/feature_version_audit.py`:
- Around line 291-292: analyze_features currently returns author_stats (with a
meta key set to 0) that main never uses; remove author_stats from
analyze_features' return and its initialization (author_stats = {} / meta
handling), and instead always derive author stats from feature_actions by
calling build_author_stats(feature_actions) where needed (e.g. inside
generate_audit_report/full-report path); update function signatures and any call
sites (analyze_features, main, generate_audit_report) to stop
expecting/propagating author_stats so author stats are constructed only via
build_author_stats and meta is no longer misleading.
- Around line 387-388: The conditional for needs_bump contains an unreachable
check (prior_ver is None) because propose_new_version(prior_ver, ...) returns
None when prior_ver is falsy; remove that dead branch and simplify the boolean
expression so it only checks that proposed_ver and new_ver are present and that
proposed_ver > new_ver. Update the assignment of needs_bump (which uses
proposed_ver, propose_new_version, prior_ver, and new_ver) to something like:
needs_bump = (proposed_ver is not None and new_ver is not None and proposed_ver
> new_ver).

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

tools/feature_version_audit.py (1)

296-303: subprocess.CalledProcessError catch is narrower than the rest of the file.

Every other subprocess.check_output call catches the generic Exception. This one only catches CalledProcessError, so an OSError (e.g., git not on PATH) would propagate uncaught and crash the script.

Proposed fix

-        except subprocess.CalledProcessError as e:
-            print(f"Error running git diff: {e}", file=sys.stderr)
+        except Exception as e:
+            print(f"Error running git diff: {e}", file=sys.stderr)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tools/feature_version_audit.py` around lines 296 - 303, The except block
around the subprocess.check_output call that builds cmd using FEATURES_DIR and
DEFAULT_FEATURE_HEADERS_DIR is too narrow (only subprocess.CalledProcessError);
change it to catch the broader Exception (e.g., except Exception as e:) so
OSError (missing git) and other failures are handled the same way as other
subprocess calls, log the error into stderr with the exception variable and set
all_changes = [] as before.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tools/feature_version_audit.py`:
- Around line 267-285: propose_new_version currently treats an empty commits
list as a patch bump which causes spurious proposals for unchanged features; fix
by either guarding the call site where propose_new_version(...) is invoked (skip
calling it when only_changed is False and the feature's changes/all_commits is
empty or when the feature has no changes/changes == []) so no bump is proposed
for untouched features, or modify propose_new_version itself to return None
immediately when commits is empty (replace the existing early return to return
None) so callers like the code that builds all_commits/changes and checks
ini_path will not receive a patch increment for unchanged features.
- Around line 661-689: The current filter for bump_suggestions uses substring
matching (any(name in s for name in bumped_ini_names)) which can incorrectly
remove suggestions when one INI basename is a suffix of another; change the
filter to match the exact filename token in the suggestion (e.g., check for the
bold-markdown anchor "**{basename}**" or extract the filename from each
suggestion via a regex and compare equality) so bumped_ini_names (built from
fa['ini_path']) is compared with the exact bolded filename in bump_suggestions
instead of using substring containment.

---

Nitpick comments:
In `@tools/feature_version_audit.py`:
- Around line 296-303: The except block around the subprocess.check_output call
that builds cmd using FEATURES_DIR and DEFAULT_FEATURE_HEADERS_DIR is too narrow
(only subprocess.CalledProcessError); change it to catch the broader Exception
(e.g., except Exception as e:) so OSError (missing git) and other failures are
handled the same way as other subprocess calls, log the error into stderr with
the exception variable and set all_changes = [] as before.