Repository navigation
fix(tui): narrow bare except clauses to specific exceptions - #154
Merged
Merged
Conversation
Replace 30 overly broad `except Exception: pass` with precise exception types across 11 TUI files: - 29× `except NoMatches:` for `query_one()` calls that may fail when widgets are not yet mounted or have been removed from the DOM - 1× `except ValueError:` for `datetime.fromisoformat()` parsing Bare `except Exception` silently swallows real bugs (TypeError, KeyError, AttributeError) that should surface during development. `query_one()` only raises `NoMatches` when the selector finds no widget — `TooManyMatches` is intentionally not caught since duplicate widget IDs indicate a real bug that should be fixed. Files changed: - components/: agents_panel, event_log, progress, token_tracker - screens/: dashboard, dashboard_v2, dashboard_v3, hud_dashboard, logs - widgets/: agent_activity, lineage_tree Co-Authored-By: Claude Opus 4.6 <[email protected]>
![ouroboros-agent[bot]](https://avatars.githubusercontent.com/in/3091302?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Review — ouroboros-agent[bot]
Verdict: APPROVE
Branch: fix/tui-narrow-bare-except | 11 files, +41/-30 | CI: all required checks passing
Issue #N Requirements
| Requirement | Status |
|---|---|
Narrow broad except Exception handlers around query_one() calls to the specific missing-widget failure mode |
MET — all 29 UI lookup sites now import and catch NoMatches instead of swallowing unrelated exceptions (src/ouroboros/tui/components/progress.py:302, src/ouroboros/tui/screens/dashboard_v2.py:488, src/ouroboros/tui/widgets/lineage_tree.py:135) |
| Preserve fallback behavior for malformed event-log timestamps while exposing unrelated runtime bugs | MET — event_log.py now falls back only on ValueError from datetime.fromisoformat(...) instead of masking arbitrary exceptions (src/ouroboros/tui/components/event_log.py:347) |
Code Findings
| # | File:Line | Severity | Confidence | Finding |
|---|---|---|---|---|
| 1 | N/A |
Low | 95% | No blocking issues found. The change consistently narrows exception handling to the expected failure mode and intentionally leaves TooManyMatches visible, which is the right tradeoff for surfacing real TUI bugs. |
Test Coverage
- CI is green across Ruff, MyPy, and Python 3.12/3.13/3.14 test jobs.
- The PR description documents a full
pytest tests/pass and a repo-wide audit of remainingexcept Exceptionusage. - I did not see a targeted regression test for the missing-widget paths, but this is acceptable here because the change is a mechanical exception narrowing with broad existing suite coverage.
Design
- Good cleanup: it reduces silent failure scope without changing the intended UI fallback behavior.
ouroboros-agent[bot]
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Replace 30 overly broad
except Exception: passwith precise exception types across 11 TUI files.except NoMatches:forquery_one()calls that may fail when widgets are not yet mounted or have been removed from the DOMexcept ValueError:fordatetime.fromisoformat()parsing inevent_log.pyWhy this matters
Bare
except Exceptionsilently swallows real bugs —TypeError,KeyError,AttributeErrorall get hidden behindpass. This makes debugging TUI issues unnecessarily difficult, since genuine errors disappear without a trace.query_one()only raisesNoMatcheswhen the CSS selector finds no widget. The other exception it can raise —TooManyMatches— is intentionally not caught, because duplicate widget IDs indicate a real bug that should surface immediately.Files changed
Components (4 files, 13 changes):
agents_panel.py— 1×NoMatches(DataTable query)event_log.py— 3×NoMatches+ 1×ValueError(datetime parsing)progress.py— 5×NoMatches(progress display updates)token_tracker.py— 3×NoMatches(summary/tier/budget widgets)Screens (5 files, 15 changes):
dashboard.py— 3×NoMatches(status widgets)dashboard_v2.py— 7×NoMatches(mini-status widgets)dashboard_v3.py— 2×NoMatches(Tree widget)hud_dashboard.py— 2×NoMatches(HUD row / Tree widget)logs.py— 1×NoMatches(VerticalScroll container)Widgets (2 files, 2 changes):
agent_activity.py— 1×NoMatches(activity display)lineage_tree.py— 1×NoMatches(Tree widget)What was intentionally left unchanged
app.py(11 instances) — wraps polling, file I/O, and state updates, notquery_one()screens/debug.py(2 instances) — usesexcept Exception as e:with logging; intentional broad catchscreens/lineage_detail.py(5 instances) — mixed patterns including git operations andas eloggingscreens/session_selector.py,lineage_selector.py—as epatterns with error handlingThese require case-by-case analysis and are better suited for a follow-up PR.
Test plan
ruff check— all checks passed (no lint issues)pytest tests/— 2516 passedgrep -rn "except Exception" src/ouroboros/tui/audit — all remaining instances verified as intentional🤖 Generated with Claude Code