fix(cli): move estimation flag validation before action lookup #1073

Closed
hamza.khyari wants to merge 5 commits from fix/estimation-flag-validation into master

5 Commits

Author SHA1 Message Date
hamza.khyari d1d8ac483f fix(cli): move estimation flag validation before action lookup
CI / lint (pull_request) Successful in 22s
CI / security (pull_request) Successful in 41s
CI / typecheck (pull_request) Successful in 55s
CI / quality (pull_request) Successful in 31s
CI / benchmark-publish (pull_request) Has been skipped
CI / build (pull_request) Successful in 21s
CI / unit_tests (pull_request) Failing after 4m6s
CI / docker (pull_request) Has been skipped
CI / integration_tests (pull_request) Failing after 4m0s
CI / benchmark-regression (pull_request) Failing after 18m51s
CI / coverage (pull_request) Failing after 19m13s
CI / e2e_tests (pull_request) Failing after 19m15s
Move the --no-estimate / --estimation-actor mutual exclusion check
and all actor format validations before service.get_action_by_name()
in the plan use command.

Previously, the flag conflict check at line 1682 ran AFTER the action
lookup at line 1655. In CI with behave-parallel (--processes 24),
mock patches on _get_lifecycle_service can race, causing the real
service to be called. The real service raises ResourceNotFoundError
for test actions, which is caught by the broad except at line 1835
and raises typer.Abort() — the estimation flag conflict message
never appears.

By validating CLI flags before any service calls, the conflict is
detected deterministically regardless of mock state.
2026-03-19 15:07:05 +00:00
aditya 97920678b9 test(estimation): harden CI test assertions and restore coverage to 97%
Fix unit test and integration test failures on Forgejo CI caused by
Rich/Click text wrapping in narrow terminal environments. The assertion
for "--no-estimate cannot be used with --estimation-actor" was split
across lines when COLUMNS was small, breaking exact substring checks.

- Normalize whitespace before assertion in cli_extensions_steps.py so
  the check is resilient to line wrapping regardless of terminal width.
- Set COLUMNS=240 in robot/helper_plan_cli_spec.py to give CLI
  subprocesses a consistent wide terminal, preventing Rich from
  reflowing output mid-assertion.
- Add 16 new BDD scenarios to estimation_coverage.feature exercising
  previously-uncovered estimation branches: validation constraints
  (max_length, min_length, frozen model), EstimationActorProtocol
  runtime check, all three exception handler paths in estimate_plan,
  parse_estimation_report generic error path, serialization of non-None
  values, EstimationError attribute preservation, and CLI dict / status
  detail rendering of estimation data.
- estimation_service.py coverage: 75% → 99% (only TYPE_CHECKING and
  Protocol stub lines remain uncovered).
- Overall coverage: 96.89% → 97.0%, meeting the project threshold.

All changes are scoped to estimation feature tests and the estimation
CLI assertion helpers introduced by this branch.

Refs: #209
2026-03-19 15:07:05 +00:00
aditya 9273f0d8de fix(estimation): address Hamza code review findings F1–F17 on PR #528
Squash 7 estimation migrations (m6_003, 992484befd85, m6_005, m6_006,
m6_007, m6_008, m7_002) into a single clean m6_003_estimation_schema
with linear dependency on m4_003_plan_env_columns. Rename column
estimation_skipped_json to estimation_skipped for naming consistency.

Domain model hardening: replace deprecated FieldValidationInfo with
ValidationInfo, change historical_basis from list[str] to tuple[str,...]
to prevent mutation on frozen model, add max_length=10_000 to rationale.

Service improvements: remove unnecessary ThreadPoolExecutor wrapping,
add EstimationActorProtocol for pluggable actor dispatch, log unexpected
exceptions at error level, document EstimationService as stateless.

Deduplicate estimation serialization via static helpers on
LifecyclePlanModel used by both from_domain() and _update_plan_row().

Remove 6 out-of-scope coverage-boost files (container_73_lines,
plan_cli_print_coverage, resume_coverage) and revert 3 unrelated
benchmark import fixes.

Add Behave scenario for update_plan_overrides atomic field setting.

ISSUES CLOSED: #209
2026-03-19 15:07:05 +00:00
aditya 377c3d46da fix(estimation): resolve migration-head test breakage and stabilize lifecycle CLI coverage
Merge divergent Alembic heads so test DB setup can run reliably, and harden lifecycle/CLI coverage step fixtures and assertions to remove parallel-format flakiness without changing runtime behavior. This unblocks unit/integration test execution for the M6 estimation branch.

ISSUES CLOSED: #209
2026-03-19 15:07:05 +00:00
aditya 50680612d5 feat(estimation): add cost and risk estimation actor
Implemented optional estimation_actor role for cost, risk, and duration
estimation during plan lifecycle. Estimates are persisted to plan metadata
and surfaced in CLI output. Key implementation details:

- EstimationOutput (Pydantic models): CostEstimate with currency, token
  estimates, and confidence ranges; RiskScore with 0-100 scale and factors;
  DurationEstimate with min/expected/max seconds. All include confidence
  levels and validation. EstimationSkipped records when estimation is opted out.

- EstimationService: stateless async service that invokes estimation actor
  (stub implementation for M6). Handles actor output parsing, error recovery,
  and fallback to EstimationSkipped on failure.

- Integration: Plan model gains estimation_output and estimation_skipped fields.
  LifecyclePlanModel adds JSON columns for persistence. PlanLifecycleService
  invokes estimation during use_action unless skip_estimation is true.

- CLI: --no-estimate flag added to 'agents plan use'. plan status displays
  cost (USD with token estimates), risk (score/100 with confidence), and
  duration (seconds with ranges) in rich format.

- Database: Alembic migration m6_003_estimation_metadata adds
  estimation_output_json and estimation_skipped_json columns to v3_plans.

- Tests: 44 BDD scenarios (features/estimation.feature) covering validation,
  lifecycle integration, persistence, CLI display, and edge cases. 18 Robot
  Framework smoke tests. 19 ASV benchmark suites for schema, serialization,
  validation, and plan integration performance.

- Documentation: docs/reference/estimation.md with schema, configuration,
  and examples. plan_cli.md updated with --no-estimate flag usage.

ISSUES CLOSED: #209
2026-03-19 15:07:05 +00:00