From 603136f05cf76509ca467ebf0cd78787f96ddc6d Mon Sep 17 00:00:00 2001 From: Sasa Junuzovic <44276455+microsasa@users.noreply.github.com> Date: Wed, 6 May 2026 14:18:11 -0700 Subject: [PATCH] fix: remove dead code in _build_active_summary (fp.model always None) Remove two instances of dead/misleading code in parser.py: 1. _build_active_summary line 979: simplified 'model = fp.model or fp.tool_model' to 'model = fp.tool_model' since fp.model is invariably None in the active-session path. 2. _build_session_summary_with_meta line 1058: simplified 'used_config = fp.model is None and fp.tool_model is None' to 'used_config = fp.tool_model is None' since fp.model is always None when no shutdowns exist. Added a unit test that explicitly documents the invariant: fp.model is None when no SESSION_SHUTDOWN events exist, and the summary model is resolved from fp.tool_model directly. Closes #1202 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/copilot_usage/parser.py | 4 ++-- tests/copilot_usage/test_parser.py | 37 ++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/src/copilot_usage/parser.py b/src/copilot_usage/parser.py index f0ed8f87..a18d993e 100644 --- a/src/copilot_usage/parser.py +++ b/src/copilot_usage/parser.py @@ -976,7 +976,7 @@ def _build_active_summary( events_path: Path | None = None, ) -> SessionSummary: """Build a :class:`SessionSummary` for a session with no shutdown data.""" - model = fp.model or fp.tool_model + model = fp.tool_model # Fall back to ~/.copilot/config.json for active sessions if model is None: @@ -1055,7 +1055,7 @@ def _build_session_summary_with_meta( used_config_fallback=False, ) - used_config = fp.model is None and fp.tool_model is None + used_config = fp.tool_model is None return _BuildMeta( _build_active_summary(fp, name, config_path, events_path=events_path), used_config_fallback=used_config, diff --git a/tests/copilot_usage/test_parser.py b/tests/copilot_usage/test_parser.py index 76568f72..d7ca8b5a 100644 --- a/tests/copilot_usage/test_parser.py +++ b/tests/copilot_usage/test_parser.py @@ -6754,6 +6754,43 @@ def test_active_session_resolves_model_via_first_pass(self, tmp_path: Path) -> N assert summary.model == "claude-sonnet-4" assert summary.is_active is True + def test_active_summary_uses_tool_model_directly_without_fp_model( + self, tmp_path: Path + ) -> None: + """Active session model comes from tool_model alone (fp.model is None). + + Confirms that _build_active_summary does not depend on fp.model + (which is only set during SESSION_SHUTDOWN processing). A session + with TOOL_EXECUTION_COMPLETE but no SESSION_SHUTDOWN must resolve + the model from fp.tool_model. + """ + tool = json.dumps( + { + "type": "tool.execution_complete", + "data": { + "toolCallId": "tc-1", + "model": "gpt-5.1", + "success": True, + }, + "id": "ev-t1", + "timestamp": "2026-03-07T10:01:00.000Z", + } + ) + p = tmp_path / "s" / "events.jsonl" + _write_events(p, _START_EVENT, _USER_MSG, _ASSISTANT_MSG, tool) + events = parse_events(p) + + # Confirm the invariant: fp.model is None when no shutdown exists + fp = _first_pass(events) + assert fp.model is None + assert fp.all_shutdowns == () + assert fp.tool_model == "gpt-5.1" + + # The summary model must match tool_model + summary = build_session_summary(events, config_path=tmp_path / "no-config.json") + assert summary.model == "gpt-5.1" + assert summary.is_active is True + def test_malformed_tool_event_skipped(self, tmp_path: Path) -> None: """A malformed tool.execution_complete is skipped; next valid one wins.""" bad_tool = json.dumps(