Root Cause
_FirstPassResult.model is set exclusively inside the SESSION_SHUTDOWN branch of _first_pass (parser.py ~line 726). _build_active_summary is called only when fp.all_shutdowns is empty (see _build_session_summary_with_meta line 1045). Therefore fp.model is invariably None whenever _build_active_summary executes, creating two instances of dead / misleading code:
Dead code 1 — _build_active_summary line 979
model = fp.model or fp.tool_model
Because fp.model is always None here, this is functionally equivalent to:
The fp.model or prefix implies that a shutdown-derived model could appear in an active session, which is architecturally incorrect.
Dead code 2 — _build_session_summary_with_meta line 1058
used_config = fp.model is None and fp.tool_model is None
Because fp.model is None is a tautology in this branch, this is functionally equivalent to:
used_config = fp.tool_model is None
The extra fp.model is None and check adds no value and obscures the invariant.
Impact
Future contributors reading these two lines may incorrectly believe that fp.model can carry a non-None value in the no-shutdown path (e.g., from some event other than SESSION_SHUTDOWN). This makes it harder to reason about the model-resolution logic and increases the risk of future bugs.
Files Affected
src/copilot_usage/parser.py — lines 979 and 1058
Spec
1. Simplify line 979 in _build_active_summary
# Before
model = fp.model or fp.tool_model
# After
model = fp.tool_model
2. Simplify line 1058 in _build_session_summary_with_meta
# Before
used_config = fp.model is None and fp.tool_model is None
# After
used_config = fp.tool_model is None
3. (Optional but recommended) Add an assertion documenting the invariant
At the top of _build_active_summary:
assert fp.model is None, (
"_build_active_summary must only be called for sessions without shutdown events; "
"fp.model is only populated by SESSION_SHUTDOWN processing"
)
This turns a silent invariant into a loud, self-documenting check.
Testing Requirements
- Add a unit test in
tests/copilot_usage/test_parser.py that builds a session summary from events that include TOOL_EXECUTION_COMPLETE (setting fp.tool_model) but no SESSION_SHUTDOWN, and asserts that the resulting SessionSummary.model matches the tool model — confirming that the simplification preserves behaviour.
- Verify existing parser tests continue to pass after the change (no regressions).
Generated by Code Health Analysis · ● 3.9M · ◷
Root Cause
_FirstPassResult.modelis set exclusively inside theSESSION_SHUTDOWNbranch of_first_pass(parser.py ~line 726)._build_active_summaryis called only whenfp.all_shutdownsis empty (see_build_session_summary_with_metaline 1045). Thereforefp.modelis invariablyNonewhenever_build_active_summaryexecutes, creating two instances of dead / misleading code:Dead code 1 —
_build_active_summaryline 979Because
fp.modelis alwaysNonehere, this is functionally equivalent to:The
fp.model orprefix implies that a shutdown-derived model could appear in an active session, which is architecturally incorrect.Dead code 2 —
_build_session_summary_with_metaline 1058Because
fp.model is Noneis a tautology in this branch, this is functionally equivalent to:The extra
fp.model is None andcheck adds no value and obscures the invariant.Impact
Future contributors reading these two lines may incorrectly believe that
fp.modelcan carry a non-Nonevalue in the no-shutdown path (e.g., from some event other thanSESSION_SHUTDOWN). This makes it harder to reason about the model-resolution logic and increases the risk of future bugs.Files Affected
src/copilot_usage/parser.py— lines 979 and 1058Spec
1. Simplify line 979 in
_build_active_summary2. Simplify line 1058 in
_build_session_summary_with_meta3. (Optional but recommended) Add an assertion documenting the invariant
At the top of
_build_active_summary:This turns a silent invariant into a loud, self-documenting check.
Testing Requirements
tests/copilot_usage/test_parser.pythat builds a session summary from events that includeTOOL_EXECUTION_COMPLETE(settingfp.tool_model) but noSESSION_SHUTDOWN, and asserts that the resultingSessionSummary.modelmatches the tool model — confirming that the simplification preserves behaviour.