Add AI bloat review advisories#271
Conversation
|
Strix is installed on this repository, but we could not run this PR security review because this workspace does not have an active plan. If you'd like to continue receiving code reviews, you can add a payment method or manage billing here. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (1)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (6)packages/**/module-package.yaml⚙️ CodeRabbit configuration file
Files:
docs/**/*.md⚙️ CodeRabbit configuration file
Files:
**/*.{js,ts,tsx,jsx,py,java,cs,go,rb,php,cpp,c,h}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
packages/**/src/**/*.py⚙️ CodeRabbit configuration file
Files:
openspec/**/*.md⚙️ CodeRabbit configuration file
Files:
tests/**/*.py⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (7)
📝 WalkthroughWalkthroughAdds an advisory ChangesCode Review AI Bloat Detection
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer (pre-commit)
participant PreCommit as pre_commit_code_review.py
participant CLI as specfact CLI
participant Semgrep as Semgrep runner
participant AI_Bloat as AI-bloat AST runner
participant Scorer as score_review
Dev->>PreCommit: commit -> run hook
PreCommit->>CLI: specfact code review --json
CLI->>Semgrep: run semgrep configs (clean-code + ai-bloat)
Semgrep->>CLI: findings (ai_bloat -> severity=info)
CLI->>AI_Bloat: run_ai_bloat(files)
AI_Bloat->>CLI: ai_bloat findings (info)
CLI->>Scorer: aggregate findings
Scorer->>PreCommit: verdict + ci_exit_code + .specfact/code-review.json
PreCommit->>Dev: exit code + stderr summary (includes ai_bloat count)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
tests/unit/test_bundle_resource_payloads.py (1)
226-248: ⚡ Quick winValidate the
categoryfield in addition toprinciple.The test validates that all rules have
principle == "ai_bloat"(line 247), but the policy pack YAML also specifiescategory: ai_bloatfor each rule. The category field drives rule categorization and severity mapping. Consider adding a parallel assertion to ensure contract completeness:assert {rule["category"] for rule in data["rules"]} == {"ai_bloat"}This guards against miscategorization if the policy pack is edited incorrectly.
🤖 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/unit/test_bundle_resource_payloads.py` around lines 226 - 248, The test function test_code_review_bundle_packages_ai_bloat_policy_pack_manifest currently asserts principle values but not the rule category; update the test to also assert that every rule's "category" equals "ai_bloat" by adding a parallel assertion against data["rules"] (e.g., verify {rule["category"] for rule in data["rules"]} == {"ai_bloat"}) so the policy pack's categorization contract is enforced alongside the existing principle check.
🤖 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.
Inline comments:
In `@docs/modules/code-review.md`:
- Around line 110-115: The new "AI-shaped bloat advisories" section documents an
ai_bloat pass but the page's orchestration/runner order is missing that pass,
causing documentation drift; update the runner/orchestration list on this page
to include the ai_bloat pass in the correct execution order so it matches the
described behavior (ensure references like ai_bloat, .specfact/code-review.json,
and the /specfact.08-simplify prompt remain consistent), keeping the
docs/**/*.md accuracy guideline in mind when editing the orchestration list.
In `@docs/quickstart-ai-bloat.md`:
- Line 35: The docs line includes hard-coded, run-specific detection counts
("144" and "0"); update the sentence in docs/quickstart-ai-bloat.md to avoid
fixed numbers by either removing the counts or marking them as example output
(e.g., "example output: X candidates, Y rewrites accepted"), and keep the
identifiers like `ai_bloat`, `specfact-code-review`, `specfact-project`, and
`/specfact.08-simplify` unchanged so readers can map the example to the rule and
rewrite path.
In `@openspec/changes/code-review-ai-bloat-detection/tasks.md`:
- Around line 66-68: The checklist items for 9.2–9.4 have incorrect indentation
causing MD005; locate the section heading "9" and the sibling checklist items
and realign lines for items "9.2", "9.3", and "9.4" so their list markers and
indentation exactly match the other checklist entries (remove or add leading
spaces as needed) to restore consistent markdown list indentation and avoid
markdownlint noise.
In
`@packages/specfact-code-review/src/specfact_code_review/tools/ai_bloat_runner.py`:
- Around line 258-263: The try/except around ast.parse in ai_bloat_runner.py
currently catches OSError and SyntaxError but not UnicodeDecodeError; update the
except to also catch UnicodeDecodeError so non-UTF8 decoding errors are
converted into a findings entry via tool_error(file_path=file_path,
message=...), mirroring the pattern used in run_ast_clean_code; locate the
ast.parse call and the findings.append(...) that uses tool_error and add
UnicodeDecodeError to the exception tuple to ensure the tool emits a graceful
tool_error instead of crashing.
---
Nitpick comments:
In `@tests/unit/test_bundle_resource_payloads.py`:
- Around line 226-248: The test function
test_code_review_bundle_packages_ai_bloat_policy_pack_manifest currently asserts
principle values but not the rule category; update the test to also assert that
every rule's "category" equals "ai_bloat" by adding a parallel assertion against
data["rules"] (e.g., verify {rule["category"] for rule in data["rules"]} ==
{"ai_bloat"}) so the policy pack's categorization contract is enforced alongside
the existing principle check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5c0e9ad1-5983-417d-b148-558335b63266
📒 Files selected for processing (40)
README.mddocs/bundles/code-review/run.mddocs/bundles/project/overview.mddocs/index.mddocs/modules/code-review.mddocs/quickstart-ai-bloat.mdopenspec/changes/code-review-ai-bloat-detection/TDD_EVIDENCE.mdopenspec/changes/code-review-ai-bloat-detection/design.mdopenspec/changes/code-review-ai-bloat-detection/proposal.mdopenspec/changes/code-review-ai-bloat-detection/specs/code-review-ai-bloat-detection/spec.mdopenspec/changes/code-review-ai-bloat-detection/tasks.mdpackages/specfact-code-review/module-package.yamlpackages/specfact-code-review/resources/policy-packs/specfact/ai-bloat-patterns.yamlpackages/specfact-code-review/resources/semgrep-rules/ai-bloat.yamlpackages/specfact-code-review/src/specfact_code_review/run/findings.pypackages/specfact-code-review/src/specfact_code_review/run/runner.pypackages/specfact-code-review/src/specfact_code_review/run/scorer.pypackages/specfact-code-review/src/specfact_code_review/tools/__init__.pypackages/specfact-code-review/src/specfact_code_review/tools/ai_bloat_runner.pypackages/specfact-code-review/src/specfact_code_review/tools/semgrep_runner.pypackages/specfact-project/module-package.yamlpackages/specfact-project/resources/prompts/specfact.08-simplify.mdscripts/pre_commit_code_review.pytests/fixtures/semgrep/bad_identity_try_except.pytests/fixtures/semgrep/bad_manual_loop_comprehension.pytests/fixtures/semgrep/bad_none_then_none.pytests/fixtures/semgrep/bad_passthrough_lambda.pytests/fixtures/semgrep/bad_single_call_wrapper.pytests/fixtures/semgrep/good_identity_try_except.pytests/fixtures/semgrep/good_manual_loop_comprehension.pytests/fixtures/semgrep/good_none_then_none.pytests/fixtures/semgrep/good_passthrough_lambda.pytests/fixtures/semgrep/good_single_call_wrapper.pytests/unit/scripts/test_pre_commit_code_review.pytests/unit/specfact_code_review/run/test_findings.pytests/unit/specfact_code_review/run/test_runner.pytests/unit/specfact_code_review/run/test_scorer.pytests/unit/specfact_code_review/tools/test_ai_bloat_runner.pytests/unit/specfact_code_review/tools/test_semgrep_runner.pytests/unit/test_bundle_resource_payloads.py
📜 Review details
⏰ 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). (3)
- GitHub Check: quality (3.13)
- GitHub Check: quality (3.11)
- GitHub Check: quality (3.12)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{js,ts,tsx,jsx,py,java,cs,go,rb,php,cpp,c,h}
📄 CodeRabbit inference engine (CLAUDE.md)
Preserve the clean-code compliance gate and its category references (naming, kiss, yagni, dry, and solid)
Files:
tests/fixtures/semgrep/good_manual_loop_comprehension.pytests/fixtures/semgrep/good_passthrough_lambda.pypackages/specfact-code-review/src/specfact_code_review/tools/__init__.pypackages/specfact-code-review/src/specfact_code_review/run/scorer.pytests/fixtures/semgrep/bad_identity_try_except.pytests/fixtures/semgrep/good_single_call_wrapper.pytests/fixtures/semgrep/bad_none_then_none.pytests/fixtures/semgrep/good_none_then_none.pytests/fixtures/semgrep/bad_manual_loop_comprehension.pypackages/specfact-code-review/src/specfact_code_review/run/runner.pytests/unit/specfact_code_review/run/test_findings.pytests/fixtures/semgrep/bad_passthrough_lambda.pypackages/specfact-code-review/src/specfact_code_review/run/findings.pytests/unit/specfact_code_review/run/test_scorer.pytests/fixtures/semgrep/bad_single_call_wrapper.pytests/unit/test_bundle_resource_payloads.pyscripts/pre_commit_code_review.pytests/unit/specfact_code_review/tools/test_ai_bloat_runner.pytests/unit/specfact_code_review/tools/test_semgrep_runner.pytests/fixtures/semgrep/good_identity_try_except.pytests/unit/scripts/test_pre_commit_code_review.pypackages/specfact-code-review/src/specfact_code_review/tools/ai_bloat_runner.pytests/unit/specfact_code_review/run/test_runner.pypackages/specfact-code-review/src/specfact_code_review/tools/semgrep_runner.py
tests/**/*.py
⚙️ CodeRabbit configuration file
tests/**/*.py: Contract-first and integration tests: migration suites, bundle validation, and flakiness.
Ensure changes to adapters or bridges have targeted coverage.
Files:
tests/fixtures/semgrep/good_manual_loop_comprehension.pytests/fixtures/semgrep/good_passthrough_lambda.pytests/fixtures/semgrep/bad_identity_try_except.pytests/fixtures/semgrep/good_single_call_wrapper.pytests/fixtures/semgrep/bad_none_then_none.pytests/fixtures/semgrep/good_none_then_none.pytests/fixtures/semgrep/bad_manual_loop_comprehension.pytests/unit/specfact_code_review/run/test_findings.pytests/fixtures/semgrep/bad_passthrough_lambda.pytests/unit/specfact_code_review/run/test_scorer.pytests/fixtures/semgrep/bad_single_call_wrapper.pytests/unit/test_bundle_resource_payloads.pytests/unit/specfact_code_review/tools/test_ai_bloat_runner.pytests/unit/specfact_code_review/tools/test_semgrep_runner.pytests/fixtures/semgrep/good_identity_try_except.pytests/unit/scripts/test_pre_commit_code_review.pytests/unit/specfact_code_review/run/test_runner.py
packages/**/module-package.yaml
⚙️ CodeRabbit configuration file
packages/**/module-package.yaml: Validate metadata: name, version, commands, dependencies, and parity with packaged src.
Call out semver and signing implications when manifests or payloads change.
Files:
packages/specfact-project/module-package.yamlpackages/specfact-code-review/module-package.yaml
packages/**/src/**/*.py
⚙️ CodeRabbit configuration file
packages/**/src/**/*.py: Focus on adapter and bridge patterns: imports from specfact_cli (models, runtime, validators),
Typer/Rich command surfaces, and clear boundaries so core upgrades do not silently break bundles.
Flag breaking assumptions about registry loading, lazy imports, and environment/mode behavior.
Files:
packages/specfact-code-review/src/specfact_code_review/tools/__init__.pypackages/specfact-code-review/src/specfact_code_review/run/scorer.pypackages/specfact-code-review/src/specfact_code_review/run/runner.pypackages/specfact-code-review/src/specfact_code_review/run/findings.pypackages/specfact-code-review/src/specfact_code_review/tools/ai_bloat_runner.pypackages/specfact-code-review/src/specfact_code_review/tools/semgrep_runner.py
docs/**/*.md
⚙️ CodeRabbit configuration file
docs/**/*.md: User-facing and cross-site accuracy: Jekyll front matter, links per documentation-url-contract,
CLI examples matching bundled commands.
Files:
docs/index.mddocs/bundles/code-review/run.mddocs/bundles/project/overview.mddocs/modules/code-review.mddocs/quickstart-ai-bloat.md
openspec/**/*.md
⚙️ CodeRabbit configuration file
openspec/**/*.md: Specification truth: proposal/tasks/spec deltas vs. bundle behavior, CHANGE_ORDER, and
drift vs. shipped modules or docs.
Files:
openspec/changes/code-review-ai-bloat-detection/TDD_EVIDENCE.mdopenspec/changes/code-review-ai-bloat-detection/proposal.mdopenspec/changes/code-review-ai-bloat-detection/tasks.mdopenspec/changes/code-review-ai-bloat-detection/design.mdopenspec/changes/code-review-ai-bloat-detection/specs/code-review-ai-bloat-detection/spec.md
scripts/**/*.py
⚙️ CodeRabbit configuration file
scripts/**/*.py: Deterministic tooling: signing, publishing, docs generation; subprocess and path safety.
Files:
scripts/pre_commit_code_review.py
🪛 LanguageTool
openspec/changes/code-review-ai-bloat-detection/TDD_EVIDENCE.md
[uncategorized] ~5-~5: The official name of this software platform is spelled with a capital “H”.
Context: ...: Refreshed GitHub hierarchy cache with python scripts/sync_github_hierarchy_cache.py. - 2026-05-20: Veri...
(GITHUB)
openspec/changes/code-review-ai-bloat-detection/proposal.md
[style] ~22-~22: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... enabled or disabled independently. - NEW: Add an IDE slash-command prompt at `...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
openspec/changes/code-review-ai-bloat-detection/tasks.md
[uncategorized] ~5-~5: The official name of this software platform is spelled with a capital “H”.
Context: ...d change scaffolding - [x] 1.1 Verify .specfact/backlog/github_hierarchy_cache.md is fresh, confirm i...
(GITHUB)
[locale-violation] ~44-~44: In American English, ‘afterward’ is the preferred variant. ‘Afterwards’ is more commonly used in British English and other dialects.
Context: ... the IDE, and suggest re-running review afterwards. - [x] 6.2 Add the new command to the ...
(AFTERWARDS_US)
🪛 markdownlint-cli2 (0.22.1)
openspec/changes/code-review-ai-bloat-detection/tasks.md
[warning] 66-66: Inconsistent indentation for list items at the same level
Expected: 1; Actual: 0
(MD005, list-indent)
[warning] 67-67: Inconsistent indentation for list items at the same level
Expected: 1; Actual: 0
(MD005, list-indent)
[warning] 68-68: Inconsistent indentation for list items at the same level
Expected: 1; Actual: 0
(MD005, list-indent)
🔇 Additional comments (29)
scripts/pre_commit_code_review.py (1)
224-233: LGTM!Also applies to: 265-281
tests/unit/scripts/test_pre_commit_code_review.py (1)
89-89: LGTM!Also applies to: 150-216
packages/specfact-code-review/resources/policy-packs/specfact/ai-bloat-patterns.yaml (1)
1-33: LGTM!packages/specfact-project/resources/prompts/specfact.08-simplify.md (1)
1-62: LGTM!packages/specfact-code-review/module-package.yaml (1)
2-2: LGTM!Also applies to: 26-27
packages/specfact-project/module-package.yaml (1)
2-2: LGTM!Also applies to: 30-31
tests/unit/test_bundle_resource_payloads.py (2)
38-38: LGTM!
266-270: LGTM!packages/specfact-code-review/src/specfact_code_review/run/findings.py (1)
27-27: LGTM!Also applies to: 85-85
tests/unit/specfact_code_review/run/test_findings.py (1)
27-27: LGTM!Also applies to: 85-85, 104-104
packages/specfact-code-review/resources/semgrep-rules/ai-bloat.yaml (1)
1-55: LGTM!tests/fixtures/semgrep/bad_identity_try_except.py (1)
1-6: LGTM!tests/fixtures/semgrep/bad_manual_loop_comprehension.py (1)
1-6: LGTM!tests/fixtures/semgrep/bad_none_then_none.py (1)
1-5: LGTM!tests/fixtures/semgrep/bad_passthrough_lambda.py (1)
1-6: LGTM!tests/fixtures/semgrep/bad_single_call_wrapper.py (1)
1-7: LGTM!tests/fixtures/semgrep/good_identity_try_except.py (1)
1-2: LGTM!tests/fixtures/semgrep/good_manual_loop_comprehension.py (1)
1-2: LGTM!tests/fixtures/semgrep/good_none_then_none.py (1)
1-2: LGTM!tests/fixtures/semgrep/good_passthrough_lambda.py (1)
1-4: LGTM!tests/fixtures/semgrep/good_single_call_wrapper.py (1)
1-1: LGTM!tests/unit/specfact_code_review/tools/test_semgrep_runner.py (1)
30-36: LGTM!Also applies to: 44-50, 140-164, 353-371
packages/specfact-code-review/src/specfact_code_review/tools/semgrep_runner.py (1)
27-31: LGTM!Also applies to: 37-37, 171-199, 203-207, 246-248, 278-279, 325-325, 361-366, 373-373, 380-393, 466-466
tests/unit/specfact_code_review/tools/test_ai_bloat_runner.py (1)
8-11: LGTM!Also applies to: 14-31, 33-47, 50-63, 66-74, 76-87, 89-100
packages/specfact-code-review/src/specfact_code_review/tools/__init__.py (1)
3-3: LGTM!Also applies to: 15-15
packages/specfact-code-review/src/specfact_code_review/run/runner.py (1)
24-24: LGTM!Also applies to: 256-256
packages/specfact-code-review/src/specfact_code_review/run/scorer.py (1)
53-54: LGTM!tests/unit/specfact_code_review/run/test_runner.py (1)
42-42: LGTM!Also applies to: 68-68, 92-92, 114-119, 152-152, 174-174, 197-197, 250-250, 292-292, 316-316, 340-340, 371-371, 427-427
tests/unit/specfact_code_review/run/test_scorer.py (1)
22-33: LGTM!Also applies to: 66-72
Summary
ai_bloatreview findings through Semgrep rules and a conservative AST runner./specfact.08-simplifyprompt support plus packaged policy resources and docs/quickstart callouts.ai_bloatinfo findings remain visible in JSON without blocking by themselves.Verification
hatch run formathatch run type-checkhatch run linthatch run yaml-linthatch run check-bundle-importshatch run validate-prompt-commandshatch run verify-modules-signature --payload-from-filesystem --enforce-version-bumphatch run contract-test(693 passed, 2 warnings)hatch run smart-test(693 passed, 2 warnings)hatch run test(693 passed, 2 warnings)openspec validate code-review-ai-bloat-detection --strictReview Gate Note
Full-scope manual review was run with:
SPECFACT_ALLOW_UNSIGNED=1 hatch run specfact code review run --bug-hunt --json --out .specfact/code-review.json --scope fullIt still fails because of pre-existing full-repository findings outside this change. Modified-artifact triage is documented in
openspec/changes/code-review-ai-bloat-detection/TDD_EVIDENCE.md; after fixes, the only modified-file findings left are existing warnings onrun_reviewcomplexity/parameter count.Fixes #269.