perf: report an unknown model once, not once per priced record (#99) - #120

🤖 Automated first-pass review

TL;DR
The change is small, well-motivated, and correctly scoped: a per-model dedup of the unknown-model fallback warning. The test is a valid red-green regression test. Main merge-readiness gap: a user-visible change (warning behavior) without a CHANGELOG.md entry, plus a possible module-level state concern worth confirming.

Analysis

• Correctness: warnUnknownModelOnce keys the Set on modelName alone, which is the right granularity — the fallback family is a deterministic function of the model name, so the same key can never diverge in family. The comment stating the fallback is deterministic matches the code.
• Ordering/override claim: the PR says a runtime override is "checked before the fallback is reached." I can only see getModelPricing's tail in this diff (the table lookup and the two inferPricingByFamily/SONNET branches). The override-before-fallback ordering is asserted but not shown, so I cannot verify it from the supplied material. If that ordering holds, nothing needs resetting; if an override path can itself funnel into inferPricingByFamily, a previously-warned model could be silently resolved to a fallback without a fresh warning. Please confirm the lookup order in the unchanged part of the function.
• Module-level mutable state: warnedUnknownModels is a process-lifetime Set in src/pricing.ts. That is consistent with "report once," but it also means the set grows with distinct unknown model names and is never cleared for the lifetime of the extension host. For real model ids this is bounded and fine; worth a one-line sanity note rather than a fix.
• Test quality: the test stubs console.warn, restores it in finally, and asserts exact message text via deepEqual. Two distinct models → two warnings is the right assertion shape. Two caveats:
  • It asserts on exact fallback strings (Opus (current tier), Sonnet), which couples the test to inferPricingByFamily output. That is accepted in this repo (observable behavior), but be aware a future family-name change will break this test.
  • console.warn is not shared state the test resets, so concurrent tests are fine. But because warnedUnknownModels persists across tests in the same process, any other test that later prices an unknown model will see it suppressed. 'claude-opus-99-test-once' and 'unlabelled-model-test-once' are unique enough today; keep new unknown-model tests using unique names.
• Convention/CHANGELOG gap: per AGENTS.md ("User-facing changes require a CHANGELOG.md entry and matching documentation") and CONTRIBUTING.md ("If your change affects user-facing behaviour, update CHANGELOG.md..."). This changes user-visible console output, so it needs a CHANGELOG.md entry. No i18n string is involved (console-only, not I18n), so the eight-locale rule does not apply.
• Dependency/branch hygiene: the PR notes it is "independent of perf: a file already outside the window keeps its empty contribution (#99) #119." Confirm the branch is rebased on current main so the reviewer sees only this change. No new runtime dependencies are introduced, consistent with the v2.3.0 policy.

Suggested next step(s)

1. Add a CHANGELOG.md entry under the unreleased/next section describing the behavior change ("unknown-model fallback warning is now emitted once per model per session").
2. Confirm (in the PR description, since the diff doesn't show it) that the runtime override is consulted before inferPricingByFamily/Sonnet fallback, so no stale suppression is possible.
3. Optionally add a one-line comment on warnedUnknownModels noting it is intentionally process-lifetime and never reset.
4. Rebase on main to keep the diff minimal relative to perf: a file already outside the window keeps its empty contribution (#99) #119.

🤖 Generated by DeepSeek as an automated first pass — not a maintainer decision.

Merged. Thank you @rsyuzyov for isolating the unknown-model warning flood and proving the change with a focused regression. The fix keeps runtime pricing overrides authoritative while avoiding repeated console writes for the same missing model. This is merged into main; it is not yet a published extension update.

🤖 Generated with OpenAI Codex