[AUTO-INF-2] Restore Ruff compliance for plan explain failure path #8943

New issue

Open

opened 2026-04-14 04:05:13 +00:00 by HAL9000 · 0 comments

HAL9000 commented

2026-04-14 04:05:13 +00:00

Owner

Summary

The nox -s lint job fails on master because ruff reports rule B904 in src/cleveragents/cli/commands/plan.py when DecisionService.get_decision() raises DecisionNotFoundError.
The handler prints a user-facing error message and immediately raises typer.Exit(1) inside the except clause without chaining the original exception, which violates bugbear rule B904.

Impact

CI cannot complete the lint stage, blocking merges while master remains red.
Local contributors running nox -s lint or ruff check src/cleveragents/cli/commands/plan.py hit the same failure, preventing pre-commit hook compliance.

Steps to Reproduce

Check out master at commit 38bcd41338ff610ba12f95cc60f14a321616cea0 (latest when the failure was captured in ci_run_12455_lint.log).
Run nox -s lint (installs ruff>=0.15,<0.16).
Observe the failure: B904 Within an except clause, raise exceptions with raise ... from err or raise ... from None.

Current Behavior

2026-04-09T22:22:24.9815687Z B904 Within an `except` clause, raise exceptions with `raise ... from err` or `raise ... from None`
2026-04-09T22:22:24.9816123Z     --> src/cleveragents/cli/commands/plan.py:4220:9
2026-04-09T22:22:24.9816425Z 4218 |     except DecisionNotFoundError:
2026-04-09T22:22:24.9816555Z 4219 |         console.print(f"[red]Error:[/red] '{identifier}' not found as a decision.")
2026-04-09T22:22:24.9816675Z 4220 |         raise typer.Exit(1)

Expected Behavior

ruff should exit cleanly with no B-series violations when linting plan.py.

Root Cause Analysis

Ruff bugbear rule B904 requires chaining (or explicitly discarding) the original exception when re-raising inside an except block.
The plan explain command swallows DecisionNotFoundError and raises typer.Exit(1) directly, so Ruff flags the handler because it loses the underlying exception context.

Proposed Fix

Refactor the handler so the control-flow exit happens outside the except block, keeping the error handling explicit and Ruff-compliant. Two concrete options:
- Convert the try/except into a contextlib.suppress(DecisionNotFoundError) block, record decision = None, and handle the console.print/typer.Exit path after the with block.
- Alternatively, catch DecisionNotFoundError as exc, emit the user message, and use raise typer.Exit(1) from exc (or from None if we intentionally discard the stack). The suppress pattern is preferred because it also enables the existing fallback that queries svc.list_decisions() when a plan ULID is supplied.
Add a targeted unit test around plan explain that exercises the "missing decision" path to ensure DecisionNotFoundError is chained or suppressed correctly and prevents regressions.

Acceptance Criteria

nox -s lint succeeds without reporting B904 for plan.py.
The failure path for agents plan explain <missing-id> still exits with status 1 and prints the same user-facing message.
A regression test covers the missing-decision path to guard against reintroducing the forbidden raise pattern.

Definition of Done

Fix merged into master with reviewer approval.
All CI stages (lint, unit, integration) are green on the fixing commit.
Documentation or changelog updated if additional behavior (decision vs plan fallback) is touched.

Duplicate Check

Keyword search (open) — Searched for "B904" via /issues?q=B904&state=open; found status trackers (#8906, #8898) and a multi-failure ticket (#8328) but none dedicated to fixing this handler.
Cross-area search — Looked at Implementation and Product Builder pool trackers (#8328, #8906); they only memoize the failure and do not provide a remediation issue for plan.py.
Closed issues search — Queried state=closed&q=B904; hits such as #3264 and #2604 address unrelated CLI error handling paths and are already resolved.
Dedup proof — See open search results: #8906, /8898, /8328. Closed search results: /8162, /3264, /2604 (different scopes).
Uncertainty avoidance — No existing open issue is scoped to resolving the plan.py lint failure; confident this report is not a duplicate.

Automated by CleverAgents Bot
Supervisor: Test Infrastructure Pool | Agent: test-infra-worker

## Summary - The `nox -s lint` job fails on master because `ruff` reports rule **B904** in `src/cleveragents/cli/commands/plan.py` when `DecisionService.get_decision()` raises `DecisionNotFoundError`. - The handler prints a user-facing error message and immediately raises `typer.Exit(1)` inside the `except` clause without chaining the original exception, which violates bugbear rule B904. ## Impact - CI cannot complete the lint stage, blocking merges while `master` remains red. - Local contributors running `nox -s lint` or `ruff check src/cleveragents/cli/commands/plan.py` hit the same failure, preventing pre-commit hook compliance. ## Steps to Reproduce 1. Check out `master` at commit `38bcd41338ff610ba12f95cc60f14a321616cea0` (latest when the failure was captured in `ci_run_12455_lint.log`). 2. Run `nox -s lint` (installs `ruff>=0.15,<0.16`). 3. Observe the failure: `B904 Within an except clause, raise exceptions with raise ... from err or raise ... from None`. ## Current Behavior ``` 2026-04-09T22:22:24.9815687Z B904 Within an `except` clause, raise exceptions with `raise ... from err` or `raise ... from None` 2026-04-09T22:22:24.9816123Z --> src/cleveragents/cli/commands/plan.py:4220:9 2026-04-09T22:22:24.9816425Z 4218 | except DecisionNotFoundError: 2026-04-09T22:22:24.9816555Z 4219 | console.print(f"[red]Error:[/red] '{identifier}' not found as a decision.") 2026-04-09T22:22:24.9816675Z 4220 | raise typer.Exit(1) ``` ## Expected Behavior - `ruff` should exit cleanly with no B-series violations when linting `plan.py`. ## Root Cause Analysis - Ruff bugbear rule **B904** requires chaining (or explicitly discarding) the original exception when re-raising inside an `except` block. - The `plan explain` command swallows `DecisionNotFoundError` and raises `typer.Exit(1)` directly, so Ruff flags the handler because it loses the underlying exception context. ## Proposed Fix - Refactor the handler so the control-flow exit happens **outside** the `except` block, keeping the error handling explicit and Ruff-compliant. Two concrete options: - Convert the `try/except` into a `contextlib.suppress(DecisionNotFoundError)` block, record `decision = None`, and handle the `console.print`/`typer.Exit` path after the `with` block. - Alternatively, catch `DecisionNotFoundError as exc`, emit the user message, and use `raise typer.Exit(1) from exc` (or `from None` if we intentionally discard the stack). The suppress pattern is preferred because it also enables the existing fallback that queries `svc.list_decisions()` when a plan ULID is supplied. - Add a targeted unit test around `plan explain` that exercises the "missing decision" path to ensure `DecisionNotFoundError` is chained or suppressed correctly and prevents regressions. ## Acceptance Criteria - [ ] `nox -s lint` succeeds without reporting B904 for `plan.py`. - [ ] The failure path for `agents plan explain <missing-id>` still exits with status 1 and prints the same user-facing message. - [ ] A regression test covers the missing-decision path to guard against reintroducing the forbidden raise pattern. ## Definition of Done - [ ] Fix merged into `master` with reviewer approval. - [ ] All CI stages (lint, unit, integration) are green on the fixing commit. - [ ] Documentation or changelog updated if additional behavior (decision vs plan fallback) is touched. ### Duplicate Check 1. **Keyword search (open)** — Searched for `"B904"` via `/issues?q=B904&state=open`; found status trackers (#8906, #8898) and a multi-failure ticket (#8328) but none dedicated to fixing this handler. 2. **Cross-area search** — Looked at Implementation and Product Builder pool trackers (#8328, #8906); they only memoize the failure and do not provide a remediation issue for `plan.py`. 3. **Closed issues search** — Queried `state=closed&q=B904`; hits such as #3264 and #2604 address unrelated CLI error handling paths and are already resolved. 4. **Dedup proof** — See open search results: https://git.cleverthis.com/cleveragents/cleveragents-core/issues/8906, /8898, /8328. Closed search results: /8162, /3264, /2604 (different scopes). 5. **Uncertainty avoidance** — No existing open issue is scoped to resolving the `plan.py` lint failure; confident this report is not a duplicate. --- **Automated by CleverAgents Bot** Supervisor: Test Infrastructure Pool | Agent: test-infra-worker

HAL9000 added this to the v3.2.0 milestone

2026-04-14 04:05:13 +00:00

HAL9000 added the

labels

2026-04-14 04:05:13 +00:00

HAL9000 referenced this issue