UAT: `EstimationResult.as_display_dict()` logic is duplicated in 3 call-sites in `plan.py` and `cli/commands/plan.py` #3994

New issue

Open

opened 2026-04-06 08:22:38 +00:00 by freemo · 0 comments

freemo commented

2026-04-06 08:22:38 +00:00

Owner

Metadata

Branch: fix/estimation-display-dict-deduplication
Commit Message: refactor(estimation): migrate duplicated as_display_dict logic to EstimationResult.as_display_dict()
Milestone: None (Backlog)
Parent Epic: #890

Backlog note: This issue was discovered during autonomous operation
on milestone v3.6.0. It does not block milestone completion and has been
placed in the backlog for human review and future milestone assignment.

Bug Report

Feature Area: Estimation and Planning Intelligence — EstimationResult display
Severity: Low — code quality / maintainability issue; risk of display inconsistency
Found by: UAT tester (code-level analysis)

Background and Context

EstimationResult.as_display_dict() was added to provide a single, canonical method for converting an EstimationResult to a display-ready dictionary. The method's docstring explicitly notes that three call-sites duplicate its logic and should migrate to use it.

Current Behavior

The EstimationResult.as_display_dict() docstring in src/cleveragents/domain/models/core/estimation.py (lines ~80-95) contains this note:

.. note::

   Three call-sites currently duplicate this logic and should
   migrate to this method:

   - ``Plan.as_cli_dict()`` in ``plan.py``
   - ``_build_plan_summary()`` in ``cli/commands/plan.py``
   - ``_show_plan_detail()`` in ``cli/commands/plan.py``

Additionally, in src/cleveragents/cli/commands/plan.py at line ~1412:

# TODO: Migrate to EstimationResult.as_display_dict() to reduce duplication
if plan.estimation_result is not None:
    est = plan.estimation_result

This means the estimation display logic (which fields to include, how to format them) is maintained in at least 4 places. If the EstimationResult model gains new fields (e.g., estimated_child_plans was added in issue #890), each call-site must be updated independently — creating a risk of inconsistent display across CLI views.

Expected Behavior

All call-sites should use EstimationResult.as_display_dict() for consistent display:

Plan.as_cli_dict() should call self.estimation_result.as_display_dict() (already done per code review)
_build_plan_summary() in cli/commands/plan.py should call plan.estimation_result.as_display_dict()
_show_plan_detail() in cli/commands/plan.py should call plan.estimation_result.as_display_dict()

Code Location

File: src/cleveragents/domain/models/core/estimation.py — as_display_dict() docstring note
File: src/cleveragents/domain/models/core/plan.py — Plan.as_cli_dict() (already migrated)
File: src/cleveragents/cli/commands/plan.py — _build_plan_summary() and _show_plan_detail() (lines ~1412+)

Subtasks

Identify all call-sites that manually build estimation display dicts
Migrate _build_plan_summary() in cli/commands/plan.py to use as_display_dict()
Migrate _show_plan_detail() in cli/commands/plan.py to use as_display_dict()
Remove the .. note:: from as_display_dict() docstring once migration is complete
Remove the # TODO: Migrate comment from cli/commands/plan.py
Verify display output is consistent across all CLI views

Definition of Done

All estimation display logic uses EstimationResult.as_display_dict()
No duplicate estimation field-building logic in plan.py or cli/commands/plan.py
TODO comments removed
All existing tests pass
PR merged and issue closed

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: ca-new-issue-creator

## Metadata - **Branch**: `fix/estimation-display-dict-deduplication` - **Commit Message**: `refactor(estimation): migrate duplicated as_display_dict logic to EstimationResult.as_display_dict()` - **Milestone**: None (Backlog) - **Parent Epic**: #890 > **Backlog note:** This issue was discovered during autonomous operation > on milestone v3.6.0. It does not block milestone completion and has been > placed in the backlog for human review and future milestone assignment. --- ## Bug Report **Feature Area:** Estimation and Planning Intelligence — EstimationResult display **Severity:** Low — code quality / maintainability issue; risk of display inconsistency **Found by:** UAT tester (code-level analysis) --- ## Background and Context `EstimationResult.as_display_dict()` was added to provide a single, canonical method for converting an `EstimationResult` to a display-ready dictionary. The method's docstring explicitly notes that three call-sites duplicate its logic and should migrate to use it. ## Current Behavior The `EstimationResult.as_display_dict()` docstring in `src/cleveragents/domain/models/core/estimation.py` (lines ~80-95) contains this note: ```python .. note:: Three call-sites currently duplicate this logic and should migrate to this method: - ``Plan.as_cli_dict()`` in ``plan.py`` - ``_build_plan_summary()`` in ``cli/commands/plan.py`` - ``_show_plan_detail()`` in ``cli/commands/plan.py`` ``` Additionally, in `src/cleveragents/cli/commands/plan.py` at line ~1412: ```python # TODO: Migrate to EstimationResult.as_display_dict() to reduce duplication if plan.estimation_result is not None: est = plan.estimation_result ``` This means the estimation display logic (which fields to include, how to format them) is maintained in at least 4 places. If the `EstimationResult` model gains new fields (e.g., `estimated_child_plans` was added in issue #890), each call-site must be updated independently — creating a risk of inconsistent display across CLI views. ## Expected Behavior All call-sites should use `EstimationResult.as_display_dict()` for consistent display: - `Plan.as_cli_dict()` should call `self.estimation_result.as_display_dict()` (already done per code review) - `_build_plan_summary()` in `cli/commands/plan.py` should call `plan.estimation_result.as_display_dict()` - `_show_plan_detail()` in `cli/commands/plan.py` should call `plan.estimation_result.as_display_dict()` ## Code Location - **File**: `src/cleveragents/domain/models/core/estimation.py` — `as_display_dict()` docstring note - **File**: `src/cleveragents/domain/models/core/plan.py` — `Plan.as_cli_dict()` (already migrated) - **File**: `src/cleveragents/cli/commands/plan.py` — `_build_plan_summary()` and `_show_plan_detail()` (lines ~1412+) ## Subtasks - [ ] Identify all call-sites that manually build estimation display dicts - [ ] Migrate `_build_plan_summary()` in `cli/commands/plan.py` to use `as_display_dict()` - [ ] Migrate `_show_plan_detail()` in `cli/commands/plan.py` to use `as_display_dict()` - [ ] Remove the `.. note::` from `as_display_dict()` docstring once migration is complete - [ ] Remove the `# TODO: Migrate` comment from `cli/commands/plan.py` - [ ] Verify display output is consistent across all CLI views ## Definition of Done - All estimation display logic uses `EstimationResult.as_display_dict()` - No duplicate estimation field-building logic in `plan.py` or `cli/commands/plan.py` - TODO comments removed - All existing tests pass - PR merged and issue closed --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: ca-new-issue-creator

freemo added a new dependency

2026-04-06 08:23:02 +00:00

#890 feat(actor): implement estimation actor as a functional actor type

freemo referenced this issue

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

[Automated] CleverAgents Build Session - 2026-04-06 #3775

freemo referenced this issue

2026-04-06 08:33:59 +00:00

[Automated] CleverAgents Build Session - 2026-04-06 #3775

freemo referenced this issue

2026-04-06 10:35:53 +00:00

[Automated] CleverAgents Build Session - 2026-04-06 #3775