fix(i18n): rename first-token metrics to TTFT - #1374
Conversation
Rename runtime fields, leaderboard keys, public status payload types, and locale keys from the historical TFFT/Ttfb spellings to TTFT/Ttft. Preserve the database ttfb_ms column and Redis rollup metric names as storage compatibility contracts.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
📝 WalkthroughWalkthrough本次变更统一 TTFT 相关字段、方法、翻译键和显示文案。更新代理会话、数据库映射、公开状态聚合、排行榜、日志组件及测试数据。数据库列名保持不变。 ChangesTTFT 命名统一
Estimated code review effort: 3 (Moderate) | ~25 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 |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| @@ -246,7 +246,7 @@ function normalizeRoutingTraceSummary(value: unknown): RoutingTraceSummaryV1 | u | |||
| outcome: summary.outcome as RoutingTraceRequestOutcome, | |||
| statusCode: summary.statusCode as number, | |||
| durationMs: summary.durationMs as number, | |||
| ttfbMs: summary.ttfbMs as number | null, | |||
| ttftMs: summary.ttftMs as number | null, | |||
There was a problem hiding this comment.
When a persisted version-1 routing trace contains the previous ttfbMs property, the changed normalizer validates only ttftMs and omits the entire summary, causing historical usage logs and pending routing-trace entries to lose TTFT and the other summary metrics.
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/types/routing-trace.ts
Line: 239-249
Comment:
**Legacy routing summaries are dropped**
When a persisted version-1 routing trace contains the previous `ttfbMs` property, the changed normalizer validates only `ttftMs` and omits the entire summary, causing historical usage logs and pending routing-trace entries to lose TTFT and the other summary metrics.
**Knowledge Base Used:**
- [Proxy request pipeline](https://app.greptile.com/ygxz/-/custom-context/knowledge-base/ding113/claude-code-hub/-/docs/proxy-pipeline.md)
- [Redis Caching and Session Tracking](https://app.greptile.com/ygxz/-/custom-context/knowledge-base/ding113/claude-code-hub/-/docs/redis-caching-and-sessions.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| state: normalizeTimelineState(value.state), | ||
| availabilityPct: normalizeNullableNumber(value.availabilityPct), | ||
| ttfbMs: normalizeNullableNumber(value.ttfbMs), | ||
| ttftMs: normalizeNullableNumber(value.ttftMs), |
There was a problem hiding this comment.
When the public status endpoint reads a cross-version Redis snapshot containing ttfbMs and latestTtfbMs, these sanitizers read only the renamed properties and normalize the existing measurements to null, causing TTFT to display as unavailable until the snapshot is rebuilt.
Knowledge Base Used: Redis Caching and Session Tracking
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/public-status/read-store.ts
Line: 126
Comment:
**Stored status metrics become null**
When the public status endpoint reads a cross-version Redis snapshot containing `ttfbMs` and `latestTtfbMs`, these sanitizers read only the renamed properties and normalize the existing measurements to null, causing TTFT to display as unavailable until the snapshot is rebuilt.
**Knowledge Base Used:** [Redis Caching and Session Tracking](https://app.greptile.com/ygxz/-/custom-context/knowledge-base/ding113/claude-code-hub/-/docs/redis-caching-and-sessions.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.![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: ca6738faa2
ℹ️ 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".
| !winnerOrigins.has(summary.winnerOrigin as RoutingTraceWinnerOrigin) || | ||
| numericKeys.some((key) => finiteNumber(summary[key]) === undefined) || | ||
| !(summary.ttfbMs === null || finiteNumber(summary.ttfbMs) !== undefined) || | ||
| !(summary.ttftMs === null || finiteNumber(summary.ttftMs) !== undefined) || |
There was a problem hiding this comment.
For every persisted routing trace created before this commit, the version-1 summary contains ttfbMs rather than ttftMs. This condition therefore treats an otherwise valid historical summary as invalid and removes it during normalization, causing existing usage-log and live-chain views to lose summary-only routing information such as winner origin, rounds, and attempt counts. Since the trace version remains unchanged, normalize ttfbMs as a backward-compatible alias for ttftMs.
Useful? React with 👍 / 👎.
| state: normalizeTimelineState(value.state), | ||
| availabilityPct: normalizeNullableNumber(value.availabilityPct), | ||
| ttfbMs: normalizeNullableNumber(value.ttfbMs), | ||
| ttftMs: normalizeNullableNumber(value.ttftMs), |
There was a problem hiding this comment.
When a fresh public-status snapshot was written by the previous release, its buckets contain ttfbMs; reading only ttftMs converts every existing value to null. A fresh v2 projection does not trigger a rebuild here, so after deployment the status page and public API report no first-token latency until the snapshot becomes stale and is rebuilt, and rolling deployments can repeat the loss. Read the historical key as a fallback here and likewise fall back from latestTtftMs to latestTtfbMs for model snapshots.
Useful? React with 👍 / 👎.
| totalTokens, | ||
| successRate: clampRatio01Nullable(entry.successRate), | ||
| avgTtfbMs: entry.avgTtfbMs ?? 0, | ||
| avgTtftMs: entry.avgTtftMs ?? 0, |
There was a problem hiding this comment.
When Redis already contains a provider leaderboard entry from the previous release, it is stored under the unchanged leaderboard:v2 key with avgTtfbMs. The new consumer receives that cached object without validation, so avgTtftMs is undefined and the leaderboard displays - and sorts the row as zero until the cache expires; during a rolling deployment, an older instance can keep repopulating the incompatible shape. Increment CACHE_SHAPE_VERSION when renaming this serialized field, as required by the cache's own shape-version contract.
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: 7
🧹 Nitpick comments (1)
src/app/[locale]/status/_components/public-status-timeline.tsx (1)
6-6: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win统一 TTFT formatter 的导入路径。
两个变更都继续使用相对路径。请改用
@/路径别名。
src/app/[locale]/status/_components/public-status-timeline.tsx#L6-L6: 使用@/app/[locale]/status/_lib/format-ttft。src/app/[locale]/status/_components/public-status-view.tsx#L31-L31: 使用@/app/[locale]/status/_lib/format-ttft。As per coding guidelines，TS/TSX 导入必须使用
@/路径别名。🤖 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]/status/_components/public-status-timeline.tsx at line 6, Update the formatTtft imports in src/app/[locale]/status/_components/public-status-timeline.tsx lines 6-6 and src/app/[locale]/status/_components/public-status-view.tsx lines 31-31 to use the `@/app/`[locale]/status/_lib/format-ttft path alias instead of relative paths.Source: Coding guidelines
🤖 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/virtualized-logs-table.test.tsx:
- Around line 605-612: 在性能 Tooltip 测试中补充对 logs.details.performance.ttfb
的断言，使其同时验证 TTFT 和 TTFB；保留现有的 TTFT 断言，并继续使用 renderPerformanceWithLog 生成的 tooltip
内容。
In `@src/lib/langfuse/trace-proxy-request.ts`:
- Around line 437-439: Update the comment adjacent to the completionStartTime
assignment in the session.ttftMs branch of generation.update to label the metric
as TTFT instead of TTFB; leave the implementation unchanged.
In `@src/lib/utils/performance-formatter.test.ts`:
- Around line 15-21: Update the test title in the calculateOutputRate case to
state that TTFT is greater than TTFB, matching the firstByteMs arguments and
existing assertions; leave the test logic unchanged.
In `@src/types/routing-trace.ts`:
- Line 65: Update normalizeRoutingTraceSummary to fall back to the legacy
summary.ttfbMs field when summary.ttftMs is absent, and always normalize the
result to ttftMs while preserving the current RoutingTraceV1 output shape. Add a
regression test covering a persisted payload that contains only ttfbMs.
In `@tests/unit/app/status/timeline-windows.test.ts`:
- Line 84: 将测试用例标题中的指标术语从“ttfb”更新为“ttft”，使其与验证的 computeAvgTtft 和 ttftMs
保持一致；不要修改测试逻辑。
In `@tests/unit/public-status/public-status-view.test.tsx`:
- Line 176: 更新 public status 测试夹具中 ttft 对应的显示文本，将标签和值从旧的 TTFB 文案统一为 TTFT，并同步修改
ttft tooltip 的文案。确保使用该夹具的渲染测试验证新的状态页显示文本。
In `@tests/unit/public-status/status-page-locale.test.ts`:
- Line 41: Update the mock values for the ttft and tooltip.ttft keys in the
status-page locale test from “TTFB” to “TTFT”, ensuring both user-visible label
assertions validate the new wording.
---
Nitpick comments:
In `@src/app/`[locale]/status/_components/public-status-timeline.tsx:
- Line 6: Update the formatTtft imports in
src/app/[locale]/status/_components/public-status-timeline.tsx lines 6-6 and
src/app/[locale]/status/_components/public-status-view.tsx lines 31-31 to use
the `@/app/`[locale]/status/_lib/format-ttft path alias instead of relative paths.
🪄 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: c728ef9c-cf88-43f3-85b2-cda1a54437e0
📒 Files selected for processing (92)
messages/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/leaderboard/_components/leaderboard-view.tsxsrc/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/index.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog/types.tssrc/app/[locale]/dashboard/logs/_components/provider-chain-popover.test.tsxsrc/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/[locale]/status/[slug]/page.tsxsrc/app/[locale]/status/_components/public-status-timeline.tsxsrc/app/[locale]/status/_components/public-status-view.tsxsrc/app/[locale]/status/_lib/format-ttft.tssrc/app/[locale]/status/_lib/timeline-windows.tssrc/app/[locale]/status/page.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/observability/discovery-metrics.tssrc/lib/public-status/aggregation-core.tssrc/lib/public-status/aggregation.tssrc/lib/public-status/openapi.tssrc/lib/public-status/payload.tssrc/lib/public-status/read-store.tssrc/lib/public-status/rollup-store.tssrc/lib/redis/live-chain-store.test.tssrc/lib/utils/performance-formatter.test.tssrc/repository/leaderboard.tssrc/repository/message-write-buffer.tssrc/repository/message.tssrc/repository/usage-logs.tssrc/types/message.tssrc/types/routing-trace.tstests/integration/proxy-hedge-lifecycle.test.tstests/unit/api/leaderboard-route.test.tstests/unit/api/public-status-api-contract.test.tstests/unit/app/status/derive-display-state.test.tstests/unit/app/status/fill-display-timeline.test.tstests/unit/app/status/timeline-windows.test.tstests/unit/dashboard-logs-virtualized-special-settings-ui.test.tsxtests/unit/dashboard-logs-warmup-ui.test.tsxtests/unit/dashboard/leaderboard-view-cache-coefficient.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/routing-trace.test.tstests/unit/proxy/session-ttfb-ttft.test.tstests/unit/public-status/aggregation-core-tps.test.tstests/unit/public-status/aggregation.test.tstests/unit/public-status/public-status-view.test.tsxtests/unit/public-status/read-store.test.tstests/unit/public-status/rebuild-worker.test.tstests/unit/public-status/rollup-store.test.tstests/unit/public-status/status-page-locale.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-replay-projection.test.tstests/unit/repository/usage-logs-sessionid-filter.test.tstests/unit/types/routing-trace.test.tstests/unit/usage-logs/export-csv.test.tstests/unit/usage-logs/export-summary.test.tstests/unit/usage-logs/export-xlsx.test.ts
| test("性能 Tooltip 保留 TTFT 和 TTFB 完整术语", () => { | ||
| const { tooltip } = renderPerformanceWithLog({ | ||
| durationMs: 1000, | ||
| tfftMs: 500, | ||
| ttftMs: 500, | ||
| firstByteMs: 250, | ||
| }); | ||
|
|
||
| expect(tooltip.textContent).toContain("logs.details.performance.tfft"); | ||
| expect(tooltip.textContent).toContain("logs.details.performance.ttft"); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
补充 TTFB 断言。
测试名称声称验证 TTFT 和 TTFB，但当前只断言 logs.details.performance.ttft。如果 logs.details.performance.ttfb 回归，测试仍会通过。
建议补充断言
expect(tooltip.textContent).toContain("logs.details.performance.ttft");
+ expect(tooltip.textContent).toContain("logs.details.performance.ttfb");📝 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.
| test("性能 Tooltip 保留 TTFT 和 TTFB 完整术语", () => { | |
| const { tooltip } = renderPerformanceWithLog({ | |
| durationMs: 1000, | |
| tfftMs: 500, | |
| ttftMs: 500, | |
| firstByteMs: 250, | |
| }); | |
| expect(tooltip.textContent).toContain("logs.details.performance.tfft"); | |
| expect(tooltip.textContent).toContain("logs.details.performance.ttft"); | |
| test("性能 Tooltip 保留 TTFT 和 TTFB 完整术语", () => { | |
| const { tooltip } = renderPerformanceWithLog({ | |
| durationMs: 1000, | |
| ttftMs: 500, | |
| firstByteMs: 250, | |
| }); | |
| expect(tooltip.textContent).toContain("logs.details.performance.ttft"); | |
| expect(tooltip.textContent).toContain("logs.details.performance.ttfb"); |
🤖 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/virtualized-logs-table.test.tsx
around lines 605 - 612, 在性能 Tooltip 测试中补充对 logs.details.performance.ttfb
的断言，使其同时验证 TTFT 和 TTFB；保留现有的 TTFT 断言，并继续使用 renderPerformanceWithLog 生成的 tooltip
内容。
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| !winnerOrigins.has(summary.winnerOrigin as RoutingTraceWinnerOrigin) || | ||
| numericKeys.some((key) => finiteNumber(summary[key]) === undefined) || | ||
| !(summary.ttfbMs === null || finiteNumber(summary.ttfbMs) !== undefined) || | ||
| !(summary.ttftMs === null || finiteNumber(summary.ttftMs) !== undefined) || |
There was a problem hiding this comment.
[HIGH] [LOGIC-BUG] Legacy version-1 routing traces lose their summary after this rename
Why this is a problem: Persisted routing_trace JSON written before this PR still stores summary.ttfbMs. normalizeRoutingTrace() is used on read paths for usage logs and routing-trace outbox payloads, so validating only summary.ttftMs makes those legacy summaries drop out entirely. Existing traces then lose winner/attempt/duration metrics even though the stored data is still valid.
Suggested fix:
const ttftMs = summary.ttftMs ?? summary.ttfbMs;
if (
!outcomes.has(summary.outcome as RoutingTraceRequestOutcome) ||
!winnerOrigins.has(summary.winnerOrigin as RoutingTraceWinnerOrigin) ||
numericKeys.some((key) => finiteNumber(summary[key]) === undefined) ||
!(ttftMs === null || finiteNumber(ttftMs) !== undefined) ||
!(summary.winnerProviderId === null || finiteNumber(summary.winnerProviderId) !== undefined) ||
!(summary.winnerRound === null || finiteNumber(summary.winnerRound) !== undefined)
) {
return undefined;
}
return {
...,
ttftMs: ttftMs as number | null,
};
| state: normalizeTimelineState(value.state), | ||
| availabilityPct: normalizeNullableNumber(value.availabilityPct), | ||
| ttfbMs: normalizeNullableNumber(value.ttfbMs), | ||
| ttftMs: normalizeNullableNumber(value.ttftMs), |
There was a problem hiding this comment.
[HIGH] [LOGIC-BUG] Existing Redis public-status snapshots normalize TTFT metrics to null
Why this is a problem: readPublicStatusPayload() always sanitizes the stored snapshot before serving /status. Snapshots already sitting in Redis still use ttfbMs and latestTtfbMs, so this rename erases valid TTFT values until a rebuild rewrites the snapshot. That produces immediate user-visible regressions on deploy even though the cached data is otherwise valid.
Suggested fix:
const ttftMs = normalizeNullableNumber(value.ttftMs ?? value.ttfbMs);
const latestTtftMs = normalizeNullableNumber(value.latestTtftMs ?? value.latestTtfbMs);
return [
{
...,
latestTtftMs,
timeline: sanitizeTimelineBuckets(value.timeline),
},
];
// inside sanitizeTimelineBuckets
return [{
...,
ttftMs,
...,
}];
| totalTokens, | ||
| successRate: clampRatio01Nullable(entry.successRate), | ||
| avgTtfbMs: entry.avgTtfbMs ?? 0, | ||
| avgTtftMs: entry.avgTtftMs ?? 0, |
There was a problem hiding this comment.
[MEDIUM] [LOGIC-BUG] Provider leaderboard payload shape changes without a cache-version bump
Why this is a problem: This line renames the provider latency field to avgTtftMs, but getLeaderboardWithCache() still reads leaderboard:v2:* entries with a raw JSON.parse() and CACHE_SHAPE_VERSION is unchanged. For up to the 60-second TTL, cached provider rows still contain avgTtfbMs, so the API/frontend won't receive avgTtftMs until those Redis entries expire or are invalidated.
Suggested fix:
// src/lib/redis/leaderboard-cache.ts
const CACHE_SHAPE_VERSION = "v3";If you want to preserve warm caches, add a small normalization step when reading cached provider entries so both field names are accepted during the rollout.
There was a problem hiding this comment.
Code Review Summary
This PR is mostly a broad rename, but it changes a few persisted and cached payload shapes without preserving existing data. I validated three rollout regressions: two affect already-stored JSON/Redis data immediately after deploy, and one affects cached leaderboard responses until their TTL expires.
PR Size: XL
- Lines changed: 935
- Files changed: 92
- Split suggestions: Separate the pure i18n/test rename sweep from the persisted-data compatibility changes in
src/types/*,src/lib/public-status/*, andsrc/repository/*. - Split suggestions: Isolate the Redis/cache-contract changes so
public-statusandleaderboardrollout behavior can be reviewed independently.
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 2 | 1 | 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][Confidence: 92]
src/types/routing-trace.ts:239only acceptssummary.ttftMs, so legacy version-1 routing traces persisted withsummary.ttfbMslose their summary on read. - [High][LOGIC-BUG][Confidence: 94]
src/lib/public-status/read-store.ts:126andsrc/lib/public-status/read-store.ts:162only read renamed TTFT fields, so existing Redis public-status snapshots servenullTTFT metrics until rebuilt. - [Medium][LOGIC-BUG][Confidence: 82]
src/repository/leaderboard.ts:728changes the provider payload field toavgTtftMs, butsrc/lib/redis/leaderboard-cache.ts:70still usesCACHE_SHAPE_VERSION = "v2", so cached provider rows can return the oldavgTtfbMsshape for up to 60 seconds after deploy.
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 Submitted
- Applied the
size/XLlabel to PR#1374and posted the summary review comment. - Added inline comments for three validated rollout regressions:
src/types/routing-trace.ts:239— legacy persisted routing traces still usesummary.ttfbMs, so the renamed normalizer drops their summaries.src/lib/public-status/read-store.ts:126— existing Redis public-status snapshots still usettfbMs/latestTtfbMs, so TTFT metrics becomenulluntil rebuilt.src/repository/leaderboard.ts:728— provider leaderboard payload renamed toavgTtftMswithout bumping the Redis cache shape version, so cached rows can serve the old field for up to 60 seconds.
- Review summary outcome: 2 high-severity compatibility bugs, 1 medium-severity cache-shape bug, and no security findings.
If you want, I can also draft the concrete fix plan for those three comments.
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6bac1d1af
ℹ️ 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".
| ? model.availabilityPct | ||
| : computeUptimePct(model.timeline); | ||
| const ttfb24h = computeAvgTtfb(model.timeline); | ||
| const ttft24h = computeAvgTtft(model.timeline); |
There was a problem hiding this comment.
During a rolling deployment, the new status-page bundle can poll an older instance of /api/public-status, whose buckets still contain ttfbMs rather than ttftMs. The refresh path passes response.groups through unchanged in toPublicStatusPayload, so computeAvgTtft treats the resulting undefined as non-null, produces NaN, and the card displays NaN ms; timeline tooltips similarly display undefined ms. Normalize the legacy wire fields in the polling response, as is already done for Redis snapshots, or retain compatible response aliases until older instances are drained.
Useful? React with 👍 / 👎.
🧪 测试结果
总体结果: ✅ 所有测试通过 |
1 participant
Summary
ttfb_mscolumn and Redisttfb_sum/ttfb_countstorage contractsValidation
bun run typecheckbun run lintbun run buildbun run test(843 files passed, 8170 tests passed, 13 skipped)tfft/TFFT/avgTtfbMs/latestTtfbMsnames scanned with no remaining matchesNotes
The database column and Redis rollup metric names remain unchanged intentionally because they are historical persistence contracts. Their TypeScript-facing fields and user-visible terminology now use TTFT.
Greptile Summary
The PR consistently renames first-token runtime and user-facing terminology from historical TFFT/Ttfb spellings to TTFT/Ttft while preserving database and Redis persistence contracts.
Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains; both previously reported compatibility issues are addressed by legacy-field fallbacks and targeted regression coverage.
Important Files Changed
ttftMswhile retaining a tested fallback for persistedttfbMsvalues.ttfb_mscolumn through the renamed TypeScript-facingttftMsfield.Reviews (2): Last reviewed commit: "fix(review): preserve legacy TTFT metric..." | Re-trigger Greptile