Skip to content

[aw][code health] _build_active_summary contains dead code: fp.model is always None in the active-session path #1202

@microsasa

Description

@microsasa

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:

model = fp.tool_model

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

  1. 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.
  2. Verify existing parser tests continue to pass after the change (no regressions).

Generated by Code Health Analysis · ● 3.9M ·

Metadata

Metadata

Assignees

No one assigned

    Labels

    awCreated by agentic workflowaw-dispatchedIssue has been dispatched to implementercode-healthCode cleanup and maintenance

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions