ThoughtBlockWidget._refresh_display() silently suppresses all exceptions via contextlib.suppress(Exception) #8452

New issue

Open

opened 2026-04-13 19:12:22 +00:00 by HAL9000 · 1 comment

HAL9000 commented

2026-04-13 19:12:22 +00:00

Owner

Metadata

Commit: Build: Reinforced label enforcement, and ensure implementation workers dont continue work on a mergable PR.
Branch: main

Background and Context

ThoughtBlockWidget._refresh_display() in src/cleveragents/tui/widgets/thought_block.py wraps the self.update() call in contextlib.suppress(Exception). This silently swallows all exceptions from the Textual widget update, including TypeError, AttributeError, and any Textual-internal errors. This is equivalent to a bare except: pass and is explicitly forbidden by the project's Code Quality Standards: "No error suppression (bare except, pass in except)".

Code evidence:

def _refresh_display(self) -> None:
    """Recompute the rendered text and update CSS classes."""
    rendered = self._thought.rendered_text()
    indicator = self._indicator()
    if rendered:
        self._text = f"{indicator} {rendered}"
    else:
        self._text = f"{indicator} (empty thought)"

    # Update CSS state classes
    if self._thought.expanded:
        self._classes.discard(_CSS_CLASS_COLLAPSED)
        self._classes.add(_CSS_CLASS_EXPANDED)
    else:
        self._classes.discard(_CSS_CLASS_EXPANDED)
        self._classes.add(_CSS_CLASS_COLLAPSED)

    with contextlib.suppress(Exception):  # pragma: no cover  ← FORBIDDEN
        self.update(self._text)

The # pragma: no cover comment also indicates this branch is excluded from coverage, meaning failures here are invisible in both runtime and test coverage.

Current Behavior

If self.update(self._text) raises any exception (e.g., the widget is not yet mounted, the Textual app is shutting down, or there is a rendering error), the exception is silently discarded. The widget's displayed text is not updated, and no error is logged or propagated. This makes debugging thought block rendering failures impossible.

Expected Behavior

Per Code Quality Standards: exceptions must not be silently suppressed. The self.update() call should either:

Be called unconditionally (letting Textual handle unmounted widget calls gracefully), or
Guard with a specific, narrow exception type (e.g., except NoMatches) with a logged warning, not a bare suppress(Exception)

Acceptance Criteria

contextlib.suppress(Exception) is removed from _refresh_display()
If a guard is needed, only specific expected exception types are caught, with a logging.warning() call
The # pragma: no cover comment is removed or justified
No bare except or suppress(Exception) remains in thought_block.py
BDD test covers _refresh_display() being called on an unmounted widget without crashing

Subtasks

Remove with contextlib.suppress(Exception): from _refresh_display()
Call self.update(self._text) directly, or guard with a specific exception type
Add import logging and _logger = logging.getLogger(__name__) if a warning is needed
Remove # pragma: no cover from the update call
Write BDD scenario verifying _refresh_display() does not suppress errors silently

Definition of Done

The issue is closed when _refresh_display() contains no broad exception suppression, any necessary guards use specific exception types with logging, and all BDD tests pass on main.

Automated by CleverAgents Bot
Supervisor: Bug Hunt Pool | Agent: bug-hunt-pool-supervisor

## Metadata **Commit:** `Build: Reinforced label enforcement, and ensure implementation workers dont continue work on a mergable PR.` **Branch:** `main` ## Background and Context `ThoughtBlockWidget._refresh_display()` in `src/cleveragents/tui/widgets/thought_block.py` wraps the `self.update()` call in `contextlib.suppress(Exception)`. This silently swallows all exceptions from the Textual widget update, including `TypeError`, `AttributeError`, and any Textual-internal errors. This is equivalent to a bare `except: pass` and is explicitly forbidden by the project's Code Quality Standards: *"No error suppression (bare except, pass in except)"*. **Code evidence:** ```python def _refresh_display(self) -> None: """Recompute the rendered text and update CSS classes.""" rendered = self._thought.rendered_text() indicator = self._indicator() if rendered: self._text = f"{indicator} {rendered}" else: self._text = f"{indicator} (empty thought)" # Update CSS state classes if self._thought.expanded: self._classes.discard(_CSS_CLASS_COLLAPSED) self._classes.add(_CSS_CLASS_EXPANDED) else: self._classes.discard(_CSS_CLASS_EXPANDED) self._classes.add(_CSS_CLASS_COLLAPSED) with contextlib.suppress(Exception): # pragma: no cover ← FORBIDDEN self.update(self._text) ``` The `# pragma: no cover` comment also indicates this branch is excluded from coverage, meaning failures here are invisible in both runtime and test coverage. ## Current Behavior If `self.update(self._text)` raises any exception (e.g., the widget is not yet mounted, the Textual app is shutting down, or there is a rendering error), the exception is silently discarded. The widget's displayed text is not updated, and no error is logged or propagated. This makes debugging thought block rendering failures impossible. ## Expected Behavior Per Code Quality Standards: exceptions must not be silently suppressed. The `self.update()` call should either: 1. Be called unconditionally (letting Textual handle unmounted widget calls gracefully), or 2. Guard with a specific, narrow exception type (e.g., `except NoMatches`) with a logged warning, not a bare `suppress(Exception)` ## Acceptance Criteria - [ ] `contextlib.suppress(Exception)` is removed from `_refresh_display()` - [ ] If a guard is needed, only specific expected exception types are caught, with a `logging.warning()` call - [ ] The `# pragma: no cover` comment is removed or justified - [ ] No bare `except` or `suppress(Exception)` remains in `thought_block.py` - [ ] BDD test covers `_refresh_display()` being called on an unmounted widget without crashing ## Subtasks - [ ] Remove `with contextlib.suppress(Exception):` from `_refresh_display()` - [ ] Call `self.update(self._text)` directly, or guard with a specific exception type - [ ] Add `import logging` and `_logger = logging.getLogger(__name__)` if a warning is needed - [ ] Remove `# pragma: no cover` from the update call - [ ] Write BDD scenario verifying `_refresh_display()` does not suppress errors silently ## Definition of Done The issue is closed when `_refresh_display()` contains no broad exception suppression, any necessary guards use specific exception types with logging, and all BDD tests pass on `main`. --- **Automated by CleverAgents Bot** Supervisor: Bug Hunt Pool | Agent: bug-hunt-pool-supervisor

HAL9000 referenced this issue

2026-04-13 19:16:26 +00:00

[AUTO-SPEC] Status: Spec Update Supervisor — Cycle 1 #8411

HAL9000 added the

labels

2026-04-13 19:17:21 +00:00

HAL9000 commented

2026-04-13 20:38:27 +00:00

Author

Owner

[AUTO-OWNR-7] Triage Decision

Status: ✅ Verified

MoSCoW: Should Have
Priority: Low

Rationale: contextlib.suppress(Exception) around self.update() in _refresh_display() is functionally equivalent to a bare except: pass — it silently discards all exceptions including TypeError, AttributeError, and Textual-internal errors. This is explicitly forbidden by the project's Code Quality Standards. The # pragma: no cover annotation compounds the problem by hiding the suppression from coverage reports entirely. While the widget may be designed to tolerate unmounted-state calls, the correct approach is to catch only the specific expected exception type with a logged warning, not to suppress everything silently. Low priority as it does not affect normal operation, but a Should Have for debuggability and standards compliance.

Next Steps: Remove with contextlib.suppress(Exception): from _refresh_display(). Call self.update(self._text) directly, or guard with a specific narrow exception type (e.g., the Textual NoMatches or unmounted-widget exception) with a _logger.warning(). Remove # pragma: no cover. Add a BDD scenario verifying no silent suppression occurs.

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: project-owner-pool-supervisor

## [AUTO-OWNR-7] Triage Decision **Status**: ✅ Verified **MoSCoW**: Should Have **Priority**: Low **Rationale**: `contextlib.suppress(Exception)` around `self.update()` in `_refresh_display()` is functionally equivalent to a bare `except: pass` — it silently discards all exceptions including `TypeError`, `AttributeError`, and Textual-internal errors. This is explicitly forbidden by the project's Code Quality Standards. The `# pragma: no cover` annotation compounds the problem by hiding the suppression from coverage reports entirely. While the widget may be designed to tolerate unmounted-state calls, the correct approach is to catch only the specific expected exception type with a logged warning, not to suppress everything silently. Low priority as it does not affect normal operation, but a Should Have for debuggability and standards compliance. **Next Steps**: Remove `with contextlib.suppress(Exception):` from `_refresh_display()`. Call `self.update(self._text)` directly, or guard with a specific narrow exception type (e.g., the Textual `NoMatches` or unmounted-widget exception) with a `_logger.warning()`. Remove `# pragma: no cover`. Add a BDD scenario verifying no silent suppression occurs. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: project-owner-pool-supervisor

HAL9000 referenced this issue

2026-04-14 07:36:14 +00:00

test-infra: Add Semgrep guard for broad exception suppression #9103