perf: reduce gateway amplification and correct TTFT/TTFB metrics - #1371
perf: reduce gateway amplification and correct TTFT/TTFB metrics#1371ding113 wants to merge 1 commit into
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
📝 WalkthroughWalkthroughChanges性能语义与数据链路
Estimated code review effort: 5 (Critical) | ~120 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 |
| } | ||
| if (this.cleanupTimer) clearInterval(this.cleanupTimer); | ||
| this.cleanupTimer = null; | ||
| await Promise.race([this.waitForDrain(), delay(5_000)]); |
There was a problem hiding this comment.
Snapshot shutdown abandons queued writes
When shutdown or backend reconfiguration occurs while the serial filesystem queue needs more than five seconds to drain, stop() returns with accepted writes still pending, causing recent session-detail snapshots to be missing after process exit or store replacement.
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/session-snapshot/filesystem-store.ts
Line: 190
Comment:
**Snapshot shutdown abandons queued writes**
When shutdown or backend reconfiguration occurs while the serial filesystem queue needs more than five seconds to drain, `stop()` returns with accepted writes still pending, causing recent session-detail snapshots to be missing after process exit or store replacement.
**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)
- [Server Lifecycle and Background Jobs](https://app.greptile.com/ygxz/-/custom-context/knowledge-base/ding113/claude-code-hub/-/docs/server-lifecycle-and-jobs.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2d045f8089
ℹ️ 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".
| UPDATE "message_request" | ||
| SET | ||
| "ttft_ms" = "ttfb_ms", | ||
| "ttfb_ms" = "first_byte_ms", | ||
| "timing_semantics_version" = 2 |
There was a problem hiding this comment.
Preserve legacy TTFT before firing the ledger trigger
When upgrading a database that already has migration 0114's trg_upsert_usage_ledger, this update fires that old trigger before it is replaced later in this file. The trigger copies the newly reassigned message_request.ttfb_ms into usage_ledger.ttfb_ms, so the subsequent ledger backfill reads an already-overwritten value and stores the old first-byte time in both ttfb_ms and ttft_ms, permanently losing historical TFFT for retained rows. Disable or replace the trigger before this update, or backfill the ledger before updating message_request.
Useful? React with 👍 / 👎.
| if (outcome === "success" && ttfbMs !== null) { | ||
| increments.push( | ||
| { groupId, modelKey, metric: "ttfb_sum", value: tfftMs }, | ||
| { groupId, modelKey, metric: "ttfb_sum", value: ttfbMs }, | ||
| { groupId, modelKey, metric: "ttfb_count", value: 1 } | ||
| ); |
There was a problem hiding this comment.
Version the rollup keys before changing timing semantics
On upgrades with existing public-status:v2 hashes, this adds actual TTFB to ttfb_sum, even though those same fields already contain legacy TFFT samples; tps_sum similarly starts combining TTFT-based rates with the previous first-byte-based rates. Because these hashes have a 32-day TTL and the Redis prefix was not changed or cleared, status rebuilds will publish blended, invalid timing metrics for up to 32 days. Write the new semantics under a new rollup namespace or explicitly rebuild the old buckets.
Useful? React with 👍 / 👎.
| await Promise.allSettled([ | ||
| schedulerState.__CCH_CACHE_EFFECTIVENESS_CURRENT_PROMISE__, | ||
| schedulerState.__CCH_REPLAY_CLEANUP_CURRENT_PROMISE__, | ||
| ]); |
There was a problem hiding this comment.
Bound scheduler joins before flushing buffered messages
If SIGTERM arrives while either DB-backed scheduler tick is slow or stuck, this unbounded Promise.allSettled prevents cleanup from reaching async-task shutdown and the message-writer flush. awaitQuiescenceBestEffort only logs after its warning interval and continues waiting, while server.js hard-exits after 28 seconds, so a routine rollout can kill the process with terminal request updates still buffered. Bound or detach these non-critical joins, or move them after the critical writer flush.
Useful? React with 👍 / 👎.
| `${t("logs.details.performance.tfft")} ${formatDuration(log.tfftMs)}`, | ||
| displayTtfbMs != null && | ||
| displayTtfbMs > 0 && | ||
| `TTFB ${formatDuration(displayTtfbMs)}`, |
There was a problem hiding this comment.
Localize the inline TTFB label
When current timing data is displayed, this literal emits user-facing text outside next-intl, even though the translated logs.details.performance.ttfb key is already used in the tooltip below. Build this label from t(...) rather than hardcoding it.
AGENTS.md reference: AGENTS.md:L14-L14
Useful? React with 👍 / 👎.
| await tx.execute(sql` | ||
| INSERT INTO background_task_cursor (task_key, cursor_at, updated_at) | ||
| VALUES (${TASK_KEY}, ${initialCursor}, NOW()) | ||
| ON CONFLICT (task_key) DO NOTHING |
There was a problem hiding this comment.
Seed the new cursor from the last aggregated window
On upgrades where provider_cache_effectiveness already contains windows, the new cursor table is empty, so this initializes it to one hour ago instead of the previous MAX(window_end). The immediate scheduler tick then writes a broad window overlapping the existing narrower windows; the unique index does not conflict because their bounds differ, and coefficient queries subsequently sum both sets, double-counting up to an hour of samples indefinitely. Seed the cursor from the latest existing window when creating its first row.
Useful? React with 👍 / 👎.
| if (bucket.sampleCount > 0 && bucket.ttftMs !== null) { | ||
| weightedSum += bucket.ttftMs * bucket.sampleCount; | ||
| sampleTotal += bucket.sampleCount; |
There was a problem hiding this comment.
Weight TTFT averages by timing-bearing samples
When a timeline bucket mixes streaming requests with non-streaming, failed, or legacy requests, bucket.ttftMs is calculated only from requests that have TTFT, while bucket.sampleCount counts all outcomes. Weighting the bucket average by that total lets a bucket with only a few TTFT measurements but many untimed requests dominate the displayed range average. Carry the TTFT sample count through the payload and use that as the weight.
Useful? React with 👍 / 👎.
| ALTER TABLE "message_request" DROP COLUMN IF EXISTS "first_byte_ms";--> statement-breakpoint | ||
| ALTER TABLE "usage_ledger" DROP COLUMN IF EXISTS "first_byte_ms"; |
There was a problem hiding this comment.
Retain legacy timing columns through the rolling upgrade
During the configured rolling deployment, the new pod runs this migration while old replicas are still serving. Those replicas still select and write first_byte_ms on both request finalization and usage-log reads, so dropping the columns here causes their SQL to fail until every old pod is terminated, losing terminal log updates and breaking dashboard queries during the rollout. Keep the legacy columns for at least one compatibility release and remove them only after all running code has stopped referencing them.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/repository/message.ts (1)
971-1010: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
findMessageRequestBySessionId主查询路径遗漏 ttfbMs/ttftMs/timingSemanticsVersion。对比同文件内的
findMessageRequestById(840-882 行,已正确加入新字段)与本函数的 ledger-only 兜底路径(1041-1043 行,已正确加入新字段),本函数针对messageRequest表的主查询路径(即最常命中的路径)显式声明了 select 字段列表,却唯独没有选择ttfbMs/ttftMs/timingSemanticsVersion。由于该处使用了显式列声明的.select({...})(而非select()全列),这三个字段在主路径命中时会始终缺失,只有走到 ledger-only 兜底分支才能拿到正确值 —— 与本 PR 统一 TTFB/TTFT 数据契约的目标直接冲突,可能导致依赖findMessageRequestBySessionId的下游逻辑(如会话级性能展示)拿到不完整的计时数据。🛠️ 建议修复
model: messageRequest.model, originalModel: messageRequest.originalModel, durationMs: messageRequest.durationMs, + ttfbMs: messageRequest.ttfbMs, + ttftMs: messageRequest.ttftMs, + timingSemanticsVersion: messageRequest.timingSemanticsVersion, costUsd: messageRequest.costUsd, costMultiplier: messageRequest.costMultiplier,🤖 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 971 - 1010, Update the explicit select list in findMessageRequestBySessionId to include messageRequest.ttfbMs, messageRequest.ttftMs, and messageRequest.timingSemanticsVersion, matching findMessageRequestById and the ledger-only fallback while preserving the existing query behavior.src/app/[locale]/dashboard/leaderboard/_components/leaderboard-view.tsx (1)
344-370: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
avgTtfbMs为空时会完全隐藏可能有效的avgTtftMs数据。当前逻辑:
val == null || val <= 0时直接返回"-",不再渲染 Tooltip,因此即便row.avgTtftMs有有效值,用户也完全看不到。avgTtfbMs/avgTtftMs是两个独立的 SQLavg(CASE WHEN timingSemanticsVersion = 2 THEN ... END)表达式,不能保证同一分组内两者总是同时非空或同时为空。建议分别判断两个值是否可用,而不是让 TTFT(本次 PR 的核心新指标)完全依赖 TTFB 是否有值。🐛 建议修复
cell: (row) => { const val = row.avgTtfbMs; - if (val == null || val <= 0) return "-"; + const ttft = row.avgTtftMs; + if ((val == null || val <= 0) && (ttft == null || ttft <= 0)) return "-"; return ( <Tooltip> <TooltipTrigger asChild> - <span className="cursor-help">{Math.round(val).toLocaleString()} ms</span> + <span className="cursor-help"> + {val != null && val > 0 + ? `${Math.round(val).toLocaleString()} ms` + : t("columns.timingUnavailable")} + </span> </TooltipTrigger> <TooltipContent className="space-y-1 text-xs"> <div> - {t("columns.avgTtfbMs")}: {Math.round(val).toLocaleString()} ms + {t("columns.avgTtfbMs")}:{" "} + {val != null && val > 0 + ? `${Math.round(val).toLocaleString()} ms` + : t("columns.timingUnavailable")} </div> <div> {t("columns.avgTtftMs")}:{" "} - {row.avgTtftMs == null + {ttft == null || ttft <= 0 ? t("columns.timingUnavailable") - : `${Math.round(row.avgTtftMs).toLocaleString()} ms`} + : `${Math.round(ttft).toLocaleString()} ms`} </div> </TooltipContent> </Tooltip> ); },🤖 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/leaderboard/_components/leaderboard-view.tsx around lines 344 - 370, Update the avgTtfbMs column cell renderer to evaluate avgTtfbMs and avgTtftMs independently. Preserve the existing TTFB display when valid, but render a Tooltip whenever either timing value is available so valid row.avgTtftMs remains visible even when avgTtfbMs is null or non-positive; show the unavailable text only for the individual missing value.
🧹 Nitpick comments (5)
src/instrumentation.ts (1)
275-304: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win建议提取共享的“受控 interval tick”辅助函数以消除重复逻辑。
startCacheEffectivenessScheduler和startReplayCleanupScheduler中的 tick 实现(stop-request 检查、current promise追踪、finally清空)结构几乎一致,仅内部执行体不同。建议提取一个通用辅助函数(如createGuardedIntervalTick(fn, { stopFlagKey, promiseKey })),减少未来维护时两处逻辑分叉的风险。♻️ 提炼共享辅助函数的思路
+function createGuardedTick(options: { + isStopRequested: () => boolean; + getCurrentPromise: () => Promise<void> | undefined; + setCurrentPromise: (p: Promise<void> | undefined) => void; + run: () => Promise<void>; +}): () => void { + return () => { + if (options.isStopRequested() || options.getCurrentPromise()) return; + const current = options + .run() + .catch(() => undefined) + .finally(() => { + if (options.getCurrentPromise() === current) options.setCurrentPromise(undefined); + }); + options.setCurrentPromise(current); + }; +}Also applies to: 330-363
🤖 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/instrumentation.ts` around lines 275 - 304, 提取一个共享的受控 interval tick 辅助函数,统一封装 stop-request 检查、current promise 记录以及 finally 中清空 promise 的逻辑;然后让 startCacheEffectivenessScheduler 和 startReplayCleanupScheduler 通过该辅助函数仅提供各自的异步执行体、stopFlagKey 和 promiseKey。保持现有停止行为、并发保护、错误处理及调度结果不变,删除两处重复的 tick 实现。src/lib/utils/performance-formatter.test.ts (1)
5-21: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win补充非法 TTFT 输入的回归测试。
当前测试覆盖了缺失和超出总耗时的 TTFT,但没有覆盖负数或
NaN。建议在修复格式化器后增加对应断言,防止无效速率重新进入仪表盘。🤖 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/utils/performance-formatter.test.ts` around lines 5 - 21, 在 performance formatter 测试中补充负数和 NaN 的非法 TTFT 回归断言,验证 calculateOutputRate 对这些输入返回 null,确保无效速率不会进入仪表盘。src/app/v1/_lib/proxy/stream-gate/stream-content-gate.ts (1)
336-364: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win为 stream-gate 的增量 SSE parser 设置解码缓冲上限。
SseFrameParser.assertBufferLimit()在maxBufferedCharacters === undefined时直接返回,内容计时观察器和 shadow 观察器都是new SseFrameParser(),遇到未完结的超长行/多行 data 且未产出 content 帧时,lineTail/dataCharacters可能持续累积;同时现有catch也不会拦截SseFrameBufferLimitError。建议为这两个 observer 设置固定上限,避免与后续解析逻辑的缓冲保护脱节。🤖 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/stream-gate/stream-content-gate.ts` around lines 336 - 364, 为 createContentTimingObserver 中的增量 SSE 解析器设置固定的 maxBufferedCharacters 上限,避免未完成的超长行或多行 data 持续累积;同时定位并更新 shadow observer 中的 SseFrameParser 实例,使用相同上限配置,保持现有 SseFrameBufferLimitError 的终止处理行为。src/repository/provider-cache-effectiveness.ts (1)
136-175: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win建议提取
getProviderCacheCoefficients与getProviderModelCacheCoefficients的公共聚合逻辑。两个函数的 select 投影、
computeCoefficientBp调用与结果组装完全同构,仅 groupBy 维度不同。建议抽取一个内部 helper(按传入的 groupBy 列构造查询并统一组装结果),避免未来两处口径漂移。♻️ 重构思路示意
-export async function getProviderCacheCoefficients({...}) { - const rows = await db.select({...}).from(...).where(...).groupBy(providerCacheEffectiveness.providerId); - const coefficients = new Map<number, ProviderCacheCoefficient>(); - for (const row of rows) { ... } - return coefficients; -} - -export async function getProviderModelCacheCoefficients({...}) { - const rows = await db.select({...}).from(...).where(...).groupBy(providerId, normalizedModel); - const coefficients: ProviderModelCacheCoefficientMap = new Map(); - for (const row of rows) { ... } - return coefficients; -} +async function aggregateCacheCoefficientRows({ start, end, byModel }: { start: Date; end: Date; byModel: boolean }) { + // 复用相同的 select/groupBy/computeCoefficientBp 逻辑,byModel 控制是否额外按 model 分组 +}Also applies to: 183-228
🤖 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/provider-cache-effectiveness.ts` around lines 136 - 175, 提取一个内部 helper,复用 getProviderCacheCoefficients 与 getProviderModelCacheCoefficients 中相同的聚合查询、computeCoefficientBp 调用及结果组装逻辑;让两个公开函数仅传入各自的 groupBy 列并保留现有返回类型与筛选条件,确保 provider 与 provider-model 两种维度行为不变。src/app/[locale]/status/_lib/timeline-windows.ts (1)
27-55: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
computeAvgTtft与computeAvgTtfb逻辑重复,建议提取公共辅助函数。两个函数除字段名(
ttfbMs/ttftMs)外完全相同。建议抽取一个按字段名参数化的通用加权平均函数,减少后续两个指标实现走偏的风险。♻️ 建议重构
-export function computeAvgTtfb(timeline: PublicStatusTimelineBucket[]): number | null { - let weightedSum = 0; - let sampleTotal = 0; - for (const bucket of timeline) { - if (bucket.sampleCount > 0 && bucket.ttfbMs !== null) { - weightedSum += bucket.ttfbMs * bucket.sampleCount; - sampleTotal += bucket.sampleCount; - } - } - if (sampleTotal === 0) { - return null; - } - return Math.round(weightedSum / sampleTotal); -} - -export function computeAvgTtft(timeline: PublicStatusTimelineBucket[]): number | null { - let weightedSum = 0; - let sampleTotal = 0; - for (const bucket of timeline) { - if (bucket.sampleCount > 0 && bucket.ttftMs !== null) { - weightedSum += bucket.ttftMs * bucket.sampleCount; - sampleTotal += bucket.sampleCount; - } - } - if (sampleTotal === 0) { - return null; - } - return Math.round(weightedSum / sampleTotal); -} +function computeWeightedAvg( + timeline: PublicStatusTimelineBucket[], + field: "ttfbMs" | "ttftMs" +): number | null { + let weightedSum = 0; + let sampleTotal = 0; + for (const bucket of timeline) { + const value = bucket[field]; + if (bucket.sampleCount > 0 && value !== null) { + weightedSum += value * bucket.sampleCount; + sampleTotal += bucket.sampleCount; + } + } + if (sampleTotal === 0) { + return null; + } + return Math.round(weightedSum / sampleTotal); +} + +export const computeAvgTtfb = (timeline: PublicStatusTimelineBucket[]) => + computeWeightedAvg(timeline, "ttfbMs"); +export const computeAvgTtft = (timeline: PublicStatusTimelineBucket[]) => + computeWeightedAvg(timeline, "ttftMs");🤖 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/_lib/timeline-windows.ts around lines 27 - 55, 提取一个按指标字段参数化的通用加权平均辅助函数,集中复用 computeAvgTtfb 和 computeAvgTtft 中的遍历、样本加权、空样本返回 null 及四舍五入逻辑;让两个公开函数仅传入对应的 ttfbMs 或 ttftMs 字段,并保持现有结果不变。
🤖 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 `@deploy/k8s/app/deployment.yaml`:
- Around line 138-145: Replace the session-snapshots hostPath volume in the
deployment’s volumeMounts/volumes configuration with a shared RWX PVC, and
configure the PVC-backed storage for SESSION_SNAPSHOT_ROOT so replicas can
access the same snapshots across nodes. Alternatively, when multi-node
deployment uses the Redis snapshot backend, remove this local hostPath
dependency and apply the corresponding Redis configuration.
In `@deploy/k8s/README.md`:
- Around line 57-61: Remove the blank line separating the adjacent blockquote
lines in the README so the entire note remains one continuous blockquote and
satisfies markdownlint MD028.
In `@drizzle/0116_lying_marvel_apes.sql`:
- Around line 13-49: 调整迁移 DO 块中 message_request 的回填顺序,避免旧触发器
trg_upsert_usage_ledger 在更新 ttfb_ms 时污染 usage_ledger.ttfb_ms:在 message_request
回填前禁用该触发器(或先删除),完成两个表的回填后恢复或创建迁移所需的新触发器,并确保触发器状态最终正确。
In `@messages/zh-CN/dashboard.json`:
- Line 366: 统一更新 firstByteToFirstToken 文案:在 messages/zh-CN/dashboard.json
第366行改为“首字节到首个有效内容”,在 messages/ru/dashboard.json 第366行改为“От первого байта до
первого валидного содержимого”,在 messages/zh-TW/dashboard.json
第366行改为“首字節到首個有效內容”。
In `@src/actions/system-config.ts`:
- Around line 219-225: 更新 system settings 保存流程中 reconfigureSessionSnapshotStore
的失败处理:不要仅记录 warn 后继续返回普通的 { ok: true, data: updated },应沿用
publicStatusProjectionWarningCode
的处理路径,将可供前端识别的失败/告警码附加到成功响应中。保留数据库与缓存已更新的行为,并确保成功重配置时响应保持现有结果。
In
`@src/app/`[locale]/dashboard/logs/_components/error-details-dialog/components/LatencyBreakdownBar.tsx:
- Around line 58-66: Replace the hardcoded “TTFB” label in the
LatencyBreakdownBar TTFB segment with the existing t("ttfb") translation,
matching the surrounding translated labels and preserving the current visibility
condition.
In `@src/app/`[locale]/dashboard/logs/_components/usage-logs-table.tsx:
- Around line 593-596: 将 usage logs 表格中 secondLine 的内联 TTFB 文案替换为 next-intl
翻译键,传入格式化后的时长插值并保持现有的非空及大于零判断;同时在五种 locale 的翻译资源中补齐该键对应的译文,确保各语言界面不再固定显示英文。
In `@src/app/`[locale]/dashboard/logs/_components/virtualized-logs-table.tsx:
- Around line 1170-1173: Update the ttfbLine construction in the virtualized
logs table to replace the hardcoded “TTFB” label with the existing
t("logs.details.performance.ttfb") translation, while preserving the current
displayTtfbMs validation and formatDuration output.
In `@src/app/`[locale]/status/_components/public-status-view.tsx:
- Around line 518-540: Update the TooltipTrigger child in the TTFB/TTFT metrics
block to use a natively focusable element, or add the required focusability and
keyboard interaction to the rendered element, while preserving the existing
tooltip content and styling.
In `@src/app/api/admin/system-config/route.ts`:
- Around line 131-137: 更新系统配置提交流程,覆盖动态导入与 reconfigureSessionSnapshotStore 的完整
try/catch;重配置失败时不要仅记录警告后继续返回
updated,而应回滚数据库/缓存配置并返回错误,确保运行时存储未成功启动时不会对外显示新配置已生效。同步检查
reconfigureSessionSnapshotStore 中 activeSelection 和 activeStore
的更新时机,采用提交成功后再更新或失败恢复旧状态的语义。
In `@src/app/v1/_lib/proxy/response-handler.ts`:
- Line 3682: 调整 Gemini 透传分支中的 observePassthroughChunk 流程,使其接入现有的
contentTimingObserver,并按首个内容帧而非首个原始字节记录 TTFT;仅在没有 observer 时保留
session.recordTtft() 的回退逻辑,确保 timingSemanticsVersion: 2 的语义与非透传路径一致。
In `@src/lib/ledger-backfill/service.ts`:
- Around line 93-95: Update the ledger backfill query’s selection conditions so
rows with missing timing fields are eligible for legacy timing backfill, even
when success_rate_outcome already exists. In the ON CONFLICT DO UPDATE branch,
persist or merge the incoming ttfb_ms, ttft_ms, and timing_semantics_version
values instead of updating only success_rate_outcome; apply the same behavior to
all corresponding conflict-update branches.
In `@src/lib/public-status/openapi.ts`:
- Around line 51-57: 同步更新 200 响应中的 filteredReady 示例模型,在其对象中补充必填字段
latestTtftMs,并使用与 publicStatusModelSchema.properties 中定义一致的类型和值格式,确保示例满足
publicStatusModelSchema.required 的约束。
In `@src/lib/session-snapshot/filesystem-store.ts`:
- Around line 84-129: 更新 enqueuePatch 中启动 start().then(() => drain()) 的异步链,为
start 失败增加 catch 处理。失败时移除对应队列任务、回滚 pendingBytes,并调用该任务的
completion;同时确保错误被消费,避免未处理拒绝及 keyTails 永久等待。
In `@src/lib/session-snapshot/store.ts`:
- Around line 80-111: 调整 getSessionSnapshotStore 与
reconfigureSessionSnapshotStore 的切换流程:不要在 target.start() 成功前更新 activeSelection 或
activeStore,也不要在后台重配置期间立即返回 resolveStore(selected);应继续返回当前 activeStore。仅当
target.start() 成功且目标仍有效时,再提交 activeSelection/activeStore 并停止其他 store,失败时保留当前可用
store,使后续配置变化仍能重试自愈。
In `@src/lib/utils/performance-formatter.ts`:
- Around line 56-67: Update both performance-formatting functions, including
shouldHideOutputRate and the rate calculation block, to reject non-finite
durationMs, outputTokens, and ttftMs values; require durationMs and outputTokens
to be positive and ttftMs to be non-negative. Preserve the existing null return
behavior for all invalid timing inputs and prevent NaN or incorrect rates from
being produced.
In `@tests/unit/proxy/session-ttfb-tfft.test.ts`:
- Around line 41-50: Update the test case around recordTtfb and recordTtft to
assert that both returned values and the corresponding session fields equal the
explicitly supplied elapsedMs values 200 and 800, rather than only comparing
each field with its returned value. Preserve the existing distinction between
TTFB and TTFT.
In `@tests/unit/proxy/stream-gate-content-gate.test.ts`:
- Around line 260-270: Update the newly added string literals in the
parameterized test around the openai-chat and gemini cases to use Biome’s
configured double-quote style, including the neutral and content values, without
changing their contents or test behavior.
In `@tests/unit/repository/message-session-readback.test.ts`:
- Around line 104-106: 更新 findMessageRequestBySessionId 的主表 messageRequest 查询
projection,补充选择 ttfbMs、ttftMs 和 timingSemanticsVersion,保持与 ledger fallback
的字段一致;同时增加主表 session 回读路径的断言,验证这三个新版计时字段能够正确返回。
---
Outside diff comments:
In `@src/app/`[locale]/dashboard/leaderboard/_components/leaderboard-view.tsx:
- Around line 344-370: Update the avgTtfbMs column cell renderer to evaluate
avgTtfbMs and avgTtftMs independently. Preserve the existing TTFB display when
valid, but render a Tooltip whenever either timing value is available so valid
row.avgTtftMs remains visible even when avgTtfbMs is null or non-positive; show
the unavailable text only for the individual missing value.
In `@src/repository/message.ts`:
- Around line 971-1010: Update the explicit select list in
findMessageRequestBySessionId to include messageRequest.ttfbMs,
messageRequest.ttftMs, and messageRequest.timingSemanticsVersion, matching
findMessageRequestById and the ledger-only fallback while preserving the
existing query behavior.
---
Nitpick comments:
In `@src/app/`[locale]/status/_lib/timeline-windows.ts:
- Around line 27-55: 提取一个按指标字段参数化的通用加权平均辅助函数,集中复用 computeAvgTtfb 和
computeAvgTtft 中的遍历、样本加权、空样本返回 null 及四舍五入逻辑;让两个公开函数仅传入对应的 ttfbMs 或 ttftMs
字段,并保持现有结果不变。
In `@src/app/v1/_lib/proxy/stream-gate/stream-content-gate.ts`:
- Around line 336-364: 为 createContentTimingObserver 中的增量 SSE 解析器设置固定的
maxBufferedCharacters 上限,避免未完成的超长行或多行 data 持续累积;同时定位并更新 shadow observer 中的
SseFrameParser 实例,使用相同上限配置,保持现有 SseFrameBufferLimitError 的终止处理行为。
In `@src/instrumentation.ts`:
- Around line 275-304: 提取一个共享的受控 interval tick 辅助函数,统一封装 stop-request 检查、current
promise 记录以及 finally 中清空 promise 的逻辑;然后让 startCacheEffectivenessScheduler 和
startReplayCleanupScheduler 通过该辅助函数仅提供各自的异步执行体、stopFlagKey 和
promiseKey。保持现有停止行为、并发保护、错误处理及调度结果不变,删除两处重复的 tick 实现。
In `@src/lib/utils/performance-formatter.test.ts`:
- Around line 5-21: 在 performance formatter 测试中补充负数和 NaN 的非法 TTFT 回归断言,验证
calculateOutputRate 对这些输入返回 null,确保无效速率不会进入仪表盘。
In `@src/repository/provider-cache-effectiveness.ts`:
- Around line 136-175: 提取一个内部 helper,复用 getProviderCacheCoefficients 与
getProviderModelCacheCoefficients 中相同的聚合查询、computeCoefficientBp
调用及结果组装逻辑;让两个公开函数仅传入各自的 groupBy 列并保留现有返回类型与筛选条件,确保 provider 与 provider-model
两种维度行为不变。
🪄 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: 4a5a40c6-207c-45ea-979e-fcadd9c9cc0a
📒 Files selected for processing (152)
.env.exampledeploy/k8s/README.mddeploy/k8s/app/deployment.yamldeploy/k8s/app/hpa.yamldocs/k8s-deployment.mddocs/performance-optimization-roadmap.mddrizzle/0116_lying_marvel_apes.sqldrizzle/meta/0114_snapshot.jsondrizzle/meta/0116_snapshot.jsondrizzle/meta/_journal.jsonmessages/en/dashboard.jsonmessages/en/settings/config.jsonmessages/en/settings/statusPage.jsonmessages/ja/dashboard.jsonmessages/ja/settings/config.jsonmessages/ja/settings/statusPage.jsonmessages/ru/dashboard.jsonmessages/ru/settings/config.jsonmessages/ru/settings/statusPage.jsonmessages/zh-CN/dashboard.jsonmessages/zh-CN/settings/config.jsonmessages/zh-CN/settings/statusPage.jsonmessages/zh-TW/dashboard.jsonmessages/zh-TW/settings/config.jsonmessages/zh-TW/settings/statusPage.jsonsrc/actions/public-status.tssrc/actions/system-config.tssrc/app/[locale]/dashboard/leaderboard/_components/leaderboard-view.tsxsrc/app/[locale]/dashboard/leaderboard/_components/success-rate-display.tssrc/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/[locale]/settings/config/_components/system-settings-form.tsxsrc/app/[locale]/settings/config/page.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/timeline-windows.tssrc/app/[locale]/status/page.tsxsrc/app/api/admin/system-config/route.tssrc/app/v1/_lib/proxy/forwarder.tssrc/app/v1/_lib/proxy/response-handler.tssrc/app/v1/_lib/proxy/session.tssrc/app/v1/_lib/proxy/stream-gate/stream-content-gate.tssrc/app/v1/_lib/proxy/warmup-guard.tssrc/drizzle/schema.tssrc/instrumentation.tssrc/lib/api-client/v1/openapi-types.gen.tssrc/lib/api/v1/schemas/system-config.tssrc/lib/cache-effectiveness/service.tssrc/lib/config/index.tssrc/lib/config/system-settings-cache.tssrc/lib/langfuse/emit-proxy-trace.tssrc/lib/langfuse/trace-proxy-request.tssrc/lib/ledger-backfill/service.tssrc/lib/ledger-backfill/trigger.sqlsrc/lib/lifecycle/shutdown.tssrc/lib/observability/discovery-metrics.tssrc/lib/price-sync/cloud-price-updater.tssrc/lib/provider-endpoints/leader-lock.tssrc/lib/provider-endpoints/probe-log-cleanup.tssrc/lib/provider-endpoints/probe-scheduler.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/leaderboard-cache.tssrc/lib/session-manager-detail-snapshots.test.tssrc/lib/session-manager.tssrc/lib/session-snapshot/filesystem-store.tssrc/lib/session-snapshot/store.tssrc/lib/session-snapshot/types.tssrc/lib/utils/performance-formatter.test.tssrc/lib/utils/performance-formatter.tssrc/lib/validation/schemas.tssrc/repository/_shared/transformers.tssrc/repository/leaderboard.tssrc/repository/message-write-buffer.tssrc/repository/message.tssrc/repository/provider-cache-effectiveness.tssrc/repository/provider-endpoints.tssrc/repository/system-config.tssrc/repository/usage-logs.tssrc/types/message.tssrc/types/provider.tssrc/types/routing-trace.tssrc/types/system-config.tstests/integration/proxy-hedge-lifecycle.test.tstests/integration/public-status/config-publish.test.tstests/unit/actions/system-config-fake-streaming-setting.test.tstests/unit/actions/system-config-non-chat-retry-setting.test.tstests/unit/actions/system-config-save.test.tstests/unit/actions/system-config-stream-gate-affinity-settings.test.tstests/unit/dashboard-logs-virtualized-special-settings-ui.test.tsxtests/unit/dashboard-logs-warmup-ui.test.tsxtests/unit/dashboard/leaderboard-success-rate-display.test.tstests/unit/dashboard/leaderboard-view-cache-coefficient.test.tsxtests/unit/error-details-dialog-warmup-ui.test.tsxtests/unit/k8s-deploy-assets-review-fixes.test.tstests/unit/langfuse/langfuse-trace.test.tstests/unit/lib/cache-effectiveness-service.test.tstests/unit/lib/config/system-settings-cache.test.tstests/unit/lib/filesystem-session-snapshot-store.test.tstests/unit/lib/performance-formatter-timing.test.tstests/unit/lib/provider-endpoints/leader-lock.test.tstests/unit/lib/provider-endpoints/probe-scheduler.test.tstests/unit/lib/session-snapshot-store.test.tstests/unit/price-sync/cloud-price-updater.test.tstests/unit/proxy/proxy-forwarder-hedge-first-byte.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-exported-finalizers.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/proxy/stream-gate-content-gate.test.tstests/unit/public-status/aggregation-core-tps.test.tstests/unit/public-status/aggregation.test.tstests/unit/public-status/read-store.test.tstests/unit/public-status/rollup-store.test.tstests/unit/public-status/system-config-publish.test.tstests/unit/redis/leaderboard-cache.test.tstests/unit/repository/leaderboard-cache-coefficient.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/provider-endpoints-probe-result.test.tstests/unit/repository/system-config-degradation-ladder.test.tstests/unit/repository/system-config-update-missing-columns.test.tstests/unit/repository/usage-logs-actual-response-model.test.tstests/unit/repository/usage-logs-sessionid-filter.test.tstests/unit/server-shutdown.test.tstests/unit/settings/system-settings-form-replay-cache-toggles.test.tsxvitest.config.ts
💤 Files with no reviewable changes (1)
- deploy/k8s/app/hpa.yaml
| volumeMounts: | ||
| - name: session-snapshots | ||
| mountPath: /var/lib/claude-code-hub/session-snapshots | ||
| volumes: | ||
| - name: session-snapshots | ||
| hostPath: | ||
| path: /var/lib/claude-code-hub/session-snapshots | ||
| type: DirectoryOrCreate |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
不要用 hostPath 作为多副本的共享快照存储。
hostPath 仅在单个节点本地持久化;副本被调度或重调度到不同节点后,Session 快照会丢失或无法读取,与 SESSION_SNAPSHOT_ROOT 的共享持久化契约不符。请改用共享 PVC(RWX),或在多节点部署时选择 Redis 快照后端。
🤖 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 `@deploy/k8s/app/deployment.yaml` around lines 138 - 145, Replace the
session-snapshots hostPath volume in the deployment’s volumeMounts/volumes
configuration with a shared RWX PVC, and configure the PVC-backed storage for
SESSION_SNAPSHOT_ROOT so replicas can access the same snapshots across nodes.
Alternatively, when multi-node deployment uses the Redis snapshot backend,
remove this local hostPath dependency and apply the corresponding Redis
configuration.
| > App 的 filesystem Session 快照不使用 PVC,而是挂载节点本地 | ||
| > `/var/lib/claude-code-hub/session-snapshots` hostPath。这个目录只在同一节点上的 Pod 间共享; | ||
| > 默认配置适用于单节点 k3s,或明确保证所有 App Pod 位于同一节点的部署。多节点集群应在系统设置中 | ||
| > 切换为 Redis,或自行提供真正的共享文件系统。PostgreSQL/Redis 仍使用 StorageClass/PVC。 | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
引用块间的空行触发 markdownlint MD028
第 57-60 行与 62-63 行是两个 > 引用块,中间用一个普通空行分隔,markdownlint-cli2 报告 MD028 (no-blanks-blockquote):不同渲染器对此处理不一致,有些会把它们合并为同一个引用块并保留内部空行,有些则视为两个独立块。
📝 建议修复
> 默认配置适用于单节点 k3s,或明确保证所有 App Pod 位于同一节点的部署。多节点集群应在系统设置中
> 切换为 Redis,或自行提供真正的共享文件系统。PostgreSQL/Redis 仍使用 StorageClass/PVC。
-
+>
> NodePort 回落模式下,`scripts/deploy-k8s.sh` 会自动跳过 `app/networkpolicy.yaml`,As per static analysis hints: [warning] 61-61: Blank line inside blockquote (MD028, no-blanks-blockquote)。
📝 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.
| > App 的 filesystem Session 快照不使用 PVC,而是挂载节点本地 | |
| > `/var/lib/claude-code-hub/session-snapshots` hostPath。这个目录只在同一节点上的 Pod 间共享; | |
| > 默认配置适用于单节点 k3s,或明确保证所有 App Pod 位于同一节点的部署。多节点集群应在系统设置中 | |
| > 切换为 Redis,或自行提供真正的共享文件系统。PostgreSQL/Redis 仍使用 StorageClass/PVC。 | |
| > App 的 filesystem Session 快照不使用 PVC,而是挂载节点本地 | |
| > `/var/lib/claude-code-hub/session-snapshots` hostPath。这个目录只在同一节点上的 Pod 间共享; | |
| > 默认配置适用于单节点 k3s,或明确保证所有 App Pod 位于同一节点的部署。多节点集群应在系统设置中 | |
| > 切换为 Redis,或自行提供真正的共享文件系统。PostgreSQL/Redis 仍使用 StorageClass/PVC。 | |
| > | |
| > NodePort 回落模式下,`scripts/deploy-k8s.sh` 会自动跳过 `app/networkpolicy.yaml`, |
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 61-61: Blank line inside blockquote
(MD028, no-blanks-blockquote)
🤖 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 `@deploy/k8s/README.md` around lines 57 - 61, Remove the blank line separating
the adjacent blockquote lines in the README so the entire note remains one
continuous blockquote and satisfies markdownlint MD028.
Source: Linters/SAST tools
| DO $migration$ | ||
| BEGIN | ||
| IF EXISTS ( | ||
| SELECT 1 | ||
| FROM information_schema.columns | ||
| WHERE table_schema = current_schema() | ||
| AND table_name = 'message_request' | ||
| AND column_name = 'first_byte_ms' | ||
| ) THEN | ||
| EXECUTE $sql$ | ||
| UPDATE "message_request" | ||
| SET | ||
| "ttft_ms" = "ttfb_ms", | ||
| "ttfb_ms" = "first_byte_ms", | ||
| "timing_semantics_version" = 2 | ||
| WHERE "first_byte_ms" IS NOT NULL | ||
| $sql$; | ||
| END IF; | ||
|
|
||
| IF EXISTS ( | ||
| SELECT 1 | ||
| FROM information_schema.columns | ||
| WHERE table_schema = current_schema() | ||
| AND table_name = 'usage_ledger' | ||
| AND column_name = 'first_byte_ms' | ||
| ) THEN | ||
| EXECUTE $sql$ | ||
| UPDATE "usage_ledger" | ||
| SET | ||
| "ttft_ms" = "ttfb_ms", | ||
| "ttfb_ms" = "first_byte_ms", | ||
| "timing_semantics_version" = 2 | ||
| WHERE "first_byte_ms" IS NOT NULL | ||
| $sql$; | ||
| END IF; | ||
| END | ||
| $migration$;--> statement-breakpoint |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -e sql . drizzle --max-depth 1 | sort
echo "---"
grep -rn "fn_upsert_usage_ledger\|ttfb_ms" drizzle/*.sql | grep -v "0116_lying_marvel_apes"Repository: ding113/claude-code-hub
Length of output: 7920
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "--- 0114 trigger definition ---"
sed -n '1,180p' drizzle/0114_overconfident_ronan.sql
echo "--- 0116 migration relevant section ---"
sed -n '1,230p' drizzle/0116_lying_marvel_apes.sql
echo "--- deterministic SQL target/update-of semantics probe from pg docs ---"
python3 - <<'PY'
print("PostgreSQL UPDATE OF target behavior: an AFTER UPDATE OF trigger fires when any listed column is mentioned in SET, INSERT, ON CONFLICT DO UPDATE, ON UPDATE SET regardless of whether value changes.")
PYRepository: ding113/claude-code-hub
Length of output: 14120
迁移回填顺序存在数据污染风险:usage_ledger.ttft_ms 会被旧触发器污染。
message_request 的 SET ... "ttfb_ms" = "first_byte_ms" 在 DO 块中执行于旧触发器(0114)仍生效期间;旧触发器监听 ttfb_ms,并将 INSERT ... ON CONFLICT ... SET ttfb_ms = EXCLUDED.ttfb_ms 同步到 usage_ledger.ttfb_ms,覆盖了其原有 TTFB。随后 usage_ledger 自身的回填再执行 "ttft_ms" = "ttfb_ms",会让这部分历史行的 usage_ledger.ttft_ms 与 usage_ledger.ttfb_ms 都等于原 first_byte_ms,破坏预期的一致性。
建议在 message_request 交换 UPDATE 前禁用 message_request 上的 trg_upsert_usage_ledger,或在 DO 块前先 DROP TRIGGER IF EXISTS trg_upsert_usage_ledger ON message_request;迁移完成后再 ENABLE / CREATE 新触发器。
🤖 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 `@drizzle/0116_lying_marvel_apes.sql` around lines 13 - 49, 调整迁移 DO 块中
message_request 的回填顺序,避免旧触发器 trg_upsert_usage_ledger 在更新 ttfb_ms 时污染
usage_ledger.ttfb_ms:在 message_request
回填前禁用该触发器(或先删除),完成两个表的回填后恢复或创建迁移所需的新触发器,并确保触发器状态最终正确。
| "segmentTotal": "总计", | ||
| "ttfb": "TTFB", | ||
| "ttft": "TTFT", | ||
| "firstByteToFirstToken": "响应头到首个有效内容", |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
统一修正 firstByteToFirstToken 的起点文案。
该字段表示从首字节(TTFB)到首个有效内容(TTFT),不应表述为从响应头开始。
messages/zh-CN/dashboard.json#L366-L366: 改为“首字节到首个有效内容”。messages/ru/dashboard.json#L366-L366: 改为“От первого байта до первого валидного содержимого”。messages/zh-TW/dashboard.json#L366-L366: 改为“首字節到首個有效內容”。
📍 Affects 3 files
messages/zh-CN/dashboard.json#L366-L366(this comment)messages/ru/dashboard.json#L366-L366messages/zh-TW/dashboard.json#L366-L366
🤖 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 `@messages/zh-CN/dashboard.json` at line 366, 统一更新 firstByteToFirstToken 文案:在
messages/zh-CN/dashboard.json 第366行改为“首字节到首个有效内容”,在 messages/ru/dashboard.json
第366行改为“От первого байта до первого валидного содержимого”,在
messages/zh-TW/dashboard.json 第366行改为“首字節到首個有效內容”。
| primeSystemSettingsCache(updated); | ||
| if (validated.sessionSnapshotStore !== undefined) { | ||
| const { reconfigureSessionSnapshotStore } = await import("@/lib/session-snapshot/store"); | ||
| await reconfigureSessionSnapshotStore(updated.sessionSnapshotStore).catch((error) => { | ||
| logger.warn("[SystemSettings] Failed to reconfigure session snapshot store", { error }); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP --type=ts -C4 '\bsessionSnapshotStore\b' -g '!tests/**'
rg -nP --type=ts -C3 'reconfigureSessionSnapshotStore'Repository: ding113/claude-code-hub
Length of output: 161
🏁 Script executed:
#!/bin/bash
set -u
echo "== repo files =="
git ls-files | sed -n '1,120p'
echo "== target file =="
if [ -f src/actions/system-config.ts ]; then
wc -l src/actions/system-config.ts
sed -n '180,245p' src/actions/system-config.ts | cat -n -v
else
fd -a 'system-config\.ts$' .
fi
echo "== broad symbol search without file filter =="
rg -n -C 3 'sessionSnapshotStore|reconfigureSessionSnapshotStore|primeSystemSettingsCache|publicStatusProjectionWarningCode' . || trueRepository: ding113/claude-code-hub
Length of output: 50379
🏁 Script executed:
#!/bin/bash
set -u
echo "== src/actions/system-config.ts relevant lines =="
sed -n '32,140p;215,310p' src/actions/system-config.ts | cat -n
echo "== src/app/api/admin/system-config/route.ts relevant lines =="
sed -n '1,180p' src/app/api/admin/system-config/route.ts | cat -n
echo "== runtime session snapshot store implementation =="
sed -n '1,150p' src/lib/session-snapshot/store.ts | cat -n
echo "== return schemas relevant =="
sed -n '250,270p' src/lib/api/v1/schemas/system-config.ts | cat -n
sed -n '1,90p' src/lib/api-client/v1/actions/system-config.ts | cat -n
echo "== saveSystemSettings callers in repo (non-test) =="
rg -n -C 2 'saveSystemSettings|updateSystemSettings|/api/v1/system/settings|/api/admin/system-config|/api/admin/system' src --glob '!**/*test*' || trueRepository: ding113/claude-code-hub
Length of output: 35943
把 sessionSnapshotStore 重配置失败转为可观测告警,避免配置已落库但运行时仍使用旧快照存储。
primeSystemSettingsCache(updated) 先提交 DB/内存缓存为新 store;reconfigureSessionSnapshotStore() 失败会只输出 warn,随后 actions/system-config.ts 仍返回 { ok: true, data: updated }。参考 publicStatusProjectionWarningCode 的处理路径,将失败码返回给前端,让管理员知道保存结果需要修复环境后重试。
🤖 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/actions/system-config.ts` around lines 219 - 225, 更新 system settings
保存流程中 reconfigureSessionSnapshotStore 的失败处理:不要仅记录 warn 后继续返回普通的 { ok: true,
data: updated },应沿用 publicStatusProjectionWarningCode
的处理路径,将可供前端识别的失败/告警码附加到成功响应中。保留数据库与缓存已更新的行为,并确保成功重配置时响应保持现有结果。
| export function getSessionSnapshotStore(): SessionSnapshotStore { | ||
| if (shuttingDown) return disabledStore; | ||
| const selected = getCachedSystemSettingsOnlyCache()?.sessionSnapshotStore ?? activeSelection; | ||
| if (selected !== activeSelection) { | ||
| void reconfigureSessionSnapshotStore(selected).catch((error) => { | ||
| logger.warn("[SessionSnapshot] Runtime store reconfiguration failed", { error }); | ||
| }); | ||
| return resolveStore(selected); | ||
| } | ||
| return activeStore; | ||
| } | ||
|
|
||
| export function reconfigureSessionSnapshotStore( | ||
| selected: SessionSnapshotStoreSetting | ||
| ): Promise<void> { | ||
| if (shuttingDown) return Promise.resolve(); | ||
|
|
||
| const target = resolveStore(selected); | ||
| activeSelection = selected; | ||
| activeStore = target; | ||
| const previousTransition = reconfigurePromise.catch(() => undefined); | ||
| reconfigurePromise = previousTransition.then(async () => { | ||
| await target.start(); | ||
| if (activeStore !== target) return; | ||
|
|
||
| const inactiveStores = [filesystemStore, redisStore, disabledStore].filter( | ||
| (store) => store !== target | ||
| ); | ||
| await Promise.all(inactiveStores.map((store) => store.stop())); | ||
| }); | ||
| return reconfigurePromise; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)session-snapshot|session-snapshot-store.test|filesystem-store|redis-store|disabled-.*snap' || true
echo "== store outline =="
ast-grep outline src/lib/session-snapshot/store.ts --view compact || true
echo "== store code =="
cat -n src/lib/session-snapshot/store.ts
echo "== tests around store =="
for f in $(git ls-files | rg 'session-snapshot-store\.test\.ts$|session-snapshot.*test\.ts$'); do
echo "--- $f"
wc -l "$f"
cat -n "$f"
done
echo "== filesystem store outline/code relevant =="
for f in $(git ls-files | rg 'filesystem-store\.ts$'); do
echo "--- $f"
wc -l "$f"
ast-grep outline "$f" --view compact || true
cat -n "$f"
done
echo "== redis store outline/code relevant =="
for f in $(git ls-files | rg 'redis.*store.*\.ts$|redis-session.*\.ts$'); do
echo "--- $f"
wc -l "$f"
ast-grep outline "$f" --view compact || true
rg -n "class|start|stop|enqueue|get|active|activeStore|store" "$f"
doneRepository: ding113/claude-code-hub
Length of output: 47369
切换 store 前不要提前提交配置状态
getSessionSnapshotStore() 检测到配置变更时立即返回 resolveStore(selected),并异步启动新 store;如果 target.start() 失败,activeSelection/activeStore 仍然指向失败目标,后面会继续对该失败 store 执行 enqueuePatch()(文件系统实现会先执行 start(),发现非目录则 accepting = false,直接返回 false 丢弃快照),而 disabled 路径同样依赖一次配置/设置变化才切换到可用的 store。
建议:仅在 target.start() 成功后再更新 activeSelection/activeStore;在后台重新配置期间继续返回当前仍在使用的 store,这样失败后下次配置变更仍可自愈。
🤖 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/session-snapshot/store.ts` around lines 80 - 111, 调整
getSessionSnapshotStore 与 reconfigureSessionSnapshotStore 的切换流程:不要在
target.start() 成功前更新 activeSelection 或 activeStore,也不要在后台重配置期间立即返回
resolveStore(selected);应继续返回当前 activeStore。仅当 target.start() 成功且目标仍有效时,再提交
activeSelection/activeStore 并停止其他 store,失败时保留当前可用 store,使后续配置变化仍能重试自愈。
| if ( | ||
| outputTokens == null || | ||
| outputTokens <= 0 || | ||
| durationMs == null || | ||
| durationMs <= 0 || | ||
| ttftMs == null | ||
| ) { | ||
| return null; | ||
| } | ||
| if (firstByteMs == null) return null; | ||
| const generationTimeMs = durationMs - firstByteMs; | ||
| const generationTimeMs = durationMs - ttftMs; | ||
| if (generationTimeMs <= 0) return null; | ||
| return outputTokens / (generationTimeMs / 1000); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
拒绝非法计时值,避免输出 NaN 或错误速率。
当前检查无法拦截 NaN 和负数 ttftMs:ttftMs = NaN 会使函数返回 NaN,负数则会扩大生成窗口。请在两个函数中校验 Number.isFinite(...),并要求 ttftMs >= 0;同时对 durationMs 和 outputTokens 做有限数校验。
建议修复
if (
outputTokens == null ||
+ !Number.isFinite(outputTokens) ||
outputTokens <= 0 ||
durationMs == null ||
+ !Number.isFinite(durationMs) ||
durationMs <= 0 ||
- ttftMs == null
+ ttftMs == null ||
+ !Number.isFinite(ttftMs) ||
+ ttftMs < 0
) {对 shouldHideOutputRate 同样增加 durationMs 和 ttftMs 的有限数、非负校验。
Also applies to: 80-92
🤖 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/utils/performance-formatter.ts` around lines 56 - 67, Update both
performance-formatting functions, including shouldHideOutputRate and the rate
calculation block, to reject non-finite durationMs, outputTokens, and ttftMs
values; require durationMs and outputTokens to be positive and ttftMs to be
non-negative. Preserve the existing null return behavior for all invalid timing
inputs and prevent NaN or incorrect rates from being produced.
| describe("ProxySession TTFB / TTFT", () => { | ||
| it("分别记录响应头与首个有效内容耗时", () => { | ||
| const session = createSession(Date.now() - 1_200); | ||
|
|
||
| const tfft = session.recordTfft(); | ||
| const ttfb = session.recordTtfb(200); | ||
| const ttft = session.recordTtft(800); | ||
|
|
||
| expect(session.tfftMs).toBe(tfft); | ||
| expect(session.firstByteMs).toBe(tfft); | ||
| expect(session.ttfbMs).toBe(ttfb); | ||
| expect(session.ttftMs).toBe(ttft); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
首个用例未验证传入的 elapsedMs 是否被实际使用。
该用例只断言 session.ttfbMs === ttfb(ttfb 就是 recordTtfb 刚返回并存入的值,属于同义反复),并未断言 ttfb/ttft 等于传入的 200/800。若 recordTtfb/recordTtft 忽略了 elapsedMs 参数、退化为始终用 Date.now() - startTime 计算,本用例仍会通过,未能覆盖用例标题所声明的"分别记录响应头与首个有效内容耗时"这一核心契约。
✅ 建议补充具体数值断言
const ttfb = session.recordTtfb(200);
const ttft = session.recordTtft(800);
+ expect(ttfb).toBe(200);
+ expect(ttft).toBe(800);
expect(session.ttfbMs).toBe(ttfb);
expect(session.ttftMs).toBe(ttft);📝 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.
| describe("ProxySession TTFB / TTFT", () => { | |
| it("分别记录响应头与首个有效内容耗时", () => { | |
| const session = createSession(Date.now() - 1_200); | |
| const tfft = session.recordTfft(); | |
| const ttfb = session.recordTtfb(200); | |
| const ttft = session.recordTtft(800); | |
| expect(session.tfftMs).toBe(tfft); | |
| expect(session.firstByteMs).toBe(tfft); | |
| expect(session.ttfbMs).toBe(ttfb); | |
| expect(session.ttftMs).toBe(ttft); | |
| }); | |
| describe("ProxySession TTFB / TTFT", () => { | |
| it("分别记录响应头与首个有效内容耗时", () => { | |
| const session = createSession(Date.now() - 1_200); | |
| const ttfb = session.recordTtfb(200); | |
| const ttft = session.recordTtft(800); | |
| expect(ttfb).toBe(200); | |
| expect(ttft).toBe(800); | |
| expect(session.ttfbMs).toBe(ttfb); | |
| expect(session.ttftMs).toBe(ttft); | |
| }); |
🤖 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/proxy/session-ttfb-tfft.test.ts` around lines 41 - 50, Update the
test case around recordTtfb and recordTtft to assert that both returned values
and the corresponding session fields equal the explicitly supplied elapsedMs
values 200 and 800, rather than only comparing each field with its returned
value. Preserve the existing distinction between TTFB and TTFT.
| it.each([ | ||
| { | ||
| family: "openai-chat" as const, | ||
| neutral: 'data: {"choices":[{"delta":{"role":"assistant"}}]}\n\n', | ||
| content: 'data: {"choices":[{"delta":{"content":"hello"}}]}\n\n', | ||
| }, | ||
| { | ||
| family: "gemini" as const, | ||
| neutral: 'data: {"usageMetadata":{"totalTokenCount":1}}\n\n', | ||
| content: 'data: {"candidates":[{"content":{"parts":[{"text":"hello"}]}}]}\n\n', | ||
| }, |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
使用 Biome 的双引号格式。
第 263、264、268、269 行新增字符串使用单引号,和仓库的双引号配置不一致。
建议修改
- neutral: 'data: {"choices":[{"delta":{"role":"assistant"}}]}\n\n',
- content: 'data: {"choices":[{"delta":{"content":"hello"}}]}\n\n',
+ neutral: "data: {\"choices\":[{\"delta\":{\"role\":\"assistant\"}}]}\n\n",
+ content: "data: {\"choices\":[{\"delta\":{\"content\":\"hello\"}}]}\n\n",
...
- neutral: 'data: {"usageMetadata":{"totalTokenCount":1}}\n\n',
- content: 'data: {"candidates":[{"content":{"parts":[{"text":"hello"}]}}]}\n\n',
+ neutral: "data: {\"usageMetadata\":{\"totalTokenCount\":1}}\n\n",
+ content: "data: {\"candidates\":[{\"content\":{\"parts\":[{\"text\":\"hello\"}]}}]}\n\n",As per coding guidelines, **/*.{ts,tsx,js,jsx} must use Biome formatting with double quotes.
📝 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.
| it.each([ | |
| { | |
| family: "openai-chat" as const, | |
| neutral: 'data: {"choices":[{"delta":{"role":"assistant"}}]}\n\n', | |
| content: 'data: {"choices":[{"delta":{"content":"hello"}}]}\n\n', | |
| }, | |
| { | |
| family: "gemini" as const, | |
| neutral: 'data: {"usageMetadata":{"totalTokenCount":1}}\n\n', | |
| content: 'data: {"candidates":[{"content":{"parts":[{"text":"hello"}]}}]}\n\n', | |
| }, | |
| it.each([ | |
| { | |
| family: "openai-chat" as const, | |
| neutral: "data: {\"choices\":[{\"delta\":{\"role\":\"assistant\"}}]}\n\n", | |
| content: "data: {\"choices\":[{\"delta\":{\"content\":\"hello\"}}]}\n\n", | |
| }, | |
| { | |
| family: "gemini" as const, | |
| neutral: "data: {\"usageMetadata\":{\"totalTokenCount\":1}}\n\n", | |
| content: "data: {\"candidates\":[{\"content\":{\"parts\":[{\"text\":\"hello\"}]}}]}\n\n", | |
| }, |
🤖 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/proxy/stream-gate-content-gate.test.ts` around lines 260 - 270,
Update the newly added string literals in the parameterized test around the
openai-chat and gemini cases to use Biome’s configured double-quote style,
including the neutral and content values, without changing their contents or
test behavior.
Source: Coding guidelines
| ttfbMs: 100, | ||
| ttftMs: 200, | ||
| timingSemanticsVersion: 2, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
补齐主表 session 回读的新版计时字段。
findMessageRequestBySessionId 的 messageRequest 查询仍未选择 ttfbMs、ttftMs 和 timingSemanticsVersion,但 ledger fallback 已选择它们。普通主表 session 回读会丢失这些值;请在主表 projection 中补齐字段,并增加主表路径断言。
🤖 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-session-readback.test.ts` around lines 104 -
106, 更新 findMessageRequestBySessionId 的主表 messageRequest 查询 projection,补充选择
ttfbMs、ttftMs 和 timingSemanticsVersion,保持与 ledger fallback 的字段一致;同时增加主表 session
回读路径的断言,验证这三个新版计时字段能够正确返回。
| return result; | ||
| } | ||
|
|
||
| async start(): Promise<void> {} |
There was a problem hiding this comment.
[HIGH] [ERROR-SILENT] Redis snapshot backend can be enabled while still unusable
Why this is a problem: The new async start(): Promise<void> {} never validates Redis availability, but reconfigureSessionSnapshotStore("redis") treats it as a successful transition. If Redis is disconnected, later enqueuePatch() / get() calls just return false / null, so the settings update appears to work while request and response snapshots are silently dropped.
Suggested fix:
async start(): Promise<void> {
const redis = getRedisClient();
if (redis?.status !== "ready") {
throw new Error("Redis session snapshot store is not ready");
}
}| if (shuttingDown) return Promise.resolve(); | ||
|
|
||
| const target = resolveStore(selected); | ||
| activeSelection = selected; |
There was a problem hiding this comment.
[HIGH] [LOGIC-BUG] Reconfiguration swaps the active store before startup succeeds
Why this is a problem: This new sequence assigns activeSelection = selected and activeStore = target before await target.start(). If start() rejects (for example, the filesystem root is invalid), the process is left pointing at the failed backend even when the previous backend was healthy. Callers only log the reconfiguration failure, so subsequent snapshot reads and writes keep using a broken store until another manual change or restart.
Suggested fix:
const previousSelection = activeSelection;
const previousStore = activeStore;
const previousTransition = reconfigurePromise.catch(() => undefined);
reconfigurePromise = previousTransition.then(async () => {
await target.start();
activeSelection = selected;
activeStore = target;
const inactiveStores = [filesystemStore, redisStore, disabledStore].filter(
(store) => store !== target
);
await Promise.all(inactiveStores.map((store) => store.stop()));
}).catch((error) => {
activeSelection = previousSelection;
activeStore = previousStore;
throw error;
});There was a problem hiding this comment.
Code Review Summary
This PR is very large and spans runtime snapshot storage, timing metrics, schedulers, migrations, and UI/status surfaces. I validated the highest-risk new code paths against full-file context and found two high-priority issues in the new session snapshot backend switching flow. Because the change set crosses several subsystems, it is difficult to reason about rollout risk as a single unit.
PR Size: XL
- Lines changed: 10140
- Files changed: 152
- Split suggestions:
- Separate timing semantics and proxy persistence changes from storage backend changes
- Separate session snapshot backend/runtime reconfiguration from scheduler and Kubernetes tuning
- Separate leaderboard/public-status reporting changes from migration and instrumentation work
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 1 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 1 | 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/lib/session-snapshot/store.ts:55—RedisSessionSnapshotStore.start()is a no-op, so switching the setting to Redis can succeed even when Redis is unavailable and snapshots will then be silently dropped.src/lib/session-snapshot/store.ts:98—reconfigureSessionSnapshotStore()swapsactiveSelection/activeStorebeforetarget.start()succeeds, so a failed backend transition can strand the process on a dead store.
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.
Posted the PR review for #1371.
- Applied the size label
size/XL. - Added 2 inline review comments on
src/lib/session-snapshot/store.ts:55andsrc/lib/session-snapshot/store.ts:98. - Submitted the required summary review with XL split suggestions and the issue matrix.
- Reported findings:
src/lib/session-snapshot/store.ts:55— Redis backend activation succeeds without validating Redis readiness, which can silently drop snapshots.src/lib/session-snapshot/store.ts:98— runtime reconfiguration swaps the active backend before startup succeeds, which can leave the process pinned to a broken store.
If you want, I can also export the posted review text into a local markdown artifact for the workflow logs.
Summary
Timing Semantics
ttfbMs: final winner HTTP response-header latencyttftMs: final winner first protocol-valid content latencytimingSemanticsVersion = 2outputTokens / ((durationMs - ttftMs) / 1000)Session Snapshot Storage
Multi-Replica Work
Migration
0116_lying_marvel_apes0115_breezy_polarisfirst_byte_msand legacyttfb_msvalues into v2 timing columns before droppingfirst_byte_msVerification
bun run lintbun run typecheckbun run validate:migrations(118 migrations)bun run openapi:checkbun run openapi:lintbun run i18n:audit-messages-no-emoji:failbun run buildDeferred
Greptile Summary
This PR revises gateway timing semantics and reduces replicated background and storage amplification.
Confidence Score: 4/5
The snapshot shutdown path should be fixed before merging because it can acknowledge store termination while accepted writes remain unfinished.
The timing, migration, and distributed aggregation changes are internally coordinated, but filesystem snapshot shutdown has a fixed five-second escape that can lose recently queued session details during process exit or backend replacement.
Files Needing Attention: src/lib/session-snapshot/filesystem-store.ts
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart LR Request[Proxy request] --> Winner[Final provider winner] Winner --> TTFB[Record response-header TTFB] Winner --> Stream[Process response content] Stream --> TTFT[Record protocol-valid-content TTFT] TTFB --> Ledger[(Message and usage ledger)] TTFT --> Ledger Request --> Queue[Bounded snapshot queue] Queue --> Gzip[Gzip and atomic filesystem write] Gzip --> Snapshot[(Session snapshot files)] Ledger --> Aggregation[Leader-locked aggregation] Aggregation --> Dashboard[Leaderboard and public status]Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "perf: reduce gateway amplification and c..." | Re-trigger Greptile
Context used (5)