PR #22 Review Notes
Structured review feedback for the multi-provider support PR:
(a) Missing unit tests for new provider files
New provider parsing/normalization files lack unit tests. Each provider's model name mapping, token counting, and cost calculation should have dedicated test coverage.
(b) mergeEntries cost distribution bug
When mixing providers with and without per-model cost breakdowns, the merge logic may incorrectly distribute costs. If provider A has detailed model_breakdown and provider B doesn't, the merged entry's breakdown will be incomplete.
(c) Model prettification not updated in all locations
post-share.ts and recap.ts need their model name display logic updated to handle new provider model names (Gemini, Qwen, Mistral, etc.). Currently they only know about Claude and OpenAI models.
(d) ModelBreakdownEntry missing provider field
When two providers use models with the same name (unlikely but possible), the breakdown entries can't be disambiguated. Adding an optional provider field to ModelBreakdownEntry would future-proof the type.
Priority
High — this PR introduces a significant feature that needs thorough review before merge.
PR #22 Review Notes
Structured review feedback for the multi-provider support PR:
(a) Missing unit tests for new provider files
New provider parsing/normalization files lack unit tests. Each provider's model name mapping, token counting, and cost calculation should have dedicated test coverage.
(b) mergeEntries cost distribution bug
When mixing providers with and without per-model cost breakdowns, the merge logic may incorrectly distribute costs. If provider A has detailed model_breakdown and provider B doesn't, the merged entry's breakdown will be incomplete.
(c) Model prettification not updated in all locations
post-share.ts and recap.ts need their model name display logic updated to handle new provider model names (Gemini, Qwen, Mistral, etc.). Currently they only know about Claude and OpenAI models.
(d) ModelBreakdownEntry missing provider field
When two providers use models with the same name (unlikely but possible), the breakdown entries can't be disambiguated. Adding an optional provider field to ModelBreakdownEntry would future-proof the type.
Priority
High — this PR introduces a significant feature that needs thorough review before merge.