Conversation
docs(code-review): finalize PR 289 signing evidence
…orecast-agent-handoff Implement cleanup forecast handoff
|
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. |
|
Warning Review limit reached
Your plan includes 5 reviews of capacity. Refill in 20 minutes and 59 seconds. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (8)
📝 WalkthroughWalkthroughAdds simplify-focused cleanup_forecast and AI‑IDE remediation handoff: new report schema fields, non‑mutating preview and optional mutation evidence, preserve‑reason detection and enrichment, CLI flags/validation, forecast aggregation, docs/skill updates, tests, and package/registry updates. ChangesCleanup forecast & remediation handoff
Sequence Diagram(s) sequenceDiagram
participant CLI
participant Runner
participant CleanupEvidence
participant Forecast
participant Report
CLI->>Runner: run --focus simplify [--preview-fixes|--with-mutation]
Runner->>CleanupEvidence: preview fixes / record mutation evidence
CleanupEvidence->>Forecast: recompute build_cleanup_forecast(findings, files)
Forecast->>Runner: attach cleanup_forecast
Runner->>Report: emit JSON report (cleanup_forecast, signal_trace, remediation_packet)
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Focus notes (correctness/security/regressions)
✨ Finishing Touches🧪 Generate unit tests (beta)
|
djm81
added
enhancement
![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: 17b0e62560
ℹ️ 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
registry/index.json (1)
1-96:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winFix registry artifact integrity: add/prune missing sha256 sidecars
- The originally called-out intermediate versions aren’t orphaned:
specfact-code-review-0.47.28.tar.gzandspecfact-code-review-0.47.29.tar.gzboth have matching*.sha256sidecars.- Registry integrity is still off: 80/177
registry/modules/*.tar.gztarballs are missing corresponding*.tar.gz.sha256, while thelatest_versionentries inregistry/index.jsondo have checksums.- Either add the missing
sha256sidecars for those tarballs or remove the unverifiable tarballs to avoid checksum/verification failures for older pinned versions.🤖 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 `@registry/index.json` around lines 1 - 96, The registry index references checksum_sha256 values (e.g., in module entries "nold-ai/specfact-code-review" with latest_version "0.47.30" and other modules like "nold-ai/specfact-codebase"/"specfact-project") but many tarballs under registry/modules/*.tar.gz lack corresponding .tar.gz.sha256 sidecar files; update the registry by either adding the missing SHA256 sidecar files for each tarball that has an entry in index.json (ensuring the sidecar contents match the checksum_sha256 field for the corresponding module id/latest_version) or remove the unverifiable tarballs that don't have matching sidecars so older pinned versions are not left unverifiable — apply this to the affected modules (e.g., specfact-code-review v0.47.28/0.47.29 and the ~80 missing artifacts) and ensure index.json checksums remain consistent with the added/removed artifacts.
🧹 Nitpick comments (4)
packages/specfact-code-review/src/specfact_code_review/run/runner.py (1)
419-434: ⚡ Quick winCache parsed AST per file in preserve-reason enrichment.
_preserve_reasons_for_findingreparses the same file for every simplify finding. On large reports this becomes a hot path and can noticeably slow--focus simplifyruns.♻️ Suggested change
-from functools import partial +from functools import lru_cache, partial @@ +@lru_cache(maxsize=256) +def _parsed_source_for_preserve(path: str) -> tuple[ast.Module, list[str]] | None: + try: + source = Path(path).read_text(encoding="utf-8") + return ast.parse(source, filename=path), source.splitlines() + except (OSError, SyntaxError, UnicodeDecodeError): + return None + def _preserve_reasons_for_finding(finding: ReviewFinding, *, load_bearing: bool) -> list[PreserveReasonEvidence]: @@ - try: - source = Path(finding.file).read_text(encoding="utf-8") - tree = ast.parse(source, filename=finding.file) - except (OSError, SyntaxError, UnicodeDecodeError): + parsed = _parsed_source_for_preserve(finding.file) + if parsed is None: return reasons - lines = source.splitlines() + tree, lines = parsed🤖 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 `@packages/specfact-code-review/src/specfact_code_review/run/runner.py` around lines 419 - 434, _preserve_reasons_for_finding currently reparses the same file for every finding which is slow; introduce a cache for parsed ASTs (e.g., a module-level dict or a `@functools.lru_cache-decorated` helper) and use it from _preserve_reasons_for_finding instead of calling Path(...).read_text/ast.parse each time; add a helper like _get_parsed_ast(file_path: str) -> Optional[ast.AST] (or similar) that returns the parsed tree or None on OSError/SyntaxError/UnicodeDecodeError and reference that helper inside _preserve_reasons_for_finding so multiple calls for the same finding.file reuse the cached AST.tests/unit/specfact_code_review/run/test_commands.py (1)
315-341: ⚡ Quick winAdd the missing negative-path test for
--preview-fixeswithout simplify focus.The new validation branch for preview-without-simplify should have a direct unit assertion, mirroring the mutation-focus rejection test, so this CLI guard can’t regress silently.
As per coding guidelines
tests/**/*.py: Contract-first and integration tests: migration suites, bundle validation, and flakiness. Ensure changes to adapters or bridges have targeted coverage.🤖 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/specfact_code_review/run/test_commands.py` around lines 315 - 341, Add a negative-path unit test that asserts the CLI rejects using --preview-fixes without --focus simplify: create a function (e.g., test_run_command_rejects_preview_fixes_without_simplify_focus) that calls runner.invoke(app, ["review", "run", "--preview-fixes", "tests/fixtures/review/clean_module.py"]) and then asserts result.exit_code == 2 and that the output contains the validation message "Use --preview-fixes only with --focus simplify"; place this new test alongside the existing tests such as test_run_command_rejects_preview_fixes_with_fix and test_run_command_rejects_with_mutation_without_simplify_focus so it covers the new validation branch.packages/specfact-code-review/src/specfact_code_review/run/forecast.py (1)
82-85: 💤 Low valueTest directory detection may miss
test/directories.The check
"tests" in file_path.partsonly matches the exact string "tests". Some Python projects usetest/(singular) as their test directory. This could misclassify test code as production LOC.If this is intentional for consistency with repo conventions, no change needed. Otherwise, consider:
♻️ Optional: broaden test detection
- if "tests" in file_path.parts: + if "tests" in file_path.parts or "test" in file_path.parts:🤖 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 `@packages/specfact-code-review/src/specfact_code_review/run/forecast.py` around lines 82 - 85, The current LOC classification only treats paths containing the segment "tests" as test files which misses directories named "test"; update the detection in the code that uses file_path.parts (the branch that increments tests vs production) to consider both "test" and "tests" (e.g., check if any part is in the set {"test","tests"}) so files under either directory are counted toward tests rather than production; adjust the condition where tests += loc and production += loc accordingly (preserving existing variables/tests and production accumulation).tests/unit/specfact_code_review/run/test_forecast.py (1)
36-58: ⚡ Quick winExpand this test to pin all fixed guidance weights.
The current assertion only locks
safe_mechanicalandpreserve. Addneeds_testsanddesign_judgmentcases so the V1 weight contract (1.0/0.6/0.25/0.0) can’t drift silently.As per coding guidelines,
openspec/changes/code-review-13-cleanup-forecast-agent-handoff/design.mddefines fixed V1 forecast weights that should be preserved.🤖 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/specfact_code_review/run/test_forecast.py` around lines 36 - 58, Add two additional findings to the test_build_cleanup_forecast_counts_loc_and_weighted_bloat (use _finding with guidance_kind="needs_tests" and guidance_kind="design_judgment") so the build_cleanup_forecast input covers all four guidance kinds, and add assertions on forecast.by_guidance_kind[...] to pin the V1 weights: safe_mechanical == 1.0, needs_tests == 0.6, design_judgment == 0.25, preserve == 0.0; reference the test name test_build_cleanup_forecast_counts_loc_and_weighted_bloat, the helper _finding, and the forecast.by_guidance_kind mapping and weight property to locate where to add the new findings and assertions.
🤖 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/bundles/code-review/run.md`:
- Around line 104-106: The docs changed the output path to
.specfact/code-review-simplify.json which will break existing consumers that
expect .specfact/code-review.json; update the documented commands so they either
write simplify output to both paths (e.g., run specfact code review with --out
.specfact/code-review-simplify.json and also --out .specfact/code-review.json)
or explicitly call out that callers must be updated, and coordinate updates to
the consumer artifacts referenced (scripts/pre_commit_code_review.py,
openspec/config.yaml, and any TDD evidence scripts that hardcode
.specfact/code-review.json) so they accept the new simplify path or the docs
state that simplify writes to the canonical .specfact/code-review.json as well.
In `@packages/specfact-code-review/src/specfact_code_review/review/commands.py`:
- Around line 34-35: Update the user-facing example/instructions so they also
emit or recommend the canonical review JSON path to stay compatible with
existing consumers: when showing the run invocation (the example containing
"specfact code review run --scope changed --focus simplify --preview-fixes
--json --out .specfact/code-review-simplify.json"), include guidance to write to
the canonical file ".specfact/code-review.json" as well (either by changing
"--out" to that path or by showing a dual-output/merge pattern), and apply the
same change to the other examples referenced around the same section (the
occurrences noted at lines 53-55).
In `@tests/unit/specfact_code_review/run/test_findings.py`:
- Around line 460-486: Add a second assertion path that constructs a
ReviewReport whose findings include handoff metadata (create a Finding instance
with signal_trace, preserve_reasons and remediation_packet populated) and assert
report.schema_version == "1.3" and that at least one finding exposes the handoff
fields (e.g., check finding.signal_trace is not None and
finding.preserve_reasons is not None and finding.remediation_packet is not
None); update
test_review_report_uses_schema_1_3_when_cleanup_forecast_is_present to include
or add a new test that uses ReviewReport, Finding, and the
signal_trace/preserve_reasons/remediation_packet symbols so the suite fails if
emitting finding-level handoff metadata does not bump schema to "1.3".
---
Outside diff comments:
In `@registry/index.json`:
- Around line 1-96: The registry index references checksum_sha256 values (e.g.,
in module entries "nold-ai/specfact-code-review" with latest_version "0.47.30"
and other modules like "nold-ai/specfact-codebase"/"specfact-project") but many
tarballs under registry/modules/*.tar.gz lack corresponding .tar.gz.sha256
sidecar files; update the registry by either adding the missing SHA256 sidecar
files for each tarball that has an entry in index.json (ensuring the sidecar
contents match the checksum_sha256 field for the corresponding module
id/latest_version) or remove the unverifiable tarballs that don't have matching
sidecars so older pinned versions are not left unverifiable — apply this to the
affected modules (e.g., specfact-code-review v0.47.28/0.47.29 and the ~80
missing artifacts) and ensure index.json checksums remain consistent with the
added/removed artifacts.
---
Nitpick comments:
In `@packages/specfact-code-review/src/specfact_code_review/run/forecast.py`:
- Around line 82-85: The current LOC classification only treats paths containing
the segment "tests" as test files which misses directories named "test"; update
the detection in the code that uses file_path.parts (the branch that increments
tests vs production) to consider both "test" and "tests" (e.g., check if any
part is in the set {"test","tests"}) so files under either directory are counted
toward tests rather than production; adjust the condition where tests += loc and
production += loc accordingly (preserving existing variables/tests and
production accumulation).
In `@packages/specfact-code-review/src/specfact_code_review/run/runner.py`:
- Around line 419-434: _preserve_reasons_for_finding currently reparses the same
file for every finding which is slow; introduce a cache for parsed ASTs (e.g., a
module-level dict or a `@functools.lru_cache-decorated` helper) and use it from
_preserve_reasons_for_finding instead of calling Path(...).read_text/ast.parse
each time; add a helper like _get_parsed_ast(file_path: str) ->
Optional[ast.AST] (or similar) that returns the parsed tree or None on
OSError/SyntaxError/UnicodeDecodeError and reference that helper inside
_preserve_reasons_for_finding so multiple calls for the same finding.file reuse
the cached AST.
In `@tests/unit/specfact_code_review/run/test_commands.py`:
- Around line 315-341: Add a negative-path unit test that asserts the CLI
rejects using --preview-fixes without --focus simplify: create a function (e.g.,
test_run_command_rejects_preview_fixes_without_simplify_focus) that calls
runner.invoke(app, ["review", "run", "--preview-fixes",
"tests/fixtures/review/clean_module.py"]) and then asserts result.exit_code == 2
and that the output contains the validation message "Use --preview-fixes only
with --focus simplify"; place this new test alongside the existing tests such as
test_run_command_rejects_preview_fixes_with_fix and
test_run_command_rejects_with_mutation_without_simplify_focus so it covers the
new validation branch.
In `@tests/unit/specfact_code_review/run/test_forecast.py`:
- Around line 36-58: Add two additional findings to the
test_build_cleanup_forecast_counts_loc_and_weighted_bloat (use _finding with
guidance_kind="needs_tests" and guidance_kind="design_judgment") so the
build_cleanup_forecast input covers all four guidance kinds, and add assertions
on forecast.by_guidance_kind[...] to pin the V1 weights: safe_mechanical == 1.0,
needs_tests == 0.6, design_judgment == 0.25, preserve == 0.0; reference the test
name test_build_cleanup_forecast_counts_loc_and_weighted_bloat, the helper
_finding, and the forecast.by_guidance_kind mapping and weight property to
locate where to add the new findings and assertions.
🪄 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: 13891b8a-32e6-4934-94d7-3d1364b145f6
⛔ Files ignored due to path filters (4)
registry/modules/specfact-code-review-0.47.28.tar.gzis excluded by!**/*.gzregistry/modules/specfact-code-review-0.47.29.tar.gzis excluded by!**/*.gzregistry/modules/specfact-code-review-0.47.30.tar.gzis excluded by!**/*.gzregistry/modules/specfact-project-0.41.18.tar.gzis excluded by!**/*.gz
📒 Files selected for processing (38)
docs/bundles/code-review/run.mddocs/modules/code-review.mddocs/quickstart-ai-bloat.mdopenspec/CHANGE_ORDER.mdopenspec/changes/code-review-12-guided-simplification-enforcement/TDD_EVIDENCE.mdopenspec/changes/code-review-13-cleanup-forecast-agent-handoff/.openspec.yamlopenspec/changes/code-review-13-cleanup-forecast-agent-handoff/README.mdopenspec/changes/code-review-13-cleanup-forecast-agent-handoff/TDD_EVIDENCE.mdopenspec/changes/code-review-13-cleanup-forecast-agent-handoff/design.mdopenspec/changes/code-review-13-cleanup-forecast-agent-handoff/proposal.mdopenspec/changes/code-review-13-cleanup-forecast-agent-handoff/specs/ai-ide-remediation-handoff/spec.mdopenspec/changes/code-review-13-cleanup-forecast-agent-handoff/specs/cleanup-forecast-review/spec.mdopenspec/changes/code-review-13-cleanup-forecast-agent-handoff/specs/review-finding-model/spec.mdopenspec/changes/code-review-13-cleanup-forecast-agent-handoff/specs/review-run-command/spec.mdopenspec/changes/code-review-13-cleanup-forecast-agent-handoff/tasks.mdpackages/specfact-code-review/module-package.yamlpackages/specfact-code-review/src/specfact_code_review/resources/skills/specfact-code-review/SKILL.mdpackages/specfact-code-review/src/specfact_code_review/review/commands.pypackages/specfact-code-review/src/specfact_code_review/rules/updater.pypackages/specfact-code-review/src/specfact_code_review/run/cleanup_evidence.pypackages/specfact-code-review/src/specfact_code_review/run/commands.pypackages/specfact-code-review/src/specfact_code_review/run/findings.pypackages/specfact-code-review/src/specfact_code_review/run/forecast.pypackages/specfact-code-review/src/specfact_code_review/run/runner.pypackages/specfact-project/module-package.yamlpackages/specfact-project/resources/prompts/specfact.08-simplify.mdregistry/index.jsonregistry/modules/specfact-code-review-0.47.28.tar.gz.sha256registry/modules/specfact-code-review-0.47.29.tar.gz.sha256registry/modules/specfact-code-review-0.47.30.tar.gz.sha256registry/modules/specfact-project-0.41.18.tar.gz.sha256tests/cli-contracts/specfact-code-review-run.scenarios.yamltests/unit/specfact_code_review/review/test_commands.pytests/unit/specfact_code_review/run/test_cleanup_evidence.pytests/unit/specfact_code_review/run/test_commands.pytests/unit/specfact_code_review/run/test_findings.pytests/unit/specfact_code_review/run/test_forecast.pytests/unit/specfact_code_review/run/test_runner.py
📜 Review details
🧰 Additional context used
📓 Path-based instructions (7)
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-13-cleanup-forecast-agent-handoff/README.mdopenspec/changes/code-review-13-cleanup-forecast-agent-handoff/TDD_EVIDENCE.mdopenspec/CHANGE_ORDER.mdopenspec/changes/code-review-13-cleanup-forecast-agent-handoff/specs/cleanup-forecast-review/spec.mdopenspec/changes/code-review-13-cleanup-forecast-agent-handoff/tasks.mdopenspec/changes/code-review-13-cleanup-forecast-agent-handoff/design.mdopenspec/changes/code-review-12-guided-simplification-enforcement/TDD_EVIDENCE.mdopenspec/changes/code-review-13-cleanup-forecast-agent-handoff/specs/ai-ide-remediation-handoff/spec.mdopenspec/changes/code-review-13-cleanup-forecast-agent-handoff/specs/review-run-command/spec.mdopenspec/changes/code-review-13-cleanup-forecast-agent-handoff/proposal.mdopenspec/changes/code-review-13-cleanup-forecast-agent-handoff/specs/review-finding-model/spec.md
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
registry/**
⚙️ CodeRabbit configuration file
registry/**: Registry and index consistency: bundle listings, version pins, and compatibility with
published module artifacts.
Files:
registry/index.jsonregistry/modules/specfact-code-review-0.47.28.tar.gz.sha256registry/modules/specfact-code-review-0.47.30.tar.gz.sha256registry/modules/specfact-code-review-0.47.29.tar.gz.sha256registry/modules/specfact-project-0.41.18.tar.gz.sha256
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/quickstart-ai-bloat.mddocs/bundles/code-review/run.mddocs/modules/code-review.md
**/*.{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:
packages/specfact-code-review/src/specfact_code_review/rules/updater.pytests/unit/specfact_code_review/review/test_commands.pytests/unit/specfact_code_review/run/test_cleanup_evidence.pytests/unit/specfact_code_review/run/test_forecast.pypackages/specfact-code-review/src/specfact_code_review/review/commands.pypackages/specfact-code-review/src/specfact_code_review/run/runner.pypackages/specfact-code-review/src/specfact_code_review/run/forecast.pytests/unit/specfact_code_review/run/test_commands.pytests/unit/specfact_code_review/run/test_runner.pypackages/specfact-code-review/src/specfact_code_review/run/commands.pypackages/specfact-code-review/src/specfact_code_review/run/cleanup_evidence.pytests/unit/specfact_code_review/run/test_findings.pypackages/specfact-code-review/src/specfact_code_review/run/findings.py
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/rules/updater.pypackages/specfact-code-review/src/specfact_code_review/review/commands.pypackages/specfact-code-review/src/specfact_code_review/run/runner.pypackages/specfact-code-review/src/specfact_code_review/run/forecast.pypackages/specfact-code-review/src/specfact_code_review/run/commands.pypackages/specfact-code-review/src/specfact_code_review/run/cleanup_evidence.pypackages/specfact-code-review/src/specfact_code_review/run/findings.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/unit/specfact_code_review/review/test_commands.pytests/unit/specfact_code_review/run/test_cleanup_evidence.pytests/unit/specfact_code_review/run/test_forecast.pytests/unit/specfact_code_review/run/test_commands.pytests/unit/specfact_code_review/run/test_runner.pytests/unit/specfact_code_review/run/test_findings.py
🪛 LanguageTool
openspec/changes/code-review-13-cleanup-forecast-agent-handoff/tasks.md
[grammar] ~25-~25: Use a hyphen to join words.
Context: .... ## 4. Preview, preserve, and mutation proof - [x] 4.1 Implement non-mutating ...
(QB_NEW_EN_HYPHEN)
[grammar] ~29-~29: Use a hyphen to join words.
Context: ...alculated. - [x] 4.3 Add opt-in mutation proof scaffolding for simplify candidate...
(QB_NEW_EN_HYPHEN)
openspec/changes/code-review-13-cleanup-forecast-agent-handoff/proposal.md
[style] ~36-~36: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ... required fields remain compatible. - Affected command surface: `specfact code revie...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔀 Multi-repo context nold-ai/specfact-cli
Findings (nold-ai/specfact-cli)
-
Pre-commit hook and repo gates expect the canonical review JSON at .specfact/code-review.json; changes that write an alternate simplify file (.specfact/code-review-simplify.json) could be ignored by existing consumers unless callers are updated to point at the new file:
- scripts/pre_commit_code_review.py — prints/reads REVIEW_JSON_OUT and expects the review JSON report path; used by pre-commit hook and scripts/pre-commit-quality-checks.sh [::nold-ai/specfact-cli::scripts/pre_commit_code_review.py:210-256, 336-347]
- scripts/setup-git-hooks.sh — documents running specfact code review as staged-file pre-commit gate [::nold-ai/specfact-cli::scripts/setup-git-hooks.sh:58-75]
- scripts/pre-commit-quality-checks.sh — orchestration of code review gate in pre-commit pipeline [::nold-ai/specfact-cli::scripts/pre-commit-quality-checks.sh:39-41,116,389-397]
-
Openspec / CI expectations reference the canonical .specfact/code-review.json; consumers of code-review output in openspec tasks and TDD evidence may need updates or parallel expectations for simplify-focused output:
- openspec/config.yaml — mandates code review JSON (dogfood) at .specfact/code-review.json for OpenSpec validations and gates [::nold-ai/specfact-cli::openspec/config.yaml:64-66,178-185]
- multiple openspec change/TDD files call
hatch run specfact code review run --json --out .specfact/code-review.jsonin CI/docs evidence — these runs will not pick up a new .specfact/code-review-simplify.json unless explicitly changed [::nold-ai/specfact-cli::openspec/** (many TDD/docs entries referencing the exact command)]
-
Docs and module-install/onboarding references expect the existing code review report path and behavior; new flags/outputs (--preview-fixes, --with-mutation, cleanup_forecast, remediation_packet, preserve_reasons) are not automatically visible to existing documentation consumers unless docs/examples are updated to point to the simplify JSON:
- docs/modules/code-review.md — documents
specfact code review run --json --out .specfact/code-review.jsonand the ai_bloat simplify workflow [::nold-ai/specfact-cli::docs/modules/code-review.md] - tests/scripts/docs/tests assert presence of
specfact code review runand review JSON in repo docs and test fixtures (dogfood self-review tests expect the canonical JSON) [::nold-ai/specfact-cli::tests/unit/scripts/test_code_review_module_docs.py; tests/unit/specfact_cli/test_dogfood_self_review.py]
- docs/modules/code-review.md — documents
-
Risk/impact summary:
- If the new simplify-focused output is emitted only to .specfact/code-review-simplify.json, automation and CI gates (pre-commit, openspec TDD evidence, docs/tests) that still read .specfact/code-review.json will not see the simplify-only findings/cleanup_forecast unless callers are adjusted to request or merge the simplify output.
- Any automation that expects schema versioning/fields in the canonical code-review JSON may need to tolerate the new optional fields (cleanup_forecast, remediation_packet, preserve_reasons, signal_trace) or be updated to read the simplify file.
Recommendation (concise): Ensure CI/pre-commit scripts and openspec/TDD instructions that currently read .specfact/code-review.json are updated (or accept) the new simplify JSON path or that specfact code review run --focus simplify --preview-fixes can also write/update the canonical .specfact/code-review.json as needed.
🔇 Additional comments (51)
packages/specfact-project/module-package.yaml (1)
2-2: LGTM!Also applies to: 30-31
tests/unit/specfact_code_review/run/test_runner.py (1)
10-17: LGTM!Also applies to: 231-317, 349-442
packages/specfact-code-review/src/specfact_code_review/review/commands.py (1)
148-157: LGTM!Also applies to: 198-200
packages/specfact-code-review/src/specfact_code_review/run/commands.py (1)
19-22: LGTM!Also applies to: 70-71, 84-85, 332-341, 658-660, 678-680, 697-704, 745-750, 866-867, 901-907, 923-924, 977-979
tests/cli-contracts/specfact-code-review-run.scenarios.yaml (1)
171-192: LGTM!tests/unit/specfact_code_review/run/test_commands.py (1)
343-395: LGTM!Also applies to: 616-617
packages/specfact-code-review/src/specfact_code_review/run/findings.py (6)
32-97: LGTM!
99-147: LGTM!
149-202: LGTM!
293-340: LGTM!
408-420: LGTM!
471-502: LGTM!packages/specfact-code-review/src/specfact_code_review/run/forecast.py (3)
1-36: LGTM!
39-69: LGTM!
97-121: LGTM!packages/specfact-code-review/src/specfact_code_review/run/cleanup_evidence.py (2)
23-58: LGTM!
60-185: LGTM!tests/unit/specfact_code_review/run/test_cleanup_evidence.py (1)
11-67: LGTM!tests/unit/specfact_code_review/run/test_findings.py (1)
9-234: LGTM!Also applies to: 488-499
tests/unit/specfact_code_review/run/test_forecast.py (1)
1-34: LGTM!Also applies to: 60-68
docs/modules/code-review.md (2)
125-125: Cross-repository breaking change: duplicate concern.Same issue flagged in
docs/bundles/code-review/run.mdlines 104-106: the new.specfact/code-review-simplify.jsonpath is not compatible with existingspecfact-cliautomation that expects.specfact/code-review.json. See first comment for coordination checklist.Also applies to: 202-206
62-66: LGTM!Also applies to: 83-85, 136-143
docs/quickstart-ai-bloat.md (2)
26-27: Cross-repository breaking change: duplicate concern.Same issue flagged in
docs/bundles/code-review/run.mdlines 104-106: the.specfact/code-review-simplify.jsonpath won't be picked up by existingspecfact-cliautomation. See first comment for coordination checklist.Also applies to: 91-91
13-13: LGTM!Also applies to: 23-29, 33-34
packages/specfact-code-review/src/specfact_code_review/resources/skills/specfact-code-review/SKILL.md (2)
11-11: Cross-repository breaking change: duplicate concern.Same issue flagged in
docs/bundles/code-review/run.mdlines 104-106: the.specfact/code-review-simplify.jsonpath won't be visible tospecfact-cliautomation. See first comment for coordination checklist.
12-14: LGTM!Also applies to: 26-26, 29-29
packages/specfact-code-review/src/specfact_code_review/rules/updater.py (2)
35-37: Cross-repository breaking change: duplicate concern.Same issue flagged in
docs/bundles/code-review/run.mdlines 104-106: the.specfact/code-review-simplify.jsonpath hardcoded in default rules won't be visible tospecfact-cliautomation. See first comment for coordination checklist.
65-65: LGTM!packages/specfact-project/resources/prompts/specfact.08-simplify.md (2)
59-60: Cross-repository breaking change: duplicate concern.Same issue flagged in
docs/bundles/code-review/run.mdlines 104-106: the.specfact/code-review-simplify.jsonpath in the prompt won't be compatible withspecfact-cliautomation. See first comment for coordination checklist.Also applies to: 127-127
62-65: LGTM!Also applies to: 79-79
docs/bundles/code-review/run.md (1)
42-43: LGTM!Also applies to: 58-60
tests/unit/specfact_code_review/review/test_commands.py (1)
44-50: LGTM!openspec/changes/code-review-13-cleanup-forecast-agent-handoff/design.md (1)
1-38: LGTM!openspec/changes/code-review-13-cleanup-forecast-agent-handoff/specs/ai-ide-remediation-handoff/spec.md (1)
1-20: LGTM!openspec/changes/code-review-13-cleanup-forecast-agent-handoff/specs/cleanup-forecast-review/spec.md (1)
1-41: LGTM!openspec/changes/code-review-13-cleanup-forecast-agent-handoff/specs/review-finding-model/spec.md (1)
1-26: LGTM!openspec/changes/code-review-13-cleanup-forecast-agent-handoff/specs/review-run-command/spec.md (1)
1-27: LGTM!openspec/changes/code-review-13-cleanup-forecast-agent-handoff/tasks.md (1)
1-45: LGTM!openspec/CHANGE_ORDER.md (1)
106-106: LGTM!openspec/changes/code-review-13-cleanup-forecast-agent-handoff/proposal.md (1)
35-35: 🏗️ Heavy liftClarify simplify-report output vs CI gate artifact
The “Affected JSON” statement for
openspec/changes/code-review-13-cleanup-forecast-agent-handoff/proposal.mdis aligned with the implementation:--focus simplifyonly populates simplify-specific fields likecleanup_forecast, and the JSON output path is determined by the caller’s--out(there’s no implicit redirection based on focus). Promotion/touched-scope gates for this change still runspecfact code review run --bug-hunt --json --out .specfact/code-review.json --scope changed, so core automation remains tied to.specfact/code-review.json, not the simplify file.
Optional: add a short clarifier that.specfact/code-review-simplify.jsonis the simplify/IDE aux artifact, while.specfact/code-review.jsonis the mandatory gate evidence.> Likely an incorrect or invalid review comment.openspec/changes/code-review-13-cleanup-forecast-agent-handoff/.openspec.yaml (1)
1-2: LGTM!openspec/changes/code-review-13-cleanup-forecast-agent-handoff/README.md (1)
1-3: LGTM!openspec/changes/code-review-13-cleanup-forecast-agent-handoff/TDD_EVIDENCE.md (1)
1-56: LGTM!openspec/changes/code-review-12-guided-simplification-enforcement/TDD_EVIDENCE.md (1)
127-129: ⚡ Quick winFix the scope of the signing-evidence concern: the
0.47.27note incode-review-12is PR#289-specific, not the0.47.30promotion version.
openspec/changes/code-review-12-guided-simplification-enforcement/TDD_EVIDENCE.mdis describing the PR#289follow-up updatingspecfact-code-reviewto0.47.27and the intended integrity model (source payload vs published tarball sidecar). Thecode-review-13promotion of0.47.30is handled by its required version-bump/signature verification gate (e.g.,verify-modules-signature --payload-from-filesystem --enforce-version-bump), so adding prose about0.47.30into thecode-review-12file is unnecessary.registry/modules/specfact-code-review-0.47.28.tar.gz.sha256 (1)
1-1: Intermediate version checksum without registry entry.This sidecar file exists for version
0.47.28, but the registry index only lists0.47.30as the latest version. If the tarball artifact is retained for rollback or historical purposes, consider documenting the retention policy; otherwise, this file should be removed to match the registry index state.registry/modules/specfact-code-review-0.47.29.tar.gz.sha256 (1)
1-1: Intermediate version checksum without registry entry.Same issue as
0.47.28: version0.47.29sidecar exists but is not indexed inregistry/index.json.packages/specfact-code-review/module-package.yaml (1)
2-2: LGTM!Also applies to: 26-27
registry/index.json (1)
5-7: LGTM!Also applies to: 81-83
registry/modules/specfact-code-review-0.47.30.tar.gz.sha256 (1)
1-1: LGTM!registry/modules/specfact-project-0.41.18.tar.gz.sha256 (2)
1-1: LGTM!
1-1: ⚡ Quick winFix release-integrity parity check for specfact-project (manifest exists, checksum mismatch).
packages/specfact-project/module-package.yamlis present and matches the promotedversion: 0.41.18, but itsintegrity.checksum(sha256:4f2905d8…9d99a2) does not match the registry/promoted tarball checksum (58d5652c6c5e…a51cd, same asregistry/index.jsonandregistry/modules/specfact-project-0.41.18.tar.gz.sha256). Confirm what artifactintegrity.checksumis supposed to cover; if it’s meant to represent the promoted tarball, this is a release-integrity mismatch.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/specfact-code-review/src/specfact_code_review/run/cleanup_evidence.py (1)
54-59: 💤 Low valueRedundant
schema_versionoverride—validator already handles this.
ReviewReport._derive_governance_fieldsruns aftermodel_copyand unconditionally setsschema_version = "1.3"whencleanup_forecastis present. The explicit"schema_version": "1.3"here is therefore redundant. Not a bug—just unnecessary.♻️ Optional cleanup
def with_refreshed_cleanup_forecast(report: ReviewReport, files: list[Path]) -> ReviewReport: return report.model_copy( update={ "cleanup_forecast": build_cleanup_forecast(report.findings, files), - "schema_version": "1.3", } )🤖 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 `@packages/specfact-code-review/src/specfact_code_review/run/cleanup_evidence.py` around lines 54 - 59, The code is redundantly setting "schema_version": "1.3" in the dict passed to report.model_copy; remove that key so model_copy only sets "cleanup_forecast": build_cleanup_forecast(report.findings, files) and let ReviewReport._derive_governance_fields set schema_version when cleanup_forecast is present; update the return statement that calls report.model_copy(...) and delete the explicit schema_version override.
🤖 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 `@packages/specfact-code-review/src/specfact_code_review/run/forecast.py`:
- Around line 82-83: The current test-path detection (if any("test" in
part.lower() for part in file_path.parts): tests += loc) is too broad and
misclassifies production files; update the check to only flag true test
paths/names (e.g., path segment equals "tests", segment startswith or equals
"test", filename startswith "test_" or endswith "_test" or filename endswith
common test suffixes like "_test.py" or ".spec.py") and leave other segments
alone so tests, reviewed_loc and ai_bloat_index are computed correctly; locate
the logic around file_path.parts, tests and loc in forecast.py and replace the
substring check with a stricter regex/explicit conditions that match typical
test directory/file naming patterns.
---
Nitpick comments:
In
`@packages/specfact-code-review/src/specfact_code_review/run/cleanup_evidence.py`:
- Around line 54-59: The code is redundantly setting "schema_version": "1.3" in
the dict passed to report.model_copy; remove that key so model_copy only sets
"cleanup_forecast": build_cleanup_forecast(report.findings, files) and let
ReviewReport._derive_governance_fields set schema_version when cleanup_forecast
is present; update the return statement that calls report.model_copy(...) and
delete the explicit schema_version override.
🪄 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: 66b824c7-e09a-4c1f-bb39-b74a3dc11874
⛔ Files ignored due to path filters (2)
registry/modules/specfact-code-review-0.47.31.tar.gzis excluded by!**/*.gzregistry/modules/specfact-code-review-0.47.32.tar.gzis excluded by!**/*.gz
📒 Files selected for processing (96)
docs/bundles/code-review/run.mdopenspec/changes/code-review-13-cleanup-forecast-agent-handoff/proposal.mdpackages/specfact-code-review/module-package.yamlpackages/specfact-code-review/src/specfact_code_review/review/commands.pypackages/specfact-code-review/src/specfact_code_review/run/cleanup_evidence.pypackages/specfact-code-review/src/specfact_code_review/run/findings.pypackages/specfact-code-review/src/specfact_code_review/run/forecast.pypackages/specfact-code-review/src/specfact_code_review/run/runner.pyregistry/index.jsonregistry/modules/specfact-backlog-0.39.0.tar.gz.sha256registry/modules/specfact-backlog-0.40.0.tar.gz.sha256registry/modules/specfact-backlog-0.40.1.tar.gz.sha256registry/modules/specfact-backlog-0.40.10.tar.gz.sha256registry/modules/specfact-backlog-0.40.11.tar.gz.sha256registry/modules/specfact-backlog-0.40.13.tar.gz.sha256registry/modules/specfact-backlog-0.40.14.tar.gz.sha256registry/modules/specfact-backlog-0.40.15.tar.gz.sha256registry/modules/specfact-backlog-0.40.2.tar.gz.sha256registry/modules/specfact-backlog-0.40.3.tar.gz.sha256registry/modules/specfact-backlog-0.40.4.tar.gz.sha256registry/modules/specfact-backlog-0.40.5.tar.gz.sha256registry/modules/specfact-backlog-0.40.6.tar.gz.sha256registry/modules/specfact-backlog-0.40.7.tar.gz.sha256registry/modules/specfact-backlog-0.40.8.tar.gz.sha256registry/modules/specfact-backlog-0.40.9.tar.gz.sha256registry/modules/specfact-code-review-0.47.31.tar.gz.sha256registry/modules/specfact-code-review-0.47.32.tar.gz.sha256registry/modules/specfact-codebase-0.39.0.tar.gz.sha256registry/modules/specfact-codebase-0.40.0.tar.gz.sha256registry/modules/specfact-codebase-0.40.1.tar.gz.sha256registry/modules/specfact-codebase-0.40.10.tar.gz.sha256registry/modules/specfact-codebase-0.40.11.tar.gz.sha256registry/modules/specfact-codebase-0.40.12.tar.gz.sha256registry/modules/specfact-codebase-0.40.13.tar.gz.sha256registry/modules/specfact-codebase-0.40.14.tar.gz.sha256registry/modules/specfact-codebase-0.40.2.tar.gz.sha256registry/modules/specfact-codebase-0.40.3.tar.gz.sha256registry/modules/specfact-codebase-0.40.4.tar.gz.sha256registry/modules/specfact-codebase-0.40.5.tar.gz.sha256registry/modules/specfact-codebase-0.40.6.tar.gz.sha256registry/modules/specfact-codebase-0.40.7.tar.gz.sha256registry/modules/specfact-codebase-0.40.8.tar.gz.sha256registry/modules/specfact-codebase-0.40.9.tar.gz.sha256registry/modules/specfact-codebase-0.41.0.tar.gz.sha256registry/modules/specfact-govern-0.39.0.tar.gz.sha256registry/modules/specfact-govern-0.40.0.tar.gz.sha256registry/modules/specfact-govern-0.40.1.tar.gz.sha256registry/modules/specfact-govern-0.40.10.tar.gz.sha256registry/modules/specfact-govern-0.40.11.tar.gz.sha256registry/modules/specfact-govern-0.40.12.tar.gz.sha256registry/modules/specfact-govern-0.40.13.tar.gz.sha256registry/modules/specfact-govern-0.40.2.tar.gz.sha256registry/modules/specfact-govern-0.40.3.tar.gz.sha256registry/modules/specfact-govern-0.40.4.tar.gz.sha256registry/modules/specfact-govern-0.40.5.tar.gz.sha256registry/modules/specfact-govern-0.40.6.tar.gz.sha256registry/modules/specfact-govern-0.40.7.tar.gz.sha256registry/modules/specfact-govern-0.40.8.tar.gz.sha256registry/modules/specfact-govern-0.40.9.tar.gz.sha256registry/modules/specfact-project-0.39.0.tar.gz.sha256registry/modules/specfact-project-0.40.0.tar.gz.sha256registry/modules/specfact-project-0.40.1.tar.gz.sha256registry/modules/specfact-project-0.40.11.tar.gz.sha256registry/modules/specfact-project-0.40.12.tar.gz.sha256registry/modules/specfact-project-0.40.13.tar.gz.sha256registry/modules/specfact-project-0.40.14.tar.gz.sha256registry/modules/specfact-project-0.40.15.tar.gz.sha256registry/modules/specfact-project-0.40.2.tar.gz.sha256registry/modules/specfact-project-0.40.20.tar.gz.sha256registry/modules/specfact-project-0.40.3.tar.gz.sha256registry/modules/specfact-project-0.40.4.tar.gz.sha256registry/modules/specfact-project-0.40.5.tar.gz.sha256registry/modules/specfact-project-0.40.6.tar.gz.sha256registry/modules/specfact-project-0.40.7.tar.gz.sha256registry/modules/specfact-project-0.40.8.tar.gz.sha256registry/modules/specfact-project-0.40.9.tar.gz.sha256registry/modules/specfact-spec-0.39.0.tar.gz.sha256registry/modules/specfact-spec-0.40.0.tar.gz.sha256registry/modules/specfact-spec-0.40.1.tar.gz.sha256registry/modules/specfact-spec-0.40.10.tar.gz.sha256registry/modules/specfact-spec-0.40.11.tar.gz.sha256registry/modules/specfact-spec-0.40.12.tar.gz.sha256registry/modules/specfact-spec-0.40.13.tar.gz.sha256registry/modules/specfact-spec-0.40.2.tar.gz.sha256registry/modules/specfact-spec-0.40.3.tar.gz.sha256registry/modules/specfact-spec-0.40.4.tar.gz.sha256registry/modules/specfact-spec-0.40.5.tar.gz.sha256registry/modules/specfact-spec-0.40.6.tar.gz.sha256registry/modules/specfact-spec-0.40.7.tar.gz.sha256registry/modules/specfact-spec-0.40.8.tar.gz.sha256registry/modules/specfact-spec-0.40.9.tar.gz.sha256tests/unit/specfact_code_review/run/test_cleanup_evidence.pytests/unit/specfact_code_review/run/test_commands.pytests/unit/specfact_code_review/run/test_findings.pytests/unit/specfact_code_review/run/test_forecast.pytests/unit/specfact_code_review/run/test_runner.py
✅ Files skipped from review due to trivial changes (61)
- registry/modules/specfact-codebase-0.41.0.tar.gz.sha256
- registry/modules/specfact-project-0.40.12.tar.gz.sha256
- registry/modules/specfact-govern-0.40.6.tar.gz.sha256
- registry/modules/specfact-backlog-0.40.2.tar.gz.sha256
- registry/modules/specfact-backlog-0.40.13.tar.gz.sha256
- registry/modules/specfact-spec-0.40.5.tar.gz.sha256
- registry/modules/specfact-spec-0.40.7.tar.gz.sha256
- registry/modules/specfact-project-0.40.5.tar.gz.sha256
- registry/modules/specfact-spec-0.40.9.tar.gz.sha256
- registry/modules/specfact-codebase-0.40.12.tar.gz.sha256
- registry/modules/specfact-govern-0.40.8.tar.gz.sha256
- registry/modules/specfact-govern-0.40.9.tar.gz.sha256
- registry/modules/specfact-project-0.40.9.tar.gz.sha256
- registry/modules/specfact-project-0.40.13.tar.gz.sha256
- registry/modules/specfact-govern-0.40.13.tar.gz.sha256
- registry/modules/specfact-backlog-0.40.4.tar.gz.sha256
- registry/modules/specfact-backlog-0.40.0.tar.gz.sha256
- registry/modules/specfact-backlog-0.40.15.tar.gz.sha256
- registry/modules/specfact-spec-0.40.12.tar.gz.sha256
- registry/modules/specfact-project-0.40.4.tar.gz.sha256
- registry/modules/specfact-project-0.40.14.tar.gz.sha256
- registry/modules/specfact-govern-0.40.10.tar.gz.sha256
- registry/modules/specfact-project-0.40.2.tar.gz.sha256
- registry/modules/specfact-codebase-0.40.8.tar.gz.sha256
- registry/modules/specfact-spec-0.40.10.tar.gz.sha256
- registry/modules/specfact-spec-0.40.13.tar.gz.sha256
- registry/modules/specfact-backlog-0.40.8.tar.gz.sha256
- registry/modules/specfact-codebase-0.40.13.tar.gz.sha256
- registry/modules/specfact-project-0.40.1.tar.gz.sha256
- registry/modules/specfact-backlog-0.40.1.tar.gz.sha256
- registry/modules/specfact-backlog-0.40.14.tar.gz.sha256
- registry/modules/specfact-code-review-0.47.32.tar.gz.sha256
- registry/modules/specfact-codebase-0.40.5.tar.gz.sha256
- registry/modules/specfact-backlog-0.40.3.tar.gz.sha256
- registry/modules/specfact-project-0.39.0.tar.gz.sha256
- registry/modules/specfact-backlog-0.40.7.tar.gz.sha256
- registry/modules/specfact-project-0.40.0.tar.gz.sha256
- registry/modules/specfact-govern-0.40.5.tar.gz.sha256
- registry/modules/specfact-backlog-0.40.6.tar.gz.sha256
- registry/modules/specfact-govern-0.40.3.tar.gz.sha256
- registry/modules/specfact-codebase-0.40.4.tar.gz.sha256
- registry/modules/specfact-govern-0.40.1.tar.gz.sha256
- registry/modules/specfact-govern-0.40.2.tar.gz.sha256
- registry/modules/specfact-govern-0.39.0.tar.gz.sha256
- registry/modules/specfact-project-0.40.7.tar.gz.sha256
- registry/modules/specfact-project-0.40.20.tar.gz.sha256
- registry/modules/specfact-codebase-0.40.11.tar.gz.sha256
- registry/modules/specfact-project-0.40.8.tar.gz.sha256
- registry/modules/specfact-project-0.40.15.tar.gz.sha256
- registry/modules/specfact-codebase-0.40.0.tar.gz.sha256
- registry/modules/specfact-spec-0.40.6.tar.gz.sha256
- registry/modules/specfact-spec-0.40.4.tar.gz.sha256
- registry/modules/specfact-codebase-0.40.9.tar.gz.sha256
- registry/modules/specfact-project-0.40.11.tar.gz.sha256
- registry/modules/specfact-spec-0.40.2.tar.gz.sha256
- registry/modules/specfact-backlog-0.40.10.tar.gz.sha256
- registry/modules/specfact-govern-0.40.4.tar.gz.sha256
- registry/modules/specfact-codebase-0.40.6.tar.gz.sha256
- registry/modules/specfact-project-0.40.3.tar.gz.sha256
- packages/specfact-code-review/module-package.yaml
- docs/bundles/code-review/run.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.12)
- GitHub Check: quality (3.13)
- GitHub Check: quality (3.11)
🧰 Additional context used
📓 Path-based instructions (5)
registry/**
⚙️ CodeRabbit configuration file
registry/**: Registry and index consistency: bundle listings, version pins, and compatibility with
published module artifacts.
Files:
registry/modules/specfact-codebase-0.40.2.tar.gz.sha256registry/modules/specfact-codebase-0.40.3.tar.gz.sha256registry/modules/specfact-code-review-0.47.31.tar.gz.sha256registry/modules/specfact-backlog-0.39.0.tar.gz.sha256registry/modules/specfact-spec-0.40.1.tar.gz.sha256registry/modules/specfact-govern-0.40.7.tar.gz.sha256registry/modules/specfact-govern-0.40.0.tar.gz.sha256registry/modules/specfact-codebase-0.39.0.tar.gz.sha256registry/modules/specfact-spec-0.40.11.tar.gz.sha256registry/modules/specfact-spec-0.39.0.tar.gz.sha256registry/modules/specfact-backlog-0.40.5.tar.gz.sha256registry/modules/specfact-spec-0.40.0.tar.gz.sha256registry/modules/specfact-codebase-0.40.1.tar.gz.sha256registry/modules/specfact-codebase-0.40.14.tar.gz.sha256registry/modules/specfact-project-0.40.6.tar.gz.sha256registry/modules/specfact-spec-0.40.3.tar.gz.sha256registry/modules/specfact-backlog-0.40.11.tar.gz.sha256registry/modules/specfact-spec-0.40.8.tar.gz.sha256registry/index.jsonregistry/modules/specfact-govern-0.40.12.tar.gz.sha256registry/modules/specfact-codebase-0.40.10.tar.gz.sha256registry/modules/specfact-backlog-0.40.9.tar.gz.sha256registry/modules/specfact-codebase-0.40.7.tar.gz.sha256registry/modules/specfact-govern-0.40.11.tar.gz.sha256
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-13-cleanup-forecast-agent-handoff/proposal.md
**/*.{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/unit/specfact_code_review/run/test_runner.pytests/unit/specfact_code_review/run/test_forecast.pytests/unit/specfact_code_review/run/test_cleanup_evidence.pypackages/specfact-code-review/src/specfact_code_review/run/forecast.pytests/unit/specfact_code_review/run/test_commands.pypackages/specfact-code-review/src/specfact_code_review/review/commands.pytests/unit/specfact_code_review/run/test_findings.pypackages/specfact-code-review/src/specfact_code_review/run/runner.pypackages/specfact-code-review/src/specfact_code_review/run/cleanup_evidence.pypackages/specfact-code-review/src/specfact_code_review/run/findings.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/unit/specfact_code_review/run/test_runner.pytests/unit/specfact_code_review/run/test_forecast.pytests/unit/specfact_code_review/run/test_cleanup_evidence.pytests/unit/specfact_code_review/run/test_commands.pytests/unit/specfact_code_review/run/test_findings.py
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/run/forecast.pypackages/specfact-code-review/src/specfact_code_review/review/commands.pypackages/specfact-code-review/src/specfact_code_review/run/runner.pypackages/specfact-code-review/src/specfact_code_review/run/cleanup_evidence.pypackages/specfact-code-review/src/specfact_code_review/run/findings.py
🔀 Multi-repo context nold-ai/specfact-cli
Findings for nold-ai/specfact-cli
-
Pre-commit / commit-gate consumers expect canonical report at .specfact/code-review.json:
- scripts/pre_commit_code_review.py — REVIEW_JSON_OUT = ".specfact/code-review.json" and writes/reads that path; used by hooks/tests. [::nold-ai/specfact-cli::scripts/pre_commit_code_review.py:3,31]
- scripts/pre-commit-quality-checks.sh — instructs running pre_commit_code_review and references .specfact/code-review.json in warnings. [::nold-ai/specfact-cli::scripts/pre-commit-quality-checks.sh:393-398]
-
CI / OpenSpec gates and docs require .specfact/code-review.json (multiple specs reference the exact out path):
- openspec/config.yaml — mandates
.specfact/code-review.jsonas mandatory evidence and example command uses--out .specfact/code-review.json. [::nold-ai/specfact-cli::openspec/config.yaml:64-66,179-185] - Many openspec specs/tasks expect the canonical JSON (examples: dogfood-self-review, review-run-command, pre-commit-review-gate). [::nold-ai/specfact-cli::openspec/specs/dogfood-self-review/spec.md:29][::nold-ai/specfact-cli::openspec/specs/review-run-command/spec.md:9-17][::nold-ai/specfact-cli::openspec/specs/pre-commit-review-gate/spec.md:11]
- openspec/config.yaml — mandates
-
Tests & docs assert presence/use of .specfact/code-review.json and a specific review command:
- tests/unit/scripts/test_code_review_module_docs.py — asserts docs contain "specfact code review run" and ".specfact/code-review.json". [::nold-ai/specfact-cli::tests/unit/scripts/test_code_review_module_docs.py:14-15]
- docs/modules/code-review.md, README.md, and multiple docs/examples show
specfact code review run --json --out .specfact/code-review.json. [::nold-ai/specfact-cli::docs/modules/code-review.md:28,43,50][::nold-ai/specfact-cli::README.md:57,62]
-
Risk: the PR introduces simplify-focused output (.specfact/code-review-simplify.json and new preview flags) and new optional fields (cleanup_forecast, remediation_packet, preserve_reasons). Existing automation, CI gates, pre-commit hooks, OpenSpec tasks, tests and docs currently read the canonical .specfact/code-review.json and will not see the simplify-only output unless callers are updated or the simplify run can also produce/merge into the canonical path.
- Evidence: many scripts/specs/tests explicitly reference the canonical path and command; pre-commit helper uses REVIEW_JSON_OUT constant. [::nold-ai/specfact-cli::scripts/pre_commit_code_review.py:31][::nold-ai/specfact-cli::openspec/config.yaml:64-66]
Recommendation (concise): Update CI/pre-commit scripts, OpenSpec commands, and tests to either:
- call the new simplify run with --out .specfact/code-review.json when appropriate, or
- accept/merge /.specfact/code-review-simplify.json into existing consumers, and ensure docs/tests are updated to the new canonical usage.
🔇 Additional comments (38)
registry/modules/specfact-codebase-0.40.2.tar.gz.sha256 (1)
1-1: LGTM!registry/modules/specfact-codebase-0.40.3.tar.gz.sha256 (1)
1-1: LGTM!registry/modules/specfact-code-review-0.47.31.tar.gz.sha256 (1)
1-1: LGTM!registry/modules/specfact-backlog-0.39.0.tar.gz.sha256 (1)
1-1: LGTM!registry/modules/specfact-spec-0.40.1.tar.gz.sha256 (1)
1-1: LGTM!registry/modules/specfact-govern-0.40.7.tar.gz.sha256 (1)
1-1: LGTM!registry/modules/specfact-govern-0.40.0.tar.gz.sha256 (1)
1-1: LGTM!registry/modules/specfact-codebase-0.39.0.tar.gz.sha256 (1)
1-1: LGTM!registry/modules/specfact-spec-0.40.11.tar.gz.sha256 (1)
1-1: LGTM!registry/modules/specfact-spec-0.39.0.tar.gz.sha256 (1)
1-1: LGTM!registry/modules/specfact-backlog-0.40.5.tar.gz.sha256 (1)
1-1: LGTM!registry/modules/specfact-spec-0.40.0.tar.gz.sha256 (1)
1-1: LGTM!registry/modules/specfact-codebase-0.40.1.tar.gz.sha256 (1)
1-1: LGTM!registry/modules/specfact-codebase-0.40.14.tar.gz.sha256 (1)
1-1: LGTM!registry/modules/specfact-project-0.40.6.tar.gz.sha256 (1)
1-1: LGTM!registry/modules/specfact-spec-0.40.3.tar.gz.sha256 (1)
1-1: LGTM!registry/modules/specfact-backlog-0.40.11.tar.gz.sha256 (1)
1-1: LGTM!registry/modules/specfact-spec-0.40.8.tar.gz.sha256 (1)
1-1: LGTM!registry/modules/specfact-govern-0.40.12.tar.gz.sha256 (1)
1-1: LGTM!registry/modules/specfact-codebase-0.40.10.tar.gz.sha256 (1)
1-1: LGTM!registry/modules/specfact-backlog-0.40.9.tar.gz.sha256 (1)
1-1: LGTM!registry/modules/specfact-codebase-0.40.7.tar.gz.sha256 (1)
1-1: LGTM!openspec/changes/code-review-13-cleanup-forecast-agent-handoff/proposal.md (1)
35-35: LGTM!registry/index.json (1)
81-83: ⚡ Quick winRegistry release-integrity check looks good for
0.47.32—registry/index.json’slatest_version/checksum_sha256fornold-ai/specfact-code-reviewmatches the correspondingregistry/modules/specfact-code-review-0.47.32.tar.gz.sha256contents (no missing or mismatched checksum files across the index).tests/unit/specfact_code_review/run/test_runner.py (1)
273-317: LGTM!Also applies to: 445-466
tests/unit/specfact_code_review/run/test_forecast.py (1)
6-91: LGTM!registry/modules/specfact-govern-0.40.11.tar.gz.sha256 (1)
1-2: LGTM!tests/unit/specfact_code_review/run/test_cleanup_evidence.py (1)
7-13: LGTM!Also applies to: 42-78, 99-136, 150-159
packages/specfact-code-review/src/specfact_code_review/run/forecast.py (1)
107-125: LGTM!tests/unit/specfact_code_review/run/test_commands.py (1)
3-4: LGTM!Also applies to: 28-33, 322-348, 357-358, 414-466
packages/specfact-code-review/src/specfact_code_review/review/commands.py (1)
34-37: LGTM!Also applies to: 56-57, 150-202
tests/unit/specfact_code_review/run/test_findings.py (1)
83-119: LGTM!Also applies to: 526-539
packages/specfact-code-review/src/specfact_code_review/run/runner.py (2)
497-503: Caching AST parses with full source content as key—memory bounded but worth noting.The
lru_cacheuses(file_path, source)as the key, meaning the entire file content string is retained for each cached entry. Withmaxsize=256, this bounds memory to ~256 source files. For typical use (multiple findings per file), this is a reasonable tradeoff, but callers processing many distinct large files in a single run could see elevated memory usage.Confirmed that callers only read the AST (no mutations), so cache correctness is preserved.
357-386: LGTM!Also applies to: 944-947
packages/specfact-code-review/src/specfact_code_review/run/cleanup_evidence.py (1)
62-95: LGTM!Also applies to: 128-160, 163-186
packages/specfact-code-review/src/specfact_code_review/run/findings.py (3)
185-202: LGTM!
294-305: LGTM!Also applies to: 336-341, 409-421
472-475: LGTM!Also applies to: 492-503
chore(modules): auto-sign module manifests
chore(registry): publish changed modules
chore(modules): auto-sign module manifests
chore(registry): publish changed modules
1 participant
Summary
Promotes current
devtomain, including the cleanup forecast / AI bloat handoff code review module updates from PR #298 and the signed module/registry artifacts already landed ondev.Refs:
code-review-13-cleanup-forecast-agent-handoffScope
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
nold-ai/specfact-code-review: promoted through0.47.30nold-ai/specfact-project: promoted through0.41.18Validation Evidence
Validation was completed on PR #298 before merge to
dev. Thedevpush workflows also ran after merge:Required local gates
hatch run formathatch run type-checkhatch run linthatch run yaml-linthatch run check-bundle-importshatch run contract-testhatch run smart-test/hatch run testcovered by PR Implement cleanup forecast handoff #298 gatesSignature + version integrity
hatch run verify-modules-signature --payload-from-filesystem --enforce-version-bumppassed on PR Implement cleanup forecast handoff #298main: manifests include CI signatures from Module Signature HardeningCI and Branch Protection
Docs / Pages
docs/)specfact-clidocs updated (not part of this modules promotion)Checklist