From f49d3c40b82882deaa72346974c0f620a2757953 Mon Sep 17 00:00:00 2001 From: Sasa Junuzovic Date: Sat, 21 Mar 2026 18:08:06 -0700 Subject: [PATCH 1/3] fix: use Loguru brace formatting instead of %s in parser.py Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/copilot_usage/parser.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/copilot_usage/parser.py b/src/copilot_usage/parser.py index 9367f03..9afe91b 100644 --- a/src/copilot_usage/parser.py +++ b/src/copilot_usage/parser.py @@ -355,7 +355,7 @@ def get_all_sessions(base_path: Path | None = None) -> list[SessionSummary]: try: events = parse_events(events_path) except (FileNotFoundError, OSError) as exc: - logger.warning("Skipping vanished session %s: %s", events_path, exc) + logger.warning("Skipping vanished session {}: {}", events_path, exc) continue if not events: continue From 00f027346125aa5a783151ff4efb9b93390ad799 Mon Sep 17 00:00:00 2001 From: Sasa Junuzovic Date: Sat, 21 Mar 2026 18:15:14 -0700 Subject: [PATCH 2/3] fix: remove broad except-Exception, add narrow OSError handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Removed broad except-Exception blocks that swallowed programming errors. Added narrow except-OSError catches around session data access only — filesystem errors on data files get a friendly message, programming bugs crash loudly. Also removes dead 'except SystemExit: raise' from session command. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/copilot_usage/cli.py | 98 ++++++++++++++++----------------- tests/copilot_usage/test_cli.py | 13 ++--- 2 files changed, 52 insertions(+), 59 deletions(-) diff --git a/src/copilot_usage/cli.py b/src/copilot_usage/cli.py index ba61842..ad0ed7d 100644 --- a/src/copilot_usage/cli.py +++ b/src/copilot_usage/cli.py @@ -313,12 +313,12 @@ def summary( path = path or ctx.obj.get("path") try: sessions = get_all_sessions(path) - render_summary( - sessions, since=ensure_aware_opt(since), until=ensure_aware_opt(until) - ) - except Exception as exc: # noqa: BLE001 - click.echo(f"Error: {exc}", err=True) + except OSError as exc: + click.echo(f"Error reading sessions: {exc}", err=True) sys.exit(1) + render_summary( + sessions, since=ensure_aware_opt(since), until=ensure_aware_opt(until) + ) # --------------------------------------------------------------------------- @@ -341,45 +341,39 @@ def session(ctx: click.Context, session_id: str, path: Path | None) -> None: path = path or ctx.obj.get("path") try: event_paths = discover_sessions(path) - if not event_paths: - click.echo("No sessions found.", err=True) - sys.exit(1) - - # Fast path: skip directories that clearly cannot match the prefix. - # Only apply the pre-filter on UUID-shaped directory names (36 chars - # with 4 dashes), where the directory name IS the session ID. - # Non-UUID dirs (e.g. test fixtures) always need a full parse. - available: list[str] = [] - for events_path in event_paths: - dir_name = events_path.parent.name - is_uuid_dir = len(dir_name) == 36 and dir_name.count("-") == 4 - if ( - len(session_id) >= 4 - and is_uuid_dir - and not dir_name.startswith(session_id) - ): - available.append(dir_name[:8]) - continue - events = parse_events(events_path) - if not events: - continue - s = build_session_summary(events, session_dir=events_path.parent) - if s.session_id.startswith(session_id): - render_session_detail(events, s) - return - if s.session_id: - available.append(s.session_id[:8]) - - click.echo(f"Error: no session matching '{session_id}'", err=True) - if available: - click.echo(f"Available: {', '.join(available)}", err=True) + except OSError as exc: + click.echo(f"Error reading sessions: {exc}", err=True) sys.exit(1) - except SystemExit: - raise - except Exception as exc: # noqa: BLE001 - click.echo(f"Error: {exc}", err=True) + if not event_paths: + click.echo("No sessions found.", err=True) sys.exit(1) + # Fast path: skip directories that clearly cannot match the prefix. + # Only apply the pre-filter on UUID-shaped directory names (36 chars + # with 4 dashes), where the directory name IS the session ID. + # Non-UUID dirs (e.g. test fixtures) always need a full parse. + available: list[str] = [] + for events_path in event_paths: + dir_name = events_path.parent.name + is_uuid_dir = len(dir_name) == 36 and dir_name.count("-") == 4 + if len(session_id) >= 4 and is_uuid_dir and not dir_name.startswith(session_id): + available.append(dir_name[:8]) + continue + events = parse_events(events_path) + if not events: + continue + s = build_session_summary(events, session_dir=events_path.parent) + if s.session_id.startswith(session_id): + render_session_detail(events, s) + return + if s.session_id: + available.append(s.session_id[:8]) + + click.echo(f"Error: no session matching '{session_id}'", err=True) + if available: + click.echo(f"Available: {', '.join(available)}", err=True) + sys.exit(1) + # --------------------------------------------------------------------------- # cost @@ -417,16 +411,16 @@ def cost( path = path or ctx.obj.get("path") try: sessions = get_all_sessions(path) - - render_cost_view( - sessions, - since=ensure_aware_opt(since), - until=ensure_aware_opt(until), - ) - except Exception as exc: # noqa: BLE001 - click.echo(f"Error: {exc}", err=True) + except OSError as exc: + click.echo(f"Error reading sessions: {exc}", err=True) sys.exit(1) + render_cost_view( + sessions, + since=ensure_aware_opt(since), + until=ensure_aware_opt(until), + ) + # --------------------------------------------------------------------------- # live @@ -447,7 +441,7 @@ def live(ctx: click.Context, path: Path | None) -> None: path = path or ctx.obj.get("path") try: sessions = get_all_sessions(path) - render_live_sessions(sessions) - except Exception as exc: # noqa: BLE001 - click.echo(f"Error: {exc}", err=True) + except OSError as exc: + click.echo(f"Error reading sessions: {exc}", err=True) sys.exit(1) + render_live_sessions(sessions) diff --git a/tests/copilot_usage/test_cli.py b/tests/copilot_usage/test_cli.py index 918d24b..0e95f16 100644 --- a/tests/copilot_usage/test_cli.py +++ b/tests/copilot_usage/test_cli.py @@ -268,7 +268,7 @@ def test_summary_invalid_path() -> None: def test_summary_error_handling(tmp_path: Path, monkeypatch: Any) -> None: - """Exercise the except-Exception branch (lines 77-79) in summary.""" + """OSError in get_all_sessions produces a friendly error message.""" def _exploding_sessions(_base: Path | None = None) -> list[object]: msg = "disk on fire" @@ -319,7 +319,7 @@ def _fake_discover(_base: Path | None = None) -> list[Path]: def test_session_error_handling(tmp_path: Path, monkeypatch: Any) -> None: - """Trigger an exception in session detail → friendly error (lines 129-131).""" + """PermissionError in discover_sessions produces a friendly error message.""" def _exploding_discover(_base: Path | None = None) -> list[Path]: msg = "permission denied" @@ -330,7 +330,6 @@ def _exploding_discover(_base: Path | None = None) -> list[Path]: result = runner.invoke(main, ["session", "anything"]) assert result.exit_code != 0 assert "permission denied" in result.output - assert "Traceback" not in (result.output or "") def test_cost_no_model_metrics(tmp_path: Path) -> None: @@ -387,11 +386,11 @@ def test_cost_zero_multiplier_model(tmp_path: Path) -> None: def test_cost_error_handling(tmp_path: Path, monkeypatch: Any) -> None: - """Exercise the except-Exception branch (lines 226-228) in cost.""" + """OSError in get_all_sessions produces a friendly error message.""" def _exploding_sessions(_base: Path | None = None) -> list[object]: msg = "cost explosion" - raise RuntimeError(msg) + raise OSError(msg) monkeypatch.setattr("copilot_usage.cli.get_all_sessions", _exploding_sessions) runner = CliRunner() @@ -401,11 +400,11 @@ def _exploding_sessions(_base: Path | None = None) -> list[object]: def test_live_error_handling(tmp_path: Path, monkeypatch: Any) -> None: - """Exercise the except-Exception branch (lines 248-250) in live.""" + """OSError in get_all_sessions produces a friendly error message.""" def _exploding_sessions(_base: Path | None = None) -> list[object]: msg = "live explosion" - raise RuntimeError(msg) + raise OSError(msg) monkeypatch.setattr("copilot_usage.cli.get_all_sessions", _exploding_sessions) runner = CliRunner() From ca2811b1bb0fda635ed12695016d430dda409194 Mon Sep 17 00:00:00 2001 From: Sasa Junuzovic Date: Sun, 22 Mar 2026 01:05:15 -0700 Subject: [PATCH 3/3] =?UTF-8?q?fix:=20address=20review=20=E2=80=94=20simpl?= =?UTF-8?q?ify=20OSError=20catch,=20guard=20parse=5Fevents,=20assert=20no?= =?UTF-8?q?=20traceback?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove redundant FileNotFoundError (subclass of OSError) in parser.py - Wrap parse_events() in session command with OSError catch to skip unreadable files - Add 'Traceback not in output' assertions to all error handling tests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/copilot_usage/cli.py | 5 ++++- src/copilot_usage/parser.py | 2 +- tests/copilot_usage/test_cli.py | 4 ++++ 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/src/copilot_usage/cli.py b/src/copilot_usage/cli.py index ad0ed7d..27dfa74 100644 --- a/src/copilot_usage/cli.py +++ b/src/copilot_usage/cli.py @@ -359,7 +359,10 @@ def session(ctx: click.Context, session_id: str, path: Path | None) -> None: if len(session_id) >= 4 and is_uuid_dir and not dir_name.startswith(session_id): available.append(dir_name[:8]) continue - events = parse_events(events_path) + try: + events = parse_events(events_path) + except OSError: + continue if not events: continue s = build_session_summary(events, session_dir=events_path.parent) diff --git a/src/copilot_usage/parser.py b/src/copilot_usage/parser.py index 9afe91b..4959acd 100644 --- a/src/copilot_usage/parser.py +++ b/src/copilot_usage/parser.py @@ -354,7 +354,7 @@ def get_all_sessions(base_path: Path | None = None) -> list[SessionSummary]: for events_path in paths: try: events = parse_events(events_path) - except (FileNotFoundError, OSError) as exc: + except OSError as exc: logger.warning("Skipping vanished session {}: {}", events_path, exc) continue if not events: diff --git a/tests/copilot_usage/test_cli.py b/tests/copilot_usage/test_cli.py index 0e95f16..1c08d5b 100644 --- a/tests/copilot_usage/test_cli.py +++ b/tests/copilot_usage/test_cli.py @@ -279,6 +279,7 @@ def _exploding_sessions(_base: Path | None = None) -> list[object]: result = runner.invoke(main, ["summary", "--path", str(tmp_path)]) assert result.exit_code != 0 assert "disk on fire" in result.output + assert "Traceback" not in (result.output or "") def test_session_no_sessions(tmp_path: Path, monkeypatch: Any) -> None: @@ -330,6 +331,7 @@ def _exploding_discover(_base: Path | None = None) -> list[Path]: result = runner.invoke(main, ["session", "anything"]) assert result.exit_code != 0 assert "permission denied" in result.output + assert "Traceback" not in (result.output or "") def test_cost_no_model_metrics(tmp_path: Path) -> None: @@ -397,6 +399,7 @@ def _exploding_sessions(_base: Path | None = None) -> list[object]: result = runner.invoke(main, ["cost", "--path", str(tmp_path)]) assert result.exit_code != 0 assert "cost explosion" in result.output + assert "Traceback" not in (result.output or "") def test_live_error_handling(tmp_path: Path, monkeypatch: Any) -> None: @@ -411,6 +414,7 @@ def _exploding_sessions(_base: Path | None = None) -> list[object]: result = runner.invoke(main, ["live", "--path", str(tmp_path)]) assert result.exit_code != 0 assert "live explosion" in result.output + assert "Traceback" not in (result.output or "") # ---------------------------------------------------------------------------