fix(review): address CodeRabbit and Gemini review comments
- Design doc: update "No UI changes" to reflect cost disclaimer
- Plan doc: fix MD001 heading level jump (### after #)
- pricing.ts: rename to PRICING_MAP, add isLiteLLMPricing type guard,
pre-compute LOWERCASE_KEY_MAP for O(1) case-insensitive lookups
- SessionContextHeader: simplify cost disclaimer JSX into single span
- Tests: fix no-empty-function lint (() => {} → () => undefined)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
parent
c507a4f9d5
commit
5ca3fc9008
6 changed files with 42 additions and 36 deletions
|
|
@ -62,7 +62,7 @@ Create a single shared pricing module that both processes import.
|
||||||
|
|
||||||
- Report costs become more accurate (tiered pricing, 206 models instead of 6)
|
- Report costs become more accurate (tiered pricing, 206 models instead of 6)
|
||||||
- Cost numbers between chat view and Session Report now agree exactly
|
- Cost numbers between chat view and Session Report now agree exactly
|
||||||
- No UI changes — same components, same layout
|
- Small UI change: Visible Context header adds a "parent only · view full cost" action when available
|
||||||
|
|
||||||
## Out of Scope
|
## Out of Scope
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -10,6 +10,8 @@
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
## Tasks
|
||||||
|
|
||||||
### Task 1: Create the shared pricing module with tests
|
### Task 1: Create the shared pricing module with tests
|
||||||
|
|
||||||
**Files:**
|
**Files:**
|
||||||
|
|
|
||||||
|
|
@ -134,10 +134,11 @@ export const SessionContextHeader = ({
|
||||||
<span className="font-medium tabular-nums" style={{ color: COLOR_TEXT_SECONDARY }}>
|
<span className="font-medium tabular-nums" style={{ color: COLOR_TEXT_SECONDARY }}>
|
||||||
{formatCostUsd(sessionMetrics.costUsd)}
|
{formatCostUsd(sessionMetrics.costUsd)}
|
||||||
</span>
|
</span>
|
||||||
<span style={{ color: COLOR_TEXT_MUTED }}> (parent only</span>
|
<span style={{ color: COLOR_TEXT_MUTED }}>
|
||||||
{onViewReport ? (
|
{' (parent only'}
|
||||||
<span>
|
{onViewReport && (
|
||||||
<span style={{ color: COLOR_TEXT_MUTED }}> · </span>
|
<>
|
||||||
|
{' · '}
|
||||||
<button
|
<button
|
||||||
onClick={onViewReport}
|
onClick={onViewReport}
|
||||||
className="underline"
|
className="underline"
|
||||||
|
|
@ -145,11 +146,10 @@ export const SessionContextHeader = ({
|
||||||
>
|
>
|
||||||
view full cost
|
view full cost
|
||||||
</button>
|
</button>
|
||||||
<span style={{ color: COLOR_TEXT_MUTED }}>)</span>
|
</>
|
||||||
</span>
|
|
||||||
) : (
|
|
||||||
<span style={{ color: COLOR_TEXT_MUTED }}>)</span>
|
|
||||||
)}
|
)}
|
||||||
|
{')'}
|
||||||
|
</span>
|
||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
</div>
|
</div>
|
||||||
|
|
|
||||||
|
|
@ -22,19 +22,28 @@ export interface DisplayPricing {
|
||||||
|
|
||||||
const TIER_THRESHOLD = 200_000;
|
const TIER_THRESHOLD = 200_000;
|
||||||
|
|
||||||
const pricingMap = pricingData as Record<string, unknown>;
|
const PRICING_MAP = pricingData as Record<string, unknown>;
|
||||||
|
|
||||||
function tryGetPricing(key: string): LiteLLMPricing | null {
|
// Pre-compute lowercase key map for O(1) case-insensitive lookups
|
||||||
const entry = pricingMap[key];
|
const LOWERCASE_KEY_MAP = new Map<string, string>();
|
||||||
if (
|
for (const key of Object.keys(PRICING_MAP)) {
|
||||||
entry &&
|
if (!LOWERCASE_KEY_MAP.has(key.toLowerCase())) {
|
||||||
|
LOWERCASE_KEY_MAP.set(key.toLowerCase(), key);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
function isLiteLLMPricing(entry: unknown): entry is LiteLLMPricing {
|
||||||
|
return (
|
||||||
|
!!entry &&
|
||||||
typeof entry === 'object' &&
|
typeof entry === 'object' &&
|
||||||
'input_cost_per_token' in entry &&
|
'input_cost_per_token' in entry &&
|
||||||
'output_cost_per_token' in entry
|
'output_cost_per_token' in entry
|
||||||
) {
|
);
|
||||||
return entry as LiteLLMPricing;
|
|
||||||
}
|
}
|
||||||
return null;
|
|
||||||
|
function tryGetPricing(key: string): LiteLLMPricing | null {
|
||||||
|
const entry = PRICING_MAP[key];
|
||||||
|
return isLiteLLMPricing(entry) ? entry : null;
|
||||||
}
|
}
|
||||||
|
|
||||||
export function getPricing(modelName: string): LiteLLMPricing | null {
|
export function getPricing(modelName: string): LiteLLMPricing | null {
|
||||||
|
|
@ -42,14 +51,9 @@ export function getPricing(modelName: string): LiteLLMPricing | null {
|
||||||
if (exact) return exact;
|
if (exact) return exact;
|
||||||
|
|
||||||
const lowerName = modelName.toLowerCase();
|
const lowerName = modelName.toLowerCase();
|
||||||
const lower = tryGetPricing(lowerName);
|
const originalKey = LOWERCASE_KEY_MAP.get(lowerName);
|
||||||
if (lower) return lower;
|
if (originalKey) {
|
||||||
|
return tryGetPricing(originalKey);
|
||||||
for (const key of Object.keys(pricingMap)) {
|
|
||||||
if (key.toLowerCase() === lowerName) {
|
|
||||||
const match = tryGetPricing(key);
|
|
||||||
if (match) return match;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
return null;
|
return null;
|
||||||
|
|
|
||||||
|
|
@ -84,7 +84,7 @@ describe('Cost Calculation', () => {
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should return 0 cost when model pricing not found', () => {
|
it('should return 0 cost when model pricing not found', () => {
|
||||||
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
|
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined);
|
||||||
const messages: ParsedMessage[] = [
|
const messages: ParsedMessage[] = [
|
||||||
{
|
{
|
||||||
type: 'assistant',
|
type: 'assistant',
|
||||||
|
|
|
||||||
|
|
@ -55,7 +55,7 @@ describe('Shared Pricing Module', () => {
|
||||||
});
|
});
|
||||||
|
|
||||||
it('should return 0 for unknown models', () => {
|
it('should return 0 for unknown models', () => {
|
||||||
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {});
|
const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => undefined);
|
||||||
const cost = calculateMessageCost('unknown-model', 1000, 500, 0, 0);
|
const cost = calculateMessageCost('unknown-model', 1000, 500, 0, 0);
|
||||||
expect(cost).toBe(0);
|
expect(cost).toBe(0);
|
||||||
expect(warnSpy).toHaveBeenCalledWith(
|
expect(warnSpy).toHaveBeenCalledWith(
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue