feat(perf): add TTFB and output rate tracking (#421)#427
Conversation
- Add ttfb_ms field to message_request table for Time To First Byte tracking - Record TTFB at first chunk arrival (streaming) or use durationMs fallback (non-streaming) - Add performance section to ErrorDetailsDialog showing TTFB, duration, and output rate - Update provider leaderboard: replace avgResponseTime with avgTtfbMs and avgTokensPerSecond - Add i18n support for all 5 locales (en, zh-CN, zh-TW, ja, ru) Implementation details: - ProxySession.recordTtfb() method with idempotent design - Gemini passthrough records TTFB at response received time - Output rate calculated as outputTokens / ((durationMs - ttfbMs) / 1000) - Display "-" for null/zero values instead of misleading "0" 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Summary of ChangesHello @ding113, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the application's performance monitoring capabilities by introducing Time To First Byte (TTFB) and output rate tracking. It involves a full-stack implementation, from updating the database schema to store these new metrics, to integrating the logic for their calculation and persistence in the backend, and finally, presenting them in a user-friendly manner within the dashboard's error details and provider leaderboard. These changes provide deeper insights into API request performance, enabling better analysis and optimization. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
github-actions
bot
added
enhancement
🧪 测试结果
总体结果: ✅ 所有测试通过 |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request is a great enhancement, adding valuable performance tracking metrics like Time To First Byte (TTFB) and output rate. The changes are comprehensive, spanning the database schema, backend logic for recording metrics, and UI updates for displaying them. The implementation for handling both streaming and non-streaming requests is solid, and the new additions to the provider leaderboard and request details dialog are very insightful. Overall, this is a high-quality contribution that significantly improves performance monitoring capabilities.
| outputTokens === null || | ||
| outputTokens === undefined || | ||
| outputTokens <= 0 || | ||
| durationMs === null || | ||
| durationMs === undefined || | ||
| ttfbMs === null || | ||
| ttfbMs === undefined || | ||
| ttfbMs >= durationMs |
There was a problem hiding this comment.
The null and undefined checks can be made more concise. Using == null is a common practice in TypeScript to check for both null and undefined at the same time, which improves readability.
outputTokens == null ||
outputTokens <= 0 ||
durationMs == null ||
ttfbMs == null ||
ttfbMs >= durationMs
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Summary
This PR adds comprehensive TTFB (Time To First Byte) and output rate tracking to the performance monitoring capabilities. The implementation is clean, consistent, and follows existing patterns in the codebase.
PR Size: XL
- Lines changed: 2,157 (2,145 additions, 12 deletions)
- Files changed: 19
Note: The large line count is primarily due to the auto-generated Drizzle snapshot file (
0040_snapshot.json~1,951 lines). The actual code changes are approximately 200 lines across 12 source files.
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Analysis Notes
Perspective 1: Comment Analyzer
- Comments are accurate and helpful (e.g.,
// ⭐ 同步记录 TTFB，与首字节超时口径一致) - JSDoc on
recordTtfb()method clearly explains the intended behavior
Perspective 2: Test Analyzer
- No new tests added for the TTFB feature. However, the PR description indicates manual test plan items are present and unit tests pass (30/30)
- The feature is primarily additive (new optional field) and integrates into existing well-tested flows
Perspective 3: Silent Failure Hunter
- TTFB recording uses appropriate fallback:
session.ttfbMs ?? durationensures non-streaming requests get a reasonable value - The
recordTtfb()method is idempotent (returns existing value if already recorded) - All error paths in
response-handler.tsproperly includettfbMsin persistence
Perspective 4: Type Design Auditor
ttfbMsis correctly typed asnumber | nullthroughout the codebase- Type consistency maintained across schema, repository, and UI layers
- No unsafe
anyusage introduced
Perspective 5: General Code Reviewer
- Logic: The TTFB calculation
Math.max(0, Date.now() - this.startTime)correctly handles edge cases - Security: No concerns - no user input handling changes
- Performance: The
avgTokensPerSecondSQL query uses appropriateCASE WHENguards to avoid division by zero - Standards: Follows existing patterns (field naming, i18n structure, repository patterns)
Perspective 6: Code Simplifier
- The
outputTokensPerSecondcalculation inerror-details-dialog.tsxis appropriately defensive with clear null/edge case handling - The complex conditional
(ttfbMs >= durationMs)guard prevents invalid rate calculations
Validated Design Decisions
- TTFB Fallback for Non-Streaming: Using
durationMsas fallback for non-streaming requests is correct since there's no "first byte" distinction - Gemini Passthrough: TTFB is recorded when
clearResponseTimeoutis called for Gemini passthrough, maintaining consistency - SQL Aggregation: The
avgTokensPerSecondquery correctly excludes invalid cases (zero output tokens, null ttfb, ttfb >= duration)
Review Coverage
- Logic and correctness - Clean
- Security (OWASP Top 10) - Clean
- Error handling - Clean
- Type safety - Clean
- Documentation accuracy - Clean
- Test coverage - Manual test plan present
- Code clarity - Good
Automated review by Claude AI
| if (sessionWithCleanup.clearResponseTimeout) { | ||
| sessionWithCleanup.clearResponseTimeout(); | ||
| // ⭐ 同步记录 TTFB，与首字节超时口径一致 | ||
| session.recordTtfb(); |
There was a problem hiding this comment.
[HIGH] [LOGIC-BUG] Gemini passthrough records ttfbMs before the first body chunk
Why this is a problem: ProxySession.recordTtfb() is documented as "Definition: first body chunk received." (src/app/v1/_lib/proxy/session.ts:249), but Gemini passthrough calls it as soon as the Response exists (src/app/v1/_lib/proxy/response-handler.ts:578). This makes ttfbMs inconsistent across streaming paths and skews avgTokensPerSecond (it uses durationMs - ttfbMs).
Suggested fix:
// Keep clearing the first-byte timeout here…
sessionWithCleanup.clearResponseTimeout();
// …but don't record TTFB until the first body chunk (already done below in the reader loop).
// session.recordTtfb(); // remove
There was a problem hiding this comment.
Code Review Summary
This PR is XL and mixes database/schema changes, proxy instrumentation, analytics queries, UI updates, and i18n, which makes review and rollback harder. One high-priority correctness issue was found that can skew the new performance metrics.
PR Size: XL
- Lines changed: 2157
- Files changed: 19
- Split suggestion: separate into (1) DB migration/schema/types, (2) proxy TTFB instrumentation, (3) dashboard/leaderboard UI + i18n.
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 1 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
None.
High Priority Issues (Should Fix)
- [HIGH] [LOGIC-BUG] Gemini passthrough records
ttfbMsbefore the first body chunk (src/app/v1/_lib/proxy/response-handler.ts:578).
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Codex AI
There was a problem hiding this comment.
- Reviewed PR
#427(feat(perf): add TTFB and output rate tracking (#421)), applied labelsize/XL. - Left 1 inline comment:
src/app/v1/_lib/proxy/response-handler.ts:578— Gemini passthrough recordsttfbMsbefore first body chunk, which can skew the new perf metrics; included a concrete code change suggestion. - Submitted the full review summary via
gh pr review.
github-project-automation
bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
1 participant
Summary
ttfb_msfield tomessage_requesttable for Time To First Byte trackingChanges
src/drizzle/schema.ts,drizzle/0040_bored_venus.sqlsrc/types/message.tssrc/repository/message.ts,src/repository/leaderboard.ts,src/repository/usage-logs.tssrc/app/v1/_lib/proxy/session.ts,src/app/v1/_lib/proxy/response-handler.tsleaderboard-view.tsx,error-details-dialog.tsx,usage-logs-table.tsx,virtualized-logs-table.tsxmessages/{en,zh-CN,zh-TW,ja,ru}/dashboard.jsonTest plan
bun run typecheckpassesbun run lintpassesbun run test -- tests/unitpasses (30/30)bun run db:migrateon target environmentCloses #421
🤖 Generated with Claude Code