feat(proxy): CCHP 网关核心能力移植——流式内容门控 / 请求分离 Replay / 最长前缀亲和与缓存效果指标#1356
Conversation
Normalizes 85+ call sites that used the pattern !x || x.y !== z to the idiomatic x?.y !== z across server actions, API routes, dashboard pages, and Redis/session utilities. Includes test formatting fixes from biome and removes two stale biome-ignore directives.
Ports four flag-gated proxy features from the upstream gateway design: - Stream content gate (STREAM_GATE_MODE): buffers SSE frames until the first valid content frame arrives before committing the response to the client, enabling provider failover on fake-200 error frames, malformed data, and empty streams - Request replay (ENABLE_REQUEST_REPLAY): caches completed streaming responses in a Redis hot layer with PG persistence so identical retried requests are served from cache without consuming rate-limit quota or provider concurrency - Prefix affinity (ENABLE_PREFIX_AFFINITY): computes chain fingerprints from request content and nominates the previously successful provider via longest-prefix matching to keep conversations sticky and improve prompt cache hit rates - Cache effectiveness metrics (ENABLE_CACHE_EFFECTIVENESS): derives theoretical vs observed cache token ratios per provider, model, and TTL bucket, aggregating them into windowed effectiveness scores displayed in the provider settings UI All features default off and are independently enabled via environment flags. Adds database migration 0109 for replay_payloads and provider_cache_effectiveness tables plus cache score columns on message_request.
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough本次变更新增流式内容门控、请求回放、前缀亲和路由和缓存效果统计，并扩展数据库、API、系统设置、排行榜展示与相关测试；同时统一多处管理员鉴权及 Redis 就绪检查写法。 Changes代理平台能力
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 |
Newer biome flagged the chained optional-access expression as unsafe. Split the access into an explicit intermediate variable so the indexing and property reads are type-safe without changing test behaviour.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // 机会式清理过期行：写入时顺带扫尾（低流量期由定时清理兜底） | ||
| await this.cleanupExpired(); | ||
| } catch (error) { | ||
| logger.warn("[ReplayStore] persistCompleted failed (replay stays redis-only)", { |
There was a problem hiding this comment.
[Critical] [ERROR-SWALLOWED] persistCompleted() swallows the durable-write failure, so callers can still publish completed
Why this is a problem: src/app/v1/_lib/proxy/replay/replay-spool.ts:149 documents that completed must only appear after the payload is durable. But this catch logs and returns normally, so ReplaySpool.completeAfterBilling() continues to writeMeta("completed") even when the PG insert failed. Once the Redis TTL expires, the replay entry disappears entirely instead of being re-servable from PG.
Suggested fix:
} catch (error) {
logger.warn("[ReplayStore] persistCompleted failed", {
error: error instanceof Error ? error.message : String(error),
replayId: row.replayId.slice(0, 12),
});
throw error;
}
| if (effectiveGroup && !checkProviderGroupMatch(provider.groupTag, effectiveGroup)) { | ||
| return null; | ||
| } | ||
| return provider; |
There was a problem hiding this comment.
[High] [LOGIC-BUG] The new affinity nomination path bypasses provider spend-limit checks
Why this is a problem: validateAffinityCandidate() claims to use the same hard validation as findReusable(), but it never runs checkCostLimitsWithLease() or checkTotalCostLimit(). That means a provider that is already over its 5h/daily/monthly/total budget can still be selected through prefix affinity, even though the weighted-random path filters those providers out in filterByLimits().
Suggested fix:
const [providerWithinLimits] = await ProxyProviderResolver.filterByLimits([provider]);
if (!providerWithinLimits) return null;
return providerWithinLimits;
| if (this.disabled) return; | ||
| if (batch.length > 0) { | ||
| const appended = await this.store.appendChunks(this.identity.replayId, batch); | ||
| if (appended !== null) this.chunkCount = appended; |
There was a problem hiding this comment.
[High] [LOGIC-BUG] A failed final Redis flush is ignored even though live attach readers still depend on that LIST tail
Why this is a problem: appendChunks() returning null means Redis is unavailable. In the normal streaming path you already treat that as a replay failure, but here the code keeps going, persists the PG payload, and then marks Redis meta as completed. Any already-attached live replay is still tailing Redis, so it can miss the final batch and receive a truncated stream while the entry advertises success.
Suggested fix:
const appended = await this.store.appendChunks(this.identity.replayId, batch);
if (appended === null) {
throw new Error("final replay flush failed");
}
this.chunkCount = appended;
There was a problem hiding this comment.
Code Review Summary
This PR introduces a large amount of new proxy behavior across replay, affinity, stream handling, metrics, and admin/UI surfaces. I found three blocking correctness issues in the newly added code paths: two in the replay completion/attach flow and one in the new prefix-affinity selector path.
PR Size: XL
- Lines changed: 16425
- Files changed: 151
- Split suggestion: Consider separating this into at least
(1)replay/storage + migration,(2)affinity/cache-effectiveness routing and aggregation, and(3)admin/UI/OpenAPI wiring so the behavioral core can be reviewed independently from the surface-area updates.
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 2 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 1 | 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)
src/app/v1/_lib/proxy/replay/replay-store.ts:189—persistCompleted()swallows the PG failure and returns normally, which letsReplaySpool.completeAfterBilling()publishcompletedwithout a durable replay payload.
High Priority Issues (Should Fix)
src/app/v1/_lib/proxy/provider-selector.ts:643— the new affinity nomination path returns a provider without running the same spend-limit gates as the weighted-random path, so prefix affinity can bypass provider cost ceilings.src/app/v1/_lib/proxy/replay/replay-spool.ts:170— the final Redis tail flush treatsappendChunks() === nullas non-fatal and can mark an entrycompletedeven though attached live replayers still depend on that missing LIST suffix.
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Codex AI
There was a problem hiding this comment.
Review Submitted
- Reviewed PR
#1356and applied thesize/XLlabel. - Posted 3 inline findings on changed lines:
- Critical: swallowed durable-write failure in
src/app/v1/_lib/proxy/replay/replay-store.ts:189 - High: affinity path bypasses provider spend-limit gates in
src/app/v1/_lib/proxy/provider-selector.ts:643 - High: final replay tail flush ignores Redis failure in
src/app/v1/_lib/proxy/replay/replay-spool.ts:170
- Critical: swallowed durable-write failure in
- Submitted the required summary review with
XLsplit suggestions and the category/severity table.
If you want, I can also do a follow-up pass after the author pushes fixes.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9cfcb72998
ℹ️ 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".
|
|
||
| // 未命中可服务条目：尝试成为 owner（跨副本 single-flight） | ||
| const ownerToken = randomUUID(); | ||
| const claimed = await store.tryClaimOwner(identity.replayId, ownerToken); |
There was a problem hiding this comment.
When ENABLE_REQUEST_REPLAY is on and a client sends x-cch-no-replay: 1 for a request that already has a completed replay entry, this path skips the read but still claims the same replayId as a new owner. Because the existing Redis LIST/PG row is not cleared or versioned before the new ReplaySpool starts RPUSHing chunks, the next Redis replay can serve the old completed payload concatenated with the new run (and PG can remain stale due to onConflictDoNothing). The bypass path should either not become an owner for an existing completed entry, or reset/replace the stored payload before spooling.
Useful? React with 👍 / 👎.
| private static async tryPrefixAffinityNomination(session: ProxySession): Promise<void> { | ||
| try { | ||
| const env = getEnvConfig(); | ||
| if (!env.ENABLE_PREFIX_AFFINITY) return; |
There was a problem hiding this comment.
If operators enable only ENABLE_CACHE_EFFECTIVENESS (it has its own env flag and scheduler), this early return prevents session.affinity from ever being populated. Stream finalization later calls computeCacheScoreFields() under the cache-effectiveness flag, but with a null affinity it writes cacheCompatibilityKey: null, so the aggregation query (WHERE cache_compatibility_key IS NOT NULL) produces no samples and the provider card stays empty. Build the fingerprint state when either cache metrics or prefix-affinity routing is enabled, while keeping routing nomination gated by ENABLE_PREFIX_AFFINITY.
Useful? React with 👍 / 👎.
| // F3a 亲和写回（与顺序路径 session 绑定块对称；胜者在 commitWinner 即确认） | ||
| void recordAffinityWinner(session, attempt.provider.id); |
There was a problem hiding this comment.
With ENABLE_PREFIX_AFFINITY on, this writes the prefix binding as soon as a hedge attempt emits the first gated content frame, before the stream has finished and before finalization knows whether the response later times out, aborts, or is reclassified as an error. If there was no prior affinity nomination, the later failure path will not tombstone this newly written binding, so subsequent matching requests can be stuck to a provider whose previous stream failed after partial output. Move this write into the terminal-success side effect, matching the sequential streaming path.
Useful? React with 👍 / 👎.
| // F2 owner spool：guard 阶段已抢到 owner 租约的请求，把客户端可见字节 | ||
| // write-behind 喂入 Redis 热层，供并发/断线的相同请求 attach 跟尾。 | ||
| const replaySpool = createReplaySpoolIfOwner(session, response); |
There was a problem hiding this comment.
This replay spool is created only after the earlier Gemini passthrough branch has already returned its response. For Gemini-format streaming requests with ENABLE_REQUEST_REPLAY enabled, the guard can still claim the owner lease, but that passthrough path never bootstraps a spool or releases the owner token, so retries see no replay meta and are blocked from becoming a new owner until the Redis lease expires. Initialize/release replay ownership in the passthrough path as well, or avoid claiming for paths that cannot spool.
Useful? React with 👍 / 👎.
| const userId = session.authState?.user?.id; | ||
| if (!keyId || !userId) return null; | ||
|
|
||
| const bodyBytes = canonicalRequestBytes(session.request); |
There was a problem hiding this comment.
The replay guard runs after the global request filter, but this identity calculation still hashes session.request.buffer, which is the original raw body whenever it is present. If an admin changes a body rewrite/removal rule during the replay TTL, the same raw request can now produce a different upstream payload while still matching the old replayId, so the guard may return a response generated for the previous filtered body. Hash the post-filter logical request (or invalidate/version by filter state) instead of preferring the raw buffer here.
Useful? React with 👍 / 👎.
|
|
||
| // F2 终态屏障：replay completed 只能出现在计费落库（onCommitted）之后； | ||
| // 任何失败终态（假 200/中断/非 2xx）立即 abort，绝不被已完成重放命中。 | ||
| if (replaySpool) { |
There was a problem hiding this comment.
This completion/abort decision is reached only after several awaited finalization steps above it (cost persistence, Redis cost tracking, pricing lookups). If any of those throws, finalizeStream exits before calling either completeAfterBilling() or abort(), leaving the replay meta in owning, the owner lease unreleased until expiry, and activeSpoolCount permanently consumed in the process. Wrap the finalization path so any exception before successful completion aborts and releases the spool.
Useful? React with 👍 / 👎.
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| try { | ||
| await db | ||
| .insert(replayPayloads) | ||
| .values({ | ||
| replayId: row.replayId, | ||
| verifier: row.verifier, | ||
| scopeTag: row.scopeTag, | ||
| keyId: row.keyId, | ||
| userId: row.userId, | ||
| format: row.format, | ||
| model: row.model, | ||
| statusCode: row.statusCode, | ||
| headersJson: row.headers, | ||
| payload: row.payload, | ||
| byteSize: row.byteSize, | ||
| sourceMessageRequestId: row.sourceMessageRequestId, | ||
| expiresAt, | ||
| }) | ||
| .onConflictDoNothing(); | ||
| // 机会式清理过期行：写入时顺带扫尾（低流量期由定时清理兜底） | ||
| await this.cleanupExpired(); | ||
| } catch (error) { | ||
| logger.warn("[ReplayStore] persistCompleted failed (replay stays redis-only)", { | ||
| error: error instanceof Error ? error.message : String(error), | ||
| replayId: row.replayId.slice(0, 12), | ||
| }); | ||
| } |
There was a problem hiding this comment.
cleanupExpired() throws (e.g., a transient DB timeout on the DELETE), the catch logs "replay stays redis-only", implying the PG payload was never persisted. In reality the INSERT committed successfully and the replay entry will be found by findCompleted(). The incorrect message could lead an on-call engineer to believe the replay entry is missing when it is not.
| try { | |
| await db | |
| .insert(replayPayloads) | |
| .values({ | |
| replayId: row.replayId, | |
| verifier: row.verifier, | |
| scopeTag: row.scopeTag, | |
| keyId: row.keyId, | |
| userId: row.userId, | |
| format: row.format, | |
| model: row.model, | |
| statusCode: row.statusCode, | |
| headersJson: row.headers, | |
| payload: row.payload, | |
| byteSize: row.byteSize, | |
| sourceMessageRequestId: row.sourceMessageRequestId, | |
| expiresAt, | |
| }) | |
| .onConflictDoNothing(); | |
| // 机会式清理过期行：写入时顺带扫尾（低流量期由定时清理兜底） | |
| await this.cleanupExpired(); | |
| } catch (error) { | |
| logger.warn("[ReplayStore] persistCompleted failed (replay stays redis-only)", { | |
| error: error instanceof Error ? error.message : String(error), | |
| replayId: row.replayId.slice(0, 12), | |
| }); | |
| } | |
| try { | |
| await db | |
| .insert(replayPayloads) | |
| .values({ | |
| replayId: row.replayId, | |
| verifier: row.verifier, | |
| scopeTag: row.scopeTag, | |
| keyId: row.keyId, | |
| userId: row.userId, | |
| format: row.format, | |
| model: row.model, | |
| statusCode: row.statusCode, | |
| headersJson: row.headers, | |
| payload: row.payload, | |
| byteSize: row.byteSize, | |
| sourceMessageRequestId: row.sourceMessageRequestId, | |
| expiresAt, | |
| }) | |
| .onConflictDoNothing(); | |
| } catch (error) { | |
| logger.warn("[ReplayStore] persistCompleted failed (replay stays redis-only)", { | |
| error: error instanceof Error ? error.message : String(error), | |
| replayId: row.replayId.slice(0, 12), | |
| }); | |
| return; | |
| } | |
| // 机会式清理过期行：写入时顺带扫尾（低流量期由定时清理兜底） | |
| try { | |
| await this.cleanupExpired(); | |
| } catch { | |
| // cleanup failure is non-fatal; TTL will expire stale rows | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/v1/_lib/proxy/replay/replay-store.ts
Line: 167-193
Comment:
Misleading error log when only the cleanup fails — the INSERT already committed. If `cleanupExpired()` throws (e.g., a transient DB timeout on the DELETE), the catch logs "replay stays redis-only", implying the PG payload was never persisted. In reality the `INSERT` committed successfully and the replay entry will be found by `findCompleted()`. The incorrect message could lead an on-call engineer to believe the replay entry is missing when it is not.
```suggestion
try {
await db
.insert(replayPayloads)
.values({
replayId: row.replayId,
verifier: row.verifier,
scopeTag: row.scopeTag,
keyId: row.keyId,
userId: row.userId,
format: row.format,
model: row.model,
statusCode: row.statusCode,
headersJson: row.headers,
payload: row.payload,
byteSize: row.byteSize,
sourceMessageRequestId: row.sourceMessageRequestId,
expiresAt,
})
.onConflictDoNothing();
} catch (error) {
logger.warn("[ReplayStore] persistCompleted failed (replay stays redis-only)", {
error: error instanceof Error ? error.message : String(error),
replayId: row.replayId.slice(0, 12),
});
return;
}
// 机会式清理过期行：写入时顺带扫尾（低流量期由定时清理兜底）
try {
await this.cleanupExpired();
} catch {
// cleanup failure is non-fatal; TTL will expire stale rows
}
```
How can I resolve this? If you propose a fix, please make it concise.| * - terminal：干净终止标记（[DONE] / message_stop 等），不开启透传 | ||
| * - neutral：bookkeeping / 未知事件，继续缓冲，由首块超时兜底 | ||
| * | ||
| * 判定优先级：sentinel(terminal) > malformed > error > content > terminal > neutral。 |
There was a problem hiding this comment.
terminal appears twice (once as sentinel(terminal) and again at the end), and the ordering doesn't match the code. The actual code evaluation order is: doneSentinel → malformed (non-JSON) → error → content → terminalRules/Events → neutral. The PR description also states a different order ("error > malformed > terminal > content > neutral"), which further adds confusion.
| * 判定优先级：sentinel(terminal) > malformed > error > content > terminal > neutral。 | |
| * 判定优先级：doneSentinel(terminal) > malformed > error > content > terminalRules > neutral。 |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/v1/_lib/proxy/stream-gate/frame-classifier.ts
Line: 11
Comment:
The priority comment is inaccurate in two ways: `terminal` appears twice (once as `sentinel(terminal)` and again at the end), and the ordering doesn't match the code. The actual code evaluation order is: doneSentinel → malformed (non-JSON) → error → content → terminalRules/Events → neutral. The PR description also states a different order ("error > malformed > terminal > content > neutral"), which further adds confusion.
```suggestion
* 判定优先级：doneSentinel(terminal) > malformed > error > content > terminalRules > neutral。
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| async completeAfterBilling(messageRequestId: number | null): Promise<void> { | ||
| if (this.disabled || this.terminal) return; | ||
| this.terminal = true; | ||
| this.clearTimer(); | ||
| const tail = this.decoder.decode(); | ||
| if (tail.length > 0) { | ||
| this.pending.push(tail); | ||
| this.parts.push(tail); | ||
| } | ||
| const batch = this.pending; | ||
| this.pending = []; | ||
| this.pendingBytes = 0; | ||
|
|
||
| this.writeChain = this.writeChain.then(async () => { | ||
| try { | ||
| if (this.disabled) return; | ||
| if (batch.length > 0) { | ||
| const appended = await this.store.appendChunks(this.identity.replayId, batch); | ||
| if (appended !== null) this.chunkCount = appended; | ||
| } | ||
| // 先写 PG（持久 payload），再翻 Redis meta 为 completed（热层可服务） | ||
| await this.store.persistCompleted({ | ||
| replayId: this.identity.replayId, | ||
| verifier: this.identity.verifier, | ||
| scopeTag: this.identity.scopeTag, | ||
| keyId: this.identity.keyId, | ||
| userId: this.identity.userId, | ||
| format: this.identity.format, | ||
| model: this.identity.model, | ||
| statusCode: this.statusCode, | ||
| headers: { "content-type": this.contentType }, | ||
| payload: this.parts.join(""), | ||
| byteSize: this.totalBytes, | ||
| sourceMessageRequestId: messageRequestId, | ||
| }); | ||
| await this.writeMeta("completed", { messageRequestId }); | ||
| logger.info("[ReplaySpool] replay entry completed", { | ||
| replayId: this.identity.replayId.slice(0, 12), | ||
| chunkCount: this.chunkCount, | ||
| byteSize: this.totalBytes, | ||
| }); | ||
| } catch (error) { | ||
| logger.warn("[ReplaySpool] complete failed, aborting entry", { | ||
| error: error instanceof Error ? error.message : String(error), | ||
| }); | ||
| await this.writeMeta("aborted", { abortReason: "complete_failed" }).catch(() => undefined); | ||
| } finally { | ||
| await this.store.releaseOwner(this.identity.replayId, this.ownerToken); | ||
| this.release(); | ||
| } | ||
| }); | ||
| await this.writeChain; | ||
| } |
There was a problem hiding this comment.
completeAfterBilling — partial-success scenario leaves PG and Redis in inconsistent state that resolves correctly but silently
If persistCompleted() succeeds but writeMeta("completed") then fails (e.g., transient Redis error), the catch block calls writeMeta("aborted"). The PG row exists as completed while Redis shows aborted. Subsequent requests will miss the Redis hot path, fall through to findCompleted(), find the PG row with a matching verifier, and serve it correctly — so the system self-heals. However this scenario is not documented and the "aborted" Redis meta will block the fast Redis path until TTL expiry. Consider adding a log at the catch site to make the inconsistency observable.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/v1/_lib/proxy/replay/replay-spool.ts
Line: 152-204
Comment:
**`completeAfterBilling` — partial-success scenario leaves PG and Redis in inconsistent state that resolves correctly but silently**
If `persistCompleted()` succeeds but `writeMeta("completed")` then fails (e.g., transient Redis error), the catch block calls `writeMeta("aborted")`. The PG row exists as `completed` while Redis shows `aborted`. Subsequent requests will miss the Redis hot path, fall through to `findCompleted()`, find the PG row with a matching verifier, and serve it correctly — so the system self-heals. However this scenario is not documented and the "aborted" Redis meta will block the fast Redis path until TTL expiry. Consider adding a log at the catch site to make the inconsistency observable.
How can I resolve this? If you propose a fix, please make it concise.| } | ||
|
|
||
| if (framesSeen > options.prebufferEventCap || bufferedBytes > options.prebufferByteCap) { | ||
| return failure("prebuffer_overflow"); | ||
| } |
There was a problem hiding this comment.
framesSeen is incremented inside the inner for (const frame of parser.push(chunk)) loop, but the overflow check framesSeen > prebufferEventCap only runs once the entire chunk's frames have been classified. A single large chunk containing many neutral frames can push framesSeen significantly past prebufferEventCap before the check fires. In practice this is only a minor over-buffer tolerance and the byte-cap guard covers the byte side independently, but it's worth noting if the event cap is meant as a strict upper bound rather than a soft trigger.
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: 196-200
Comment:
**Overflow check runs after processing the entire chunk, not per-frame**
`framesSeen` is incremented inside the inner `for (const frame of parser.push(chunk))` loop, but the overflow check `framesSeen > prebufferEventCap` only runs once the entire chunk's frames have been classified. A single large chunk containing many neutral frames can push `framesSeen` significantly past `prebufferEventCap` before the check fires. In practice this is only a minor over-buffer tolerance and the byte-cap guard covers the byte side independently, but it's worth noting if the event cap is meant as a strict upper bound rather than a soft trigger.
How can I resolve this? If you propose a fix, please make it concise.![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (6)
src/actions/provider-cache-effectiveness.ts (1)
9-9: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value统一改用
@/路径别名。 这些新增相对导入不符合项目导入规范。
src/actions/provider-cache-effectiveness.ts#L9-L9: 将./types改为@/actions/types。src/lib/api-client/v1/actions/provider-cache-effectiveness.ts#L1-L2: 将本地模块改为对应的@/lib/api-client/v1/actions/...路径。src/app/[locale]/settings/providers/_components/provider-cache-effectiveness-card.test.tsx#L11-L11: 使用该组件的@/app/...绝对别名。src/app/[locale]/settings/providers/_components/provider-rich-list-item.tsx#L85-L85: 使用该组件的@/app/...绝对别名。As per coding guidelines, "Use path alias
@/to map to ./src/ for imports".🤖 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/provider-cache-effectiveness.ts` at line 9, Replace the relative imports with the `@/` path alias at all affected sites: in src/actions/provider-cache-effectiveness.ts:9 use `@/actions/types`; in src/lib/api-client/v1/actions/provider-cache-effectiveness.ts:1-2 use the corresponding `@/lib/api-client/v1/actions/`... paths; in src/app/[locale]/settings/providers/_components/provider-cache-effectiveness-card.test.tsx:11 use the component’s `@/app/`... path; and in src/app/[locale]/settings/providers/_components/provider-rich-list-item.tsx:85 use the component’s `@/app/`... path.Source: Coding guidelines
src/lib/api/v1/schemas/provider-cache-effectiveness.ts (1)
2-2: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win将内部导入改为
@/路径别名。此处从
src/内部使用了相对导入，和仓库导入约定不一致。建议修改
-import { IsoDateTimeStringSchema } from "./_common"; +import { IsoDateTimeStringSchema } from "`@/lib/api/v1/schemas/_common`";As per coding guidelines,
Use path alias@/to map to ./src/ for imports.🤖 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/api/v1/schemas/provider-cache-effectiveness.ts` at line 2, 将 provider-cache-effectiveness.ts 中的 IsoDateTimeStringSchema 导入从相对路径改为使用仓库约定的 `@/` 路径别名，并保持导入的模块与行为不变。Source: Coding guidelines
tests/api/v1/providers/providers.cache-effectiveness.test.ts (1)
1-161: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win将该 API 集成测试移入认可的测试目录。
tests/api/v1/...不在规范允许的位置；请移至tests/integration/（并相应更新test-utils导入路径），以保持测试发现和分类一致。As per coding guidelines,
Use Vitest for unit testing with tests located in tests/unit/, tests/integration/, or source-adjacent in src/**/*.test.ts.🤖 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/api/v1/providers/providers.cache-effectiveness.test.ts` around lines 1 - 161, Move the v1 provider cache effectiveness integration test from the current tests/api/v1 location into the approved tests/integration directory, preserving its test behavior and filename. Update the relative import of callV1Route to match the new location, and ensure any test discovery references use the relocated path.Source: Coding guidelines
src/lib/redis/redis-list-store.ts (1)
5-5: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win改用
@/别名导入 Redis 客户端。
./client不符合仓库对 TypeScript 导入路径的要求；请改为@/lib/redis/client。🤖 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/redis/redis-list-store.ts` at line 5, Update the Redis client import in redis-list-store.ts to use the repository alias path "`@/lib/redis/client`" instead of the relative "./client" path.Source: Coding guidelines
src/app/v1/_lib/proxy/replay/replay-store.ts (1)
164-203: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win每次
persistCompleted都同步执行全表清理，与定时清理重复且拖累写路径。
instrumentation.ts已注册每 10 分钟的cleanupExpired调度（见 startReplayCleanupScheduler）。在此对每个完成的 replay 都await this.cleanupExpired()会在每次写入后附加一次表级DELETE ... WHERE expires_at < now()，增加往返与写锁开销。另外cleanupExpired()抛错时会落入外层catch，记录成 “persistCompleted failed (replay stays redis-only)”，而此时 insert 其实已提交，日志有误导性。建议：将机会式清理改为按概率抽样触发，并与 insert 的错误处理解耦，避免清理失败被误报为持久化失败。
♻️ 参考改法
- .onConflictDoNothing(); - // 机会式清理过期行：写入时顺带扫尾（低流量期由定时清理兜底） - await this.cleanupExpired(); - } catch (error) { + .onConflictDoNothing(); + } catch (error) { logger.warn("[ReplayStore] persistCompleted failed (replay stays redis-only)", { error: error instanceof Error ? error.message : String(error), replayId: row.replayId.slice(0, 12), }); + return; } + // 机会式清理与主写入解耦，且抽样触发（定时清理兜底） + if (Math.random() < 0.05) { + this.cleanupExpired().catch((error) => { + logger.warn("[ReplayStore] opportunistic cleanup failed", { + error: error instanceof Error ? error.message : String(error), + }); + }); + } }🤖 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-store.ts` around lines 164 - 203, Remove the unconditional awaited cleanupExpired call from persistCompleted. If opportunistic cleanup is retained, trigger it only through low-probability sampling after a successful insert and isolate its failure with separate handling/logging so cleanup errors cannot be reported as persistCompleted failures; leave the insert error path responsible only for persistence failures.src/repository/provider-cache-effectiveness.ts (1)
28-29: 🚀 Performance & Scalability | 🔵 Trivial排序键与索引不匹配，
ORDER BY window_end无法利用现有索引。
idx_provider_cache_effectiveness_window建立在(provider_id, model, window_start DESC)上，但此查询按window_end DESC, id DESC排序，因此规划器无法用该索引满足排序，需额外 sort。请确认排序键应为windowEnd还是windowStart：若确为windowEnd，建议为(provider_id, window_end DESC, id DESC)增补覆盖索引以支撑providerId过滤 + 排序 + limit 的常见路径。建议方向
- 若语义应按窗口结束时间倒序：新增
(provider_id, window_end DESC, id DESC)索引，或调整现有索引列。- 若应按窗口开始时间倒序：将查询改为
desc(providerCacheEffectiveness.windowStart)以复用现有索引。🤖 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 28 - 29, Resolve the ordering/index mismatch in the query using providerCacheEffectiveness: confirm whether results should be ordered by window end or window start. If window start is the intended semantic, change the orderBy expression to desc(providerCacheEffectiveness.windowStart) while preserving the id tie-breaker; otherwise retain windowEnd and add or adjust an index covering provider_id, window_end DESC, and id DESC.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/app/`[locale]/settings/providers/_components/provider-cache-effectiveness-card.tsx:
- Around line 35-49: Update the provider cache effectiveness query around
getProviderCacheEffectivenessWindows and CACHE_EFFECTIVENESS_FETCH_LIMIT so the
server query retrieves the latest window for the requested providerId or current
provider set. Remove reliance on a fixed global limit followed by client-side
data.find filtering, ensuring providers outside the first global 200 results
still receive their latest window.
In `@src/app/v1/_lib/proxy/provider-selector.ts`:
- Around line 602-644: Update validateAffinityCandidate to apply the same
amount-limit checks used by the findReusable path before returning the provider,
so providers exceeding 5-hour, daily, weekly, monthly, or total limits are
rejected. Reuse the existing limit-checking helper and align the nearby comment
describing parity with findReusable to match the actual validation scope.
In `@src/app/v1/_lib/proxy/replay/replay-spool.ts`:
- Around line 100-117: Protect both shared writeChain continuations from
store-operation failures. In enqueueFlush, wrap appendChunks, writeMeta, and
renewOwnerLease in try/catch and call disable("flush_error") on exceptions; in
the bootstrap path at src/app/v1/_lib/proxy/replay/replay-spool.ts lines
141-146, wrap writeMeta("owning") and call disable on failure so later writes
remain usable and rejections are handled.
In `@src/lib/redis/redis-list-store.ts`:
- Around line 59-62: 更新 rpushBatch 中的 Redis 写入流程，将 RPUSH 与 EXPIRE
合并为原子脚本或事务操作，避免追加成功后 TTL 设置失败导致 key 永久保留；保留无有效 ttlSeconds 时仅追加的行为，并补充覆盖追加成功但 TTL
设置失败场景的回归测试。
---
Nitpick comments:
In `@src/actions/provider-cache-effectiveness.ts`:
- Line 9: Replace the relative imports with the `@/` path alias at all affected
sites: in src/actions/provider-cache-effectiveness.ts:9 use `@/actions/types`; in
src/lib/api-client/v1/actions/provider-cache-effectiveness.ts:1-2 use the
corresponding `@/lib/api-client/v1/actions/`... paths; in
src/app/[locale]/settings/providers/_components/provider-cache-effectiveness-card.test.tsx:11
use the component’s `@/app/`... path; and in
src/app/[locale]/settings/providers/_components/provider-rich-list-item.tsx:85
use the component’s `@/app/`... path.
In `@src/app/v1/_lib/proxy/replay/replay-store.ts`:
- Around line 164-203: Remove the unconditional awaited cleanupExpired call from
persistCompleted. If opportunistic cleanup is retained, trigger it only through
low-probability sampling after a successful insert and isolate its failure with
separate handling/logging so cleanup errors cannot be reported as
persistCompleted failures; leave the insert error path responsible only for
persistence failures.
In `@src/lib/api/v1/schemas/provider-cache-effectiveness.ts`:
- Line 2: 将 provider-cache-effectiveness.ts 中的 IsoDateTimeStringSchema
导入从相对路径改为使用仓库约定的 `@/` 路径别名，并保持导入的模块与行为不变。
In `@src/lib/redis/redis-list-store.ts`:
- Line 5: Update the Redis client import in redis-list-store.ts to use the
repository alias path "`@/lib/redis/client`" instead of the relative "./client"
path.
In `@src/repository/provider-cache-effectiveness.ts`:
- Around line 28-29: Resolve the ordering/index mismatch in the query using
providerCacheEffectiveness: confirm whether results should be ordered by window
end or window start. If window start is the intended semantic, change the
orderBy expression to desc(providerCacheEffectiveness.windowStart) while
preserving the id tie-breaker; otherwise retain windowEnd and add or adjust an
index covering provider_id, window_end DESC, and id DESC.
In `@tests/api/v1/providers/providers.cache-effectiveness.test.ts`:
- Around line 1-161: Move the v1 provider cache effectiveness integration test
from the current tests/api/v1 location into the approved tests/integration
directory, preserving its test behavior and filename. Update the relative import
of callV1Route to match the new location, and ensure any test discovery
references use the relocated path.
🪄 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
Run ID: 85bdd47b-8653-404e-a83c-b79a84e18360
📒 Files selected for processing (121)
drizzle/0109_redundant_fixer.sqldrizzle/meta/0109_snapshot.jsondrizzle/meta/_journal.jsonmessages/en/settings/providers/list.jsonmessages/ja/settings/providers/list.jsonmessages/ru/settings/providers/list.jsonmessages/zh-CN/settings/providers/list.jsonmessages/zh-TW/settings/providers/list.jsonsrc/actions/admin-user-insights.tssrc/actions/audit-logs.tssrc/actions/client-versions.tssrc/actions/dispatch-simulator.tssrc/actions/error-rules.tssrc/actions/keys.tssrc/actions/model-prices.tssrc/actions/notification-bindings.tssrc/actions/notifications.tssrc/actions/provider-cache-effectiveness.tssrc/actions/provider-endpoints.tssrc/actions/provider-groups.tssrc/actions/providers.tssrc/actions/public-status.tssrc/actions/rate-limit-stats.tssrc/actions/sensitive-words.tssrc/actions/system-config.tssrc/actions/users.tssrc/actions/webhook-targets.tssrc/app/[locale]/dashboard/_components/user/actions/reset-user-5h-limit.tssrc/app/[locale]/dashboard/leaderboard/user/[userId]/page.tsxsrc/app/[locale]/dashboard/providers/page.tsxsrc/app/[locale]/dashboard/quotas/keys/page.tsxsrc/app/[locale]/dashboard/quotas/providers/page.tsxsrc/app/[locale]/dashboard/quotas/users/page.tsxsrc/app/[locale]/dashboard/rate-limits/page.tsxsrc/app/[locale]/dashboard/sessions/[sessionId]/messages/page.tsxsrc/app/[locale]/dashboard/sessions/page.tsxsrc/app/[locale]/internal/data-gen/page.tsxsrc/app/[locale]/settings/client-versions/page.tsxsrc/app/[locale]/settings/providers/_components/model-multi-select.tsxsrc/app/[locale]/settings/providers/_components/provider-cache-effectiveness-card.test.tsxsrc/app/[locale]/settings/providers/_components/provider-cache-effectiveness-card.tsxsrc/app/[locale]/settings/providers/_components/provider-rich-list-item.tsxsrc/app/api/admin/database/export/route.tssrc/app/api/admin/database/import/route.tssrc/app/api/admin/database/status/route.tssrc/app/api/admin/log-cleanup/manual/route.tssrc/app/api/admin/log-level/route.tssrc/app/api/admin/system-config/route.tssrc/app/api/availability/current/route.tssrc/app/api/availability/endpoints/probe-logs/route.tssrc/app/api/availability/endpoints/route.tssrc/app/api/availability/route.tssrc/app/api/internal/data-gen/route.tssrc/app/api/prices/cloud-model-count/route.tssrc/app/api/prices/route.tssrc/app/api/prices/vendors/route.tssrc/app/api/v1/resources/providers/handlers.tssrc/app/api/v1/resources/providers/router.tssrc/app/v1/_lib/codex/session-completer.tssrc/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/fake-streaming/response-validator.tssrc/app/v1/_lib/proxy/forwarder.tssrc/app/v1/_lib/proxy/guard-pipeline.tssrc/app/v1/_lib/proxy/openai-image-compat.tssrc/app/v1/_lib/proxy/provider-selector.tssrc/app/v1/_lib/proxy/replay/replay-guard.tssrc/app/v1/_lib/proxy/replay/replay-identity.tssrc/app/v1/_lib/proxy/replay/replay-spool.tssrc/app/v1/_lib/proxy/replay/replay-store.tssrc/app/v1/_lib/proxy/response-handler.tssrc/app/v1/_lib/proxy/session.tssrc/app/v1/_lib/proxy/stream-gate/frame-classifier.tssrc/app/v1/_lib/proxy/stream-gate/sse-frames.tssrc/app/v1/_lib/proxy/stream-gate/stream-content-gate.tssrc/components/customs/model-vendor-icon.tsxsrc/drizzle/schema.tssrc/instrumentation.tssrc/lib/api-client/v1/actions/provider-cache-effectiveness.tssrc/lib/api-client/v1/openapi-types.gen.tssrc/lib/api/v1/action-migration-matrix.tssrc/lib/api/v1/schemas/provider-cache-effectiveness.tssrc/lib/auth-session-store/redis-session-store.tssrc/lib/cache-effectiveness/gate.tssrc/lib/cache-effectiveness/service.tssrc/lib/config/env.schema.tssrc/lib/provider-endpoints/leader-lock.tssrc/lib/public-status/scheduler.tssrc/lib/rate-limit/lease-service.tssrc/lib/rate-limit/service.tssrc/lib/redis/cost-cache-cleanup.tssrc/lib/redis/redis-kv-store.tssrc/lib/redis/redis-list-store.tssrc/lib/request-identity.tssrc/lib/session-manager.tssrc/lib/session-tracker.tssrc/repository/message-write-buffer.tssrc/repository/message.tssrc/repository/provider-cache-effectiveness.tssrc/types/message.tssrc/types/provider-cache-effectiveness.tstests/api/v1/providers/providers.cache-effectiveness.test.tstests/unit/api/v1/action-migration-matrix.test.tstests/unit/lib/cache-effectiveness-gate.test.tstests/unit/lib/redis-list-store.test.tstests/unit/lib/request-identity.test.tstests/unit/proxy/affinity-fingerprint.test.tstests/unit/proxy/affinity-recorder.test.tstests/unit/proxy/affinity-store.test.tstests/unit/proxy/provider-selector-affinity-priority.test.tstests/unit/proxy/replay-guard.test.tstests/unit/proxy/replay-identity.test.tstests/unit/proxy/replay-spool.test.tstests/unit/proxy/replay-store.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/proxy/stream-gate-sse-frames.test.tstests/unit/repository/message-hedge-loser-cost.test.tstests/unit/server-response-write-backpressure.test.ts
💤 Files with no reviewable changes (2)
- src/components/customs/model-vendor-icon.tsx
- tests/unit/repository/message-hedge-loser-cost.test.ts
| const { data, isLoading } = useQuery({ | ||
| queryKey: CACHE_EFFECTIVENESS_QUERY_KEY, | ||
| queryFn: async () => { | ||
| const result = await getProviderCacheEffectivenessWindows({ | ||
| limit: CACHE_EFFECTIVENESS_FETCH_LIMIT, | ||
| }); | ||
| if (!result.ok) throw new Error(result.error); | ||
| return result.data; | ||
| }, | ||
| staleTime: 60_000, | ||
| refetchOnWindowFocus: false, | ||
| }); | ||
|
|
||
| // 列表按 windowEnd 倒序,首个匹配行即该 provider 最近窗口 | ||
| const latest = data?.find((window) => window.providerId === providerId); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
固定全局上限会遗漏部分供应商的最新窗口。
接口按全局 windowEnd 排序后只返回 200 条，随后才按 providerId 筛选。窗口总数超过 200 时，未落入前 200 条的供应商会被错误显示为“暂无缓存数据”。应改为服务端按供应商返回最新窗口，或按当前供应商集合查询，而不是截断全局列表后在客户端过滤。
🤖 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]/settings/providers/_components/provider-cache-effectiveness-card.tsx
around lines 35 - 49, Update the provider cache effectiveness query around
getProviderCacheEffectivenessWindows and CACHE_EFFECTIVENESS_FETCH_LIMIT so the
server query retrieves the latest window for the requested providerId or current
provider set. Remove reliance on a fixed global limit followed by client-side
data.find filtering, ensuring providers outside the first global 200 results
still receive their latest window.
# Conflicts: # drizzle/meta/0109_snapshot.json # drizzle/meta/_journal.json
Add provider_cache_effectiveness table to track cache hit-rate metrics per provider/model/TTL-bucket over time windows, including theoretical vs observed cache tokens and confidence-adjusted effectiveness scores. Add replay_payloads table for storing verifiable request replay payloads with expiry-based cleanup support. Add cache scoring columns to message_request (cache_compatibility_key, cache_score_eligible, cache_score_excluded_reason, theoretical_cache_tokens, cache_ttl_bucket) to record per-request cache eligibility and effectiveness data. Migration renumbered to 0110 after a prior dev migration claimed 0109.
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d70547cbbf
ℹ️ 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".
| AND mr.created_at >= ${windowStart} | ||
| AND mr.created_at < ${windowEnd} |
There was a problem hiding this comment.
When a stream runs longer than the 5-minute safety lag, or finalization is otherwise delayed, its message_request row can have created_at inside a window that gets aggregated by other completed rows while cache_compatibility_key is still null. Since later runs advance windowStart from MAX(window_end) but continue filtering on mr.created_at, that late-finalized row is never picked up, permanently dropping valid cache-effectiveness samples. Use a terminal/update timestamp or an overlapping rescan so long streams are not skipped.
Useful? React with 👍 / 👎.
| const effectiveGroup = getEffectiveProviderGroup(session); | ||
| if (effectiveGroup && !checkProviderGroupMatch(provider.groupTag, effectiveGroup)) { | ||
| return null; | ||
| } | ||
| return provider; |
There was a problem hiding this comment.
When ENABLE_PREFIX_AFFINITY is on, an affinity hit returns the provider here after schedule/circuit/model/group checks but without the provider cost-limit checks that both session reuse and weighted-random selection run via RateLimitService. If a provider reaches its 5h/daily/total spend cap after an affinity binding was written, matching requests keep being routed to it until the Redis binding expires, bypassing the admin cap. Reuse the same cost-limit validation before accepting the affinity candidate.
Useful? React with 👍 / 👎.
| if (batch.length > 0) { | ||
| const appended = await this.store.appendChunks(this.identity.replayId, batch); | ||
| if (appended !== null) this.chunkCount = appended; |
There was a problem hiding this comment.
If earlier chunks have already been flushed and this final appendChunks returns null for a transient Redis failure, the code still persists the full payload and marks Redis meta as completed. tryServe() serves any non-empty Redis chunk list before consulting PG, so a retry can receive only the already-flushed prefix as a completed replay. Treat a failed final append as a hot-layer abort/delete, or avoid publishing completed in Redis and let PG serve the full payload.
Useful? React with 👍 / 👎.
| // === 前缀亲和提名（flag 门控；优先级：显式 session 绑定 > 亲和 > 加权随机）=== | ||
| if (!session.provider) { | ||
| await ProxyProviderResolver.tryPrefixAffinityNomination(session); |
There was a problem hiding this comment.
When an existing session binding is found, this branch skips tryPrefixAffinityNomination() entirely. That method is also the only place that populates session.affinity, so terminal recordAffinityWinner() no-ops for session-reused turns; after the first bound request, deeper conversation prefixes are never written and cache-effectiveness fields are excluded as no_affinity_key. Compute the fingerprint state for eligible chat requests even when the provider was already selected, while keeping nomination itself below explicit session reuse.
Useful? React with 👍 / 👎.
| // F3a 亲和写回（非流式成功；流式由 finalizeStream 的终态副作用负责） | ||
| void recordAffinityWinner(session, currentProvider.id); |
There was a problem hiding this comment.
For raw passthrough endpoints such as /v1/messages/count_tokens and /v1/responses/compact, a successful non-stream response reaches this same success path, so with prefix affinity enabled it records the request prefix as if a real generation had warmed or validated that provider. Those utility calls can overwrite chat affinity and steer later generations to the provider that merely served token counting/compaction. Gate affinity nomination/writeback on the default chat endpoint policy.
Useful? React with 👍 / 👎.
| await redis.set( | ||
| `cch:replay:owner:${replayId}`, | ||
| ownerToken, | ||
| "EX", | ||
| OWNER_LEASE_TTL_SECONDS, | ||
| "XX" |
There was a problem hiding this comment.
If an owner stream is quiet long enough for the 45s lease to expire, a retry can claim a new owner token while the original upstream is still alive. The next flush from the old owner uses SET ... XX, which overwrites whatever token is currently present instead of checking it matches ownerToken, so two owners can append to the same replay chunk list and publish mixed data. Renew with a compare-and-expire script, and stop spooling if ownership was lost.
Useful? React with 👍 / 👎.
| const appended = await this.store.appendChunks(this.identity.replayId, batch); | ||
| if (appended === null) { | ||
| // Redis 不可用：本次 replay 放弃（已写块靠 TTL 清理） | ||
| this.disable("redis_unavailable"); | ||
| return; | ||
| } | ||
| this.chunkCount = appended; | ||
| await this.writeMeta("owning"); | ||
| await this.store.renewOwnerLease(this.identity.replayId, this.ownerToken); |
There was a problem hiding this comment.
When disable() runs while an earlier appendChunks() is in flight, it deletes the replay entry and releases the owner, but this continuation resumes after the append and still writes fresh owning meta and renews the lease. A payload that exceeded REPLAY_MAX_PAYLOAD_BYTES can therefore leave a live-looking partial replay entry that later attachers follow until it stalls. Check this.disabled again after the await, or sequence cleanup through the write chain.
Useful? React with 👍 / 👎.
| const shadowGateObserver = (() => { | ||
| if (resolveStreamGateMode() !== "shadow") return null; | ||
| if (session.getEndpointPolicy().kind === "raw_passthrough") return null; | ||
| const family = mapProviderTypeToFamily(provider.providerType); | ||
| if (!family) return null; | ||
| return createShadowGateObserver({ | ||
| family, | ||
| providerId: provider.id, |
There was a problem hiding this comment.
This shadow observer is created only after the earlier Gemini passthrough branch has already returned, but STREAM_GATE_MODE=enforce is applied in the forwarder before that branch and therefore can affect Gemini passthrough traffic. Operators running shadow mode before enforce will get no divergence/error signal for exactly those Gemini streams, making the rollout data incomplete. Move the shadow observer into the passthrough pump or into the forwarder gate path.
Useful? React with 👍 / 👎.
Introduce two new system settings: streamGateMode (off/shadow/enforce) for buffering upstream output until the first valid content frame and auto-failing over on error or empty streams; and affinityIgnoreClientSessionId to force longest-prefix affinity for fingerprintable requests, skipping client session ID binding. Both settings default on and are driven by a proxy runtime settings snapshot warmed at startup. The stream gate now checks per-frame event caps inside the neutral branch. The replay subsystem gains compare-and-expire lease renewal, atomic RPUSH+EXPIRE via Lua, PG-before-completed ordering, and proper ownership release on all decline paths. Also adds cacheCoefficientBp to leaderboard entries, bumps the leaderboard cache shape to v2, enables cache effectiveness by default, and removes the per-provider cache effectiveness card.
Surface cacheCoefficientBp as a new sortable column on both the provider usage and provider cache hit rate leaderboards. The cache hit rate board now defaults to coefficient DESC with nulls last. Bumps the Redis cache shape version to v2 to prevent stale payloads.
The inline cache effectiveness card on provider list items duplicated leaderboard metrics. Remove the card component, its test, and related i18n strings. Clean up import paths in the provider cache effectiveness action and API client modules.
冲突解决：system_settings 新列与 dev 的 Discovery 设置合并（阶梯我方列在最外层）、 provider-selector 保留双方新增方法、迁移让号待重生成。 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Introduce provider_batch_apply_operations, provider_cache_effectiveness, and replay_payloads tables. Add cache scoring columns to message_request and stream gate mode plus affinity settings to system_settings. Regenerate this migration as 0112 following dev branch renumbering and repair the broken prevId chain in the 0110 snapshot.
Replace inline chunk concatenation in the forwarder with a call to buildBufferedPrefixStream, matching the multi-chunk stream builder introduced by the merged Discovery code. Extend both affinity test SessionManager mocks with getSessionBindingSnapshot and isSessionProviderCoolingDown stubs that report unavailable versioned bindings with legacy fallback enabled, so findReusable falls through to getSessionProvider and the original test intent is preserved.
第二轮推送说明（需求变更 + 全部评审意见修复）需求变更
评审修复（全部处置，明细见 PR 描述末节）
合并 dev 时的额外发现：dev 的 本地全量验证（合并 dev 最新 104e42d 后）：typecheck / lint / build / 全量单测 7805 / integration / test:v1 377 + critical coverage / openapi:check+lint / validate:migrations(114) 全绿；coverage 与 dev 基线一致。 |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Test layout and Vitest configuration =="
fd -a '^vitest\.config\.(ts|js|mts|mjs)$|^package\.json$' . | sort
if [ -f vitest.config.ts ]; then
nl -ba vitest.config.ts | sed -n '90,125p'
fi
printf '\nExisting API v1 test files (sample):\n'
fd -a -t f . tests/api/v1 2>/dev/null | head -40 || true
printf '\nExisting integration test files (sample):\n'
fd -a -t f . tests/integration 2>/dev/null | head -20 || true
echo "== Current changed-file summary =="
git diff --stat HEAD^ HEAD
printf '\n== Relevant changed paths ==\n'
git diff --name-only HEAD^ HEAD | rg '(^src/(app/v1/_lib/proxy|lib/(cache-effectiveness|config)|app/\[locale\]/(dashboard|settings)|drizzle|instrumentation)|^tests/(api/v1|unit/proxy)|^vitest\.config\.ts)' || true
echo "== Structural outlines for targeted implementation files =="
for f in \
src/app/v1/_lib/proxy/stream-gate/stream-content-gate.ts \
src/app/v1/_lib/proxy/provider-selector.ts \
src/app/v1/_lib/proxy/replay/replay-spool.ts \
src/lib/config/env.schema.ts \
src/instrumentation.ts \
src/lib/cache-effectiveness/service.ts
do
if [ -f "$f" ]; then
echo "--- $f ---"
ast-grep outline "$f" --items all --view compact | head -120
fi
done
echo "== Migration journal tail and migrations 0109-0112 =="
if [ -f drizzle/meta/_journal.json ]; then
tail -80 drizzle/meta/_journal.json
fi
fd -a -t f '^(0109|0110|0111|0112).*' drizzle drizzle/meta 2>/dev/null | sortLength of output: 1919 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "== Test layout and Vitest configuration =="
printf '%s\n' "-- vitest.config.ts lines 90-125 --"
sed -n '90,125p' vitest.config.ts
printf '%s\n' "-- package scripts related to v1/API tests --"
rg -n -C2 '"(test:v1|test:api|test:integration)"' package.json || true
printf '%s\n' "-- existing API v1 tests (sample) --"
fd -a -t f . tests/api/v1 2>/dev/null | sort | head -40 || true
printf '%s\n' "-- existing integration tests (sample) --"
fd -a -t f . tests/integration 2>/dev/null | sort | head -20 || true
echo "== Current changed-file summary =="
git diff --stat HEAD^ HEAD
printf '%s\n' "== Relevant changed paths =="
git diff --name-only HEAD^ HEAD | rg '(^src/(app/v1/_lib/proxy|lib/(cache-effectiveness|config)|app/\[locale\]/(dashboard|settings)|drizzle|instrumentation)|^tests/(api/v1|unit/proxy)|^vitest\.config\.ts)' || true
echo "== Targeted implementation outlines =="
for f in \
src/app/v1/_lib/proxy/stream-gate/stream-content-gate.ts \
src/app/v1/_lib/proxy/provider-selector.ts \
src/app/v1/_lib/proxy/replay/replay-spool.ts \
src/lib/config/env.schema.ts \
src/instrumentation.ts \
src/lib/cache-effectiveness/service.ts
do
if [ -f "$f" ]; then
echo "--- $f ---"
ast-grep outline "$f" --items all --view compact | head -100 || true
fi
done
echo "== Migration journal tail and migrations 0109-0112 =="
if [ -f drizzle/meta/_journal.json ]; then
tail -80 drizzle/meta/_journal.json
fi
fd -a -t f '^(0109|0110|0111|0112).*' drizzle drizzle/meta 2>/dev/null | sort || trueLength of output: 12235
已核对：
本次沙箱可见的仅是最新提交的 3 个文件，因此我无法据此独立复核你列出的全部修复与全量验证结果；但该目录归属问题已确认无需调整。 ✏️ Learnings added
|
🧪 测试结果
总体结果: ✅ 所有测试通过 |
| streamGateMode: "enforce", | ||
| affinityIgnoreClientSessionId: true, |
There was a problem hiding this comment.
DEFAULT_SETTINGS.streamGateMode = "enforce" and affinityIgnoreClientSessionId = true (mirrored by the migration SQL DEFAULT 'enforce' NOT NULL / DEFAULT true NOT NULL) mean both features activate immediately on all existing deployments upon running the migration. resolveStreamGateMode() reads the DB-cached setting first (getCachedProxyRuntimeSettings()?.streamGateMode), so it returns 'enforce' from the DB row, overriding the env var default of 'off'.
Concrete impact on migration day: (1) Every SSE response is now buffered until a valid content frame arrives, adding latency and triggering provider failovers for streams that previously passed through unmolested. (2) skipSessionBinding = affinityIgnoreClientSessionId && fingerprintable evaluates to true for the majority of chat requests, so existing session-bound provider assignments are silently dropped in favour of a Redis affinity lookup that returns null (no data yet), effectively replacing deterministic session reuse with random provider selection.
The PR description explicitly promises 全部功能由环境变量门控且默认关闭（暗合并，行为零变化）. To make this claim true, the migration default and DEFAULT_SETTINGS should both be 'off' / false, matching the env-var defaults (STREAM_GATE_MODE='off', ENABLE_PREFIX_AFFINITY=false).
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/config/system-settings-cache.ts
Line: 125-126
Comment:
**Defaults contradict the stated "dark launch / zero behavior change" intent**
`DEFAULT_SETTINGS.streamGateMode = "enforce"` and `affinityIgnoreClientSessionId = true` (mirrored by the migration SQL `DEFAULT 'enforce' NOT NULL` / `DEFAULT true NOT NULL`) mean both features activate immediately on all existing deployments upon running the migration. `resolveStreamGateMode()` reads the DB-cached setting first (`getCachedProxyRuntimeSettings()?.streamGateMode`), so it returns `'enforce'` from the DB row, overriding the env var default of `'off'`.
Concrete impact on migration day: (1) Every SSE response is now buffered until a valid content frame arrives, adding latency and triggering provider failovers for streams that previously passed through unmolested. (2) `skipSessionBinding = affinityIgnoreClientSessionId && fingerprintable` evaluates to `true` for the majority of chat requests, so existing session-bound provider assignments are silently dropped in favour of a Redis affinity lookup that returns `null` (no data yet), effectively replacing deterministic session reuse with random provider selection.
The PR description explicitly promises 全部功能由环境变量门控且默认关闭（暗合并，行为零变化）. To make this claim true, the migration default and `DEFAULT_SETTINGS` should both be `'off'` / `false`, matching the env-var defaults (`STREAM_GATE_MODE='off'`, `ENABLE_PREFIX_AFFINITY=false`).
How can I resolve this? If you propose a fix, please make it concise.| } catch { | ||
| return { streamGateMode: "off", affinityIgnoreClientSessionId: true }; | ||
| } | ||
| } | ||
|
|
||
| export async function getProxyRuntimeSettings(): Promise<ProxyRuntimeSettings> { | ||
| try { |
There was a problem hiding this comment.
envFallback() hardcodes affinityIgnoreClientSessionId: true, ignoring ENABLE_PREFIX_AFFINITY
When getCachedSystemSettings() throws and lastKnown is null (e.g., first request during DB startup delay), envFallback() is called and returns affinityIgnoreClientSessionId: true regardless of the ENABLE_PREFIX_AFFINITY env var. Since isAffinityRoutingEnabledWith is ENABLE_PREFIX_AFFINITY || settings.affinityIgnoreClientSessionId, the env var ENABLE_PREFIX_AFFINITY=false has no effect in the fallback path — affinity routing (and skipSessionBinding) are always on. The field should read env.ENABLE_PREFIX_AFFINITY rather than a hardcoded literal.
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/lib/system-settings/proxy-runtime.ts
Line: 32-38
Comment:
**`envFallback()` hardcodes `affinityIgnoreClientSessionId: true`, ignoring `ENABLE_PREFIX_AFFINITY`**
When `getCachedSystemSettings()` throws and `lastKnown` is `null` (e.g., first request during DB startup delay), `envFallback()` is called and returns `affinityIgnoreClientSessionId: true` regardless of the `ENABLE_PREFIX_AFFINITY` env var. Since `isAffinityRoutingEnabledWith` is `ENABLE_PREFIX_AFFINITY || settings.affinityIgnoreClientSessionId`, the env var `ENABLE_PREFIX_AFFINITY=false` has no effect in the fallback path — affinity routing (and `skipSessionBinding`) are always on. The field should read `env.ENABLE_PREFIX_AFFINITY` rather than a hardcoded literal.
How can I resolve this? If you propose a fix, please make it concise.| ALTER TABLE "system_settings" ADD COLUMN "stream_gate_mode" varchar(10) DEFAULT 'enforce' NOT NULL;--> statement-breakpoint | ||
| ALTER TABLE "system_settings" ADD COLUMN "affinity_ignore_client_session_id" boolean DEFAULT true NOT NULL;--> statement-breakpoint |
There was a problem hiding this comment.
When PostgreSQL ALTER TABLE … ADD COLUMN … DEFAULT 'enforce' NOT NULL runs, all existing rows in system_settings are immediately set to stream_gate_mode = 'enforce'. Because resolveStreamGateMode() reads the DB-cached value first (taking precedence over the STREAM_GATE_MODE env var whose default is 'off'), stream content gating in full enforce mode activates for every deployment the moment this migration runs. The same is true for affinity_ignore_client_session_id DEFAULT true. If the intent is a safe dark launch, both defaults should be 'off' / false to match the env-var defaults, with operators opting in by updating the system settings.
Prompt To Fix With AI
This is a comment left during a code review.
Path: drizzle/0112_complex_sabra.sql
Line: 55-56
Comment:
**Migration defaults activate both features on all existing deployments immediately**
When PostgreSQL `ALTER TABLE … ADD COLUMN … DEFAULT 'enforce' NOT NULL` runs, all existing rows in `system_settings` are immediately set to `stream_gate_mode = 'enforce'`. Because `resolveStreamGateMode()` reads the DB-cached value first (taking precedence over the `STREAM_GATE_MODE` env var whose default is `'off'`), stream content gating in full enforce mode activates for every deployment the moment this migration runs. The same is true for `affinity_ignore_client_session_id DEFAULT true`. If the intent is a safe dark launch, both defaults should be `'off'` / `false` to match the env-var defaults, with operators opting in by updating the system settings.
How can I resolve this? If you propose a fix, please make it concise.Remove the FNV-1a hash-based rollout gate and its env schema entry so Discovery eligibility is controlled solely by the system settings switch (discoveryEnabled) and discoveryConcurrency, both defaulting to off. The percentage-based canary added operational complexity without providing value beyond the authoritative database feature switch.
|
追加变更（应需求）：Discovery（session 首请求并发探索多供应商，dev 新功能）的控制面收敛为纯系统设置——移除 |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/app/v1/_lib/proxy/response-handler.ts (1)
4470-4498: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift在所有终态持久化分支关闭 live observability。
这些分支已写入终态详情，却未执行
closeLiveObservability()；live chain/routing trace 会残留至 TTL 到期，已完成请求可能持续显示为进行中。应在持久化成功后统一调用清理，并处理清理失败。
src/app/v1/_lib/proxy/response-handler.ts#L4470-L4498: 流式终态详情写入后关闭 live observability。src/app/v1/_lib/proxy/response-handler.ts#L2597-L2614: Gemini 非流式透传失败终态写入后关闭 live observability。src/app/v1/_lib/proxy/response-handler.ts#L2779-L2785: 非流式中止终态写入后关闭 live observability。src/app/v1/_lib/proxy/response-handler.ts#L3101-L3107: 普通非流式终态写入后关闭 live observability。src/app/v1/_lib/proxy/response-handler.ts#L6058-L6083: 无 usage 分支返回前关闭 live observability。🤖 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/response-handler.ts` around lines 4470 - 4498, After each terminal persistence path succeeds, call closeLiveObservability() and handle any cleanup failure without preventing the completed response flow. Apply this to src/app/v1/_lib/proxy/response-handler.ts ranges 4470-4498, 2597-2614, 2779-2785, and 3101-3107 after their terminal detail writes, and to 6058-6083 before returning from the no-usage branch; use the existing response-handler/session symbols and ensure every listed path performs the cleanup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/repository/leaderboard.ts`:
- Around line 709-713: 在 findProviderLeaderboardWithTimezone 和
findProviderCacheHitRateLeaderboardWithTimezone 中分别为
getProviderCacheCoefficients 调用增加 try/catch；查询失败时记录 warning，并将 cacheCoefficients
降级为空 Map，使相关字段回退为 null，同时确保排行榜核心数据继续正常返回。
---
Outside diff comments:
In `@src/app/v1/_lib/proxy/response-handler.ts`:
- Around line 4470-4498: After each terminal persistence path succeeds, call
closeLiveObservability() and handle any cleanup failure without preventing the
completed response flow. Apply this to src/app/v1/_lib/proxy/response-handler.ts
ranges 4470-4498, 2597-2614, 2779-2785, and 3101-3107 after their terminal
detail writes, and to 6058-6083 before returning from the no-usage branch; use
the existing response-handler/session symbols and ensure every listed path
performs the cleanup.
🪄 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: 58d727d5-6951-49ef-9ff0-f0e2a472f0ee
📒 Files selected for processing (74)
drizzle/0112_complex_sabra.sqldrizzle/meta/0110_snapshot.jsondrizzle/meta/0112_snapshot.jsondrizzle/meta/_journal.jsonmessages/en/dashboard.jsonmessages/en/settings/config.jsonmessages/ja/dashboard.jsonmessages/ja/settings/config.jsonmessages/ru/dashboard.jsonmessages/ru/settings/config.jsonmessages/zh-CN/dashboard.jsonmessages/zh-CN/settings/config.jsonmessages/zh-TW/dashboard.jsonmessages/zh-TW/settings/config.jsonsrc/actions/provider-cache-effectiveness.tssrc/actions/system-config.tssrc/app/[locale]/dashboard/leaderboard/_components/leaderboard-view.tsxsrc/app/[locale]/settings/config/_components/system-settings-form.tsxsrc/app/[locale]/settings/config/page.tsxsrc/app/api/admin/system-config/route.tssrc/app/v1/_lib/proxy/affinity/affinity-recorder.tssrc/app/v1/_lib/proxy/affinity/config.tssrc/app/v1/_lib/proxy/forwarder.tssrc/app/v1/_lib/proxy/provider-selector.tssrc/app/v1/_lib/proxy/replay/replay-guard.tssrc/app/v1/_lib/proxy/replay/replay-identity.tssrc/app/v1/_lib/proxy/replay/replay-spool.tssrc/app/v1/_lib/proxy/replay/replay-store.tssrc/app/v1/_lib/proxy/response-handler.tssrc/app/v1/_lib/proxy/session.tssrc/app/v1/_lib/proxy/stream-gate/frame-classifier.tssrc/app/v1/_lib/proxy/stream-gate/stream-content-gate.tssrc/drizzle/schema.tssrc/instrumentation.tssrc/lib/api-client/v1/actions/provider-cache-effectiveness.tssrc/lib/api-client/v1/openapi-types.gen.tssrc/lib/api/v1/schemas/provider-cache-effectiveness.tssrc/lib/api/v1/schemas/system-config.tssrc/lib/cache-effectiveness/service.tssrc/lib/config/env.schema.tssrc/lib/config/system-settings-cache.tssrc/lib/redis/leaderboard-cache.tssrc/lib/redis/redis-list-store.tssrc/lib/request-identity.tssrc/lib/session-manager.tssrc/lib/session-tracker.tssrc/lib/system-settings/proxy-runtime.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/system-config.tssrc/types/message.tssrc/types/system-config.tstests/unit/actions/system-config-stream-gate-affinity-settings.test.tstests/unit/api/leaderboard-route.test.tstests/unit/dashboard/leaderboard-view-cache-coefficient.test.tsxtests/unit/lib/redis-list-store.test.tstests/unit/proxy/affinity-recorder.test.tstests/unit/proxy/provider-selector-affinity-ignore-session.test.tstests/unit/proxy/provider-selector-affinity-priority.test.tstests/unit/proxy/replay-guard.test.tstests/unit/proxy/replay-identity.test.tstests/unit/proxy/replay-spool.test.tstests/unit/proxy/replay-store.test.tstests/unit/proxy/stream-gate-content-gate.test.tstests/unit/proxy/stream-gate-mode-resolution.test.tstests/unit/redis/leaderboard-cache.test.tstests/unit/repository/leaderboard-cache-coefficient.test.tstests/unit/repository/leaderboard-provider-metrics.test.tstests/unit/repository/system-config-degradation-ladder.test.tstests/unit/repository/system-config-update-missing-columns.test.ts
🚧 Files skipped from review as they are similar to previous changes (26)
- src/lib/api/v1/schemas/provider-cache-effectiveness.ts
- src/lib/api-client/v1/actions/provider-cache-effectiveness.ts
- src/actions/provider-cache-effectiveness.ts
- src/lib/config/env.schema.ts
- src/app/v1/_lib/proxy/affinity/affinity-recorder.ts
- src/app/api/admin/system-config/route.ts
- src/repository/message-write-buffer.ts
- src/app/v1/_lib/proxy/replay/replay-identity.ts
- src/repository/message.ts
- src/types/message.ts
- tests/unit/proxy/provider-selector-affinity-priority.test.ts
- src/drizzle/schema.ts
- src/instrumentation.ts
- tests/unit/proxy/affinity-recorder.test.ts
- src/lib/request-identity.ts
- src/app/v1/_lib/proxy/replay/replay-spool.ts
- src/lib/cache-effectiveness/service.ts
- tests/unit/proxy/replay-guard.test.ts
- src/lib/session-tracker.ts
- tests/unit/proxy/replay-identity.test.ts
- src/lib/redis/redis-list-store.ts
- tests/unit/proxy/stream-gate-content-gate.test.ts
- src/app/v1/_lib/proxy/replay/replay-guard.ts
- src/app/v1/_lib/proxy/forwarder.ts
- src/app/v1/_lib/proxy/stream-gate/stream-content-gate.ts
- src/app/v1/_lib/proxy/stream-gate/frame-classifier.ts
| // F3b 缓存系数合并（只加列不改序：用量榜保持 cost DESC） | ||
| const cacheCoefficients = await getProviderCacheCoefficients( | ||
| resolveLeaderboardWindow(period, timezone, dateRange) | ||
| ); | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
为 getProviderCacheCoefficients 调用添加异常兜底，避免辅助指标故障拖垂整个排行榜。
findProviderLeaderboardWithTimezone（709-713 行）与 findProviderCacheHitRateLeaderboardWithTimezone（863-867 行）新增的 getProviderCacheCoefficients 调用均未做异常捕获。缓存系数只是展示用的辅助指标，一旦该查询报错（如迁移未就位、DB 连接异常），会使 queryDatabase 整体失败；而 getLeaderboardWithCache 的降级重试会复现同一异常并直接向上抛出，最终导致用量榜和缓存命中率榜的核心数据（成本、成功率等）也一并无法加载。建议将该调用包裹 try/catch，失败时降级为空 Map（对应字段全部回退为 null）并记录警告日志，两处保持一致处理。
🛡️ 建议修复（两处均需应用）
- const cacheCoefficients = await getProviderCacheCoefficients(
- resolveLeaderboardWindow(period, timezone, dateRange)
- );
+ const cacheCoefficients = await getProviderCacheCoefficients(
+ resolveLeaderboardWindow(period, timezone, dateRange)
+ ).catch((error) => {
+ logger.warn("[Leaderboard] Failed to load cache coefficients, falling back to null", {
+ error,
+ });
+ return new Map<number, ProviderCacheCoefficient>();
+ });Also applies to: 863-867
🤖 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/leaderboard.ts` around lines 709 - 713, 在
findProviderLeaderboardWithTimezone 和
findProviderCacheHitRateLeaderboardWithTimezone 中分别为
getProviderCacheCoefficients 调用增加 try/catch；查询失败时记录 warning，并将 cacheCoefficients
降级为空 Map，使相关字段回退为 null，同时确保排行榜核心数据继续正常返回。
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aec4c764bb
ℹ️ 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".
| "session", | ||
| "warmup", | ||
| "requestFilter", | ||
| "replayAttach", |
There was a problem hiding this comment.
When ENABLE_REQUEST_REPLAY is enabled and a provider/group request filter rewrites headers or body, this new replay step can serve a completed response before providerRequestFilter ever runs or a provider is selected. That means a replay hit can return bytes generated under an older or different provider-specific filtered payload, bypassing the current provider filter rules for the same client request; include the provider/filter snapshot in the replay identity or only attach after provider-specific filtering has been applied.
Useful? React with 👍 / 👎.
| if (hedgeGateFamily) { | ||
| const gate = await runStreamContentGate(attempt.reader, { | ||
| family: hedgeGateFamily, | ||
| providerId: attempt.provider.id, | ||
| providerName: attempt.provider.name, | ||
| ...resolveStreamGateCaps(), | ||
| }); |
There was a problem hiding this comment.
In hedge mode with STREAM_GATE_MODE=enforce, this await now waits for the first content frame after the normal response timeout was cleared at header receipt, while the hedge threshold only launches another provider and does not abort this attempt. If the only remaining provider sends slow neutral SSE frames/pings or then goes quiet, no response-handler idle timer exists yet and the request can hang far beyond the configured first-byte/hedge threshold; keep a first-content timeout armed or abort the gated attempt when the threshold expires.
Useful? React with 👍 / 👎.
| const key = `${affinity.scopeTag}:${fp}`; | ||
| const theoreticalCacheTokens = tip ? Math.floor(tip.prefixBytes / BYTES_PER_TOKEN) : null; |
There was a problem hiding this comment.
When affinity lookup falls back to an ancestor prefix (for example a system-only match for a new conversation), cacheCompatibilityKey is built from that matched fingerprint but theoreticalCacheTokens still uses the current tip size. Those samples compare cache reads for the shorter matched prefix against the whole request prefix, so provider cache effectiveness is systematically understated for sys/ancestor matches; carry the matched boundary's prefixBytes and score that prefix instead.
Useful? React with 👍 / 👎.
| const parts: string[] = [SEP, role]; | ||
| parts.push(SEP, "content:", serializeUnknownContent(msg.content)); |
There was a problem hiding this comment.
For OpenAI chat requests that include inline image/audio data in content, this path serializes the entire content array into the affinity fingerprint, unlike the Anthropic/Gemini branches that hash media payloads first. Because fingerprinting now runs on the provider-selection path when affinity/cache metrics are enabled, multimodal requests with large data URLs can allocate and hash megabyte-scale strings on every request; normalize OpenAI media parts to compact digests before appending them to the fingerprint bytes.
Useful? React with 👍 / 👎.
| const claimed = await store.tryClaimOwner(identity.replayId, ownerToken); | ||
| if (claimed) { | ||
| // 清掉上一 owner 异常退出遗留的旧 LIST 残块，防止与新流拼接 | ||
| await store.deleteChunks(identity.replayId); | ||
| session.replayState = { identity, ownerToken, role: "owner" }; |
There was a problem hiding this comment.
Because this owner lease is claimed before rateLimit, provider selection, and message context creation, any later guard rejection exits the pipeline without creating a spool or calling releaseReplayOwnership(). For a rate-limited or otherwise blocked streaming request, the replay owner key then remains until its 45s TTL with no meta for attachers, so immediate identical retries cannot become the owner or replay; claim ownership after the blocking guards or add a pipeline cleanup on downstream guard failure.
Useful? React with 👍 / 👎.
| affinity: session.affinity, | ||
| succeeded: effectiveStatusCode >= 200 && effectiveStatusCode < 300, | ||
| usageObservable: usageForCost?.input_tokens != null, | ||
| streamTruncated: !streamEndedNormally, |
There was a problem hiding this comment.
This marks a stream cache-score observable whenever input tokens exist, even when the upstream never reported cache_read_input_tokens. Those rows become eligible, and the aggregation later coalesces the missing cache-read value to zero, making providers that simply omit cache-read telemetry look like they have no cache effectiveness; require the cache-read counter to be present (zero is fine) before treating the sample as observable.
Useful? React with 👍 / 👎.
…ect.hasOwn Collapse verbose !x || x.prop !== val patterns into x?.prop !== val across session binding, session manager, provider repository, and discovery settings validation. Replace Object.prototype.hasOwnProperty.call with Object.hasOwn. Apply the same optional-chaining cleanup to test-side guards in provider undo and error-details-dialog tests. No behavioural change.
Integration auth tests now skip DB-dependent cases via test.skipIf when DSN is unset, while new mock-based cases cover token kind detection, migration flags, opaque session contracts, scoped sessions, and validateKey user-status boundaries without a database. Add a new my-usage-actions unit test file that mocks the repository and infrastructure layers to exercise metadata, logs, stats, quota, and IP-geo action branches. Update the my-usage coverage config to include these and related auth unit test files so thresholds are met even when no database is available.
Add a full-chain guard pipeline test that validates step ordering, replay cache short-circuiting, and raw-passthrough fallback selection. Expand session-id error handler tests to cover settings load failures, fake-streaming error decoration, response input normalization, missing session-id skipping, and non-Error pipeline throws. Add edge-case coverage for attachSessionIdToErrorResponse and attachSessionIdToErrorMessage. Update coverage configs to reference the renamed guard-pipeline test file and trim source-file lists to match actual coverage scope.
Add test cases for dashboard log URL query serialization (actualResponseModelMismatch flag, excludeStatusCode200 priority) and time-range utilities (timezone-aware clock formatting, quick date ranges, date overflow rejection). Add session binding tests for Buffer/null Lua result parsing and numeric-to-string field normalization with corrupt-result detection. Add clipboard read support tests covering SSR, secure-context gating, and readText error boundaries.
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/unit/proxy/responses-session-id.test.ts (1)
86-90: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win补充
clone().text()读取失败的测试。当前 mock 只让
response.clone()抛错，没有覆盖 clone 成功后text()reject 的路径；而被测实现同时处理这两种失败。建议新增一个 mock，使clone()返回的对象的text()抛错，并继续断言原始响应可读取。依据
src/app/v1/_lib/proxy/error-session-id.ts:10-61的实现，该函数分别处理克隆和读取阶段的异常。🤖 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/responses-session-id.test.ts` around lines 86 - 90, 在“returns original response when error body cannot be read”测试中补充 clone 成功但其返回对象的 text() reject 的场景，覆盖 error-session-id 处理读取阶段异常的路径；继续断言函数返回原始 response 且原始响应仍可读取，并保留现有 clone() 抛错场景。
🤖 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/responses-session-id.test.ts`:
- Around line 86-90: 在“returns original response when error body cannot be
read”测试中补充 clone 成功但其返回对象的 text() reject 的场景，覆盖 error-session-id
处理读取阶段异常的路径；继续断言函数返回原始 response 且原始响应仍可读取，并保留现有 clone() 抛错场景。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 75981fc1-5d66-4c72-b14c-baf59313e4a5
📒 Files selected for processing (20)
src/app/[locale]/dashboard/logs/_components/error-details-dialog.test.tsxsrc/lib/redis/session-binding.tssrc/lib/session-manager.tssrc/lib/validation/discovery-settings.tssrc/repository/provider.tstests/api/action-adapter-openapi.unit.test.tstests/configs/include-session-id-in-errors.config.tstests/configs/my-usage.config.tstests/configs/proxy-guard-pipeline.config.tstests/integration/auth.test.tstests/unit/actions/my-usage-actions-unit.test.tstests/unit/actions/providers-patch-actions-contract.test.tstests/unit/actions/providers-undo-engine.test.tstests/unit/dashboard-logs-query-utils.test.tstests/unit/dashboard-logs-time-range-utils.test.tstests/unit/lib/redis/session-binding.test.tstests/unit/lib/utils/clipboard.test.tstests/unit/proxy/guard-pipeline-full-chain.test.tstests/unit/proxy/proxy-handler-session-id-error.test.tstests/unit/proxy/responses-session-id.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/lib/session-manager.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7aa785787e
ℹ️ 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".
| } | ||
| try { | ||
| if (!(await isAffinityRoutingEnabled())) return; | ||
| await getAffinityStore().tombstone(affinity.scopeTag, affinity.matchedFp, "failover"); |
There was a problem hiding this comment.
When a conversation-level affinity hit fails, this only tombstones the matched fingerprint. Because recordAffinityWinner() writes both the tip and the sys fingerprint to the same provider, the next request with the same prefix will skip the tombstoned tip and immediately accept the shallower sys binding for that same failed provider, so failover self-healing still sends at least the next wave of matching requests back to the bad provider. Tombstone/skip the shallower binding when it points at the failed provider as well, or carry the failed provider id through lookup.
Useful? React with 👍 / 👎.
| gt(providerCacheEffectiveness.windowEnd, start), | ||
| lte(providerCacheEffectiveness.windowEnd, end) |
There was a problem hiding this comment.
When an aggregation window straddles the requested leaderboard period boundary, filtering only by windowEnd > start includes the whole window even though the rest of the provider leaderboard filters usage rows by exact created_at bounds. This can skew daily/custom/monthly cache coefficients and even the provider-cache-hit ranking, especially on the first 1-hour lookback window after enabling the scheduler or after an off-schedule run crosses midnight. Require fully contained windows or aggregate/query buckets aligned to the leaderboard period.
Useful? React with 👍 / 👎.
| if (verdict === "content") { | ||
| return { committed: true, prefixChunks: buffered, framesSeen, readerDone: false }; |
There was a problem hiding this comment.
With STREAM_GATE_MODE=enforce, if an upstream sends a large neutral prelude and the first content frame in the same network chunk, this returns committed before the later prebufferByteCap check runs. That defeats the configured byte cap and can buffer/forward a much larger pre-content prefix than intended instead of failing over. Check bufferedBytes immediately after adding the chunk, or before accepting a content verdict from an over-cap buffer.
Useful? React with 👍 / 👎.
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
1 participant
CCHP 网关核心能力移植：流式内容门控 / 请求分离 Replay / 最长前缀亲和 + 缓存效果指标
按获批实施计划，将 CCHP（本项目的 Go 重写版，团队拥有全部版权）的三项网关能力以 TypeScript 重写方式移植回本项目。
F1 流式内容门控（系统设置「流式内容门控」，默认 enforce）
stream-gate/frame-classifier.ts：对照 CCHPcontent_signals.json规则表移植的逐帧分类器（anthropic / openai-chat / openai-responses / gemini），判定优先级 doneSentinel(terminal) > malformed > error > content > terminalRules > neutral，未知一律 neutral（fail-open）。StreamPrecommitError→ 走既有错误归类 → 供应商切换，客户端零字节感知。streamGateMode（off/shadow/enforce，默认 enforce），热路径经内存快照读取（60s TTL + 开机预热 + 保存即失效），envSTREAM_GATE_MODE仅作无快照时的兜底。F2 请求分离 + Replay（
ENABLE_REQUEST_REPLAY，默认关闭）replayId = sha256(过滤后规范化请求体 ⊕ scopeTag(keyId|format|model) ⊕ endpoint ⊕ stream ⊕ idempotencyKey?)（评审修复：基于 requestFilter 之后的逻辑体，过滤规则变更自然产生新身份），另存异盐 verifier 供 attach 严格复核。replay_payloads完成持久层；RPUSH+EXPIRE 单条 Lua 原子化（评审修复）。rateLimit之前（命中不占限流配额、不占供应商并发、不计费）；x-cch-no-replay: 1绕过读取且不再覆写已完成条目（评审修复）。F3a 最长前缀亲和路由（系统设置「忽略客户端 Session ID」，默认开启）
F_sys = H(system+tools 归一化)，F_i = H(F_{i-1} || msg_i)；易变 ID 剥离，anthropiccache_control断点额外入界，窗口上限 64。ENABLE_PREFIX_AFFINITY与该开关任一开启即启用亲和。F3b 缓存效果计费模拟（
ENABLE_CACHE_EFFECTIVENESS默认开启，仅指标展示）provider_cache_effectiveness，5 分钟调度；窗口安全滞后 5→15 分钟（评审修复：降低长流迟到终态漏采）。GET /api/v1/providers/cache-effectiveness端点保留。数据库
0112（additive、幂等；因 dev 先后占用 0110/0111 两次让号重生成）：replay_payloads、provider_cache_effectiveness两新表 +message_request五个可空列 +system_settings新增stream_gate_mode（默认 'enforce'）与affinity_ignore_client_session_id（默认 true，已接入列降级阶梯）。0110_daffy_rawhide_kid快照 prevId 误指 0108（与 0109 同父分叉），导致任何人跑db:generate都会报 collision；已修正 prevId 并在 0112 中幂等重发 0109 的provider_batch_apply_operations（dev 的 0110/0111 快照缺失该表，重发使快照链自愈；IF NOT EXISTS，已应用环境零影响）。测试与验证
首轮 ~380 个新用例基础上，本轮新增/更新约 120 个（亲和限额与忽略开关、replay 生命周期与原子性回归、排行榜系数定点公式与排序、系统设置三段式）。本地全量验证：build / lint / typecheck / 全量单测 / integration / test:v1 + critical coverage / openapi:check + lint / validate:migrations(113) 全绿；coverage 与 e2e 与 dev 基线一致（存量）。
评审意见处置明细
已修复（全部）：
persistCompleted吞错 → 抛错走 abort，机会式清理移除（调度器兜底）@/别名；orderBy windowEnd → windowStart（吻合既有索引）驳回 ×1：「
tests/api/v1/移入 tests/integration/」——tests/api/是本仓既有约定（vitest.config.ts:109收录为 API 测试，test:v1专跑），非规范偏离。明示偏差与后续
ENABLE_REQUEST_REPLAY），建议内部 key 灰度后再放量；WS/v1/responses与 fake-streaming 路径首期不共享 replay 入口（与计划一致）。🤖 Generated with Claude Code
Greptile Summary
This PR ports three CCHP gateway capabilities into TypeScript: F1 per-stream content gating (enforce/shadow/off), F2 request-replay/deduplication with a Redis-spool + PostgreSQL cold-store, and F3a/F3b longest-prefix affinity routing with cache-effectiveness metrics.
StreamPrecommitErrortriggers provider failover; shadow mode runs the same classifier without blocking.completedflip; the verifier hash prevents cross-serving on 32-bitreplayIdcollisions.provider_cache_effectivenessusing integer-only SQL arithmetic and a transaction-scoped advisory lock.Confidence Score: 4/5
PR is safe to merge with awareness of the three P2 findings and the seven issues flagged in prior review threads.
The core billing-terminal barrier, Redis Lua atomics, verifier collision guard, and integer bp math are all sound. Three minor P2 observations do not affect correctness. Seven prior-thread items remain open but are known.
stream-content-gate.ts (shadow observer coverage), replay-spool.ts (abort/teardown race), drizzle/0112_complex_sabra.sql (enforce default on migration)
Important Files Changed
Reviews (5): Last reviewed commit: "test: expand dashboard logs, session bin..." | Re-trigger Greptile