feat: Store and display reasoning tokens separately by H2Shami · Pull Request #5429 · Helicone/helicone

H2Shami · 2025-12-18T01:47:10Z

Ticket

Link to the ticket(s) this pull request addresses.

Component/Service

What part of Helicone does this affect?

Type of Change

Testing

Technical Considerations

Database migrations included (if needed)
API changes documented
Breaking changes noted
Performance impact assessed
Security implications reviewed

Dependencies

No external dependencies added
Dependencies added and documented
Environment variables added/modified

Deployment Notes

No special deployment steps required
Database migrations need to run
Environment variable changes required
Coordination with other teams needed

Context

UX + more info

Screenshots / Demos

Before	After

Misc. Review Notes

vercel · 2025-12-18T01:47:15Z

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Review	Updated (UTC)
helicone	Ready	Preview, Comment	Dec 22, 2025 9:59pm
helicone-bifrost	Ready	Preview, Comment	Dec 22, 2025 9:59pm
helicone-eu	Ready	Preview, Comment	Dec 22, 2025 9:59pm

claude · 2025-12-18T01:47:29Z

Claude finished @H2Shami's task —— View job

PR Review: Store and Display Reasoning Tokens Separately

Reviewing this PR for critical issues, code quality, and best practices...

Review Tasks:

Examine database migration and schema changes
Review token calculation logic in backend
Check frontend UI implementation
Assess NULL handling issues from previous reviews
Evaluate test coverage
Check for breaking changes or security issues
Provide final assessment with confidence score

Issues Found:

🚨 CRITICAL: NULL Handling in ClickHouse Queries (Lines 198, 281)

Problem: The total_tokens calculation (prompt_tokens + completion_tokens + reasoning_tokens) will return NULL if any operand is NULL in ClickHouse
Impact: This breaks total token calculations for requests without reasoning tokens
Fix: Use ifNull(reasoning_tokens, 0) in the calculation:

(prompt_tokens + completion_tokens + ifNull(reasoning_tokens, 0)) AS total_tokens

This affects:

/valhalla/jawn/src/lib/stores/request/request.ts:198
/valhalla/jawn/src/lib/stores/request/request.ts:281

⚠️ Schema Issue: Migration Inconsistency

Problem: Migration file schema_78_reasoning_tokens.sql sets DEFAULT 0 but reviewer noted it should not be Nullable
Current: reasoning_tokens Int64 DEFAULT 0 (nullable with default)
Better: reasoning_tokens Int64 NOT NULL DEFAULT 0 (non-nullable with default)
Context: Following pattern from schema_76_size.sql which also uses DEFAULT 0 without explicit NOT NULL

✅ Positive Findings:

Comprehensive test coverage in HandlerContext.test.ts with 328 new lines of tests
Clean separation of reasoning tokens from completion tokens logic
Proper handling in getReasoningTokens() function returns null for missing data
Frontend UI properly handles missing values with "not found" display
Type definitions correctly updated across all packages
No security issues found - reasoning tokens are handled within existing auth context
Breaking changes are minimal and additive (new column, new API field)

Performance & Code Quality:

Good:

Token calculation functions are well-optimized with null checks
Database queries maintain proper indexing patterns
Frontend filter definition properly added to static definitions

Minor Suggestions:

Consider adding JSDoc comments to getReasoningTokens() function explaining the "thinking" token concept
Frontend display could benefit from better formatting for large reasoning token counts

Score: 6/10 - Has one critical NULL handling bug that must be fixed before merge

Suggestions Summary:

MUST FIX: Add ifNull(reasoning_tokens, 0) to total_tokens calculations in request.ts (lines 198, 281)
SHOULD FIX: Consider making reasoning_tokens NOT NULL in migration for consistency
NICE TO HAVE: Add JSDoc documentation for reasoning tokens functions

The implementation is solid overall with good test coverage and proper separation of concerns. The critical NULL handling issue is straightforward to fix and then this will be ready to merge.

Copilot

Pull request overview

This PR adds support for storing and displaying reasoning tokens separately from completion tokens. Reasoning tokens (also known as "thinking" tokens) are used by models like o1 that perform internal reasoning before generating responses. The implementation ensures these tokens are tracked independently for proper cost calculation and analytics.

Key changes:

Added reasoning_tokens field to database schema and API types
Modified token calculation logic to exclude reasoning tokens from completion tokens
Updated total_tokens calculation to include reasoning tokens separately
Added UI column and filter for reasoning tokens in the requests table

Reviewed changes

Copilot reviewed 28 out of 28 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
clickhouse/migrations/schema_77_reasoning_tokens.sql	Adds `reasoning_tokens` column to ClickHouse table
valhalla/jawn/src/lib/handlers/HandlerContext.ts	Implements `getReasoningTokens` function and updates token calculations
valhalla/jawn/src/lib/handlers/LoggingHandler.ts	Integrates reasoning tokens into logging logic
valhalla/jawn/src/lib/stores/request/request.ts	Updates SQL queries to include reasoning_tokens in calculations
valhalla/jawn/src/lib/handlers/tests/HandlerContext.test.ts	Adds comprehensive tests for token calculation functions
web/components/templates/requests/initialColumns.tsx	Adds "Reasoning Tokens" column to requests table
web/filterAST/filterUIDefinitions/staticDefinitions.ts	Adds filter definition for reasoning tokens
packages/llm-mapper/types.ts	Adds reasoning_tokens to type definitions
Multiple API/type files	Updates generated types and schemas to include reasoning_tokens

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-18T01:51:03Z

valhalla/jawn/src/lib/stores/request/request.ts

      ai_gateway_body_mapping,
      time_to_first_token,
-      (prompt_tokens + completion_tokens) AS total_tokens,
+      (prompt_tokens + completion_tokens + reasoning_tokens) AS total_tokens,


The total_tokens calculation may produce incorrect results when reasoning_tokens is NULL. In ClickHouse, NULL + any number = NULL. Consider using COALESCE or ifNull to handle NULL values:
(prompt_tokens + completion_tokens + ifNull(reasoning_tokens, 0)) AS total_tokens

This ensures that when reasoning_tokens is NULL (for models that don't support reasoning tokens), it's treated as 0 rather than making the entire sum NULL.

Suggested change

(prompt_tokens + completion_tokens + reasoning_tokens) AS total_tokens,

(prompt_tokens + completion_tokens + ifNull(reasoning_tokens, 0)) AS total_tokens,

Copilot · 2025-12-18T01:51:03Z

valhalla/jawn/src/lib/stores/request/request.ts

      ai_gateway_body_mapping,
      time_to_first_token,
-      (prompt_tokens + completion_tokens) AS total_tokens,
+      (prompt_tokens + completion_tokens + reasoning_tokens) AS total_tokens,


The total_tokens calculation may produce incorrect results when reasoning_tokens is NULL. In ClickHouse, NULL + any number = NULL. Consider using COALESCE or ifNull to handle NULL values:
(prompt_tokens + completion_tokens + ifNull(reasoning_tokens, 0)) AS total_tokens

This ensures that when reasoning_tokens is NULL (for models that don't support reasoning tokens), it's treated as 0 rather than making the entire sum NULL.

Suggested change

(prompt_tokens + completion_tokens + reasoning_tokens) AS total_tokens,

(prompt_tokens + completion_tokens + ifNull(reasoning_tokens, 0)) AS total_tokens,

…e-reasoning

connortbot · 2025-12-18T19:14:52Z

clickhouse/migrations/schema_78_reasoning_tokens.sql

@@ -0,0 +1,2 @@
+ALTER TABLE request_response_rmt
+ADD COLUMN reasoning_tokens Nullable(Int64) DEFAULT 0;


Should not be Nullable

feat: Store and display reasoning tokens separately

b92a29d

Copilot AI review requested due to automatic review settings December 18, 2025 01:47

vercel bot deployed to Preview – helicone-eu December 18, 2025 01:47 View deployment

Copilot started reviewing on behalf of H2Shami December 18, 2025 01:47 View session

vercel bot deployed to Preview – helicone December 18, 2025 01:48 View deployment

vercel bot deployed to Preview – helicone-bifrost December 18, 2025 01:50 View deployment

Copilot AI reviewed Dec 18, 2025

View reviewed changes

H2Shami added 2 commits December 17, 2025 18:25

Merge branch 'main' of https://github.com/Helicone/helicone into stor…

290f413

…e-reasoning

rename migration file

db9f2d5

vercel bot deployed to Preview – helicone-eu December 18, 2025 02:29 View deployment

vercel bot deployed to Preview – helicone December 18, 2025 02:29 View deployment

vercel bot deployed to Preview – helicone-bifrost December 18, 2025 02:32 View deployment

connortbot reviewed Dec 18, 2025

View reviewed changes

connortbot approved these changes Dec 18, 2025

View reviewed changes

change row to be non nullable

c40ab70

vercel bot deployed to Preview – helicone December 22, 2025 21:56 View deployment

vercel bot deployed to Preview – helicone-eu December 22, 2025 21:56 View deployment

vercel bot deployed to Preview – helicone-bifrost December 22, 2025 21:59 View deployment

H2Shami merged commit 8882351 into main Dec 29, 2025
14 checks passed

H2Shami deleted the store-reasoning branch December 29, 2025 21:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Store and display reasoning tokens separately#5429

feat: Store and display reasoning tokens separately#5429
H2Shami merged 4 commits intomainfrom
store-reasoning

H2Shami commented Dec 18, 2025

Uh oh!

vercel bot commented Dec 18, 2025 •

edited

Loading

Uh oh!

claude bot commented Dec 18, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 18, 2025

Uh oh!

Copilot AI Dec 18, 2025

Uh oh!

connortbot Dec 18, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	(prompt_tokens + completion_tokens + reasoning_tokens) AS total_tokens,
	(prompt_tokens + completion_tokens + ifNull(reasoning_tokens, 0)) AS total_tokens,

		@@ -0,0 +1,2 @@
		ALTER TABLE request_response_rmt
		ADD COLUMN reasoning_tokens Nullable(Int64) DEFAULT 0;

Conversation

H2Shami commented Dec 18, 2025

Ticket

Component/Service

Type of Change

Testing

Technical Considerations

Dependencies

Deployment Notes

Context

Screenshots / Demos

Misc. Review Notes

Uh oh!

vercel bot commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

claude bot commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Review: Store and Display Reasoning Tokens Separately

Review Tasks:

Issues Found:

Performance & Code Quality:

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key changes:

Reviewed changes

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

connortbot Dec 18, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

vercel bot commented Dec 18, 2025 •

edited

Loading

claude bot commented Dec 18, 2025 •

edited

Loading