Feature/code review ai bloat detection#270
Conversation
Introduces ai_bloat as a sixth principle category in the code-review pipeline alongside naming/kiss/yagni/dry/solid/clean_code/architecture. Adds a packaged semgrep rule pack for pattern-shape detectors, an AST runner for semantic detectors, a parallel ai-bloat-patterns policy pack, and a /specfact.08-simplify slash-command prompt that drives LLM-mediated rewrites with per-change human confirmation. Findings are advisory-only; pre-commit warns but never blocks. Tracks: #269 (sub-issue of Feature #175 under Modules Epic #162).
Extends tasks.md with section 8 covering root README callout, quickstart
walkthrough with before/after evidence, modules.specfact.io homepage
callout, follow-up tracking issue for the core specfact-cli README, and
explicit honest-framing guardrails ("bloat detection tuned for the shapes
AI code commonly produces" — never an is-this-AI classifier).
Section 9 absorbs the original passing-evidence section. Proposal Impact
section updated to reflect the broader docs scope.
Tracks: #269.
djm81
added
codebase
|
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. |
📝 WalkthroughOverviewSpecification PR adding an Bundle and module surfaceAffected/mentioned bundles/modules (expected packaging changes):
New module surface items:
Behavior vs specfact-cli APIs:
Commands/adapters impact:
Manifest and integrityModule-package and registry impacts:
Signature and verification:
Registry/manifest consequences:
Cross-repo: specfact-cli and imports/contractsSpecfact-cli impacts:
Contract/test expectations:
Dev-deps and path assumptions:
DocumentationDocs to add/update:
Documentation URL contract:
OpenSpec metadata & scenario coverageOpenSpec change ID: code-review-ai-bloat-detection (design/spec/tasks included). Scenario coverage required by spec.md (high level):
Implementation readiness / Taskstasks.md enumerates steps: GitHub scaffolding, spec-first failing tests, semgrep/AST implementation, policy-pack, pre-commit hook update, slash-command authoring, docs, evidence capture, CI/signing, and PR completion checklist (self-review, no unrelated files, backward-compat notes). All tasks are blocking; no implementation code present in this PR. CI / Branch protection (summary)Required CI jobs and gates (as listed in PR):
WalkthroughAdds a complete specification, proposal, and task checklist for a new ChangesAI Bloat Detection Feature Specification
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
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 unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7900a99423
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
openspec/changes/code-review-ai-bloat-detection/design.md (1)
29-31:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSame counting error in Decision 1 heading.
The decision rationale repeats "sixth principle category" when the context establishes seven existing categories.
📊 Proposed fix
-### Decision 1: Introduce `ai_bloat` as a sixth principle category, not a sub-mode of `kiss`/`yagni` +### Decision 1: Introduce `ai_bloat` as a new principle category, not a sub-mode of `kiss`/`yagni`🤖 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 `@openspec/changes/code-review-ai-bloat-detection/design.md` around lines 29 - 31, The heading for Decision 1 incorrectly says "sixth principle category" even though the context lists seven existing categories; update the Decision 1 text to reference the correct ordinal (e.g., "eighth" or "seventh" as appropriate) or rephrase to avoid ordinal counting; change the sentence that mentions "sixth principle category" (near the Decision 1 heading and the sentence that starts with "The detectors target...") to use the correct ordinal or a non-counting phrase and keep the references to the new `ai_bloat` category and existing `kiss`/`yagni` modes intact so `ai_bloat`, `kiss`, and `yagni` remain clearly named.openspec/changes/code-review-ai-bloat-detection/proposal.md (1)
18-18:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winCategory count mismatch: proposal claims "sixth" but context lists seven existing.
Line 3 enumerates seven existing principle categories, making
ai_bloatthe eighth (or "new"), not sixth. This repeats the counting error fromdesign.mdand creates spec inconsistency.📊 Proposed fix
-- **NEW**: Introduce `ai_bloat` as a sixth principle category alongside `naming | kiss | yagni | dry | solid | clean_code`, surfaced through the existing `ReviewFinding` transport and `.specfact/code-review.json` evidence file. Findings are emitted at `advisory` severity only; they never block commits. +- **NEW**: Introduce `ai_bloat` as a new principle category alongside `naming | kiss | yagni | dry | solid | clean_code | architecture`, surfaced through the existing `ReviewFinding` transport and `.specfact/code-review.json` evidence file. Findings are emitted at `advisory` severity only; they never block commits.🤖 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 `@openspec/changes/code-review-ai-bloat-detection/proposal.md` at line 18, The sentence introducing ai_bloat misstates its ordinal — the prose says "sixth" but the preceding list contains seven existing categories, so ai_bloat would be the eighth; update the wording to the correct ordinal (e.g., change "sixth" to "eighth" or rework the list so there are actually six before adding ai_bloat) and make the same correction wherever `ai_bloat`, `ReviewFinding`, `.specfact/code-review.json`, or `design.md` refer to "sixth" to keep the spec consistent.openspec/changes/code-review-ai-bloat-detection/specs/code-review-ai-bloat-detection/spec.md (1)
96-96:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winSpecification claims "six categories" but defines seven earlier.
Line 7 lists seven existing categories (
naming | kiss | yagni | dry | solid | clean_code | architecture), yet line 96 statesai_bloatis "distinct from the existing six categories." This inconsistency in the MODIFIED requirement contradicts the specification's own enumeration.📊 Proposed fix
-- **AND** SHALL state that its principle category is `ai_bloat`, distinct from the existing six categories +- **AND** SHALL state that its principle category is `ai_bloat`, distinct from the existing categories🤖 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 `@openspec/changes/code-review-ai-bloat-detection/specs/code-review-ai-bloat-detection/spec.md` at line 96, The spec incorrectly states "existing six categories" while the earlier enumeration lists seven categories (naming | kiss | yagni | dry | solid | clean_code | architecture); update the text that references the count so it matches the enumeration or adjust the enumeration to six. Specifically, change the phrase that says "existing six categories" to "existing seven categories" (or remove/modify a category in the list) so that the `ai_bloat` requirement and the listed categories are consistent; ensure the terms `ai_bloat` and the category list (naming | kiss | yagni | dry | solid | clean_code | architecture) are reconciled.
🤖 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 `@openspec/changes/code-review-ai-bloat-detection/design.md`:
- Around line 5-14: The document miscounts existing categories as "six" while
listing seven (`naming`, `kiss`, `yagni`, `dry`, `solid`, `clean_code`,
`architecture`), so update the text to state the correct ordinal (either
"eighth" if adding `ai_bloat` to become eight, or "sixth/seventh" depending on
intended target) and make the same correction wherever "sixth" appears;
specifically reference the ReviewFinding category set and replace each incorrect
"sixth" occurrence with the accurate ordinal and ensure the new `ai_bloat`
addition to the ReviewFinding accepted values is described consistently with the
listed categories.
---
Duplicate comments:
In `@openspec/changes/code-review-ai-bloat-detection/design.md`:
- Around line 29-31: The heading for Decision 1 incorrectly says "sixth
principle category" even though the context lists seven existing categories;
update the Decision 1 text to reference the correct ordinal (e.g., "eighth" or
"seventh" as appropriate) or rephrase to avoid ordinal counting; change the
sentence that mentions "sixth principle category" (near the Decision 1 heading
and the sentence that starts with "The detectors target...") to use the correct
ordinal or a non-counting phrase and keep the references to the new `ai_bloat`
category and existing `kiss`/`yagni` modes intact so `ai_bloat`, `kiss`, and
`yagni` remain clearly named.
In `@openspec/changes/code-review-ai-bloat-detection/proposal.md`:
- Line 18: The sentence introducing ai_bloat misstates its ordinal — the prose
says "sixth" but the preceding list contains seven existing categories, so
ai_bloat would be the eighth; update the wording to the correct ordinal (e.g.,
change "sixth" to "eighth" or rework the list so there are actually six before
adding ai_bloat) and make the same correction wherever `ai_bloat`,
`ReviewFinding`, `.specfact/code-review.json`, or `design.md` refer to "sixth"
to keep the spec consistent.
In
`@openspec/changes/code-review-ai-bloat-detection/specs/code-review-ai-bloat-detection/spec.md`:
- Line 96: The spec incorrectly states "existing six categories" while the
earlier enumeration lists seven categories (naming | kiss | yagni | dry | solid
| clean_code | architecture); update the text that references the count so it
matches the enumeration or adjust the enumeration to six. Specifically, change
the phrase that says "existing six categories" to "existing seven categories"
(or remove/modify a category in the list) so that the `ai_bloat` requirement and
the listed categories are consistent; ensure the terms `ai_bloat` and the
category list (naming | kiss | yagni | dry | solid | clean_code | architecture)
are reconciled.
🪄 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: 0ffd5b0a-e714-438f-bb73-ec8f54b5bc19
📒 Files selected for processing (6)
.gitignoreopenspec/CHANGE_ORDER.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.md
📜 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.11)
- GitHub Check: quality (3.12)
- GitHub Check: quality (3.13)
🧰 Additional context used
📓 Path-based instructions (1)
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/CHANGE_ORDER.mdopenspec/changes/code-review-ai-bloat-detection/specs/code-review-ai-bloat-detection/spec.mdopenspec/changes/code-review-ai-bloat-detection/tasks.mdopenspec/changes/code-review-ai-bloat-detection/proposal.mdopenspec/changes/code-review-ai-bloat-detection/design.md
🪛 LanguageTool
openspec/changes/code-review-ai-bloat-detection/specs/code-review-ai-bloat-detection/spec.md
[locale-violation] ~71-~71: In American English, ‘afterward’ is the preferred variant. ‘Afterwards’ is more commonly used in British English and other dialects.
Context: ...tool, and suggest re-running the review afterwards. The prompt SHALL NOT edit files autono...
(AFTERWARDS_US)
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: ...nd change scaffolding - [ ] 1.1 Verify .specfact/backlog/github_hierarchy_cache.md is fresh, confirm t...
(GITHUB)
[locale-violation] ~37-~37: 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. - [ ] 6.2 Add the new command to the p...
(AFTERWARDS_US)
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)
[style] ~39-~39: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...line against seeded bloat fixtures. - Affected docs: reference docs for the `ai_bloa...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (3)
openspec/CHANGE_ORDER.md (1)
103-103: LGTM!.gitignore (1)
49-50: LGTM!openspec/changes/code-review-ai-bloat-detection/tasks.md (1)
1-60: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@openspec/changes/code-review-ai-bloat-detection/proposal.md`:
- Line 40: The phrase "warn-only" in the release impact sentence is inconsistent
with the rest of the change-set which constrains ai_bloat findings to
severity=info; update the wording to "info-only" and ensure references to the
new resources (`specfact-code-review` and `specfact-project`) state that
`ai_bloat` findings are additive and info-only (never warning/error) so the
sentence aligns with the `ai_bloat` severity contract.
🪄 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: 1974bda5-f14b-446c-ba99-7c9ba6abcb58
📒 Files selected for processing (4)
openspec/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.md
✅ Files skipped from review due to trivial changes (1)
- openspec/changes/code-review-ai-bloat-detection/design.md
📜 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.11)
- GitHub Check: quality (3.13)
- GitHub Check: quality (3.12)
🧰 Additional context used
📓 Path-based instructions (1)
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/proposal.mdopenspec/changes/code-review-ai-bloat-detection/specs/code-review-ai-bloat-detection/spec.mdopenspec/changes/code-review-ai-bloat-detection/tasks.md
🪛 LanguageTool
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)
[style] ~39-~39: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...line against seeded bloat fixtures. - Affected docs: reference docs for the `ai_bloa...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
openspec/changes/code-review-ai-bloat-detection/specs/code-review-ai-bloat-detection/spec.md
[locale-violation] ~72-~72: In American English, ‘afterward’ is the preferred variant. ‘Afterwards’ is more commonly used in British English and other dialects.
Context: ...tool, and suggest re-running the review afterwards. The prompt SHALL NOT edit files autono...
(AFTERWARDS_US)
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: ...nd change scaffolding - [ ] 1.1 Verify .specfact/backlog/github_hierarchy_cache.md is fresh, confirm t...
(GITHUB)
[locale-violation] ~42-~42: 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. - [ ] 6.2 Add the new command to the p...
(AFTERWARDS_US)
🔀 Multi-repo context nold-ai/specfact-cli
nold-ai/specfact-cli — relevant consumers & integration points
-
Semgrep rules/resources (consumed/bundled):
- tools/semgrep/README.md (docs about tools/semgrep/*.yml and bundling) [::nold-ai/specfact-cli::tools/semgrep/README.md:9,13-17]
- src/specfact_cli/resources/semgrep/ (packaged semgrep resources referenced from code) [::nold-ai/specfact-cli::src/specfact_cli/resources/semgrep/]
- pyproject.toml—hatch scripts / CLI shortcuts invoking semgrep configs (scan, scan-all, scan-fix) [::nold-ai/specfact-cli::pyproject.toml:233-236,351]
- .gitignore / .semgrep entries (packaging / artifacts) [::nold-ai/specfact-cli::.gitignore:124-125]
-
Semgrep runner / extraction / mapping to ReviewFinding:
- src/specfact_cli/validators/repro_checker.py — semgrep invocation, extraction, and integration into repro checks (_extract_semgrep_findings, _repro_checks_append_semgrep) [::nold-ai/specfact-cli::src/specfact_cli/validators/repro_checker.py:346-355,1003-1030,1116-1121]
- src/specfact_cli/analyzers/code_analyzer.py — semgrep-driven analysis, finding categorisation, evidence extraction and mapping into features (many helpers: _run_semgrep_patterns, _filter_relevant_semgrep_findings, _categorise_semgrep_finding, _apply_semgrep_findings_to_feature) [::nold-ai/specfact-cli::src/specfact_cli/analyzers/code_analyzer.py:138-146,462-478,555-588,962-1018,1160-1244,1475-1536]
- openspec/specs/semgrep-runner/spec.md — spec requires semgrep output mapped to List[ReviewFinding] with tool="semgrep" and category="clean_code" (relevant for adding ai_bloat category) [::nold-ai/specfact-cli::openspec/specs/semgrep-runner/spec.md:11-17,21-28]
-
Tests & integration points that will need updates or validation coverage:
- tests/unit/validators/test_repro_checker.py — unit tests creating semgrep configs and asserting semgrep calls / extraction [::nold-ai/specfact-cli::tests/unit/validators/test_repro_checker.py:267-316]
- tests/unit/analyzers/test_code_analyzer.py & tests/e2e/test_semgrep_integration_e2e.py — tests verify semgrep env, integration, and processing; adding new rule IDs / category (ai_bloat) may require test fixtures and expectations updates [::nold-ai/specfact-cli::tests/unit/analyzers/test_code_analyzer.py:16-79][::nold-ai/specfact-cli::tests/e2e/test_semgrep_integration_e2e.py:22-78]
- tests/e2e/test_github_action_workflow.py, tests/integration/... — CI/workflow tests that assert semgrep-driven comments/annotations will be relevant [::nold-ai/specfact-cli::tests/e2e/test_github_action_workflow.py:55-362]
-
Packaging / generation of semgrep rules:
- src/specfact_cli/importers/speckit_converter.py — generate_semgrep_rules/export and workflow generator references (used by import/generation flows) [::nold-ai/specfact-cli::src/specfact_cli/importers/speckit_converter.py:338-352]
- src/specfact_cli/generators/workflow_generator.py — generate_semgrep_rules and resource fallback logic (resources vs tools) [::nold-ai/specfact-cli::src/specfact_cli/generators/workflow_generator.py:99-115]
-
Pre-commit / scripts referencing semgrep runtime config:
- scripts/pre_commit_code_review.py — config of SEMGREP_LOG_FILE and cache paths used by pre-commit review flow (ensure ai_bloat advisories propagate and aren't filtered) [::nold-ai/specfact-cli::scripts/pre_commit_code_review.py:186-188]
- scripts/license_allowlist.yaml and docs reference semgrep as an allowed dependency (packaging/licensing considerations) [::nold-ai/specfact-cli::scripts/license_allowlist.yaml:39][::nold-ai/specfact-cli::docs/getting-started/installation.md:407]
-
Spec & policy pack places to update for ai_bloat:
- openspec/specs/clean-code-semgrep-rules/spec.md and other openspec specs reference categories/principles and may need ai_bloat additions or a new ai-bloat-patterns pack (the PR’s proposal to add ai_bloat should align with these specs) [::nold-ai/specfact-cli::openspec/specs/clean-code-semgrep-rules/spec.md:1-16]
- openspec/specs/semgrep-runner/spec.md requires mapping semgrep output to ReviewFinding — will need acceptance of category="ai_bloat" and severity semantics [::nold-ai/specfact-cli::openspec/specs/semgrep-runner/spec.md:11-17]
Summary assessment
- The repo already has a mature semgrep integration surface: runner/specs, code that parses semgrep JSON into ReviewFinding-like structures, bundled resources and tests. Adding ai_bloat will require:
- new semgrep rule files placed under src/specfact_cli/resources/semgrep/ (and/or tools/semgrep/) and packaging updates,
- updates to categorisation logic/tests to accept category/principle "ai_bloat" and ensure severity/default_mode (advisory/info) semantics are preserved,
- test fixtures and CI workflow adjustments where semgrep expectations/assertions assume existing categories/rule-ids,
- policy-pack registration (openspec resources/policy-packs) and any contract tests referencing rule IDs.
🔇 Additional comments (2)
openspec/changes/code-review-ai-bloat-detection/specs/code-review-ai-bloat-detection/spec.md (1)
5-98: LGTM!openspec/changes/code-review-ai-bloat-detection/tasks.md (1)
11-65: LGTM!
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@openspec/changes/code-review-ai-bloat-detection/proposal.md`:
- Around line 35-41: Add an explicit note about required cross-repo coordination
with specfact-cli: update the Impact section to state that specfact-cli must be
updated to accept the new ai_bloat category, including changes to the
category-mapping logic in code_analyzer.py (the category-mapping block that
currently enumerates expected categories), the semgrep_runner.py rule/spec that
enforces category="clean_code" (to allow category="ai_bloat"), and related test
fixtures that assert category values; mention tests and provide or link to the
tracking issue for those specfact-cli changes.
🪄 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: ea283118-3f13-4996-96e1-5f14e9efee20
📒 Files selected for processing (1)
openspec/changes/code-review-ai-bloat-detection/proposal.md
📜 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.12)
- GitHub Check: quality (3.11)
🧰 Additional context used
📓 Path-based instructions (1)
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/proposal.md
🪛 LanguageTool
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)
[style] ~39-~39: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...line against seeded bloat fixtures. - Affected docs: reference docs for the `ai_bloa...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔀 Multi-repo context nold-ai/specfact-cli
nold-ai/specfact-cli — findings
- No existing ai_bloat identifiers found (search returned no matches). [::nold-ai/specfact-cli::]
- Semgrep rules directory present: src/specfact_cli/resources/semgrep (contains async.yml, code-quality.yml, feature-detection.yml) — new ai-bloat.yaml should be placed/packaged here. [::nold-ai/specfact-cli::src/specfact_cli/resources/semgrep/]
- Semgrep-runner spec enforces mapping semgrep → List[ReviewFinding] with category="clean_code" (spec.md header). Adding ai_bloat requires updating spec or allowing category="ai_bloat". [::nold-ai/specfact-cli::openspec/specs/semgrep-runner/spec.md]
- Semgrep extraction/parsing is implemented in _extract_semgrep_findings and callers:
- src/specfact_cli/validators/repro_checker.py::_extract_semgrep_findings (parsing JSON/text → structured findings). New rule IDs will be consumed here. [::nold-ai/specfact-cli::src/specfact_cli/validators/repro_checker.py:346]
- src/specfact_cli/analyzers/code_analyzer.py contains semgrep helpers and categorisation: _run_semgrep_patterns, _filter_relevant_semgrep_findings, and category-mapping logic (many category checks currently expect existing categories like "clean_code", "api", etc.). These locations likely need updates to accept/category-map ai_bloat and ensure severity/default_mode semantics. [::nold-ai/specfact-cli::src/specfact_cli/analyzers/code_analyzer.py:555,962,1195-1228]
- Tests reference semgrep and category expectations and will need fixture/spec updates:
- tests/unit/analyzers/test_code_analyzer.py (semgrep runner tests). [::nold-ai/specfact-cli::tests/unit/analyzers/test_code_analyzer.py:1-90]
- tests/e2e/test_semgrep_integration_e2e.py and other e2e/CI tests that assert semgrep-driven findings/workflow. [::nold-ai/specfact-cli::tests/e2e/test_semgrep_integration_e2e.py]
- tests/unit/specfact_cli/test_clean_code_principle_gates.py (clean-code principle gates/pack references). Policy-pack and principle documentation will need an ai_bloat parallel pack or spec updates. [::nold-ai/specfact-cli::tests/unit/specfact_cli/test_clean_code_principle_gates.py]
- Policy/spec surfaces referencing clean-code packs/principles are numerous (openspec and .cursor rules). Adding an ai-bloat-patterns policy-pack and updating openspec/specs to accept principle/category "ai_bloat" is required. Examples: .cursor/rules/clean-code-principles.mdc and openspec specs. [::nold-ai/specfact-cli::.cursor/rules/clean-code-principles.mdc][::nold-ai/specfact-cli::openspec/specs/clean-code-semgrep-rules/spec.md]
Implication summary
- Add semgrep rule file(s) (ai-bloat.yaml) under src/specfact_cli/resources/semgrep (and packaging).
- Wire new rule IDs into semgrep parsing (repro_checker / code_analyzer) so ReviewFinding.category/principle accepts "ai_bloat" and preserves severity="info"/advisory semantics.
- Update tests and openspec/policy-pack references to include ai_bloat rule IDs and advisory behavior.
- Update specs (semgrep-runner and related) if they hardcode/assume category="clean_code".
🔇 Additional comments (1)
openspec/changes/code-review-ai-bloat-detection/proposal.md (1)
40-40: LGTM!
| ## Impact | ||
|
|
||
| - **Affected code**: `packages/specfact-code-review/` (new runner, new semgrep rules, new policy pack, manifest version bump, runner orchestration wiring); `packages/specfact-project/` (new packaged prompt resource and manifest payload). | ||
| - **Affected tests**: targeted unit tests per detector (one fixture pair per heuristic), semgrep rule fixture tests, policy-pack reference contract tests, and an integration test that exercises the end-to-end review pipeline against seeded bloat fixtures. | ||
| - **Affected docs**: reference docs for the `ai_bloat` category, the slash command, and the `advisory`-only model under code-review and review-run on modules.specfact.io, cross-referenced from `clean-code-principles` docs; root `README.md` headline callout; a quickstart walkthrough page demonstrating the end-to-end review → simplify → re-review flow with before/after evidence captured from a real repo; a modules.specfact.io homepage callout. All marketing copy uses the framing "bloat detection tuned for the shapes AI code commonly produces" and does not claim AI-authorship classification. A parallel callout in the core `specfact-cli` README is tracked as a follow-up issue out of scope for this PR. | ||
| - **Release impact**: patch version bumps and signature/registry updates for `specfact-code-review` (new resources) and `specfact-project` (new prompt resource); no breaking change to existing finding consumers because `ai_bloat` findings from both packages are purely additive and info-only (emitted at `severity=info`, never `warning` or `error`). | ||
|
|
There was a problem hiding this comment.
Document the required cross-repo coordination with specfact-cli.
The Impact section claims "no breaking change to existing finding consumers" (line 40), but linked repository findings show that specfact-cli requires coordinated updates to consume the new ai_bloat category. Specifically:
code_analyzer.pyhas category-mapping logic (lines 1195–1228) that currently expects existing categories and needs updates to accept/category-mapai_bloatsemgrep_runner.pyspec enforcescategory="clean_code"and needs updating to allowcategory="ai_bloat"- Test fixtures assert category expectations and will need updates
Without mentioning this cross-repo dependency, the release plan is incomplete and risks shipping modules that emit findings the CLI cannot properly consume.
Recommendation: Add a bullet under "Affected code" or "Release impact" noting that specfact-cli requires coordinated updates (spec, category-mapping, tests) to consume the new category, and link the corresponding tracking issue if one exists.
As per coding guidelines, tone instructions require focus on "specfact_cli impact" and linked repository findings show "Adding ai_bloat requires updating spec or allowing category='ai_bloat'" in specfact-cli.
🧰 Tools
🪛 LanguageTool
[style] ~39-~39: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...line against seeded bloat fixtures. - Affected docs: reference docs for the `ai_bloa...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🤖 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 `@openspec/changes/code-review-ai-bloat-detection/proposal.md` around lines 35
- 41, Add an explicit note about required cross-repo coordination with
specfact-cli: update the Impact section to state that specfact-cli must be
updated to accept the new ai_bloat category, including changes to the
category-mapping logic in code_analyzer.py (the category-mapping block that
currently enumerates expected categories), the semgrep_runner.py rule/spec that
enforces category="clean_code" (to allow category="ai_bloat"), and related test
fixtures that assert category values; mention tests and provide or link to the
tracking issue for those specfact-cli changes.
1 participant
Summary
Introduce AI code bloat detection and review with --fix parameter to remove the moat.
Refs:
Scope
packages/registry/index.json,packages/*/module-package.yaml).github/workflows/*)docs/*,README.md,AGENTS.md)scripts/sign-modules.py,scripts/verify-modules-signature.py)Bundle Impact
List impacted bundles and version updates:
nold-ai/specfact-project:old -> newnold-ai/specfact-backlog:old -> newnold-ai/specfact-codebase:old -> newnold-ai/specfact-spec:old -> newnold-ai/specfact-govern:old -> newValidation Evidence
Paste command output snippets or link workflow runs.
Required local gates
hatch run formathatch run type-checkhatch run linthatch run yaml-linthatch run check-bundle-importshatch run contract-testhatch run smart-test(orhatch run test)Signature + version integrity (required)
hatch run verify-modules-signature --payload-from-filesystem --enforce-version-bumppasses (matches PRs targetingdev)main, also confirmed:hatch run verify-modules-signature --require-signature --payload-from-filesystem --enforce-version-bump(and/or approval triggeredsign-modules-on-approvalfor same-repo PRs)dev/mainsame-repo PRs)CI and Branch Protection
verify-module-signaturessign-modules-on-approval(on approval, same-repo PRs todev/mainonly)quality (3.11)quality (3.12)quality (3.13)Docs / Pages
docs/)docs-pages.yml, if changed)specfact-clidocs updated (if applicable)Checklist