feat: gateway feature follow-ups — runtime toggles, gate hardening, affinity/replay observability#1360
Conversation
Add replay_enabled and cache_effectiveness_enabled columns to system_settings so operators can flip these features without redeploying. Database values override the ENABLE_REQUEST_REPLAY and ENABLE_CACHE_EFFECTIVENESS environment variables; null defers to the environment default. Replay identity derivation, spool creation, and the cache effectiveness scheduler all consume the runtime toggle. The settings form exposes both switches with descriptive labels across all supported locales, and the degradation ladder covers the new columns for databases that have not yet migrated.
Add per-read idle timeout to the stream content gate so silent upstream connections fail fast with a 524 streaming_idle_timeout error, matching post-commit behavior. Exclude openai-responses lifecycle echo frames (response.created, response.in_progress, response.queued) from the prebuffer byte cap so large request bodies reflected in the first SSE event do not trigger false overflow. Capture commit markers (frame/chunk index, event name, buffered bytes, gate wait) for observability when not in high-concurrency mode. Invoke an onFirstByte callback on the first non-empty upstream chunk to preserve first-byte timeout semantics. Raise the default byte cap from 256 KB to 10 MB and exempt SSE event views from line and character limits in CodeDisplay so large echo frames render correctly.
Remove the F_sys fingerprint from the longest-prefix lookup sequence and winner write-back. System-prompt-only matches caused over-broad cross-conversation stickiness, binding unrelated dialogs to the same upstream. Only conversation-message boundaries participate in affinity matching now. Drop the now-unused tier field from AffinityHint. Record matched boundary details (depth, prefix bytes, fingerprint) on the provider chain item for observability. Suppress duplicate initial_selection entries when the provider was already nominated by session reuse or affinity hit.
Replace the Codex-only reasoning effort component with a unified ThinkingEffortDisplay that extracts effort from both Codex reasoning.effort and Anthropic output_config.effort audit entries. Codex takes priority when both are present. Make the reasoning effort column toggleable in the logs table instead of always visible. Column visibility state persists per user and table.
Visualize prefix affinity hits in the logic trace timeline and provider chain popover with matched depth, prefix bytes, and fingerprint. Display stream gate commit markers (trigger frame, buffered bytes, gate wait) on committed chain entries. Add a replay serve info panel showing when a request was served from cache without an upstream call. Add i18n strings for all new UI elements across en, ja, ru, zh-CN, and zh-TW locales.
Delete the Codex-only reasoning effort display component and its test. The component was replaced by the unified ThinkingEffortDisplay that handles both Codex and Anthropic effort audit entries.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
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
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 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 |
| // neutral: 继续缓冲;请求回显帧的载荷不计入字节上限(内存仍占用,由回显体积自然有界) | ||
| if (isRequestEchoFrame(options.family, frame.eventName, frame.data)) { | ||
| echoExcludedBytes += Buffer.byteLength(frame.data, "utf8"); |
There was a problem hiding this comment.
Echo payloads bypass buffer cap
When a large Responses request is reflected in a pre-content lifecycle frame, the gate retains the entire chunk but subtracts the echo payload from its byte-cap calculation, allowing one stream to retain far more than the configured 10 MiB and concurrent requests to exhaust gateway memory.
Knowledge Base Used: Proxy request pipeline
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/v1/_lib/proxy/stream-gate/stream-content-gate.ts
Line: 261-263
Comment:
**Echo payloads bypass buffer cap**
When a large Responses request is reflected in a pre-content lifecycle frame, the gate retains the entire chunk but subtracts the echo payload from its byte-cap calculation, allowing one stream to retain far more than the configured 10 MiB and concurrent requests to exhaust gateway memory.
**Knowledge Base Used:** [Proxy request pipeline](https://app.greptile.com/ygxz/-/custom-context/knowledge-base/ding113/claude-code-hub/-/docs/proxy-pipeline.md)
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Fixed in d8fe85e: 豁免额度现以 prebufferByteCap 为自身上限(bufferedBytes - min(echoExcluded, cap) > cap),单流缓冲总量被硬性压在 2×cap 内,伪装回显帧无法再撑爆内存。已补单测(echo flood 必溢出 + 额度内豁免仍生效)。
| export function getCachedProxyRuntimeSettings(): ProxyRuntimeSettings | null { | ||
| return lastKnown; | ||
| } | ||
|
|
||
| /** F3b 有效开关(同步):系统设置覆写优先,无快照时跟随 env。 */ | ||
| export function isCacheEffectivenessEnabled(): boolean { | ||
| return lastKnown?.cacheEffectivenessEnabled ?? envCacheEffectivenessDefault(); |
There was a problem hiding this comment.
Runtime toggle snapshot stays stale
When an administrator changes either new toggle after lastKnown is populated, saving clears the system-settings cache but not this snapshot, so replay and cache-effectiveness hot paths continue applying the previous setting until a later asynchronous refresh.
Knowledge Base Used:
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/system-settings/proxy-runtime.ts
Line: 84-90
Comment:
**Runtime toggle snapshot stays stale**
When an administrator changes either new toggle after `lastKnown` is populated, saving clears the system-settings cache but not this snapshot, so replay and cache-effectiveness hot paths continue applying the previous setting until a later asynchronous refresh.
**Knowledge Base Used:**
- [Database Schema & Repository Layer](https://app.greptile.com/ygxz/-/custom-context/knowledge-base/ding113/claude-code-hub/-/docs/database-schema.md)
- [Proxy request pipeline](https://app.greptile.com/ygxz/-/custom-context/knowledge-base/ding113/claude-code-hub/-/docs/proxy-pipeline.md)
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
Fixed in 4f89df8: replay guard ensure() 每请求先 await getProxyRuntimeSettings() 刷新快照(底层系统设置缓存保存时即失效,常态为缓存命中);cache-effectiveness 调度器 tick 同步改为异步刷新。两处热路径不再依赖请求路径保鲜。
🧪 测试结果
总体结果: ✅ 所有测试通过 |
| initialSettings.affinityIgnoreClientSessionId | ||
| ); | ||
| // null = 尚未覆写(跟随环境变量默认:Replay 关 / 缓存模拟开);保存后写显式值 | ||
| const [replayEnabled, setReplayEnabled] = useState(initialSettings.replayEnabled ?? false); |
There was a problem hiding this comment.
[HIGH] [LOGIC-BUG] Nullable overrides are coerced to concrete booleans on load
Why this is a problem: replayEnabled and cacheEffectivenessEnabled are defined as boolean | null, where null means "follow the environment variable". Coercing null to false/true here, then always sending those fields in the save payload, means saving any unrelated setting silently replaces the inherited state with an explicit override. The same coercion on the success path also makes it impossible to return the UI to null after an override has been saved.
Suggested fix:
type OverrideState = "inherit" | "enabled" | "disabled";
const [replayEnabled, setReplayEnabled] = useState<OverrideState>(
initialSettings.replayEnabled == null
? "inherit"
: initialSettings.replayEnabled
? "enabled"
: "disabled"
);
const replayEnabledPayload =
replayEnabled === "inherit" ? null : replayEnabled === "enabled";Use the same tri-state mapping for cacheEffectivenessEnabled so the form can round-trip null instead of destroying the env-fallback state.
There was a problem hiding this comment.
Fixed in 620e401: state 保留 boolean | null,未触碰的开关保存时原样传 null(跟随环境变量),仅用户切换后才落显式覆写;checked 仅在展示层回退 env 默认值。新增单测 system-settings-form-replay-cache-toggles 覆盖 null round-trip 与显式覆写两条路径。未引入三态下拉——展示简洁性优先,持久化语义已完整保留。
| /** F2 有效开关:系统设置覆写优先(同步快照),null/无快照时跟随 env。 */ | ||
| export function isReplayEnabled(): boolean { | ||
| try { | ||
| return getCachedProxyRuntimeSettings()?.replayEnabled ?? getEnvConfig().ENABLE_REQUEST_REPLAY; |
There was a problem hiding this comment.
[HIGH] [LOGIC-BUG] Replay attach reads a stale runtime flag before the provider step refreshes it
Why this is a problem: replayAttach runs before the provider guard, but getProxyRuntimeSettings() is only refreshed later in provider-selector.ts. After an admin changes replayEnabled, the first request can still see the old lastKnown value here and either miss replay when it should attach, or attach when replay was just disabled.
Suggested fix:
import { getProxyRuntimeSettings } from "@/lib/system-settings/proxy-runtime";
export async function deriveReplayIdentity(session: ProxySession): Promise<ReplayIdentity | null> {
const { replayEnabled } = await getProxyRuntimeSettings();
if (!replayEnabled) return null;
// existing identity derivation...
}If you want to keep the helper synchronous elsewhere, move the effective-flag check into ProxyReplayGuard.ensure() and await getProxyRuntimeSettings() there before calling the pure identity builder.
There was a problem hiding this comment.
Fixed in 4f89df8: ProxyReplayGuard.ensure() 在 deriveReplayIdentity 之前 await getProxyRuntimeSettings(),管理端保存后的首个请求即读到新值;identity 推导保持同步纯函数。
| const intervalMs = 5 * 60 * 1000; | ||
|
|
||
| instrumentationState.__CCH_CACHE_EFFECTIVENESS_INTERVAL_ID__ = setInterval(() => { | ||
| if (!isCacheEffectivenessEnabled()) return; |
There was a problem hiding this comment.
[MEDIUM] [LOGIC-BUG] The scheduler checks a stale snapshot, so DB toggle changes do not take effect reliably
Why this is a problem: isCacheEffectivenessEnabled() only reads the in-memory lastKnown snapshot or env fallback. That snapshot is not invalidated when system settings are saved, and this scheduler never calls getProxyRuntimeSettings() itself. In a low-traffic deployment, toggling cacheEffectivenessEnabled in the UI can leave aggregation permanently on or off until some later request happens to refresh runtime settings.
Suggested fix:
const { getProxyRuntimeSettings } = await import("@/lib/system-settings/proxy-runtime");
instrumentationState.__CCH_CACHE_EFFECTIVENESS_INTERVAL_ID__ = setInterval(() => {
void (async () => {
const { cacheEffectivenessEnabled } = await getProxyRuntimeSettings();
if (!cacheEffectivenessEnabled) return;
await aggregateCacheEffectiveness();
})().catch((error) => {
logger.warn("[Instrumentation] Cache effectiveness aggregation tick failed", {
error: error instanceof Error ? error.message : String(error),
});
});
}, intervalMs);That keeps the background worker aligned with the DB-backed runtime toggle instead of waiting for a request path to refresh lastKnown.
There was a problem hiding this comment.
Fixed in 4f89df8: tick 改为 await getProxyRuntimeSettings() 后判断 cacheEffectivenessEnabled,零流量实例的 DB 开关切换也能在下一 tick 生效;错误仍仅记日志。
There was a problem hiding this comment.
Code Review Summary
This XL PR spans proxy-runtime behavior, system-settings plumbing, and dashboard observability in one stack. I found three issues in the new runtime-toggle paths: one UI round-trip bug and two places where the new DB-backed flags are read from stale in-memory state, so admin changes do not take effect reliably.
PR Size: XL
- Lines changed: 7037
- Files changed: 73
- Split suggestions: separate (1) schema/API/runtime-toggle plumbing, (2) replay/affinity/stream-gate proxy behavior, and (3) dashboard/i18n/logic-trace UI so regressions are easier to isolate.
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)
src/app/[locale]/settings/config/_components/system-settings-form.tsx:218— [HIGH] [LOGIC-BUG] The new replay/cache switches coerce nullable overrides to booleans, so saving unrelated settings destroys the "follow env" state.src/app/v1/_lib/proxy/replay/replay-identity.ts:42— [HIGH] [LOGIC-BUG] Replay attach now readsreplayEnabledfrom a stale sync snapshot before the provider step refreshes it, so the first request after an admin toggle can still use the old behavior.src/instrumentation.ts:271— [MEDIUM] [LOGIC-BUG] The cache-effectiveness scheduler checks only the stale sync snapshot, so toggling the DB-backed flag in a low-traffic deployment will not start/stop aggregation until a later request refreshes runtime settings.
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.
Actionable comments posted: 10
🧹 Nitpick comments (3)
src/app/v1/_lib/proxy/replay/replay-spool.ts (1)
4-4: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win统一使用
@/路径别名导入本地模块。当前改动仍使用
./replay-identity,请改为@/app/v1/_lib/proxy/replay/replay-identity。As per coding guidelines: TypeScript 文件应使用
@/映射到./src/的路径别名。🤖 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/replay/replay-spool.ts` at line 4, Update the import of isReplayEnabled and ReplayIdentity in replay-spool.ts to use the `@/app/v1/_lib/proxy/replay/replay-identity` alias instead of the relative ./replay-identity path.Source: Coding guidelines
tests/unit/proxy/stream-gate-content-gate.test.ts (1)
281-290: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win变量命名与实际语义相反,容易误导后续维护者。
bigNonEcho的 event 名为response.in_progress,而response.in_progress正是openai-responses的回显事件之一(参见tests/unit/proxy/stream-gate-frame-classifier.test.ts中isRequestEchoFrame对response.created/response.in_progress/response.queued均返回true的用例)。因此该帧实际会被计入echoExcludedBytes,测试断言echo_excluded_bytes > 4000正是由它贡献的;真正未被排除、触发溢出的其实是oversizedTail(response.output_item.added)。变量名与其角色相反,容易让人误以为“非回显帧本身导致了排除字节数上升”。建议将
bigNonEcho改名为例如bigEchoFrame(或bigInProgressEcho),使变量名与其在断言中的实际作用一致。✏️ 命名澄清建议
- const bigNonEcho = `event: response.in_progress\ndata: {"type":"response.in_progress","response":{"instructions":"${bigPayload}"}}\n\n`; + const bigEchoFrame = `event: response.in_progress\ndata: {"type":"response.in_progress","response":{"instructions":"${bigPayload}"}}\n\n`; const oversizedTail = `event: response.output_item.added\ndata: {"item":"${"y".repeat(4096)}"}\n\n`; - const reader = readerFromChunks([bigNonEcho, oversizedTail, RESPONSES_DELTA]); + const reader = readerFromChunks([bigEchoFrame, oversizedTail, RESPONSES_DELTA]);🤖 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 281 - 290, Rename the `bigNonEcho` fixture in the overflow test to reflect that the `response.in_progress` frame is an echo frame, such as `bigEchoFrame`, and update its use when constructing `readerFromChunks`. Leave `oversizedTail` and the assertions unchanged.messages/en/dashboard.json (1)
262-262: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win不要将纯箭头符号作为翻译文案。
arrowPrefix仅表示视觉连接符,不包含需要本地化的语义。建议在组件中直接使用↳,仅将相邻的实际文本交给 i18n,避免各语言资源产生不必要的差异。🤖 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/en/dashboard.json` at line 262, 移除翻译资源中的 arrowPrefix 配置,并更新所有使用 arrowPrefix 的组件,直接渲染固定的视觉符号,仅通过 i18n 翻译相邻的实际文本内容。Source: Learnings
🤖 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 `@messages/zh-CN/dashboard.json`:
- Line 556: 更新 dashboard.json 中 desc 文案的标点格式,将英文半角括号和逗号统一替换为中文全角标点,保持原有文字内容不变。
In `@messages/zh-CN/provider-chain.json`:
- Line 63: Update the affinity_hit translation value to use full-width
parentheses, matching the punctuation style used by resource_not_found and the
rest of messages/zh-CN/provider-chain.json.
In `@messages/zh-TW/dashboard.json`:
- Around line 553-557: Update the replayServe.desc translation in the
messages/zh-TW/dashboard.json resource to replace both half-width commas with
full-width Chinese commas, preserving the rest of the Traditional Chinese text
unchanged.
In `@messages/zh-TW/provider-chain.json`:
- Line 63: 將 messages/zh-TW/provider-chain.json 中 affinity_hit 的半角括號改為與
resource_not_found 一致的全角括號,保留其餘文案內容不變。
In `@src/app/`[locale]/dashboard/logs/_components/provider-chain-popover.tsx:
- Around line 516-521: Update the affinity-hit detection near isAffinityHit to
follow the chain[0]-based precedence used by
isSessionReuseFlow/isAffinityHitFlow, preventing intermediate successful
requests from being misclassified; also extend getItemStatus to recognize the
affinity_hit reason and return the appropriate affinity-selection status for
hedge/retry visualization.
In `@src/app/`[locale]/dashboard/logs/_components/virtualized-logs-table.tsx:
- Line 48: Update the ThinkingEffortDisplay import in the virtualized logs table
to use the repository’s `@/` path alias instead of a relative path, while
preserving the imported symbol and module.
In `@src/app/`[locale]/settings/config/_components/system-settings-form.tsx:
- Around line 217-221: 保留 system-settings-form 中 replayEnabled 和
cacheEffectivenessEnabled 的 null 三态语义,不要在 useState 初始化时通过 ?? false/true
将“跟随环境变量”折叠为布尔值;调整开关状态及保存逻辑,使未修改时继续提交 null/省略对应字段并跟随环境变量,仅在用户实际修改开关时持久化显式开启或关闭值。
In `@src/app/v1/_lib/proxy/forwarder.ts`:
- Around line 4713-4726: Update the hedge path’s !gate.committed handling in the
runStreamContentGate flow to match the serial path: when gate.error is a
StreamPrecommitError with gateReason "idle_timeout", convert it to the
established 524 ProxyError with the "streaming_idle_timeout" classification
before throwing. Preserve the existing behavior for all other gate errors.
In `@src/app/v1/_lib/proxy/stream-gate/stream-content-gate.ts`:
- Around line 261-273: 更新 prebuffer 缓冲逻辑及 isRequestEchoFrame
判定:只有经过完整字段校验、确认属于真实请求回显的帧才可计入
echoExcludedBytes。为回显豁免增加独立累计上限,超过该上限或无法确认真实性的帧不得继续豁免,仍须按 bufferedBytes 计入
prebufferByteCap;保留逐帧事件数量上限。
In `@src/lib/system-settings/proxy-runtime.ts`:
- Around line 88-91: 修复 isCacheEffectivenessEnabled 的运行时快照不会刷新的问题:让缓存效果调度器的每个
tick 异步调用 getProxyRuntimeSettings() 获取最新设置后再判断,或在系统设置缓存失效流程中同步清空
lastKnown;确保设置关闭后无需后续代理请求也能停止聚合。
---
Nitpick comments:
In `@messages/en/dashboard.json`:
- Line 262: 移除翻译资源中的 arrowPrefix 配置,并更新所有使用 arrowPrefix 的组件,直接渲染固定的视觉符号,仅通过 i18n
翻译相邻的实际文本内容。
In `@src/app/v1/_lib/proxy/replay/replay-spool.ts`:
- Line 4: Update the import of isReplayEnabled and ReplayIdentity in
replay-spool.ts to use the `@/app/v1/_lib/proxy/replay/replay-identity` alias
instead of the relative ./replay-identity path.
In `@tests/unit/proxy/stream-gate-content-gate.test.ts`:
- Around line 281-290: Rename the `bigNonEcho` fixture in the overflow test to
reflect that the `response.in_progress` frame is an echo frame, such as
`bigEchoFrame`, and update its use when constructing `readerFromChunks`. Leave
`oversizedTail` and the assertions 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: 38051857-7f0e-4db9-982c-52365172c06c
📒 Files selected for processing (73)
drizzle/0113_reflective_centennial.sqldrizzle/meta/0113_snapshot.jsondrizzle/meta/_journal.jsonmessages/en/dashboard.jsonmessages/en/provider-chain.jsonmessages/en/settings/config.jsonmessages/ja/dashboard.jsonmessages/ja/provider-chain.jsonmessages/ja/settings/config.jsonmessages/ru/dashboard.jsonmessages/ru/provider-chain.jsonmessages/ru/settings/config.jsonmessages/zh-CN/dashboard.jsonmessages/zh-CN/provider-chain.jsonmessages/zh-CN/settings/config.jsonmessages/zh-TW/dashboard.jsonmessages/zh-TW/provider-chain.jsonmessages/zh-TW/settings/config.jsonsrc/actions/system-config.tssrc/app/[locale]/dashboard/logs/_components/codex-reasoning-effort-display.test.tsxsrc/app/[locale]/dashboard/logs/_components/column-visibility-dropdown.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog/components/LogicTraceTab.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog/components/SummaryTab.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog/types.tssrc/app/[locale]/dashboard/logs/_components/provider-chain-popover.tsxsrc/app/[locale]/dashboard/logs/_components/thinking-effort-display.test.tsxsrc/app/[locale]/dashboard/logs/_components/thinking-effort-display.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]/settings/config/_components/system-settings-form.tsxsrc/app/[locale]/settings/config/page.tsxsrc/app/v1/_lib/proxy/affinity/affinity-recorder.tssrc/app/v1/_lib/proxy/affinity/affinity-store.tssrc/app/v1/_lib/proxy/affinity/fingerprint.tssrc/app/v1/_lib/proxy/forwarder.tssrc/app/v1/_lib/proxy/provider-selector.tssrc/app/v1/_lib/proxy/replay/replay-identity.tssrc/app/v1/_lib/proxy/replay/replay-spool.tssrc/app/v1/_lib/proxy/response-handler.tssrc/app/v1/_lib/proxy/session.tssrc/app/v1/_lib/proxy/stream-finalization.tssrc/app/v1/_lib/proxy/stream-gate/frame-classifier.tssrc/app/v1/_lib/proxy/stream-gate/stream-content-gate.tssrc/components/ui/__tests__/code-display.test.tsxsrc/components/ui/code-display.tsxsrc/drizzle/schema.tssrc/instrumentation.tssrc/lib/api-client/v1/openapi-types.gen.tssrc/lib/api/v1/schemas/system-config.tssrc/lib/column-visibility.test.tssrc/lib/column-visibility.tssrc/lib/config/env.schema.tssrc/lib/config/system-settings-cache.tssrc/lib/system-settings/proxy-runtime.tssrc/lib/utils/thinking-effort.tssrc/lib/validation/schemas.tssrc/repository/_shared/transformers.tssrc/repository/system-config.tssrc/types/message.tssrc/types/system-config.tstests/unit/proxy/affinity-fingerprint.test.tstests/unit/proxy/affinity-recorder.test.tstests/unit/proxy/affinity-store.test.tstests/unit/proxy/provider-selector-affinity-ignore-session.test.tstests/unit/proxy/provider-selector-affinity-priority.test.tstests/unit/proxy/provider-selector-select-provider-by-type.test.tstests/unit/proxy/stream-gate-content-gate.test.tstests/unit/proxy/stream-gate-forwarder-integration.test.tstests/unit/proxy/stream-gate-frame-classifier.test.tstests/unit/repository/system-config-degradation-ladder.test.tstests/unit/repository/system-config-update-missing-columns.test.ts
💤 Files with no reviewable changes (1)
- src/app/[locale]/dashboard/logs/_components/codex-reasoning-effort-display.test.tsx
| import { ErrorDetailsDialog } from "./error-details-dialog"; | ||
| import { ModelDisplayWithRedirect } from "./model-display-with-redirect"; | ||
| import { ProviderChainPopover } from "./provider-chain-popover"; | ||
| import { ThinkingEffortDisplay } from "./thinking-effort-display"; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
改用 @/ 路径别名。
本次新增导入仍使用相对路径,违反仓库对 TSX 文件的导入约定。请改为:
建议修复
-import { ThinkingEffortDisplay } from "./thinking-effort-display";
+import { ThinkingEffortDisplay } from "`@/app/`[locale]/dashboard/logs/_components/thinking-effort-display";📝 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.
| import { ThinkingEffortDisplay } from "./thinking-effort-display"; | |
| import { ThinkingEffortDisplay } from "`@/app/`[locale]/dashboard/logs/_components/thinking-effort-display"; |
🤖 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.tsx at
line 48, Update the ThinkingEffortDisplay import in the virtualized logs table
to use the repository’s `@/` path alias instead of a relative path, while
preserving the imported symbol and module.
Source: Coding guidelines
There was a problem hiding this comment.
Skipped: 本文件同级组件导入(error-details-dialog、model-display-with-redirect、provider-chain-popover)全部使用相对路径,新增导入遵循了文件内既有惯例;@/ 别名在本仓库用于跨目录导入。单改这一行反而制造不一致。
There was a problem hiding this comment.
Posted the review on PR #1360 and applied the size/XL label.
- Flagged 3 issues on diff lines:
src/app/[locale]/settings/config/_components/system-settings-form.tsx:218— nullable settings are coerced to booleans, sonullfallback state is lost.src/app/v1/_lib/proxy/replay/replay-identity.ts:42— replay enablement can read a stale runtime snapshot before the provider refresh path runs.src/instrumentation.ts:271— cache-effectiveness scheduling also relies on a stale snapshot, so DB toggles may not take effect promptly.
- Summary review comment is posted on the PR, with split suggestions for the XL scope.
If you want, I can turn these into concrete patch suggestions next.
Replace half-width parentheses with full-width CJK equivalents in Simplified and Traditional Chinese dashboard and provider-chain translations for typographic consistency.
Request echo frames were fully excluded from the prebuffer byte cap, allowing a malicious or oversized echo flood to grow the buffer without bound. The exemption is now capped at prebufferByteCap, so echo bytes beyond that threshold count normally and trigger prebuffer_overflow. This keeps worst-case buffer memory at 2x cap.
…uler ticks The replay guard read a synchronous cached snapshot that could stay stale on low-traffic instances where no request path refreshed it, delaying admin toggle of replayEnabled. The guard now calls getProxyRuntimeSettings() on each request to pull the latest override. The cache-effectiveness scheduler tick similarly switches from the synchronous snapshot to an async refresh so toggles take effect without relying on request-path cache warming.
The form coerced null (follow-env-var) to boolean defaults on init and after fetch, so saving any unrelated field silently wrote an explicit override. The state now retains null until the user toggles a switch, and the checked prop falls back to the env default only for display.
The hedge (racing) path threw the raw StreamPrecommitError on gate idle timeout instead of building the 524 streaming_idle_timeout ProxyError used by the serial path. Extract buildStreamingIdleTimeoutError and apply it in both paths so circuit-breaker and failover logic see the same error classification regardless of racing mode.
…opover Affinity-hit detection used chain.find(), which could match a mid-chain retry and mislabel it as cache reuse. Align with session reuse by checking only chain[0]. Add a dedicated teal DatabaseZap status icon for affinity_hit items so they are visually distinct.
Review feedback addressed (a596229..f442fc8)All 15 inline comments replied individually. Summary of the 6 fix commits:
Nitpicks from the review body:
Verification: build, Biome lint, tsgo typecheck, full vitest (7911 tests), test:v1 coverage gate, and all 10 named coverage configs green locally. 🤖 Generated with Claude Code |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/unit/proxy/replay-guard.test.ts (1)
63-72: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win让运行时设置 mock 保持真实快照契约。
当前
getProxyRuntimeSettings()返回replayEnabled: false,但getCachedProxyRuntimeSettings()永远返回null,导致测试始终回退到ENABLE_REQUEST_REPLAY环境变量。即使ensure()不再刷新运行时快照,现有用例也可能继续通过,无法覆盖本 PR 的核心运行时覆写路径。建议让同步 getter 返回最近一次刷新结果,或增加显式断言验证replayEnabled: false的覆写行为。🤖 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/replay-guard.test.ts` around lines 63 - 72, 更新 replay-guard 测试中的 getProxyRuntimeSettings 与 getCachedProxyRuntimeSettings mock,使同步 getter 返回最近一次异步刷新得到的运行时快照,而不是始终返回 null。确保 replayEnabled: false 能通过运行时设置覆盖 ENABLE_REQUEST_REPLAY,并在相关用例中显式断言该覆写路径生效。
🤖 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.
Nitpick comments:
In `@tests/unit/proxy/replay-guard.test.ts`:
- Around line 63-72: 更新 replay-guard 测试中的 getProxyRuntimeSettings 与
getCachedProxyRuntimeSettings mock,使同步 getter 返回最近一次异步刷新得到的运行时快照,而不是始终返回 null。确保
replayEnabled: false 能通过运行时设置覆盖 ENABLE_REQUEST_REPLAY,并在相关用例中显式断言该覆写路径生效。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: f8db087c-a725-4c91-8495-7aa6a27bcc28
📒 Files selected for processing (13)
messages/zh-CN/dashboard.jsonmessages/zh-CN/provider-chain.jsonmessages/zh-TW/dashboard.jsonmessages/zh-TW/provider-chain.jsonsrc/app/[locale]/dashboard/logs/_components/provider-chain-popover.tsxsrc/app/[locale]/settings/config/_components/system-settings-form.tsxsrc/app/v1/_lib/proxy/forwarder.tssrc/app/v1/_lib/proxy/replay/replay-guard.tssrc/app/v1/_lib/proxy/stream-gate/stream-content-gate.tssrc/instrumentation.tstests/unit/proxy/replay-guard.test.tstests/unit/proxy/stream-gate-content-gate.test.tstests/unit/settings/system-settings-form-replay-cache-toggles.test.tsx
🚧 Files skipped from review as they are similar to previous changes (10)
- messages/zh-CN/dashboard.json
- messages/zh-TW/provider-chain.json
- messages/zh-CN/provider-chain.json
- src/instrumentation.ts
- src/app/[locale]/settings/config/_components/system-settings-form.tsx
- src/app/[locale]/dashboard/logs/_components/provider-chain-popover.tsx
- messages/zh-TW/dashboard.json
- tests/unit/proxy/stream-gate-content-gate.test.ts
- src/app/v1/_lib/proxy/forwarder.ts
- src/app/v1/_lib/proxy/stream-gate/stream-content-gate.ts
🧪 测试结果
总体结果: ✅ 所有测试通过 |
Summary
Follow-ups for the CCHP gateway features merged in #1356 (F1 stream gate / F2 replay / F3a prefix affinity / F3b cache-effectiveness), addressing the production
prebuffer_overflow502s and closing UI/observability gaps.F1 Stream Gate hardening
STREAM_GATE_PREBUFFER_BYTE_CAP, max 64MiB) — the old cap was deterministically tripped by openai-responses request-echo frames on large-context requests.response.created/response.in_progress/response.queuedecho the full request body; they no longer count toward the cap).onFirstByte), so providerfirstByteTimeoutStreamingMs(e.g. 30s) no longer misfires as "Stream response timeout" while thinking models emit neutral frames before the first content frame. Gate-phase silence is instead governed by the provider's streaming idle timeout (idleTimeoutMs), isomorphic with the post-commit 524 path.F3a Prefix affinity correctness + visibility
F_sysis excluded from both lookup and write-back, so requests sharing only a system prompt are never glued to the same provider across conversations.initial_selectionfor every request: when session reuse or an affinity hit already recorded the sticky selection, the spuriousinitial_selectionentry is suppressed.reasons.affinity_hit,selectionMethods.prefix_affinity) in 5 locales.F2/F3b runtime toggles (DB-backed, env fallback)
system_settingscolumnsreplay_enabled/cache_effectiveness_enabled(migration0113, degradation-ladder aware);nullfollowsENABLE_REQUEST_REPLAY/ENABLE_CACHE_EFFECTIVENESS.replay_serverequests now render a dedicated cache-hit panel in request details instead of the misleading orange "blocked" banner.Dashboard
extractThinkingEffortInfoutil, same logic as SummaryTab). Column is now toggleable in show/hide columns, narrower by default, tooltip reads "thinking effort of the model request".Verification
bun run build/bun run typecheck/bun run lintgreenbun run test:v1(coverage gate) green; affected coverage configs (thinking-effort-conflict-rectifier, my-usage, proxy-guard-pipeline) greenisRequestEchoFrame, sys-exclusion fingerprint/store/recorder semantics, sticky-selection chain dedup, SSE large-content rendering🤖 Generated with Claude Code
Greptile Summary
Adds runtime-configurable gateway features and expands stream-gate, affinity, replay, cache-effectiveness, and dashboard observability.
Confidence Score: 5/5
The PR appears safe to merge because no additional blocking failure eligible for this follow-up review was identified.
No blocking failure remains within the scope of this follow-up review.
Important Files Changed
Reviews (2): Last reviewed commit: "fix(ui): scope affinity-hit detection to..." | Re-trigger Greptile
Context used (3)