refactor(token): use nullish coalescing - #861
Conversation
![terragon-labs[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
…r token counts Replace all instances of logical OR (||) with nullish coalescing (??) when assigning token count variables in extractTokenUsage function. This ensures that zero values are correctly handled and only null or undefined values fall back to null or zero defaults. Co-authored-by: terragon-labs[bot] <terragon-labs[bot]@users.noreply.github.com>
WalkthroughUpdated extract-token-usage to use nullish coalescing (??) instead of logical OR (||) when assigning token metrics across provider branches (Google AI Studio, Anthropic, OpenAI/default). Adjusted total token calculations to sum nullable components where applicable. No public API/signature changes. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/gateway/src/chat/tools/extract-token-usage.ts (1)
27-42: Bug: totalTokens can be stale when completionTokens is estimated.You compute totalTokens before the estimation branch; if estimation runs, totalTokens doesn’t include the estimated completionTokens.
Apply this diff to compute total after the possible estimation:
- // Calculate total including reasoning tokens - totalTokens = - (promptTokens ?? 0) + - (completionTokens ?? 0) + - (reasoningTokens ?? 0); - // If candidatesTokenCount is missing and we have content, estimate it if (completionTokens === null && fullContent) { const estimation = estimateTokens( provider, [], fullContent, null, null, ); completionTokens = estimation.calculatedCompletionTokens; } + // Calculate total including reasoning tokens (after potential estimation) + totalTokens = + (promptTokens ?? 0) + + (completionTokens ?? 0) + + (reasoningTokens ?? 0);
🧹 Nitpick comments (4)
apps/gateway/src/chat/tools/extract-token-usage.ts (4)
47-51: Anthropic totals: confirm whether to include reasoning and cache tokens.
- totalTokens currently excludes reasoningTokens; for parity with Google’s path you may want to include them.
- cachedTokens only captures cache_read_input_tokens; decide if you also want cache_creation_input_tokens represented.
If inclusion is desired, minimally:
- totalTokens = (promptTokens ?? 0) + (completionTokens ?? 0); + // Include reasoning tokens if present for parity + totalTokens = + (promptTokens ?? 0) + + (completionTokens ?? 0) + + (reasoningTokens ?? 0);If you prefer to expose both cache components without breaking the shape, consider adding an extra optional field (e.g., cacheCreationTokens) in a separate change.
56-61: OpenAI: add a safe fallback when total_tokens is absent.Some responses omit total_tokens; compute it from parts to avoid null totals.
if (data.usage) { promptTokens = data.usage.prompt_tokens ?? null; completionTokens = data.usage.completion_tokens ?? null; totalTokens = data.usage.total_tokens ?? null; reasoningTokens = data.usage.reasoning_tokens ?? null; cachedTokens = data.usage.prompt_tokens_details?.cached_tokens ?? null; + // Fallback if provider omitted total + if (totalTokens == null) { + totalTokens = + (promptTokens ?? 0) + + (completionTokens ?? 0) + + (reasoningTokens ?? 0); + } }
13-17: Add explicit types to avoid implicit any/null widening.Make intent clear and keep strict typing.
- let promptTokens = null; - let completionTokens = null; - let totalTokens = null; - let reasoningTokens = null; - let cachedTokens = null; + let promptTokens: number | null = null; + let completionTokens: number | null = null; + let totalTokens: number | null = null; + let reasoningTokens: number | null = null; + let cachedTokens: number | null = null;
9-11: Avoidanyfor input payload; consider narrowing per provider.Typing
dataas a discriminated union (per provider) or using type guards will prevent silent shape drift.I can draft minimal types for Google AI Studio, Anthropic, and OpenAI usage blocks plus narrowers if you want.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/gateway/src/chat/tools/extract-token-usage.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always use top-level import; never use require() or dynamic imports
Files:
apps/gateway/src/chat/tools/extract-token-usage.ts
{apps/{api,gateway}/src,packages/db}/**/*.ts?(x)
📄 CodeRabbit inference engine (CLAUDE.md)
Use Drizzle ORM with the latest object syntax
Files:
apps/gateway/src/chat/tools/extract-token-usage.ts
apps/{api,gateway}/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
For read queries, use db().query.
.findMany() or db().query..findFirst() Files:
apps/gateway/src/chat/tools/extract-token-usage.ts{apps/api,apps/gateway,apps/ui,apps/docs,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Always use top-level import; never use require() or dynamic imports (e.g., import(), next/dynamic)
Files:
apps/gateway/src/chat/tools/extract-token-usage.ts{apps/api,apps/gateway,packages/db}/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
{apps/api,apps/gateway,packages/db}/**/*.ts: Use Drizzle ORM with the latest object syntax for database access
For reads, use db().query..findMany() or db().query.
.findFirst() Files:
apps/gateway/src/chat/tools/extract-token-usage.ts🔇 Additional comments (1)
apps/gateway/src/chat/tools/extract-token-usage.ts (1)
22-26: Good switch to nullish coalescing — preserves zero-token counts.Using ?? avoids collapsing 0 to null and matches the PR goal. No issues here.
Also applies to: 28-30
1 participant
Summary
extractTokenUsageto use the nullish coalescing operator (??) instead of logical OR (||)Changes
Token Extraction Logic
|| nullwith?? nullto correctly handle zero token counts?? 0instead of|| 0for accurate summationTest plan
🌿 Generated by Terry
ℹ️ Tag @terragon-labs to ask questions and address PR feedback
📎 Task: https://www.terragonlabs.com/task/8462a22d-8a2c-43d2-9427-a73697314921
Summary by CodeRabbit
Bug Fixes
Refactor