fix(metrics): 拆分 TTFB 与 TFFT 双指标，TPS 改用真 TTFB 计算 - #1362
Conversation
These files predate the locally installed biome version and were reformatted by `bun run lint:fix`. Split out from the TTFB/TFFT change so that diff stays reviewable. No behavioral change. Co-Authored-By: Claude <noreply@anthropic.com>
The stream content gate made the existing ttfb_ms column measure time to first content token (TFFT), not time to first byte (TTFB). Using TFFT as the generation-window start for TPS excluded upstream queuing and neutral frames, systematically inflating throughput numbers. Add first_byte_ms column to message_request and usage_ledger. The proxy session now records true TTFB from the stream gate first-byte callback (committed only for the winning attempt) alongside TFFT from the first chunk handed to the response handler. TPS, output rate, and leaderboard throughput calculations now use firstByteMs as the denominator start. Rows persisted before the gate shipped have NULL first_byte_ms and return null instead of falling back to total duration. Dashboard and status page labels renamed from TTFB to TFFT; the latency breakdown bar gains a three-segment view (TTFB, token wait, generation).
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
📝 WalkthroughWalkthrough本次变更将单一 Changes计时指标迁移
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d1caa4dcb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const tps = computeTokensPerSecond({ | ||
| outputTokens: input.event.outputTokens, | ||
| durationMs: input.event.durationMs, | ||
| ttfbMs, | ||
| firstByteMs: normalizeNumber(input.event.firstByteMs), | ||
| }); |
There was a problem hiding this comment.
On deployments with existing public-status Redis rollups, this starts calculating TPS from firstByteMs but continues incrementing and reading the existing tps_sum/tps_count fields, whose stored samples were calculated from the legacy TFFT basis. Consequently, current buckets can mix both formulas and older buckets continue reporting the old metric for the configured status window. Version or clear/rebuild the TPS rollup fields when introducing the new calculation.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/lib/langfuse/trace-proxy-request.ts (1)
189-200: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win将
tokenGenerationMs的生成窗口起点改为firstByteMs，与输出速率计算口径保持一致。Langfuse 里已经同时记录
tfftMs和firstByteMs，但其他生成窗口计算（src/lib/utils/performance-formatter.ts、src/repository/leaderboard.ts）都已改用真实 TTFB 作为起点，避免因内容门控让 TFFT 滞后而抬高速率/缩短生成时间。这里用durationMs - session.tfftMs会保留同样的口径偏差。♻️ 可能的修改
- tokenGenerationMs: session.tfftMs != null ? Math.max(0, durationMs - session.tfftMs) : null, + tokenGenerationMs: + session.firstByteMs != null ? Math.max(0, durationMs - session.firstByteMs) : null,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/langfuse/trace-proxy-request.ts` around lines 189 - 200, Update the tokenGenerationMs calculation in the timingBreakdown object to use session.firstByteMs as the generation-window start, preserving the existing non-negative clamp and null behavior. Leave tfftFromForwardMs based on session.tfftMs unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/app/`[locale]/dashboard/logs/_components/error-details-dialog/components/LatencyBreakdownBar.tsx:
- Around line 46-64: Update the TTFB segment handling in LatencyBreakdownBar so
a missing firstByteMs does not display the fallback tfftMs value with the “TTFB”
label. Prefer hiding that segment, matching PerformanceTab’s treatment of
missing performance.ttfb data; otherwise use a neutral label for the fallback
and ensure the displayed segments remain consistent with the actual metrics.
In `@src/repository/message.ts`:
- Around line 1032-1033: 在 findMessageRequestBySessionId 的主 messageRequest
查询中补充投影 usageLedger.tfftMs 和 usageLedger.firstByteMs
对应的两个时延字段，确保命中主表提前返回时也携带这些指标；保持 ledger fallback 的现有行为不变。
In `@tests/unit/public-status/aggregation.test.ts`:
- Around line 37-38: Update the latency fixtures and assertions across
tests/unit/public-status/aggregation.test.ts (37-38, 55-56, 106-107, 232-233)
and tests/unit/public-status/rollup-store.test.ts (45-46, 153-154, 230-231,
315-316, 353-354) so tfftMs and firstByteMs use distinct, contract-consistent
values. Add or adjust success assertions to verify TFFT and TPS independently,
including fallback cases, while keeping failure and empty-result fixtures
complete and ensuring rollup assertions distinguish ttfb_* from tps_* sources.
In `@tests/unit/repository/message-public-status-rollup.test.ts`:
- Line 443: 补齐 firstByteMs 的测试覆盖：在
tests/unit/repository/message-public-status-rollup.test.ts 的
443-443、492-492、580-580、664-664、753-753 行相关 fixture 中加入非空 firstByteMs，并在
472-472、516-516、607-607、779-779 行断言对应 rollup、fallback、retry 和配置不可用事件保留该字段；在
tests/unit/repository/message-public-readback.test.ts 的 64-64、108-108 行以及
tests/unit/repository/message-session-readback.test.ts 的 104-104 行，为
MESSAGE_ROW、LEDGER_ROW 和 session ledger fixture 加入并断言 firstByteMs，确保 readback 与
rollup 链路完整传递 timing payload。
---
Outside diff comments:
In `@src/lib/langfuse/trace-proxy-request.ts`:
- Around line 189-200: Update the tokenGenerationMs calculation in the
timingBreakdown object to use session.firstByteMs as the generation-window
start, preserving the existing non-negative clamp and null behavior. Leave
tfftFromForwardMs based on session.tfftMs unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 4274d6a8-3c2a-45c6-8f2c-9dbf0d14f0b0
📒 Files selected for processing (68)
drizzle/0114_overconfident_ronan.sqldrizzle/meta/0114_snapshot.jsondrizzle/meta/_journal.jsonmessages/en/dashboard.jsonmessages/en/settings/statusPage.jsonmessages/ja/dashboard.jsonmessages/ja/settings/statusPage.jsonmessages/ru/dashboard.jsonmessages/ru/settings/statusPage.jsonmessages/zh-CN/dashboard.jsonmessages/zh-CN/settings/statusPage.jsonmessages/zh-TW/dashboard.jsonmessages/zh-TW/settings/statusPage.jsonsrc/app/[locale]/dashboard/logs/_components/error-details-dialog.test.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog/components/LatencyBreakdownBar.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog/components/PerformanceTab.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog/components/SummaryTab.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog/index.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog/types.tssrc/app/[locale]/dashboard/logs/_components/usage-logs-table.test.tsxsrc/app/[locale]/dashboard/logs/_components/usage-logs-table.tsxsrc/app/[locale]/dashboard/logs/_components/virtualized-logs-table.test.tsxsrc/app/[locale]/dashboard/logs/_components/virtualized-logs-table.tsxsrc/app/v1/_lib/proxy/forwarder.tssrc/app/v1/_lib/proxy/response-handler.tssrc/app/v1/_lib/proxy/session.tssrc/app/v1/_lib/proxy/warmup-guard.tssrc/drizzle/schema.tssrc/lib/langfuse/emit-proxy-trace.tssrc/lib/langfuse/trace-proxy-request.tssrc/lib/ledger-backfill/service.tssrc/lib/ledger-backfill/trigger.sqlsrc/lib/public-status/aggregation-core.tssrc/lib/public-status/aggregation.tssrc/lib/public-status/rollup-store.tssrc/lib/utils/performance-formatter.test.tssrc/lib/utils/performance-formatter.tssrc/repository/leaderboard.tssrc/repository/message-write-buffer.tssrc/repository/message.tssrc/repository/usage-logs.tssrc/types/message.tstests/unit/dashboard-logs-virtualized-special-settings-ui.test.tsxtests/unit/dashboard-logs-warmup-ui.test.tsxtests/unit/error-details-dialog-warmup-ui.test.tsxtests/unit/langfuse/langfuse-trace.test.tstests/unit/proxy/response-handler-abort-listener-cleanup.test.tstests/unit/proxy/response-handler-client-abort-drain.test.tstests/unit/proxy/response-handler-endpoint-circuit-isolation.test.tstests/unit/proxy/response-handler-lease-decrement.test.tstests/unit/proxy/response-handler-non200.test.tstests/unit/proxy/session-ttfb-tfft.test.tstests/unit/public-status/aggregation-core-tps.test.tstests/unit/public-status/aggregation.test.tstests/unit/public-status/rollup-store.test.tstests/unit/repository/leaderboard-provider-metrics.test.tstests/unit/repository/leaderboard-timezone-parentheses.test.tstests/unit/repository/leaderboard-tps-basis.test.tstests/unit/repository/leaderboard-user-model-stats.test.tstests/unit/repository/message-public-readback.test.tstests/unit/repository/message-public-status-rollup.test.tstests/unit/repository/message-session-readback.test.tstests/unit/repository/message-terminal-public-status-seam.test.tstests/unit/repository/message-terminal-write-apis.test.tstests/unit/repository/message-usage-logs-query.test.tstests/unit/repository/message-write-buffer.test.tstests/unit/repository/usage-logs-actual-response-model.test.tstests/unit/repository/usage-logs-sessionid-filter.test.ts
| // 历史行没有真 TTFB：首段退化为整个 TFFT，中间段消失 | ||
| const ttfbMs = | ||
| firstByteMs !== null && firstByteMs >= 0 && firstByteMs <= tfftMs ? firstByteMs : tfftMs; | ||
| const tokenWaitMs = tfftMs - ttfbMs; | ||
| const generationMs = durationMs - tfftMs; | ||
|
|
||
| const percent = (ms: number) => (ms / durationMs) * 100; | ||
| // Minimum width for visibility (3%) | ||
| const minWidth = 3; | ||
| const adjustedTtfbPercent = Math.max(ttfbPercent, ttfbMs > 0 ? minWidth : 0); | ||
| const adjustedGenerationPercent = Math.max(generationPercent, generationMs > 0 ? minWidth : 0); | ||
| const width = (ms: number) => Math.max(percent(ms), ms > 0 ? minWidth : 0); | ||
|
|
||
| const segments = [ | ||
| { | ||
| key: "ttfb", | ||
| ms: ttfbMs, | ||
| label: t("segmentTtfb"), | ||
| barClass: "bg-blue-500", | ||
| dotClass: "bg-blue-500", | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
历史行的“TTFB”分段标签具有误导性。
当 firstByteMs 为空（历史行）时，ttfbMs 回退为整个 tfftMs，但该分段仍固定使用 t("segmentTtfb")（“TTFB”）标签展示，等于把完整的 TFFT 时长打上“TTFB”标签。同一个 Tab 内，PerformanceTab.tsx 的详细指标表对相同缺失数据的处理方式是直接隐藏 performance.ttfb 行，而不是用错误标签展示回退值，两处行为不一致，容易让用户误以为该值就是真实首字节时间。
💡 建议修复：回退场景下改用中性标签或隐藏该分段
const segments = [
{
key: "ttfb",
ms: ttfbMs,
- label: t("segmentTtfb"),
+ label: firstByteMs !== null && firstByteMs <= tfftMs ? t("segmentTtfb") : t("segmentTfft"),
barClass: "bg-blue-500",
dotClass: "bg-blue-500",
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // 历史行没有真 TTFB：首段退化为整个 TFFT，中间段消失 | |
| const ttfbMs = | |
| firstByteMs !== null && firstByteMs >= 0 && firstByteMs <= tfftMs ? firstByteMs : tfftMs; | |
| const tokenWaitMs = tfftMs - ttfbMs; | |
| const generationMs = durationMs - tfftMs; | |
| const percent = (ms: number) => (ms / durationMs) * 100; | |
| // Minimum width for visibility (3%) | |
| const minWidth = 3; | |
| const adjustedTtfbPercent = Math.max(ttfbPercent, ttfbMs > 0 ? minWidth : 0); | |
| const adjustedGenerationPercent = Math.max(generationPercent, generationMs > 0 ? minWidth : 0); | |
| const width = (ms: number) => Math.max(percent(ms), ms > 0 ? minWidth : 0); | |
| const segments = [ | |
| { | |
| key: "ttfb", | |
| ms: ttfbMs, | |
| label: t("segmentTtfb"), | |
| barClass: "bg-blue-500", | |
| dotClass: "bg-blue-500", | |
| }, | |
| // 历史行没有真 TTFB：首段退化为整个 TFFT，中间段消失 | |
| const ttfbMs = | |
| firstByteMs !== null && firstByteMs >= 0 && firstByteMs <= tfftMs ? firstByteMs : tfftMs; | |
| const tokenWaitMs = tfftMs - ttfbMs; | |
| const generationMs = durationMs - tfftMs; | |
| const percent = (ms: number) => (ms / durationMs) * 100; | |
| // Minimum width for visibility (3%) | |
| const minWidth = 3; | |
| const width = (ms: number) => Math.max(percent(ms), ms > 0 ? minWidth : 0); | |
| const segments = [ | |
| { | |
| key: "ttfb", | |
| ms: ttfbMs, | |
| label: firstByteMs !== null && firstByteMs <= tfftMs ? t("segmentTtfb") : t("segmentTfft"), | |
| barClass: "bg-blue-500", | |
| dotClass: "bg-blue-500", | |
| }, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/app/`[locale]/dashboard/logs/_components/error-details-dialog/components/LatencyBreakdownBar.tsx
around lines 46 - 64, Update the TTFB segment handling in LatencyBreakdownBar so
a missing firstByteMs does not display the fallback tfftMs value with the “TTFB”
label. Prefer hiding that segment, matching PerformanceTab’s treatment of
missing performance.ttfb data; otherwise use a neutral label for the fallback
and ensure the displayed segments remain consistent with the actual metrics.
| tfftMs: usageLedger.tfftMs, | ||
| firstByteMs: usageLedger.firstByteMs, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
补齐主查询的两项时延字段。
findMessageRequestBySessionId 的主 messageRequest 查询（第 965-1000 行）未选择 tfftMs 和 firstByteMs，命中主表时会提前返回，导致按 session 查询的日志缺失这两项指标；只有 ledger fallback 才能返回它们。请在主查询中同时投影这两个字段。
建议修改
originalModel: messageRequest.originalModel,
durationMs: messageRequest.durationMs,
+ tfftMs: messageRequest.tfftMs,
+ firstByteMs: messageRequest.firstByteMs,
costUsd: messageRequest.costUsd,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/repository/message.ts` around lines 1032 - 1033, 在
findMessageRequestBySessionId 的主 messageRequest 查询中补充投影 usageLedger.tfftMs 和
usageLedger.firstByteMs 对应的两个时延字段，确保命中主表提前返回时也携带这些指标；保持 ledger fallback 的现有行为不变。
| tfftMs: 200, | ||
| firstByteMs: 200, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
统一使用不同值验证 TFFT/TTFB 分离。
这些跨文件夹具都让 tfftMs 与 firstByteMs 相等，无法验证聚合和 rollup 是否从正确字段计算各自指标。
- tests/unit/public-status/aggregation.test.ts#L37-L38：至少加入一个不同值的成功请求，并断言 TFFT 与 TPS。
- tests/unit/public-status/aggregation.test.ts#L55-L56：避免失败请求夹具继续掩盖字段交换问题。
- tests/unit/public-status/aggregation.test.ts#L106-L107：保持失败场景的延迟字段与新契约一致。
- tests/unit/public-status/aggregation.test.ts#L232-L233：使用不同值并断言成功 fallback 的 TFFT 与 TPS。
- tests/unit/public-status/rollup-store.test.ts#L45-L46：使用不同值验证
ttfb_*与tps_*来源。 - tests/unit/public-status/rollup-store.test.ts#L153-L154：为成功 group 的延迟和吞吐断言提供可区分输入。
- tests/unit/public-status/rollup-store.test.ts#L230-L231：避免 hash/bucket 场景掩盖指标字段混用。
- tests/unit/public-status/rollup-store.test.ts#L315-L316：为 pipeline 失败场景保持完整的新字段夹具。
- tests/unit/public-status/rollup-store.test.ts#L353-L354：为无结果场景保持完整的新字段夹具。
📍 Affects 2 files
tests/unit/public-status/aggregation.test.ts#L37-L38(this comment)tests/unit/public-status/aggregation.test.ts#L55-L56tests/unit/public-status/aggregation.test.ts#L106-L107tests/unit/public-status/aggregation.test.ts#L232-L233tests/unit/public-status/rollup-store.test.ts#L45-L46tests/unit/public-status/rollup-store.test.ts#L153-L154tests/unit/public-status/rollup-store.test.ts#L230-L231tests/unit/public-status/rollup-store.test.ts#L315-L316tests/unit/public-status/rollup-store.test.ts#L353-L354
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/unit/public-status/aggregation.test.ts` around lines 37 - 38, Update
the latency fixtures and assertions across
tests/unit/public-status/aggregation.test.ts (37-38, 55-56, 106-107, 232-233)
and tests/unit/public-status/rollup-store.test.ts (45-46, 153-154, 230-231,
315-316, 353-354) so tfftMs and firstByteMs use distinct, contract-consistent
values. Add or adjust success assertions to verify TFFT and TPS independently,
including fallback cases, while keeping failure and empty-result fixtures
complete and ensuring rollup assertions distinguish ttfb_* from tps_* sources.
| const finalDetails = { | ||
| statusCode: 200, | ||
| ttfbMs: 200, | ||
| tfftMs: 200, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
统一补齐 firstByteMs 的测试覆盖。
这些迁移只更新了 tfftMs，却没有验证新增的 firstByteMs 是否沿 readback 和 rollup 链路传递。
- tests/unit/repository/message-public-status-rollup.test.ts#L443-L443：在成功终态
finalDetails中加入非空firstByteMs。 - tests/unit/repository/message-public-status-rollup.test.ts#L472-L472：断言 rollup event 包含该
firstByteMs。 - tests/unit/repository/message-public-status-rollup.test.ts#L492-L492：为 persisted seed fallback 加入
firstByteMs。 - tests/unit/repository/message-public-status-rollup.test.ts#L516-L516：断言 fallback event 保留
firstByteMs。 - tests/unit/repository/message-public-status-rollup.test.ts#L580-L580：为 retryable seed 场景加入
firstByteMs。 - tests/unit/repository/message-public-status-rollup.test.ts#L607-L607：断言重试写入事件包含该字段。
- tests/unit/repository/message-public-status-rollup.test.ts#L664-L664：为非 public-status projection 场景保持完整 timing payload。
- tests/unit/repository/message-public-status-rollup.test.ts#L753-L753：为配置暂不可用场景加入
firstByteMs。 - tests/unit/repository/message-public-status-rollup.test.ts#L779-L779：断言该场景的 event 传递
firstByteMs。 - tests/unit/repository/message-public-readback.test.ts#L64-L64：在
MESSAGE_ROW中加入并断言firstByteMs。 - tests/unit/repository/message-public-readback.test.ts#L108-L108：在
LEDGER_ROW中加入并断言firstByteMs。 - tests/unit/repository/message-session-readback.test.ts#L104-L104：在 session ledger fixture 中加入并断言
firstByteMs。
📍 Affects 3 files
tests/unit/repository/message-public-status-rollup.test.ts#L443-L443(this comment)tests/unit/repository/message-public-status-rollup.test.ts#L472-L472tests/unit/repository/message-public-status-rollup.test.ts#L492-L492tests/unit/repository/message-public-status-rollup.test.ts#L516-L516tests/unit/repository/message-public-status-rollup.test.ts#L580-L580tests/unit/repository/message-public-status-rollup.test.ts#L607-L607tests/unit/repository/message-public-status-rollup.test.ts#L664-L664tests/unit/repository/message-public-status-rollup.test.ts#L753-L753tests/unit/repository/message-public-status-rollup.test.ts#L779-L779tests/unit/repository/message-public-readback.test.ts#L64-L64tests/unit/repository/message-public-readback.test.ts#L108-L108tests/unit/repository/message-session-readback.test.ts#L104-L104
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/unit/repository/message-public-status-rollup.test.ts` at line 443, 补齐
firstByteMs 的测试覆盖：在 tests/unit/repository/message-public-status-rollup.test.ts 的
443-443、492-492、580-580、664-664、753-753 行相关 fixture 中加入非空 firstByteMs，并在
472-472、516-516、607-607、779-779 行断言对应 rollup、fallback、retry 和配置不可用事件保留该字段；在
tests/unit/repository/message-public-readback.test.ts 的 64-64、108-108 行以及
tests/unit/repository/message-session-readback.test.ts 的 104-104 行，为
MESSAGE_ROW、LEDGER_ROW 和 session ledger fixture 加入并断言 firstByteMs，确保 readback 与
rollup 链路完整传递 timing payload。
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| if (isFirstChunk) { | ||
| session.recordTtfb(); | ||
| session.recordTfft(); |
There was a problem hiding this comment.
[HIGH] [LOGIC-BUG] Ungated streams still record tfftMs on the first chunk\n\nWhy this is a problem: In shadow/off/raw_passthrough paths, the first chunk can still be a neutral SSE frame rather than the first token. The surrounding shadow-gate observer exists because first byte and first decisive content can diverge, so this line will under-report TFFT anywhere the gate is bypassed while the UI/logs now label the value as TFFT.\n\nSuggested fix:\nts\nconst decisive = shadowGateObserver?.observe(value);\nif (isFirstChunk) {\n session.recordFirstByte(Date.now());\n isFirstChunk = false;\n}\nif (decisive === "content") {\n session.recordTfft();\n}\n\nIf bypassed modes cannot surface the first content frame, leave tfftMs unset there instead of persisting first-chunk timing as TFFT.
| outputTokens: normalizedUsage.output_tokens, | ||
| ttfbMs: session.ttfbMs ?? duration, | ||
| tfftMs: session.tfftMs ?? duration, | ||
| firstByteMs: session.firstByteMs ?? duration, |
There was a problem hiding this comment.
[HIGH] [LOGIC-BUG] Non-stream paths persist firstByteMs as total duration\n\nWhy this is a problem: There is no non-stream recordFirstByte() path in this PR, so session.firstByteMs ?? duration serializes every JSON/non-stream response as TTFB == durationMs. That is not the true first-byte latency this PR is trying to expose, and it also collapses any generation-window math that depends on firstByteMs.\n\nSuggested fix:\nts\nconst terminalDetails = {\n // ...\n firstByteMs: session.firstByteMs,\n};\n\nThen record the real first byte earlier (for example when the chosen fetch() call resolves and response headers arrive). If a path cannot observe true TTFB, persist null instead of duration. The same ?? duration fallback is duplicated in the other terminal-details builders in this file.
There was a problem hiding this comment.
Code Review Summary
This PR is large and the TTFB/TFFT split is wired through most layers consistently, but two high-confidence timing regressions remain in the new proxy capture path. As written, bypassed streams can still report first-chunk time as TFFT, and non-stream responses can still persist firstByteMs as total duration, so the renamed metrics are still incorrect on those paths.
PR Size: XL
- Lines changed: 6584
- Files changed: 68
- Split suggestions: Separate this into (1) schema/ledger migration, (2) proxy timing capture semantics, and (3) dashboard/i18n/test updates to make the behavior change reviewable and reduce regression surface.
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 2 | 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)
src/app/v1/_lib/proxy/response-handler.ts:4573— bypassed streaming paths still callrecordTfft()on the first chunk, so neutral SSE prefix frames are now mislabeled as TFFT.src/app/v1/_lib/proxy/response-handler.ts:6183— non-stream terminal persistence still falls back tofirstByteMs ?? duration, which serializesTTFB == total durationinstead of true first-byte latency.
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.
Review Posted
- Applied the
size/XLlabel to PR#1362and submitted the required summary review. - Filed 2 high-priority inline comments:
src/app/v1/_lib/proxy/response-handler.ts:4573— bypassed streaming paths still record TFFT on the first chunk, so neutral SSE prefix frames can be mislabeled as first token.src/app/v1/_lib/proxy/response-handler.ts:6183— non-stream terminal persistence still falls back tofirstByteMs ?? duration, so TTFB can be stored as total duration.
- Included XL split suggestions in the summary: schema/ledger migration, proxy timing capture, and dashboard/i18n/test updates.
- No additional findings survived the confidence and full-context validation pass.
Record TFFT when valid Responses content commits through the stream gate across sequential, legacy hedge, and Discovery transport paths. Previously TFFT was not captured at the gate-commit boundary, conflating first-byte and first-token timing. commitWinner now receives a contentGateCommitted flag so TFFT is recorded only when the content gate has committed the winner, not when a raw first chunk is forwarded. Winner TTFB remains distinct and is preserved at its original recording point. Integration tests verify TFFT and TTFB stay separate before any downstream read across sequential and first-byte hedge paths, and under Discovery winner selection.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/v1/_lib/proxy/forwarder.ts (1)
1716-1721: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win清除首字节计时器改变了门控超时语义，上方注释已过时。
onFirstByte现在在收到任意字节时就调用runtime.clearResponseTimeout?.()，因此 1745-1761 的「首个有效内容超时」（524）实际上只在上游一个字节都没发时才会触发；持续输出中性帧但迟迟不给内容的供应商将由idleTimeoutMs而非首字节计时器兜底。但 1688-1690 处的说明仍写着「首字节计时器（doForward 设置，response-handler 读到首字节才清除）在门控期间继续生效，天然升级为『首个有效内容超时』」，与新行为直接矛盾。建议同步更新该注释，明确当前兜底责任已交给静默超时。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/v1/_lib/proxy/forwarder.ts` around lines 1716 - 1721, 更新 forwarder 中描述首字节计时器与门控超时语义的注释，使其反映 onFirstByte 会在任意字节到达时调用 runtime.clearResponseTimeout?.()；明确持续输出中性帧时首个有效内容超时不再由该计时器兜底，而由 idleTimeoutMs 的静默超时负责。不要修改现有运行逻辑。
🧹 Nitpick comments (1)
tests/integration/proxy-hedge-lifecycle.test.ts (1)
109-114: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value建议以真实默认设置为基底再覆盖字段。
...actual只展开模块导出，返回的设置对象仅含这 4 个键；如果被测路径读取其它运行时设置项会拿到undefined，且未来新增字段时该 mock 会静默漂移。可考虑在返回值中先展开actual.getCachedProxyRuntimeSettings()（或模块导出的默认值常量）再覆盖streamGateMode。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/integration/proxy-hedge-lifecycle.test.ts` around lines 109 - 114, 更新 getCachedProxyRuntimeSettings mock，以真实的默认运行时设置作为返回值基础，再覆盖测试所需的 affinityIgnoreClientSessionId、cacheEffectivenessEnabled、replayEnabled 和 state.streamGateMode。优先调用实际的 getCachedProxyRuntimeSettings 或使用模块导出的默认设置，避免遗漏新增配置字段。
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/app/v1/_lib/proxy/forwarder.ts`:
- Around line 1765-1768: 补齐非门控路径的 first-byte 采集，避免 session.recordFirstByte
未被调用。更新 src/app/v1/_lib/proxy/forwarder.ts:1765-1768，在 response-handler 观察到首块时补充
first-byte 记录；同时更新 src/app/v1/_lib/proxy/forwarder.ts:4765-4778，在
readFirstReadableChunk 获取首个非空块后设置 attempt.firstByteAt（仅在未设置时），使 commitWinner
能记录该指标，并保持 TFFT 的现有兜底逻辑不变。
---
Outside diff comments:
In `@src/app/v1/_lib/proxy/forwarder.ts`:
- Around line 1716-1721: 更新 forwarder 中描述首字节计时器与门控超时语义的注释，使其反映 onFirstByte
会在任意字节到达时调用 runtime.clearResponseTimeout?.()；明确持续输出中性帧时首个有效内容超时不再由该计时器兜底，而由
idleTimeoutMs 的静默超时负责。不要修改现有运行逻辑。
---
Nitpick comments:
In `@tests/integration/proxy-hedge-lifecycle.test.ts`:
- Around line 109-114: 更新 getCachedProxyRuntimeSettings
mock，以真实的默认运行时设置作为返回值基础，再覆盖测试所需的
affinityIgnoreClientSessionId、cacheEffectivenessEnabled、replayEnabled 和
state.streamGateMode。优先调用实际的 getCachedProxyRuntimeSettings
或使用模块导出的默认设置，避免遗漏新增配置字段。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: fe4a7857-3575-4e03-8ea3-427ffcf7a860
📒 Files selected for processing (2)
src/app/v1/_lib/proxy/forwarder.tstests/integration/proxy-hedge-lifecycle.test.ts
| if (gateFirstByteAt !== null) { | ||
| session.recordFirstByte(gateFirstByteAt); | ||
| } | ||
| session.recordTfft(); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
门控关闭时 first-byte 指标缺失：firstByteAt 只在 stream content gate 回调中采集。 串行与 hedge 路径都只在 runStreamContentGate 的 onFirstByte 里记录首字节时刻，因此当 streamGateMode !== "enforce"、raw_passthrough 或 mapProviderTypeToFamily 返回 null 时，session.recordFirstByte() 永不被调用，first_byte_ms 落库为 null；按本 PR 的口径，这类记录将不再返回输出速率。而 discovery 路径（L6545）是从原始首个非空字节采集的，三条路径口径不一致。
src/app/v1/_lib/proxy/forwarder.ts#L1765-L1768：非门控分支（未进入gateFamily分支时）也应在 response-handler 观测到首块前后补一次 first-byte 采集，或明确记录该路径为「无 first-byte」的既定取舍。src/app/v1/_lib/proxy/forwarder.ts#L4765-L4778：非门控分支已通过readFirstReadableChunk拿到首个非空块，可在此设置attempt.firstByteAt ??= Date.now()，使commitWinner的recordFirstByte生效（TFFT 仍由 response-handler 首块兜底）。
♻️ hedge 非门控分支的最小改法
// 保留首块：若本 attempt 落败且需要计费，drain 时需要补回首块的 usage。
attempt.firstChunk = firstChunk.value;
+ // 非门控路径：首个非空块即真实首字节，否则 first_byte_ms 恒为 null
+ attempt.firstByteAt ??= Date.now();
await commitWinner(attempt, [firstChunk.value], false);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (gateFirstByteAt !== null) { | |
| session.recordFirstByte(gateFirstByteAt); | |
| } | |
| session.recordTfft(); | |
| // 保留首块：若本 attempt 落败且需要计费，drain 时需要补回首块的 usage。 | |
| attempt.firstChunk = firstChunk.value; | |
| // 非门控路径：首个非空块即真实首字节，否则 first_byte_ms 恒为 null | |
| attempt.firstByteAt ??= Date.now(); | |
| await commitWinner(attempt, [firstChunk.value], false); |
📍 Affects 1 file
src/app/v1/_lib/proxy/forwarder.ts#L1765-L1768(this comment)src/app/v1/_lib/proxy/forwarder.ts#L4765-L4778
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/v1/_lib/proxy/forwarder.ts` around lines 1765 - 1768, 补齐非门控路径的
first-byte 采集，避免 session.recordFirstByte 未被调用。更新
src/app/v1/_lib/proxy/forwarder.ts:1765-1768，在 response-handler 观察到首块时补充
first-byte 记录；同时更新 src/app/v1/_lib/proxy/forwarder.ts:4765-4778，在
readFirstReadableChunk 获取首个非空块后设置 attempt.firstByteAt（仅在未设置时），使 commitWinner
能记录该指标，并保持 TFFT 的现有兜底逻辑不变。
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1871d59533
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| if (isFirstChunk) { | ||
| session.recordTtfb(); | ||
| session.recordTfft(); |
There was a problem hiding this comment.
When streamGateMode is off or shadow, the forwarder does not wait for a content frame, so this first chunk may be an SSE ping, metadata, or usage-only frame rather than a token. Calling recordTfft() here therefore stores TTFB as TFFT for those supported modes, making the newly relabeled UI and average-TFFT metrics inaccurate whenever a neutral prefix precedes content; record only firstByteMs here and set TFFT when content is actually observed, or leave it unavailable when content classification is disabled.
Useful? React with 👍 / 👎.
| const ttfbMs = | ||
| firstByteMs !== null && firstByteMs >= 0 && firstByteMs <= tfftMs ? firstByteMs : tfftMs; |
There was a problem hiding this comment.
For every pre-migration row, firstByteMs is intentionally null, so this fallback substitutes the stored TFFT value and then renders it using the segment's TTFB label. The resulting bar claims a true first-byte measurement exists even though the detailed metrics correctly omit TTFB for the same row; render the fallback segment as TFFT or omit the unavailable TTFB segment instead.
Useful? React with 👍 / 👎.
🧪 测试结果
总体结果: ✅ 所有测试通过 |
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
1 participant
背景
流式输出门禁（
runStreamContentGate，DB 默认enforce）上线后，上游 Response 会被拦住直到首个内容帧才交给 response handler。而session.ttfbMs正是在 response handler 见到的第一个 chunk 上打的时间戳——所以ttfb_ms这一列现在存的其实是 TFFT（time to first token），不是 TTFB。真正的首字节时刻其实被观测到了（stream-content-gate.ts的onFirstByte），但只用来清超时定时器，随后被丢弃。连带后果：
duration - ttfb_ms，分母排除了上游排队与中性帧窗口，TPS 系统性偏高（实测样本 120.48 vs 真实 89.45，虚高 34%）。改动
1. 双指标分开存
ttfb_ms（沿用，不改名）tfftMsfirst_byte_ms（新增）firstByteMs保留
ttfb_ms列名是为了避开重命名迁移的部署窗口风险与 plpgsql 名字捕获陷阱（RENAME COLUMN不会重写函数体，NEW.ttfb_ms会静默指向新列）。TS 侧不再存在名为ttfbMs的字段，因此不会有人把两者搞反；列名的历史债由schema.ts与 leaderboard SQL 的注释兜底。迁移
0114：两张表各ADD COLUMN IF NOT EXISTS，重建fn_upsert_usage_ledger()与触发器列清单。不回填——first_byte_ms IS NULL正是「无真 TTFB」的判据。2. 采集真 TTFB
ProxySession新增recordFirstByte(atEpochMs)；recordTtfb()改名recordTfft()，并在firstByteMs未设时补齐（覆盖门控关闭、shadow、raw_passthrough、非 SSE、Gemini 直通——这些路径下两者本就同一时刻）。forwarder 三处提交点：串行门控、hedge 竞速
commitWinner、discovery 竞速。首字节时刻先挂在 attempt 局部变量上，确认该次尝试会被服务后才写入 session——否则 failover 场景会记到失败尝试的时间，低估 TTFB 并放大 TPS 分母。3. TPS 改用真 TTFB
computeTokensPerSecond、calculateOutputRate、shouldHideOutputRate、leaderboard 聚合 SQL 全部改用firstByteMs。error-details-dialog/types.ts里那份与performance-formatter不一致的重复实现已删除，统一到后者。4. UI
请求详情耗时区新增 TTFB 行，原 TTFB 行改名 TFFT；延迟分解条从两段变三段（TTFB / 等待首 Token / 生成），历史行优雅降级回两段。日志表、排行榜、公开状态页只做 TTFB→TFFT 标签替换。5 语言文案同步，顺带把
LatencyBreakdownBar与 Output Tokens 的硬编码英文接上 i18n（CLAUDE.md 规则 3 的存量违规）。需要注意的行为变化
first_byte_ms为 NULL 时输出速率返回 null，不再回退到总耗时。排行榜与状态页的 TPS 在新数据积累起来之前会大面积为空——这是预期结果，旧口径的数值本身就是虚高的。伪流式与 replay 路径今天不记录该指标，因此这些行也会失去输出速率。/api/v1/resources/usage-logs的 JSON 键变了：ttfbMs→tfftMs，并新增firstByteMs。该字段从未被 OpenAPI 声明（响应 schema 是z.record(z.unknown())），未提供别名，以免把ttfbMs这个名字重新引回代码。刻意未改名
RoutingTraceSummaryV1.ttfbMs：已落库的版本化 JSON payload，改名会读不出历史 trace。payload.ts/openapi.ts的ttfbMs/latestTtfbMs：对外 JSON Schema 契约，只改展示标签。ttfb_sum/ttfb_count：既有键编码，改名会作废最多 32 天的已积累桶。验证
bun run build/lint/typecheck全绿；bun run test7927 passed（唯一失败的language-switcher.test.tsx在 dev 基线上同样失败，与本次改动无关）。test:v1单独跑，全部通过阈值；openapi:check/openapi:lint/validate-migrations通过。first_byte_ms（单列 UPDATE 也能同步）、INSERT/UPDATE 都正确投影进usage_ledger、TPS 聚合排除first_byte_ms IS NULL的历史行。🤖 Generated with Claude Code
Greptile Summary
This PR separates true TTFB from TFFT throughout the proxy metrics pipeline.
first_byte_msto request and usage-ledger persistence while retainingttfb_msfor TFFT.Confidence Score: 5/5
The PR appears safe to merge because no blocking failure eligible for this follow-up review remains.
No blocking failure remains.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR U[Upstream response] --> F[Proxy forwarder] F --> B[Record first byte] F --> T[Record first content token] B --> S[firstByteMs / first_byte_ms] T --> X[tfftMs / ttfb_ms] S --> TPS[TPS calculation] S --> UI[Latency breakdown] X --> UI S --> DB[(message_request)] X --> DB DB --> L[(usage_ledger)] L --> LB[Leaderboard] L --> PS[Public status]Reviews (2): Last reviewed commit: "fix(proxy): record TFFT at Responses con..." | Re-trigger Greptile
Context used (4)