fix(cli): make plan explain accept plan_id for plan-level explanation #1057

Merged
brent.edwards merged 2 commits from bugfix/m3-plan-explain-plan-id into master 2026-03-19 22:10:23 +00:00
Member

Summary

Fixes plan explain to accept both decision IDs and plan IDs as the positional argument, matching the M3 acceptance test usage pattern plan explain <plan_id>.

Problem

explain_decision_cmd only accepted a decision ULID. When the M3 acceptance test passed a plan ID, svc.get_decision(plan_id) returned None, causing "Decision not found" error with exit code 1.

Fix

  1. Renamed parameter decision_ididentifier
  2. Tries svc.get_decision(identifier) first (backward compat)
  3. Falls back to svc.list_decisions(identifier) treating it as a plan_id, explaining the root decision
  4. Clear error if neither resolves

Quality Gates

Session Result
nox -s lint PASS
nox -s typecheck PASS (0 errors)
nox -s unit_tests (explain features) 46/46 PASS

Closes #968

## Summary Fixes `plan explain` to accept both decision IDs and plan IDs as the positional argument, matching the M3 acceptance test usage pattern `plan explain <plan_id>`. ### Problem `explain_decision_cmd` only accepted a decision ULID. When the M3 acceptance test passed a plan ID, `svc.get_decision(plan_id)` returned `None`, causing "Decision not found" error with exit code 1. ### Fix 1. Renamed parameter `decision_id` → `identifier` 2. Tries `svc.get_decision(identifier)` first (backward compat) 3. Falls back to `svc.list_decisions(identifier)` treating it as a plan_id, explaining the root decision 4. Clear error if neither resolves ### Quality Gates | Session | Result | |---------|--------| | `nox -s lint` | PASS | | `nox -s typecheck` | PASS (0 errors) | | `nox -s unit_tests` (explain features) | 46/46 PASS | Closes #968
brent.edwards added this to the v3.2.0 milestone 2026-03-18 19:41:37 +00:00
freemo approved these changes 2026-03-19 04:55:26 +00:00
Dismissed
freemo left a comment

Code Review — PR #1057 fix(cli): make plan explain accept plan_id for plan-level explanation

Clean fix. The fallback chain (try get_decision(identifier) → fall back to list_decisions(identifier) as plan_id → pick root decision) is well-designed with proper error handling via contextlib.suppress(DecisionNotFoundError). Clear error message when neither lookup resolves. Test mock updated to cover the new fallback path.

Approved. No issues found.

## Code Review — PR #1057 `fix(cli): make plan explain accept plan_id for plan-level explanation` Clean fix. The fallback chain (try `get_decision(identifier)` → fall back to `list_decisions(identifier)` as plan_id → pick root decision) is well-designed with proper error handling via `contextlib.suppress(DecisionNotFoundError)`. Clear error message when neither lookup resolves. Test mock updated to cover the new fallback path. **Approved.** No issues found.
brent.edwards force-pushed bugfix/m3-plan-explain-plan-id from e9e40ecd71
All checks were successful
CI / benchmark-publish (pull_request) Has been skipped
CI / lint (pull_request) Successful in 23s
CI / quality (pull_request) Successful in 28s
CI / build (pull_request) Successful in 40s
CI / typecheck (pull_request) Successful in 45s
CI / security (pull_request) Successful in 1m16s
CI / unit_tests (pull_request) Successful in 3m23s
CI / integration_tests (pull_request) Successful in 3m49s
CI / docker (pull_request) Successful in 55s
CI / e2e_tests (pull_request) Successful in 4m40s
CI / coverage (pull_request) Successful in 6m48s
CI / benchmark-regression (pull_request) Successful in 37m30s
to 5b1c19f942
Some checks failed
CI / typecheck (pull_request) Successful in 49s
CI / lint (pull_request) Successful in 1m12s
CI / quality (pull_request) Successful in 28s
CI / security (pull_request) Successful in 1m2s
CI / benchmark-publish (pull_request) Has been skipped
CI / build (pull_request) Successful in 15s
CI / e2e_tests (pull_request) Successful in 5m24s
CI / unit_tests (pull_request) Failing after 5m55s
CI / docker (pull_request) Has been skipped
CI / integration_tests (pull_request) Failing after 6m1s
CI / coverage (pull_request) Successful in 8m33s
CI / benchmark-regression (pull_request) Successful in 41m59s
2026-03-19 20:58:06 +00:00
Compare
brent.edwards dismissed freemo's review 2026-03-19 21:50:29 +00:00
Reason:

New commits pushed, approval review dismissed automatically according to repository settings

brent.edwards force-pushed bugfix/m3-plan-explain-plan-id from 0fef703b67
Some checks failed
CI / lint (pull_request) Successful in 17s
CI / benchmark-publish (pull_request) Has been skipped
CI / quality (pull_request) Successful in 27s
CI / build (pull_request) Successful in 16s
CI / typecheck (pull_request) Successful in 54s
CI / security (pull_request) Successful in 58s
CI / integration_tests (pull_request) Failing after 3m6s
CI / unit_tests (pull_request) Failing after 3m45s
CI / docker (pull_request) Has been skipped
CI / e2e_tests (pull_request) Successful in 5m23s
CI / coverage (pull_request) Successful in 8m12s
CI / benchmark-regression (pull_request) Has been cancelled
to eeadb0ca61
Some checks failed
CI / quality (pull_request) Successful in 34s
CI / lint (pull_request) Successful in 36s
CI / benchmark-publish (pull_request) Has been skipped
CI / typecheck (pull_request) Successful in 41s
CI / security (pull_request) Successful in 42s
CI / build (pull_request) Successful in 16s
CI / unit_tests (pull_request) Successful in 2m8s
CI / integration_tests (pull_request) Has been cancelled
CI / e2e_tests (pull_request) Has been cancelled
CI / benchmark-regression (pull_request) Has been cancelled
CI / docker (pull_request) Has been cancelled
CI / coverage (pull_request) Has been cancelled
2026-03-19 22:00:48 +00:00
Compare
Merge branch 'master' into bugfix/m3-plan-explain-plan-id
All checks were successful
CI / lint (pull_request) Successful in 23s
CI / benchmark-publish (pull_request) Has been skipped
CI / typecheck (pull_request) Successful in 32s
CI / quality (pull_request) Successful in 30s
CI / build (pull_request) Successful in 29s
CI / security (pull_request) Successful in 55s
CI / e2e_tests (pull_request) Successful in 5m39s
CI / coverage (pull_request) Successful in 5m13s
CI / unit_tests (pull_request) Successful in 5m53s
CI / integration_tests (pull_request) Successful in 5m59s
CI / docker (pull_request) Successful in 1m8s
CI / benchmark-regression (pull_request) Successful in 41m1s
59bdb4b31c
brent.edwards scheduled this pull request to auto merge when all checks succeed 2026-03-19 22:04:39 +00:00
brent.edwards deleted branch bugfix/m3-plan-explain-plan-id 2026-03-19 22:10:24 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Reference
cleveragents/cleveragents-core!1057
No description provided.