Implement cleanup forecast handoff#298
Conversation
|
Strix is installed on this repository, but we could not run this PR security review because this workspace does not have an active plan. If you'd like to continue receiving code reviews, you can add a payment method or manage billing here. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
✅ Files skipped from review due to trivial changes (1)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (5)packages/**/module-package.yaml⚙️ CodeRabbit configuration file
Files:
registry/**⚙️ CodeRabbit configuration file
Files:
**/*.{js,ts,tsx,jsx,py,java,cs,go,rb,php,cpp,c,h}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
packages/**/src/**/*.py⚙️ CodeRabbit configuration file
Files:
tests/**/*.py⚙️ CodeRabbit configuration file
Files:
🔇 Additional comments (4)
📝 WalkthroughPull Request Summary: Cleanup Forecast Handoff ImplementationOpenSpec ChangeID: Bundle & Module Surface ChangesCLI Command InterfaceFile:
Core Data ModelsFile:
Evidence & Forecast HelpersFile:
File:
Review Orchestration UpdatesFile:
File:
No Breaking Changes to Public Runtime API
Manifest & Integrity UpdatesModule Versions
Registry & SignaturesFiles Updated:
Cross-Repo Integration Points
Documentation & Contract UpdatesCLI Contract TestsFile:
Skill & Prompt Updates
Documentation Files
Unit & Integration Test CoverageNew test files:
Verification: TDD evidence documented in OpenSpec with failing→passing test progression, full suite gates (format, type-check, lint, yaml-lint, bundle checks, signature verification, pytest suites, openspec validation, contract tests, smart-test). OpenSpec Specifications
Summary for Maintainers✅ Backward Compatible: All new fields optional; schema versions self-managed; no specfact-cli API changes WalkthroughAdds cleanup-forecast reporting, remediation-packet handoff metadata, non-mutating preview and opt-in mutation evidence, preserve-detection heuristics, CLI flags (--preview-fixes, --with-mutation), docs/specs updates, tests, and package/registry bumps for simplify-focused code review runs. ChangesCleanup Forecast & Remediation Handoff
OpenSpec & Design
Docs, Skills & Prompts
Tests
Packaging & Registry
Estimated code review effort 🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related issues
Possibly related PRs
Focus (reviewer) notes: validate model schema changes and schema-version gating, verify deterministic reviewed-LOC counting and weighted forecast math, confirm preview helpers do not modify tracked files, ensure mutation evidence is marked inconclusive when tooling is missing or timed out, and check CLI validation rules (preview vs fix, mutation requires simplify) plus packaging/registry integrity updates. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
djm81
added
documentation
![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: bdc3cd2be6
ℹ️ 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: 5
🤖 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-13-cleanup-forecast-agent-handoff/design.md`:
- Line 13: The prose taxonomy in the preserve reasons sentence uses human terms
that don't match the emitted enum names (e.g., “explicit marker” / “spec/domain
wrapper” vs. spec_linked, domain_wrapper), so update the sentence in design.md
to use the exact enum tokens used by the schema (for example replace “explicit
marker” with spec_linked, “spec/domain wrapper” with domain_wrapper, and ensure
all listed categories map 1:1 to emitted values like contract, public_api,
protocol_abc, cli_callback, compatibility_shim, load_bearing_mutation, etc.),
and verify any adjacent examples or tests reference the same enum identifiers to
eliminate schema-contract drift.
In
`@openspec/changes/code-review-13-cleanup-forecast-agent-handoff/specs/ai-ide-remediation-handoff/spec.md`:
- Line 18: The spec requires assistants to sort findings by guidance_kind and
inspect cleanup_forecast and remediation packets before editing, but
_RUN_INSTRUCTIONS in
packages/specfact-code-review/src/specfact_code_review/review/commands.py does
not mention this; update the _RUN_INSTRUCTIONS string to explicitly require
sorting findings by guidance_kind, to instruct inspection of cleanup_forecast,
and to enforce following remediation packets before making edits (reference the
_RUN_INSTRUCTIONS constant in commands.py and any helper functions that format
or emit findings output so tests and docs match the openspec).
In `@packages/specfact-code-review/src/specfact_code_review/run/findings.py`:
- Around line 149-154: Add schema-level validation to enforce invariants: in
ReviewedLoc ensure total equals production + tests (use a pydantic
root_validator or `@validator` for 'total' to raise if mismatch) and keep
non-negative checks; in DeletionEstimate ensure low <= expected <= high and all
three are >= 0 (add validators or a root_validator to validate ordering and
non-negativity). Reference the ReviewedLoc and DeletionEstimate classes and add
clear validation error messages so malformed forecasts (e.g., inverted ranges or
inconsistent totals) fail schema validation.
In `@packages/specfact-code-review/src/specfact_code_review/run/forecast.py`:
- Around line 81-84: The try/except in _reviewed_loc_for_files currently only
catches OSError around calling _count_python_loc(file_path), so a
UnicodeDecodeError from Path.read_text will abort processing; update the
exception handling to also catch UnicodeDecodeError (or UnicodeError) when
calling _count_python_loc and in the analogous block at lines 92-96 so
unreadable legacy-encoded .py files are skipped and the loop continues.
🪄 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: 732f6025-7465-47d0-a0f3-7f9941cc0dec
⛔ Files ignored due to path filters (2)
registry/modules/specfact-code-review-0.47.28.tar.gzis excluded by!**/*.gzregistry/modules/specfact-project-0.41.18.tar.gzis excluded by!**/*.gz
📒 Files selected for processing (35)
docs/bundles/code-review/run.mddocs/modules/code-review.mddocs/quickstart-ai-bloat.mdopenspec/CHANGE_ORDER.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-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)
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-project-0.41.18.tar.gz.sha256registry/modules/specfact-code-review-0.47.28.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/README.mdopenspec/changes/code-review-13-cleanup-forecast-agent-handoff/proposal.mdopenspec/changes/code-review-13-cleanup-forecast-agent-handoff/TDD_EVIDENCE.mdopenspec/changes/code-review-13-cleanup-forecast-agent-handoff/specs/ai-ide-remediation-handoff/spec.mdopenspec/CHANGE_ORDER.mdopenspec/changes/code-review-13-cleanup-forecast-agent-handoff/specs/review-run-command/spec.mdopenspec/changes/code-review-13-cleanup-forecast-agent-handoff/specs/review-finding-model/spec.mdopenspec/changes/code-review-13-cleanup-forecast-agent-handoff/tasks.mdopenspec/changes/code-review-13-cleanup-forecast-agent-handoff/specs/cleanup-forecast-review/spec.mdopenspec/changes/code-review-13-cleanup-forecast-agent-handoff/design.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/review/test_commands.pypackages/specfact-code-review/src/specfact_code_review/rules/updater.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/forecast.pytests/unit/specfact_code_review/run/test_findings.pypackages/specfact-code-review/src/specfact_code_review/review/commands.pytests/unit/specfact_code_review/run/test_commands.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/findings.pypackages/specfact-code-review/src/specfact_code_review/run/commands.pytests/unit/specfact_code_review/run/test_runner.py
tests/**/*.py
⚙️ CodeRabbit configuration file
tests/**/*.py: Contract-first and integration tests: migration suites, bundle validation, and flakiness.
Ensure changes to adapters or bridges have targeted coverage.
Files:
tests/unit/specfact_code_review/review/test_commands.pytests/unit/specfact_code_review/run/test_findings.pytests/unit/specfact_code_review/run/test_commands.pytests/unit/specfact_code_review/run/test_forecast.pytests/unit/specfact_code_review/run/test_cleanup_evidence.pytests/unit/specfact_code_review/run/test_runner.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/run/runner.pypackages/specfact-code-review/src/specfact_code_review/run/cleanup_evidence.pypackages/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/findings.pypackages/specfact-code-review/src/specfact_code_review/run/commands.py
docs/**/*.md
⚙️ CodeRabbit configuration file
docs/**/*.md: User-facing and cross-site accuracy: Jekyll front matter, links per documentation-url-contract,
CLI examples matching bundled commands.
Files:
docs/quickstart-ai-bloat.mddocs/modules/code-review.mddocs/bundles/code-review/run.md
🪛 LanguageTool
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)
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)
🔇 Additional comments (27)
openspec/changes/code-review-13-cleanup-forecast-agent-handoff/proposal.md (1)
1-49: 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/.openspec.yaml (1)
1-3: LGTM!openspec/changes/code-review-13-cleanup-forecast-agent-handoff/README.md (1)
1-4: LGTM!openspec/changes/code-review-13-cleanup-forecast-agent-handoff/TDD_EVIDENCE.md (1)
1-56: LGTM!openspec/changes/code-review-13-cleanup-forecast-agent-handoff/tasks.md (1)
1-45: LGTM!openspec/CHANGE_ORDER.md (1)
106-106: LGTM!packages/specfact-code-review/module-package.yaml (1)
2-2: LGTM!Also applies to: 26-27
packages/specfact-project/module-package.yaml (1)
2-2: LGTM!Also applies to: 30-31
registry/index.json (1)
5-7: LGTM!Also applies to: 81-83
registry/modules/specfact-code-review-0.47.28.tar.gz.sha256 (1)
1-1: LGTM!registry/modules/specfact-project-0.41.18.tar.gz.sha256 (1)
1-1: LGTM!docs/modules/code-review.md (1)
62-66: LGTM!Also applies to: 83-85, 125-125, 136-138, 202-214
docs/bundles/code-review/run.md (1)
42-43: LGTM!Also applies to: 58-60, 98-98, 104-105, 116-116, 146-146, 149-149
docs/quickstart-ai-bloat.md (1)
13-13: LGTM!Also applies to: 23-23, 26-26, 29-29, 33-33, 41-46, 52-52, 91-91
packages/specfact-code-review/src/specfact_code_review/resources/skills/specfact-code-review/SKILL.md (1)
11-14: LGTM!Also applies to: 26-26, 29-29
packages/specfact-project/resources/prompts/specfact.08-simplify.md (1)
59-59: LGTM!Also applies to: 62-62, 64-64, 70-70, 79-79, 117-117, 127-127
packages/specfact-code-review/src/specfact_code_review/rules/updater.py (1)
35-37: LGTM!Also applies to: 65-65
tests/unit/specfact_code_review/run/test_findings.py (1)
9-21: LGTM!Also applies to: 64-67, 156-234, 460-486
tests/unit/specfact_code_review/run/test_forecast.py (1)
1-58: LGTM!tests/unit/specfact_code_review/run/test_cleanup_evidence.py (1)
1-67: LGTM!tests/unit/specfact_code_review/run/test_runner.py (1)
10-17: LGTM!Also applies to: 231-271, 273-317, 349-398
tests/cli-contracts/specfact-code-review-run.scenarios.yaml (1)
171-191: LGTM!tests/unit/specfact_code_review/review/test_commands.py (1)
47-49: LGTM!tests/unit/specfact_code_review/run/test_commands.py (1)
315-395: LGTM!Also applies to: 616-617
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 `@packages/specfact-code-review/src/specfact_code_review/run/runner.py`:
- Around line 593-595: The helper _is_stub_function currently requires at least
one non-docstring statement (uses bool(body)), causing docstring-only Protocol
methods to be treated as non-stubs; remove the bool(body) check and simply
return all(_is_stub_statement(statement) for statement in body) so an empty body
(docstring-only) yields True. Update the _is_stub_function implementation (which
uses _is_docstring_statement and _is_stub_statement) to drop the bool(body)
guard and rely on all(...) to correctly mark docstring-only methods as stubs.
🪄 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: 23c73e29-f6ba-48b9-a854-c31d568f1dea
⛔ Files ignored due to path filters (1)
registry/modules/specfact-code-review-0.47.29.tar.gzis excluded by!**/*.gz
📒 Files selected for processing (12)
openspec/changes/code-review-13-cleanup-forecast-agent-handoff/design.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/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-code-review-0.47.29.tar.gz.sha256tests/unit/specfact_code_review/review/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 (3)
- registry/modules/specfact-code-review-0.47.29.tar.gz.sha256
- packages/specfact-code-review/module-package.yaml
- openspec/changes/code-review-13-cleanup-forecast-agent-handoff/design.md
📜 Review details
🧰 Additional context used
📓 Path-based instructions (4)
registry/**
⚙️ CodeRabbit configuration file
registry/**: Registry and index consistency: bundle listings, version pins, and compatibility with
published module artifacts.
Files:
registry/index.json
**/*.{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/review/test_commands.pytests/unit/specfact_code_review/run/test_forecast.pypackages/specfact-code-review/src/specfact_code_review/run/forecast.pypackages/specfact-code-review/src/specfact_code_review/review/commands.pytests/unit/specfact_code_review/run/test_runner.pytests/unit/specfact_code_review/run/test_findings.pypackages/specfact-code-review/src/specfact_code_review/run/findings.pypackages/specfact-code-review/src/specfact_code_review/run/runner.py
tests/**/*.py
⚙️ CodeRabbit configuration file
tests/**/*.py: Contract-first and integration tests: migration suites, bundle validation, and flakiness.
Ensure changes to adapters or bridges have targeted coverage.
Files:
tests/unit/specfact_code_review/review/test_commands.pytests/unit/specfact_code_review/run/test_forecast.pytests/unit/specfact_code_review/run/test_runner.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/findings.pypackages/specfact-code-review/src/specfact_code_review/run/runner.py
🔇 Additional comments (9)
registry/index.json (1)
81-83: LGTM!tests/unit/specfact_code_review/review/test_commands.py (1)
44-44: LGTM!tests/unit/specfact_code_review/run/test_forecast.py (1)
60-67: LGTM!packages/specfact-code-review/src/specfact_code_review/run/forecast.py (1)
48-55: LGTM!Also applies to: 80-80
packages/specfact-code-review/src/specfact_code_review/review/commands.py (1)
40-40: LGTM!tests/unit/specfact_code_review/run/test_runner.py (1)
351-390: LGTM!Also applies to: 392-397, 406-417, 419-419
tests/unit/specfact_code_review/run/test_findings.py (1)
488-499: LGTM!packages/specfact-code-review/src/specfact_code_review/run/findings.py (1)
156-160: LGTM!Also applies to: 170-174
packages/specfact-code-review/src/specfact_code_review/run/runner.py (1)
480-487: LGTM!Also applies to: 582-590, 598-613
1 participant
Summary
Verification
Closes #297