[aw][code health] `ModelMetrics`, `RequestMetrics`, `TokenUsage` are mutable Pydantic models used as internal accumulators — vio [Content truncated due to length]

[microsasa]

Root Cause

ModelMetrics, RequestMetrics, and TokenUsage in src/copilot_usage/models.py are Pydantic BaseModel subclasses with no ConfigDict(frozen=True). They are used purely as internal accumulator structs — not to validate external JSON — making them a violation of the coding guideline:

Pydantic at the Boundary, Plain Python Internally — Internal intermediate state uses frozen dataclasses with slots=True.

The add_to_model_metrics(target, source) function is designed to mutate target in-place (it returns None):

# models.py lines 129–137
def add_to_model_metrics(target: ModelMetrics, source: ModelMetrics) -> None:
    target.requests.count += source.requests.count
    target.requests.cost  += source.requests.cost
    target.usage.input_tokens  += source.usage.input_tokens
    target.usage.output_tokens += source.usage.output_tokens

All call sites use the convention of calling copy_model_metrics(mm) before add_to_model_metrics(copy, mm) so that cached SessionSummary.model_metrics values are never the mutation target. However, this discipline is unenforced — nothing in the type system or runtime prevents a future call site from passing a live cached ModelMetrics as target, which would silently corrupt _SESSION_CACHE entries.

The three affected classes have no Pydantic validators or special Pydantic features — they could be plain frozen dataclasses today with zero semantic change.

Scope

File	Change
src/copilot_usage/models.py	Replace BaseModel with @dataclasses.dataclass(frozen=True, slots=True) for TokenUsage, RequestMetrics, ModelMetrics; change add_to_model_metrics signature to return ModelMetrics instead of mutating None; update copy_model_metrics to use dataclasses.replace
src/copilot_usage/parser.py	Update 2 call sites (parser.py:923, parser.py:925) to capture the return value of add_to_model_metrics
src/copilot_usage/report.py	Update 2 call sites (report.py:272, report.py:274)
src/copilot_usage/models.py	Update merge_session_model_metrics call site (models.py:165, models.py:167)
tests/copilot_usage/test_models.py	Update tests that directly assign copy.requests.count = 999 etc. — these rely on mutable Pydantic fields; change to construct a new instance instead

Spec

1. Replace the three Pydantic BaseModel subclasses with frozen dataclasses:

`@dataclasses`.dataclass(frozen=True, slots=True)
class TokenUsage:
    input_tokens: int = 0
    output_tokens: int = 0

`@dataclasses`.dataclass(frozen=True, slots=True)
class RequestMetrics:
    count: int = 0
    cost: float = 0.0

`@dataclasses`.dataclass(frozen=True, slots=True)
class ModelMetrics:
    requests: RequestMetrics = dataclasses.field(default_factory=RequestMetrics)
    usage: TokenUsage = dataclasses.field(default_factory=TokenUsage)

2. Change add_to_model_metrics to return a new instance:

def add_to_model_metrics(target: ModelMetrics, source: ModelMetrics) -> ModelMetrics:
    return ModelMetrics(
        requests=RequestMetrics(
            count=target.requests.count + source.requests.count,
            cost=target.requests.cost + source.requests.cost,
        ),
        usage=TokenUsage(
            input_tokens=target.usage.input_tokens + source.usage.input_tokens,
            output_tokens=target.usage.output_tokens + source.usage.output_tokens,
        ),
    )

3. Update all 5 call sites to capture the return value, e.g.:

result[model_name] = add_to_model_metrics(result[model_name], mm)

4. Simplify copy_model_metrics — with frozen dataclasses, a copy is just constructing a new instance (or can be omitted in favour of using the frozen instance directly).

Testing Requirement

• All existing TestAddToModelMetrics tests in test_models.py must be updated: tests that currently perform direct attribute mutation (e.g. copy.requests.count = 999) must be rewritten to construct fresh instances.
• Add a regression test that verifies add_to_model_metrics(a, b) does not modify a or b — this was not testable before (mutation was the entire point).
• Verify TestCopyModelMetrics (independence tests) still pass after the dataclass conversion.
• Run the full test suite to confirm no regressions in parser, report, and e2e tests.

Generated by Code Health Analysis · ● 4.8M · ◷