cleveragents/cleveragents-core

Fork 3

Proposal: update specification — correction dry-run uses analyze_impact() instead of generate_dry_run_report() #4938

New issue

Open

opened 2026-04-08 23:01:17 +00:00 by HAL9000 · 2 comments

HAL9000 commented

2026-04-08 23:01:17 +00:00

Owner

Spec Update Proposal

Type: Incorrect implementation deviation — spec requires fix to implementation
Triggered by: Code analysis of src/cleveragents/cli/commands/plan.py and src/cleveragents/application/services/correction_service.py
Spec section: §agents plan correct --dry-run (lines ~15200–15320)

What Changed in the Implementation

The correct_decision CLI function (lines 3267–3304 of plan.py) calls svc.analyze_impact() directly for dry-run mode, bypassing generate_dry_run_report():

if dry_run:
    # Analyze and display impact
    impact = svc.analyze_impact(
        request.correction_id,
        decision_tree=decision_tree,
        influence_edges=influence_edges,
    )
    # ... shows "Correction Impact (Dry Run)" panel

The CorrectionService.generate_dry_run_report() method exists and produces a CorrectionDryRunReport with:

warnings: list of human-readable warnings
estimated_recompute_time_seconds: estimated wall-clock seconds
decisions_to_invalidate: decisions that would be marked invalid
impact.excluded_decisions: decisions NOT affected (preserved)

What Spec Section Needs Updating

This is NOT a spec update — the spec is correct. The implementation is wrong.

The spec at lines 15200–15254 shows the correct dry-run output:

╭─ Dry Run — Correction Preview ─────────────────────────────────────╮
│ ⚠  This is a preview only. No changes will be made.                │
╰────────────────────────────────────────────────────────────────────╯

╭─ Would Revert ───────────────────────────────────────╮
│ Decisions to invalidate: 3                           │
│   01HXM9A1..  strategy_choice    "sync pattern"      │
│   01HXM9A2..  implementation_choice  "requests"      │
│   01HXM9A3..  tool_invocation    write_file ×4       │
│ Child plans to roll back: 2                          │
│ Artifacts to archive: 5 files                        │
│ Unaffected decisions: 2 (will be kept)               │
╰──────────────────────────────────────────────────────╯

╭─ Estimated Cost ──────────╮
│ Re-strategize: ~$0.012    │
│ Re-execute: ~$0.035       │
│ Total: ~$0.047            │
│ ETA: ~4 minutes           │
╰───────────────────────────╯

To execute this correction, remove --dry-run and add --yes

The spec §Correction Safety (line 28956) also guarantees: "Create a new attempt revision (increment plan.attempt)".

Rationale

This is an incorrect deviation from the spec. The current implementation:

Calls analyze_impact() instead of generate_dry_run_report() — missing warnings, recompute time, and the "remove --dry-run" hint
Incorrectly mutates correction status (PENDING → ANALYZING) during dry-run, whereas generate_dry_run_report() preserves the original status
Does not show decisions_to_invalidate or excluded_decisions in the output

Additionally, CorrectionService.execute_revert() and execute_append() do not increment plan.attempt, violating the spec's safety guarantee at line 28956.

Scope

This proposal does NOT require a spec change. It documents two implementation fixes needed:

src/cleveragents/cli/commands/plan.py — correct_decision() dry-run branch must call svc.generate_dry_run_report() instead of svc.analyze_impact()
src/cleveragents/application/services/correction_service.py — execute_revert() and execute_append() must increment plan.attempt before returning

The spec text is already correct and does not need modification.

Note: Issues #4914 and #4915 (UAT findings) already document these bugs. This proposal confirms the spec is correct and the implementation needs fixing.

Definition of Done

correct_decision() dry-run branch calls generate_dry_run_report() and displays the full preview panel
Dry-run output includes warnings, estimated recompute time, decisions to invalidate, unaffected decisions, and the "remove --dry-run" hint
Dry-run does NOT mutate correction status
execute_revert() and execute_append() increment plan.attempt
All tests pass

Automated by CleverAgents Bot
Supervisor: Spec Evolution | Agent: spec-updater

## Spec Update Proposal **Type:** Incorrect implementation deviation — spec requires fix to implementation **Triggered by:** Code analysis of `src/cleveragents/cli/commands/plan.py` and `src/cleveragents/application/services/correction_service.py` **Spec section:** §agents plan correct --dry-run (lines ~15200–15320) --- ## What Changed in the Implementation The `correct_decision` CLI function (lines 3267–3304 of `plan.py`) calls `svc.analyze_impact()` directly for dry-run mode, bypassing `generate_dry_run_report()`: ```python if dry_run: # Analyze and display impact impact = svc.analyze_impact( request.correction_id, decision_tree=decision_tree, influence_edges=influence_edges, ) # ... shows "Correction Impact (Dry Run)" panel ``` The `CorrectionService.generate_dry_run_report()` method exists and produces a `CorrectionDryRunReport` with: - `warnings`: list of human-readable warnings - `estimated_recompute_time_seconds`: estimated wall-clock seconds - `decisions_to_invalidate`: decisions that would be marked invalid - `impact.excluded_decisions`: decisions NOT affected (preserved) --- ## What Spec Section Needs Updating **This is NOT a spec update — the spec is correct. The implementation is wrong.** The spec at lines 15200–15254 shows the correct dry-run output: ``` ╭─ Dry Run — Correction Preview ─────────────────────────────────────╮ │ ⚠ This is a preview only. No changes will be made. │ ╰────────────────────────────────────────────────────────────────────╯ ╭─ Would Revert ───────────────────────────────────────╮ │ Decisions to invalidate: 3 │ │ 01HXM9A1.. strategy_choice "sync pattern" │ │ 01HXM9A2.. implementation_choice "requests" │ │ 01HXM9A3.. tool_invocation write_file ×4 │ │ Child plans to roll back: 2 │ │ Artifacts to archive: 5 files │ │ Unaffected decisions: 2 (will be kept) │ ╰──────────────────────────────────────────────────────╯ ╭─ Estimated Cost ──────────╮ │ Re-strategize: ~$0.012 │ │ Re-execute: ~$0.035 │ │ Total: ~$0.047 │ │ ETA: ~4 minutes │ ╰───────────────────────────╯ To execute this correction, remove --dry-run and add --yes ``` The spec §Correction Safety (line 28956) also guarantees: "Create a new attempt revision (increment `plan.attempt`)". --- ## Rationale This is an **incorrect deviation** from the spec. The current implementation: 1. Calls `analyze_impact()` instead of `generate_dry_run_report()` — missing warnings, recompute time, and the "remove --dry-run" hint 2. Incorrectly mutates correction status (PENDING → ANALYZING) during dry-run, whereas `generate_dry_run_report()` preserves the original status 3. Does not show `decisions_to_invalidate` or `excluded_decisions` in the output Additionally, `CorrectionService.execute_revert()` and `execute_append()` do not increment `plan.attempt`, violating the spec's safety guarantee at line 28956. --- ## Scope This proposal does NOT require a spec change. It documents two implementation fixes needed: 1. **`src/cleveragents/cli/commands/plan.py`** — `correct_decision()` dry-run branch must call `svc.generate_dry_run_report()` instead of `svc.analyze_impact()` 2. **`src/cleveragents/application/services/correction_service.py`** — `execute_revert()` and `execute_append()` must increment `plan.attempt` before returning The spec text is already correct and does not need modification. **Note:** Issues #4914 and #4915 (UAT findings) already document these bugs. This proposal confirms the spec is correct and the implementation needs fixing. --- ## Definition of Done - [ ] `correct_decision()` dry-run branch calls `generate_dry_run_report()` and displays the full preview panel - [ ] Dry-run output includes warnings, estimated recompute time, decisions to invalidate, unaffected decisions, and the "remove --dry-run" hint - [ ] Dry-run does NOT mutate correction status - [ ] `execute_revert()` and `execute_append()` increment `plan.attempt` - [ ] All tests pass --- **Automated by CleverAgents Bot** Supervisor: Spec Evolution | Agent: spec-updater

HAL9000 added the

labels

2026-04-08 23:01:21 +00:00

freemo referenced this issue

2026-04-08 23:37:33 +00:00

[AUTO-LIAISON] Human Liaison Status (Cycle 1) #4941

freemo commented

2026-04-08 23:38:32 +00:00

Owner

This issue has the "Needs Feedback" label and is a proposal awaiting human review. I will not modify its state — a human must approve or reject it.

Summary of the proposal:
This is a spec-updater finding that the spec is correct but the implementation deviates. Specifically:

correct_decision() dry-run branch calls analyze_impact() instead of generate_dry_run_report() — missing warnings, recompute time, and the "remove --dry-run" hint
execute_revert() and execute_append() do not increment plan.attempt, violating the spec's safety guarantee

Note: This proposal does NOT require a spec change — it documents implementation bugs. Issues #4914 and #4915 already track these bugs. This proposal confirms the spec is correct.

To approve: Remove the "Needs Feedback" label and add "State/Verified", or comment with explicit approval.

Automated by CleverAgents Bot
Supervisor: Human Liaison | Agent: human-liaison

This issue has the "Needs Feedback" label and is a proposal awaiting human review. I will not modify its state — a human must approve or reject it. **Summary of the proposal:** This is a spec-updater finding that the spec is correct but the implementation deviates. Specifically: 1. `correct_decision()` dry-run branch calls `analyze_impact()` instead of `generate_dry_run_report()` — missing warnings, recompute time, and the "remove --dry-run" hint 2. `execute_revert()` and `execute_append()` do not increment `plan.attempt`, violating the spec's safety guarantee **Note:** This proposal does NOT require a spec change — it documents implementation bugs. Issues #4914 and #4915 already track these bugs. This proposal confirms the spec is correct. **To approve:** Remove the "Needs Feedback" label and add "State/Verified", or comment with explicit approval. --- **Automated by CleverAgents Bot** Supervisor: Human Liaison | Agent: human-liaison

freemo referenced this issue

2026-04-08 23:41:00 +00:00

[AUTO-WATCHDOG] System Health Report (Cycle 1) #4968

HAL9000 commented

2026-04-09 00:20:50 +00:00

Author

Owner

This is a proposal awaiting human review (needs feedback label). I will not modify its state — a human must approve or reject it.

Summary of proposal: This is actually an implementation fix proposal, not a spec change. The spec is correct; the implementation deviates:

correct_decision() dry-run calls analyze_impact() instead of generate_dry_run_report() — missing warnings, recompute time estimate, and the "remove --dry-run" hint
execute_revert() and execute_append() do not increment plan.attempt — violates spec safety guarantee

Issues #4914 and #4915 already document these bugs. This proposal confirms the spec is correct and the implementation needs fixing.

For human review: Please comment with approval or rejection, or remove the Needs Feedback label to proceed with implementation.

Automated by CleverAgents Bot
Supervisor: Human Liaison | Agent: human-liaison

This is a proposal awaiting human review (`needs feedback` label). I will not modify its state — a human must approve or reject it. **Summary of proposal:** This is actually an **implementation fix proposal**, not a spec change. The spec is correct; the implementation deviates: 1. `correct_decision()` dry-run calls `analyze_impact()` instead of `generate_dry_run_report()` — missing warnings, recompute time estimate, and the "remove --dry-run" hint 2. `execute_revert()` and `execute_append()` do not increment `plan.attempt` — violates spec safety guarantee Issues #4914 and #4915 already document these bugs. This proposal confirms the spec is correct and the implementation needs fixing. **For human review:** Please comment with approval or rejection, or remove the `Needs Feedback` label to proceed with implementation. --- **Automated by CleverAgents Bot** Supervisor: Human Liaison | Agent: human-liaison

HAL9000 referenced this issue

2026-04-09 00:59:59 +00:00

[AUTO-LIAISON] Human Liaison Status (Cycle 1) #4983

HAL9000 referenced this issue

2026-04-09 01:23:43 +00:00

[AUTO-LIAISON] Human Liaison Status (Cycle 10) #5131