feat: 统一前缀亲和 Session 与 Replay 使用记录语义 - #1372
Merged
Merged
Conversation
Contributor
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
📝 WalkthroughWalkthroughChangesSession identity 与账本同步
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 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Contributor
🧪 测试结果
总体结果: ✅ 所有测试通过 |
![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: 101c215e7d
ℹ️ 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".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
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/provider-selector.ts (1)
205-263: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift会话复用命中时补全
affinity.generation，避免亲和终态写回被 CAS 拒绝。当
affinityRoutingEnabled开启但affinityIgnoreClientSessionId关闭时，findReusable(session)会先选中供应商并设置session.provider，导致tryPrefixAffinityNomination(session)被跳过，session.affinity.generation保持null。成功终态调用recordAffinityWinner(session, providerId)时，AffinityStore.put()因expectedGeneration为假值直接返回false，tip 绑定不再随对话推进更新。无论复用是否已选定供应商，只要保留session.affinity且即将触发亲和写回，应在写前端补一次 generation 读取。同时建议在AffinityStore.put()/tombstone()返回false时记录 debug 日志，便于观测 CAS 拒绝。🤖 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/provider-selector.ts` around lines 205 - 263, Update provider selection in src/app/v1/_lib/proxy/provider-selector.ts:205-263 so session reuse preserves affinity state and refreshes session.affinity.generation before affinity winner write-back, even when findReusable sets session.provider and skips tryPrefixAffinityNomination. Update src/app/v1/_lib/proxy/affinity/affinity-recorder.ts:30-36 to record debug logs whenever AffinityStore.put() or tombstone() returns false because of CAS rejection.
🧹 Nitpick comments (10)
src/app/[locale]/dashboard/logs/_components/filters/status-filters.test.tsx (1)
10-16: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win将 hook mock 的导入路径改为
@/别名。第 10 行使用了跨目录相对路径。请改用映射到
src/的@/app/[locale]/dashboard/logs/_hooks/use-lazy-filter-options路径。建议修改
-vi.mock("../../_hooks/use-lazy-filter-options", () => ({ +vi.mock("`@/app/`[locale]/dashboard/logs/_hooks/use-lazy-filter-options", () => ({🤖 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/filters/status-filters.test.tsx around lines 10 - 16, Update the useLazyStatusCodes mock in the test to import the hook through the `@/app/`[locale]/dashboard/logs/_hooks/use-lazy-filter-options alias instead of the relative ../../_hooks/use-lazy-filter-options path.Source: Coding guidelines
src/lib/ledger-backfill/service.ts (1)
190-202: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win建议：让冲突更新列集合与触发器保持一致。
fn_upsert_usage_ledger的ON CONFLICT会同步status_code、blocked_by、cost_multiplier、session_id等列，这里只同步了部分列。结果是：某一行因session_identity差异被选入 batch 时，同一行上已存在的status_code或blocked_by漂移不会被修复，backfill 与触发器会产生不同的行状态。建议把两处的更新列集合对齐，或从单一定义生成，避免后续新增列时再次出现遗漏。
🤖 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/ledger-backfill/service.ts` around lines 190 - 202, Update the ON CONFLICT update clause in the ledger backfill upsert to match the column set synchronized by fn_upsert_usage_ledger, including status_code, blocked_by, cost_multiplier, session_id, and any other trigger-managed columns. Prefer reusing a shared column definition if available so future additions cannot diverge.drizzle/0116_gigantic_zombie.sql (1)
30-113: 🚀 Performance & Scalability | 🔵 Trivial提示：事务内回退路径会长时间持锁。
如果 preflight 未成功执行（marker 缺失），DO 块会在 Drizzle 迁移事务内以非 CONCURRENTLY 方式重建 9 个索引，随后第 93-113 行还会对
usage_ledger与message_request做全表关联更新。在大表部署上，这两步都会持有 ACCESS EXCLUSIVE / 行锁并阻塞写入，直到迁移事务提交。建议在发布说明中标注该回退路径的预期停机时间，并在运维手册中说明先确认 preflight 成功（marker 存在）再执行升级。对于超大
usage_ledger，可考虑把第 93-113 行的回填改为迁移后的分批任务（复用src/lib/ledger-backfill/service.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 `@drizzle/0116_gigantic_zombie.sql` around lines 30 - 113, Document the transactional fallback in the release notes and operations guidance, including its expected downtime and the requirement to verify the migration marker before upgrading. For the existing-row synchronization after the index block, consider moving the full-table UPDATE into a post-migration batched backfill using the pagination logic from ledger-backfill service, while preserving all identity, replay-provenance, and cost normalization updates.src/drizzle/schema.ts (1)
557-557: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value应用仓库的 Biome 格式。
共享根因是这些变更未遵守配置的双引号和 100 字符行宽规则。
src/drizzle/schema.ts#L557-L557: 将类型字面量改为双引号。src/drizzle/schema.ts#L1179-L1179: 将类型字面量改为双引号。tests/unit/drizzle/session-identity-indexes.test.ts#L20-L23: 将参数化测试拆分为不超过 100 字符的行。As per coding guidelines, use Biome for code formatting with configuration: double quotes, trailing commas, 2-space indent, 100 character line width.
🤖 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/drizzle/schema.ts` at line 557, Apply the configured Biome formatting: in src/drizzle/schema.ts lines 557-557 and 1179-1179, change the session identity type literals to double quotes; in tests/unit/drizzle/session-identity-indexes.test.ts lines 20-23, reformat the parameterized test so each line stays within 100 characters while preserving trailing commas and two-space indentation.Source: Coding guidelines
src/app/api/v1/resources/sessions/handlers.ts (1)
42-61: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value考虑提取重复的查询解析逻辑。
5 个 handler 中都重复了相同的
SessionSequenceQuerySchema.safeParse和fromZodError判断代码块。可以提取一个共享的辅助函数,减少重复,便于后续统一维护查询参数校验逻辑。♻️ 提取共享辅助函数的思路
+function parseSessionSequenceQuery(c: Context) { + return SessionSequenceQuerySchema.safeParse({ + requestSequence: c.req.query("requestSequence"), + sourceSessionId: c.req.query("sourceSessionId"), + }); +}各 handler 可改为调用
parseSessionSequenceQuery(c)并在!query.success时统一返回fromZodError(...)。Also applies to: 63-82, 84-102, 126-144, 146-163
🤖 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/api/v1/resources/sessions/handlers.ts` around lines 42 - 61, 提取共享的 parseSessionSequenceQuery 辅助函数，集中处理 requestSequence 和 sourceSessionId 的读取及 SessionSequenceQuerySchema.safeParse 校验；更新 getSessionDetail 及其余 4 个相关 handler 使用该辅助函数，并保留现有 query 失败时通过 fromZodError 返回错误响应的行为。src/repository/message.ts (1)
1377-1387: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
resolveSessionIdentity会加载 identity 下的全部请求行。查询没有
limit，也没有去重。一个长期存在的 identity 可能对应成千上万条message_request记录。该函数只需要最新的sessionId、scopeTag、fingerprint以及指纹集合。终止流程（src/actions/active-sessions.tsLine 1304）在批量循环内对每个 identity 调用一次，成本会叠加。建议改为聚合查询，例如用
DISTINCT取指纹集合，并用单独的limit 1查询取最新行。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/repository/message.ts` around lines 1377 - 1387, 更新 resolveSessionIdentity 中的查询，避免加载 identity 对应的全部 message_request 行：通过单独的 limit 1 查询获取最新请求的 sessionId、scopeTag 和 fingerprint，并使用 DISTINCT 聚合获取 fingerprintChain 所需的指纹集合；保持现有 identity 与未删除请求的过滤条件不变。src/actions/active-sessions.ts (1)
1301-1323: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win批量终止在循环内串行执行 DB 查询与动态导入。
循环对每个 identity 依次
await resolveSessionIdentity，并在命中 prefix 分支时重复await import(...)。批量选中大量 Session 时，请求耗时随数量线性增长。建议把
getAffinityStore的导入提到循环外，并对各 identity 的处理做有上限的并发（例如分批Promise.all）。♻️ 建议的调整方向
const { SessionTracker } = await import("`@/lib/session-tracker`"); + const { getAffinityStore } = await import("`@/app/v1/_lib/proxy/affinity/affinity-store`"); let successCount = 0; for (const identity of allowedSessionIds) { const resolution = await resolveSessionIdentity(identity); if ( resolution?.identityKind === "prefix_affinity" && resolution.scopeTag && resolution.fingerprint ) { - const { getAffinityStore } = await import("`@/app/v1/_lib/proxy/affinity/affinity-store`"); const invalidated = await getAffinityStore().invalidate(resolution.scopeTag, [🤖 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/active-sessions.ts` around lines 1301 - 1323, 优化批量终止流程：将 getAffinityStore 的动态导入移到 allowedSessionIds 循环外并复用导入结果；重构每个 identity 的处理逻辑，使 resolveSessionIdentity、affinityStore.invalidate、SessionManager.terminateSession 及相关终止操作通过有上限的分批并发执行，避免无限制 Promise.all，同时保持 successCount 统计和 prefix_affinity 分支行为不变。src/lib/session-tracker.ts (1)
795-809: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win建议提取共用的并发计数实现。
incrementObservedConcurrentCount与incrementConcurrentCount的逻辑相同，只有键前缀不同。decrementObservedConcurrentCount与decrementConcurrentCount也是这样。请提取一个接受完整键的私有方法，让两组方法复用它。这样可以避免后续只修改一侧导致行为分叉。同时建议把
incr与expire放入同一个 pipeline。若expire单独失败，该计数键会失去 TTL 并长期残留。♻️ 建议的重构方向
+ private static async incrementCount(key: string): Promise<void> { + const redis = getRedisClient(); + if (redis?.status !== "ready") return; + try { + const pipeline = redis.pipeline(); + pipeline.incr(key); + pipeline.expire(key, 600); + await pipeline.exec(); + } catch (error) { + logger.error("SessionTracker: Failed to increment concurrent count", { error, key }); + } + } + static async incrementObservedConcurrentCount(sessionIdentity: string): Promise<void> { - const redis = getRedisClient(); - if (redis?.status !== "ready" || !sessionIdentity) return; - - try { - const key = `observed_session:${sessionIdentity}:concurrent_count`; - await redis.incr(key); - await redis.expire(key, 600); - } catch (error) { - logger.error("SessionTracker: Failed to increment observed concurrent count", { - error, - sessionIdentity, - }); - } + if (!sessionIdentity) return; + await SessionTracker.incrementCount(`observed_session:${sessionIdentity}:concurrent_count`); }Also applies to: 837-851
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/session-tracker.ts` around lines 795 - 809, 提取一个接受完整 Redis key 的私有并发计数增减方法，让 incrementObservedConcurrentCount、incrementConcurrentCount、decrementObservedConcurrentCount 和 decrementConcurrentCount 复用同一实现，仅由调用方构造不同前缀的 key；在该共享方法中使用 Redis pipeline 将 incr/decr 与 expire 一起提交，并保留现有错误日志和输入校验行为。src/app/v1/_lib/proxy/replay/replay-guard.ts (1)
99-109: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win审计写入阻塞了已完成条目的重放响应。
Line 102-108 和 Line 128-133 在返回
buildStaticResponse之前await了writeAuditRow。writeAuditRow内部包含一次 INSERT，若携带sourceRequestId还会awaittryMaterializeAudit（一次 UPDATE）。这两次数据库往返都发生在客户端收到已缓存响应之前。Replay 的设计目标是零成本、低延迟地重放已完成的响应。实时 attach 路径（
buildLiveAttachResponse调用observeLiveAuditCompletion）已经采用了非阻塞方式记录审计。已完成条目的两条同步路径应保持一致，将审计写入改为 fire-and-forget，不阻塞响应返回。♻️ 建议修复（以 redis_completed 分支为例，pg_completed 同理）
if (meta.status === "completed") { const chunks = await store.readChunks(identity.replayId, 0); if (chunks && chunks.length > 0) { - await ProxyReplayGuard.writeAuditRow( - session, - identity, - meta.statusCode, - "redis_completed", - meta.messageRequestId - ); + void ProxyReplayGuard.writeAuditRow( + session, + identity, + meta.statusCode, + "redis_completed", + meta.messageRequestId + ).catch((error) => { + logger.warn("[ReplayGuard] audit row write failed", { + error: error instanceof Error ? error.message : String(error), + }); + }); return ProxyReplayGuard.buildStaticResponse(meta, chunks.join("")); }Also applies to: 125-134
🤖 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-guard.ts` around lines 99 - 109, Update both completed replay branches in the replay guard, including the paths returning buildStaticResponse for "redis_completed" and "pg_completed", so writeAuditRow is triggered without awaiting it. Preserve the existing audit arguments and return the cached response immediately, matching the non-blocking behavior used by buildLiveAttachResponse and observeLiveAuditCompletion.src/app/v1/_lib/proxy/affinity/affinity-recorder.ts (1)
30-36: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win记录
put()/tombstone()的失败结果。
put()和tombstone()现在返回布尔值，表示 CAS 写入是否成功。当前代码忽略该返回值。当affinity.generation为null或已过期时，写入会静默失败，且没有任何日志。建议在返回
false时记录一条 debug 日志，包含scopeTag和providerId，以便观测 CAS 拒绝的发生频率。这与provider-selector.ts中session.affinity.generation在会话复用路径下始终为null的问题相关，此处是排查该问题的关键观测点。Also applies to: 63-68
🤖 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/affinity/affinity-recorder.ts` around lines 30 - 36, 检查 affinity recorder 中调用 getAffinityStore().put() 和 tombstone() 的结果，并在返回 false 时记录 debug 日志；日志需包含 affinity.scopeTag 和 providerId。保留现有写入参数与流程，仅补充对 CAS 写入失败结果的观测。
🤖 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/actions/active-sessions.ts`:
- Around line 677-700: Update the existence-check branching in the session
request flow around resolveSessionRequestLocator to use the normalized locator
result rather than the raw requestSequence input. Align the condition with
getSessionDetails by consistently using locatorResult.locator.requestSequence,
while preserving the existing checks for specific-request messages and
any-session messages.
- Around line 25-33: 统一 activeSessions 的身份信息来源：修改 getAllSessions 及其聚合/缓存流程，不要仅按
pfx: 前缀过滤或推导身份，而应保留并传递数据库提供的 sessionIdentityKind 与
sessionFingerprint；列表映射直接使用这些字段，确保与 messageRequest.sessionIdentityKind 和
resolveSessionIdentity 的终止路径一致。
In `@src/app/`[locale]/dashboard/logs/_components/usage-logs-view-virtualized.tsx:
- Line 172: Update hasStatsFilters in the usage logs view so replayFilter ===
"all" is treated as no filter and does not trigger the statistics panel;
preserve the existing activeFilterCount behavior and all other filter
conditions.
In `@src/app/v1/_lib/proxy-handler.ts`:
- Around line 26-44: 在 trackObservedSession 中为
SessionTracker.trackObservedSession 和 SessionManager.storeSessionInfo 两个
fire-and-forget 调用追加 .catch 错误处理，确保 Promise 拒绝被捕获并按 response-handler.ts
中的既有模式处理。
In `@src/app/v1/_lib/proxy/response-handler.ts`:
- Around line 6515-6521: 在 response handler 的
SessionTracker.refreshObservedSession 调用前校验
session.getSessionIdentityMetadata().identity 非空；仅当 identity 有效时才刷新 observed
session，避免无 Session ID 或 Prefix Affinity 身份的请求使用空字符串键。保持现有错误捕获与日志行为不变。
In `@src/lib/config/system-settings-cache.ts`:
- Around line 57-63: Update getFallbackStreamGateMode so its catch path returns
"enforce" instead of "off" when getEnvConfig() fails, preserving the PR’s
default Stream Gate behavior while leaving successful configuration reads
unchanged.
In `@src/lib/migrations/session-replay-index-preflight.ts`:
- Around line 96-104: Update runSessionReplayIndexPreflight in
src/lib/migrations/session-replay-index-preflight.ts at lines 96-104 to check
whether both message_request and usage_ledger exist before calling
ensurePreflightColumns, returning immediately if either is missing. Update
src/lib/migrate.ts at lines 212-215 to implement the required
MigrationIndexPreflightExecutor table-existence query using to_regclass, while
keeping the preflight invocation before migrate().
In `@src/repository/_shared/ledger-conditions.ts`:
- Line 20: Separate the billing filter from session existence and ownership
checks: keep LEDGER_BILLING_CONDITION with isReplay = false for billing, quota,
dashboard, and operational aggregates, but update aggregateSessionStats and
aggregateMultipleSessionStats or their session-access callers to use a condition
that includes replay ledger rows for getSessionMessages, getSessionDetails,
getSessionRequests, terminateActiveSession, terminateActiveSessionsBatch,
getSessionOriginChain, and getSessionResponse.
In `@src/repository/message.ts`:
- Around line 320-366: Update the WHERE clause in
materializeReplayAuditFromSource to require both replay and source records to
have deleted_at IS NULL, require source.is_replay = FALSE, and require
replayRequestId and sourceRequestId to differ; preserve the existing status and
error filters and return behavior.
In `@src/types/message.ts`:
- Around line 290-296: 在 MessageRequest 接口中补充仓储层 returning 已返回的 isReplay 与
replaySourceRequestId 字段，使调用方能够读取 Replay 标记和来源请求 ID；保持与 CreateMessageRequestData
中 is_replay、replay_source_request_id 的类型和可空性对应。
---
Outside diff comments:
In `@src/app/v1/_lib/proxy/provider-selector.ts`:
- Around line 205-263: Update provider selection in
src/app/v1/_lib/proxy/provider-selector.ts:205-263 so session reuse preserves
affinity state and refreshes session.affinity.generation before affinity winner
write-back, even when findReusable sets session.provider and skips
tryPrefixAffinityNomination. Update
src/app/v1/_lib/proxy/affinity/affinity-recorder.ts:30-36 to record debug logs
whenever AffinityStore.put() or tombstone() returns false because of CAS
rejection.
---
Nitpick comments:
In `@drizzle/0116_gigantic_zombie.sql`:
- Around line 30-113: Document the transactional fallback in the release notes
and operations guidance, including its expected downtime and the requirement to
verify the migration marker before upgrading. For the existing-row
synchronization after the index block, consider moving the full-table UPDATE
into a post-migration batched backfill using the pagination logic from
ledger-backfill service, while preserving all identity, replay-provenance, and
cost normalization updates.
In `@src/actions/active-sessions.ts`:
- Around line 1301-1323: 优化批量终止流程：将 getAffinityStore 的动态导入移到 allowedSessionIds
循环外并复用导入结果；重构每个 identity 的处理逻辑，使
resolveSessionIdentity、affinityStore.invalidate、SessionManager.terminateSession
及相关终止操作通过有上限的分批并发执行，避免无限制 Promise.all，同时保持 successCount 统计和 prefix_affinity
分支行为不变。
In `@src/app/`[locale]/dashboard/logs/_components/filters/status-filters.test.tsx:
- Around line 10-16: Update the useLazyStatusCodes mock in the test to import
the hook through the
`@/app/`[locale]/dashboard/logs/_hooks/use-lazy-filter-options alias instead of
the relative ../../_hooks/use-lazy-filter-options path.
In `@src/app/api/v1/resources/sessions/handlers.ts`:
- Around line 42-61: 提取共享的 parseSessionSequenceQuery 辅助函数，集中处理 requestSequence 和
sourceSessionId 的读取及 SessionSequenceQuerySchema.safeParse 校验；更新 getSessionDetail
及其余 4 个相关 handler 使用该辅助函数，并保留现有 query 失败时通过 fromZodError 返回错误响应的行为。
In `@src/app/v1/_lib/proxy/affinity/affinity-recorder.ts`:
- Around line 30-36: 检查 affinity recorder 中调用 getAffinityStore().put() 和
tombstone() 的结果，并在返回 false 时记录 debug 日志；日志需包含 affinity.scopeTag 和
providerId。保留现有写入参数与流程，仅补充对 CAS 写入失败结果的观测。
In `@src/app/v1/_lib/proxy/replay/replay-guard.ts`:
- Around line 99-109: Update both completed replay branches in the replay guard,
including the paths returning buildStaticResponse for "redis_completed" and
"pg_completed", so writeAuditRow is triggered without awaiting it. Preserve the
existing audit arguments and return the cached response immediately, matching
the non-blocking behavior used by buildLiveAttachResponse and
observeLiveAuditCompletion.
In `@src/drizzle/schema.ts`:
- Line 557: Apply the configured Biome formatting: in src/drizzle/schema.ts
lines 557-557 and 1179-1179, change the session identity type literals to double
quotes; in tests/unit/drizzle/session-identity-indexes.test.ts lines 20-23,
reformat the parameterized test so each line stays within 100 characters while
preserving trailing commas and two-space indentation.
In `@src/lib/ledger-backfill/service.ts`:
- Around line 190-202: Update the ON CONFLICT update clause in the ledger
backfill upsert to match the column set synchronized by fn_upsert_usage_ledger,
including status_code, blocked_by, cost_multiplier, session_id, and any other
trigger-managed columns. Prefer reusing a shared column definition if available
so future additions cannot diverge.
In `@src/lib/session-tracker.ts`:
- Around line 795-809: 提取一个接受完整 Redis key 的私有并发计数增减方法，让
incrementObservedConcurrentCount、incrementConcurrentCount、decrementObservedConcurrentCount
和 decrementConcurrentCount 复用同一实现，仅由调用方构造不同前缀的 key；在该共享方法中使用 Redis pipeline 将
incr/decr 与 expire 一起提交，并保留现有错误日志和输入校验行为。
In `@src/repository/message.ts`:
- Around line 1377-1387: 更新 resolveSessionIdentity 中的查询，避免加载 identity 对应的全部
message_request 行：通过单独的 limit 1 查询获取最新请求的 sessionId、scopeTag 和 fingerprint，并使用
DISTINCT 聚合获取 fingerprintChain 所需的指纹集合；保持现有 identity 与未删除请求的过滤条件不变。
🪄 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: 49f53ecd-66d3-4000-8edb-6cdb15ff5459
📒 Files selected for processing (128)
drizzle/0116_gigantic_zombie.sqldrizzle/meta/0116_snapshot.jsondrizzle/meta/_journal.jsonmessages/en/dashboard.jsonmessages/en/errors.jsonmessages/ja/dashboard.jsonmessages/ja/errors.jsonmessages/ru/dashboard.jsonmessages/ru/errors.jsonmessages/zh-CN/dashboard.jsonmessages/zh-CN/errors.jsonmessages/zh-TW/dashboard.jsonmessages/zh-TW/errors.jsonsrc/actions/active-sessions.tssrc/actions/concurrent-sessions.tssrc/actions/session-origin-chain.tssrc/actions/session-response.tssrc/app/[locale]/dashboard/_components/bento/live-sessions-panel.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog.test.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog/components/LogicTraceTab.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog/index.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog/types.tssrc/app/[locale]/dashboard/logs/_components/filters/active-filters-display.tsxsrc/app/[locale]/dashboard/logs/_components/filters/status-filters.test.tsxsrc/app/[locale]/dashboard/logs/_components/filters/status-filters.tsxsrc/app/[locale]/dashboard/logs/_components/filters/types.tssrc/app/[locale]/dashboard/logs/_components/usage-logs-filters.tsxsrc/app/[locale]/dashboard/logs/_components/usage-logs-stats-panel.tsxsrc/app/[locale]/dashboard/logs/_components/usage-logs-table.test.tsxsrc/app/[locale]/dashboard/logs/_components/usage-logs-view-virtualized.test.tsxsrc/app/[locale]/dashboard/logs/_components/usage-logs-view-virtualized.tsxsrc/app/[locale]/dashboard/logs/_components/virtualized-logs-table.test.tsxsrc/app/[locale]/dashboard/logs/_components/virtualized-logs-table.tsxsrc/app/[locale]/dashboard/logs/_utils/logs-query.test.tssrc/app/[locale]/dashboard/logs/_utils/logs-query.tssrc/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/request-list-sidebar.tsxsrc/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client-actions.test.tsxsrc/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client.tsxsrc/app/[locale]/dashboard/sessions/_components/active-sessions-table.tsxsrc/app/[locale]/dashboard/sessions/_components/session-messages-dialog.tsxsrc/app/api/v1/resources/sessions/handlers.tssrc/app/api/v1/resources/sessions/router.tssrc/app/api/v1/resources/usage-logs/handlers.tssrc/app/v1/_lib/proxy-handler.tssrc/app/v1/_lib/proxy/affinity/affinity-recorder.tssrc/app/v1/_lib/proxy/affinity/affinity-store.tssrc/app/v1/_lib/proxy/message-service.test.tssrc/app/v1/_lib/proxy/message-service.tssrc/app/v1/_lib/proxy/provider-selector.tssrc/app/v1/_lib/proxy/replay/replay-guard.tssrc/app/v1/_lib/proxy/response-handler.tssrc/app/v1/_lib/proxy/session-guard.tssrc/app/v1/_lib/proxy/session.tssrc/drizzle/schema.tssrc/lib/api-client/v1/actions/active-sessions.tssrc/lib/api-client/v1/actions/session-origin-chain.tssrc/lib/api-client/v1/actions/session-response.tssrc/lib/api-client/v1/openapi-types.gen.tssrc/lib/api/v1/schemas/sessions.tssrc/lib/api/v1/schemas/usage-logs.tssrc/lib/availability/availability-service.tssrc/lib/config/env.schema.tssrc/lib/config/system-settings-cache.tssrc/lib/ledger-backfill/service.tssrc/lib/ledger-backfill/trigger.sqlsrc/lib/migrate.tssrc/lib/migrations/session-replay-index-preflight.tssrc/lib/proxy-status-tracker.tssrc/lib/redis/active-session-keys.tssrc/lib/request-identity.tssrc/lib/session-request-locator.tssrc/lib/session-tracker.tssrc/lib/utils/error-messages.tssrc/repository/_shared/ledger-conditions.tssrc/repository/_shared/usage-log-filters.tssrc/repository/cache-hit-rate-alert.tssrc/repository/key.tssrc/repository/message.tssrc/repository/provider.tssrc/repository/usage-logs.tssrc/types/message.tssrc/types/session.tstests/api/v1/sessions/sessions.test.tstests/api/v1/usage-logs/usage-logs.test.tstests/integration/ledger-consistency.test.tstests/integration/usage-ledger.test.tstests/unit/actions/active-sessions-detail-snapshots.test.tstests/unit/actions/active-sessions-requests.test.tstests/unit/actions/active-sessions-special-settings.test.tstests/unit/actions/active-sessions-termination.test.tstests/unit/actions/session-origin-chain-integration.test.tstests/unit/actions/session-origin-chain.test.tstests/unit/actions/session-response.test.tstests/unit/api/v1/api-client-actions.test.tstests/unit/drizzle/session-identity-indexes.test.tstests/unit/drizzle/session-replay-migration.test.tstests/unit/drizzle/usage-ledger-cost-indexes.test.tstests/unit/i18n/session-request-errors.test.tstests/unit/lib/availability-service.test.tstests/unit/lib/cache-effectiveness-gate.test.tstests/unit/lib/config/system-settings-cache.test.tstests/unit/lib/env-stream-gate-mode.test.tstests/unit/lib/proxy-status-tracker.test.tstests/unit/lib/session-replay-index-preflight.test.tstests/unit/lib/session-request-locator.test.tstests/unit/lib/session-tracker-cleanup.test.tstests/unit/proxy/affinity-recorder.test.tstests/unit/proxy/affinity-store.test.tstests/unit/proxy/connected-non-reader-lifetime.test.tstests/unit/proxy/hedge-error-pipeline.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/proxy-forwarder-hedge-first-byte.test.tstests/unit/proxy/proxy-handler-concurrency-ownership.test.tstests/unit/proxy/proxy-handler-public-errors.test.tstests/unit/proxy/proxy-handler-session-id-error.test.tstests/unit/proxy/replay-guard.test.tstests/unit/repository/cache-hit-rate-alert-integer-cast.test.tstests/unit/repository/message-aggregate-session-stats.test.tstests/unit/repository/message-replay-audit-terminal.test.tstests/unit/repository/message-session-readback.test.tstests/unit/repository/message-session-request-query.test.tstests/unit/repository/usage-logs-replay-filter.test.tstests/unit/repository/usage-logs-replay-projection.test.tstests/unit/repository/warmup-stats-exclusion.test.tstests/unit/usage-ledger/backfill.test.tstests/unit/usage-ledger/trigger.test.ts
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review Summary
This XL PR introduces substantial new session/replay semantics, but I found two regressions in the newly added plumbing that should be fixed before merge. One silently disables stream gating during a cold-cache fallback, and the other rewrites replay audit rows into the source session identity, which can misroute session detail and termination flows.
PR Size: XL
- Lines changed: 10463
- Files changed: 128
- Split suggestion: Extract replay audit/materialization + usage-log projection into one PR and active-session/prefix-identity aggregation + session management changes into another PR.
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 1 | 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/lib/config/system-settings-cache.ts:61— cold-cache fallback catches environment parse failures and silently downgradesstreamGateModetooff, which hides config errors and changes proxy behavior without any log signal.
High Priority Issues (Should Fix)
src/repository/message.ts:344— replay audit materialization overwrites the replay row’ssession_identityand affinity metadata with the source request’s values, which can make later replay rows hijack default session detail / locator / termination resolution.
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Codex AI
Contributor
There was a problem hiding this comment.
- Reviewed PR
#1372, applied thesize/XLlabel, and submitted the required review summary on the PR. - Posted a critical inline comment on
src/lib/config/system-settings-cache.ts:61for a silent fallback that can downgradestreamGateModetooffon a cold-cache settings miss when any unrelated env parse fails. - Posted a high-priority inline comment on
src/repository/message.ts:344where replay audit materialization rewritessession_identity/affinity metadata to the source request, which can misroute later session detail and termination lookups. - Summary report includes the XL split suggestion to separate replay-audit changes from active-session/prefix-identity aggregation changes.
- No other findings cleared the reporting threshold after validation against surrounding code and tests.
If you want, I can also draft the concrete code fixes for the two flagged issues.
Owner
Author
|
@codex review @coderabbitai review |
|
✅ Action performedReview finished.
|
Contributor
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/repository/message.ts (1)
1608-1621: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win统一批量会话聚合的定位条件。
aggregateSessionStats只用inArray(ledgerSessionIdentity, sessionIds)，只匹配前缀粘附的 observed identity；但getAllSessions传入的SessionManager.getAllSessionIds()还会包含物理session_id。把这些物理 ID 传入后，统计、provider、model、cacheTtl 查询都匹配不到，同一条物理 session 在单会话查询中却可能通过ledgerSessionLookup返回。批量版本应对每个sessionId使用与单会话相同的ledgerSessionLookup，或改造查询条件为or(...)。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/repository/message.ts` around lines 1608 - 1621, 统一 aggregateSessionStats 的会话定位逻辑：不要仅使用 inArray(ledgerSessionIdentity, sessionIds)，而应为每个 sessionId 通过 ledgerSessionLookup 解析并构造对应的 OR 条件，使传入的物理 session_id 与前缀粘附的 observed identity 都能匹配。确保统计及相关批量查询沿用单会话查询的定位行为。
🧹 Nitpick comments (2)
src/actions/active-sessions.ts (1)
1310-1316: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win批量终止改为串行执行，可考虑限制并发度。
每个 identity 都要串行执行
resolveSessionIdentity查询与 Redis 终止操作。批量选中数量较大时，总耗时随 identity 数量线性增长。建议按固定块大小并发处理，与SessionManager.terminateSessionsBatch的分块策略保持一致。♻️ 建议的分块并发写法
let successCount = 0; - for (const identity of allowedSessionIds) { - const resolution = await resolveSessionIdentity(identity); - if (await terminateResolvedSessionIdentity(identity, resolution)) { - successCount += 1; - } - } + const CHUNK_SIZE = 20; + for (let i = 0; i < allowedSessionIds.length; i += CHUNK_SIZE) { + const chunk = allowedSessionIds.slice(i, i + CHUNK_SIZE); + const outcomes = await Promise.all( + chunk.map(async (identity) => { + const resolution = await resolveSessionIdentity(identity); + return terminateResolvedSessionIdentity(identity, resolution); + }) + ); + successCount += outcomes.filter(Boolean).length; + }🤖 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/active-sessions.ts` around lines 1310 - 1316, Update the batch termination loop around resolveSessionIdentity and terminateResolvedSessionIdentity to process allowedSessionIds in fixed-size concurrent chunks, matching the chunking strategy used by SessionManager.terminateSessionsBatch. Await each chunk before starting the next, preserve successCount semantics, and retain per-identity resolution followed by termination.src/app/v1/_lib/proxy/session-guard.ts (1)
184-191: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win复用单次 affinity lookup 结果，避免同一请求执行两次 Redis lookup。
session-guard创建session.affinity后先调用getAffinityStore().lookup，随后ProxyProviderResolver的lookupAffinityState会再次用相同scopeTag和指纹链调用同一AffinityStore.lookup。lookup 命中活跃绑定时会写 generation、迁移旧值和刷新绑定 TTL；未命中也会生成并写入 fresh generation。建议只在会话 identity 构建时做一次 lookup，将identityFp、generation（以及需要的 affinity result）缓存在session.affinity上，后续选择/写回路径直接复用。🤖 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/session-guard.ts` around lines 184 - 191, 在 session.affinity 创建流程中复用已有的 getAffinityStore().lookup 结果，缓存完整 affinity result 及其 identityFp、generation；更新 ProxyProviderResolver.lookupAffinityState，使后续选择和写回路径优先读取 session.affinity 的缓存，避免使用相同 scopeTag 与指纹链再次执行 AffinityStore.lookup，同时保留现有命中、迁移、TTL 刷新和未命中生成 generation 的行为。
🤖 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/actions/active-sessions.ts`:
- Around line 62-72: Update the physical-session termination loop around
SessionManager.terminateSession so sources with empty providerIds are not
silently skipped; invoke termination using the supported unscoped behavior and
preserve cleanup of session info, concurrency indexes, and provider bindings.
Ensure the invalidate path only reports success according to the actual
termination result, or document the intentional
affinity-invalidated-as-terminated behavior if that is the established contract.
In `@src/app/`[locale]/dashboard/logs/_components/virtualized-logs-table.test.tsx:
- Around line 107-117: Remove the hardcoded “Replay source” text from the
ErrorDetailsDialog mocks and expose replaySourceRequestId through a
data-replay-source-request-id attribute instead. Update the related assertions
in src/app/[locale]/dashboard/logs/_components/virtualized-logs-table.test.tsx
lines 107-117 and
src/app/[locale]/dashboard/logs/_components/usage-logs-table.test.tsx lines
48-58 to verify that attribute while retaining the existing isReplay validation.
In `@src/lib/session-manager.ts`:
- Around line 2924-2931: Review the ownership check in the session termination
flow around expectedKeyId and distinguish a missing owner mirror (keyId is null)
from an actual owner change. Ensure expired owner keys do not prevent cleanup of
stale sessions when the intended semantics allow it, while still rejecting
mismatched non-null owners; if fail-closed behavior is intentional, document
that distinction explicitly at this check.
In `@src/repository/message.ts`:
- Around line 1447-1499: Update listPhysicalSessionSourcesForIdentity and its
sourcesBySession aggregation so a physical session cannot silently retain the
first row’s userId/keyId when records span multiple keys. Confirm and enforce
the one-to-one session/key relationship, or group results by the (sessionId,
keyId) pair so each returned PhysicalSessionSource carries the matching key
ownership data while still aggregating providerIds.
---
Outside diff comments:
In `@src/repository/message.ts`:
- Around line 1608-1621: 统一 aggregateSessionStats 的会话定位逻辑：不要仅使用
inArray(ledgerSessionIdentity, sessionIds)，而应为每个 sessionId 通过
ledgerSessionLookup 解析并构造对应的 OR 条件，使传入的物理 session_id 与前缀粘附的 observed identity
都能匹配。确保统计及相关批量查询沿用单会话查询的定位行为。
---
Nitpick comments:
In `@src/actions/active-sessions.ts`:
- Around line 1310-1316: Update the batch termination loop around
resolveSessionIdentity and terminateResolvedSessionIdentity to process
allowedSessionIds in fixed-size concurrent chunks, matching the chunking
strategy used by SessionManager.terminateSessionsBatch. Await each chunk before
starting the next, preserve successCount semantics, and retain per-identity
resolution followed by termination.
In `@src/app/v1/_lib/proxy/session-guard.ts`:
- Around line 184-191: 在 session.affinity 创建流程中复用已有的 getAffinityStore().lookup
结果，缓存完整 affinity result 及其 identityFp、generation；更新
ProxyProviderResolver.lookupAffinityState，使后续选择和写回路径优先读取 session.affinity
的缓存，避免使用相同 scopeTag 与指纹链再次执行 AffinityStore.lookup，同时保留现有命中、迁移、TTL 刷新和未命中生成
generation 的行为。
🪄 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: beac7130-0e0f-4a5b-987f-73ff9a3b8b0a
📒 Files selected for processing (63)
drizzle/0116_gigantic_zombie.sqlpackage.jsonscripts/migrate.tssrc/actions/active-sessions.tssrc/app/[locale]/dashboard/logs/_components/error-details-dialog.test.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog/components/LogicTraceTab.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog/components/MetadataTab.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog/components/SummaryTab.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog/index.tsxsrc/app/[locale]/dashboard/logs/_components/error-details-dialog/types.tssrc/app/[locale]/dashboard/logs/_components/usage-logs-table.test.tsxsrc/app/[locale]/dashboard/logs/_components/usage-logs-table.tsxsrc/app/[locale]/dashboard/logs/_components/usage-logs-view-virtualized.test.tsxsrc/app/[locale]/dashboard/logs/_components/usage-logs-view-virtualized.tsxsrc/app/[locale]/dashboard/logs/_components/virtualized-logs-table.test.tsxsrc/app/[locale]/dashboard/logs/_components/virtualized-logs-table.tsxsrc/app/v1/_lib/proxy-handler.tssrc/app/v1/_lib/proxy/affinity/affinity-recorder.tssrc/app/v1/_lib/proxy/affinity/affinity-store.tssrc/app/v1/_lib/proxy/provider-selector.tssrc/app/v1/_lib/proxy/replay/replay-guard.tssrc/app/v1/_lib/proxy/response-handler.tssrc/app/v1/_lib/proxy/session-guard.tssrc/app/v1/_lib/proxy/session.tssrc/lib/cache/session-cache.tssrc/lib/config/system-settings-cache.tssrc/lib/ledger-backfill/service.tssrc/lib/migrate.tssrc/lib/migrations/session-replay-index-preflight.tssrc/lib/rate-limit/service.tssrc/lib/redis/lua-scripts.tssrc/lib/session-manager.tssrc/repository/_shared/ledger-conditions.tssrc/repository/_shared/transformers.test.tssrc/repository/_shared/transformers.tssrc/repository/activity-stream.tssrc/repository/message.tssrc/repository/usage-logs.tssrc/types/message.tstests/unit/actions/active-sessions-detail-snapshots.test.tstests/unit/actions/active-sessions-termination.test.tstests/unit/drizzle/session-replay-migration.test.tstests/unit/lib/cache-effectiveness-gate.test.tstests/unit/lib/config/system-settings-cache.test.tstests/unit/lib/rate-limit/provider-session-release.test.tstests/unit/lib/session-replay-index-preflight.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/proxy-handler-concurrency-ownership.test.tstests/unit/proxy/replay-guard.test.tstests/unit/proxy/response-handler-lease-decrement.test.tstests/unit/proxy/session-guard-warmup-intercept.test.tstests/unit/repository/activity-stream-replay.test.tstests/unit/repository/message-aggregate-multiple-session-stats.test.tstests/unit/repository/message-aggregate-session-stats.test.tstests/unit/repository/message-public-readback.test.tstests/unit/repository/message-replay-audit-terminal.test.tstests/unit/repository/message-session-readback.test.tstests/unit/repository/message-session-request-query.test.tstests/unit/repository/usage-logs-actual-response-model.test.tstests/unit/usage-ledger/backfill.test.ts
💤 Files with no reviewable changes (1)
- drizzle/0116_gigantic_zombie.sql
🚧 Files skipped from review as they are similar to previous changes (20)
- tests/unit/lib/cache-effectiveness-gate.test.ts
- src/app/[locale]/dashboard/logs/_components/error-details-dialog/types.ts
- src/app/[locale]/dashboard/logs/_components/virtualized-logs-table.tsx
- src/app/v1/_lib/proxy-handler.ts
- src/app/v1/_lib/proxy/affinity/affinity-recorder.ts
- src/app/v1/_lib/proxy/response-handler.ts
- tests/unit/repository/message-replay-audit-terminal.test.ts
- tests/unit/drizzle/session-replay-migration.test.ts
- src/lib/ledger-backfill/service.ts
- tests/unit/proxy/provider-selector-affinity-ignore-session.test.ts
- src/types/message.ts
- tests/unit/usage-ledger/backfill.test.ts
- src/app/[locale]/dashboard/logs/_components/usage-logs-view-virtualized.test.tsx
- src/app/v1/_lib/proxy/session.ts
- tests/unit/actions/active-sessions-termination.test.ts
- tests/unit/actions/active-sessions-detail-snapshots.test.ts
- src/app/v1/_lib/proxy/replay/replay-guard.ts
- tests/unit/proxy/replay-guard.test.ts
- tests/unit/proxy/affinity-store.test.ts
- src/repository/usage-logs.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88cf76db09
ℹ️ 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".
Contributor
🧪 测试结果
总体结果: ✅ 所有测试通过 |
Owner
Author
|
第二轮 CodeRabbit review body 中的 3 项建议均已在
相关聚焦测试、全量 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/repository/message.ts (1)
1722-1741: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value移除永不触发的分支，简化
requestedSessionIds归并。Line 1722 已确保
userInfoMap一定包含row.session_id。因此 Line 1736 的continue永不触发，Line 1737-1740 只在同一 canonical identity 命中多个请求 ID 时生效。建议合并为 else 分支，避免读者误认为存在早退路径。♻️ 建议的简化
canonicalByRequested.set(row.requested_session_id, row.session_id); - if (!userInfoMap.has(row.session_id)) { - userInfoMap.set(row.session_id, { + const existing = userInfoMap.get(row.session_id); + if (!existing) { + userInfoMap.set(row.session_id, { sessionId: row.session_id, requestedSessionIds: [row.requested_session_id], userName: row.user_name, userId: row.user_id, keyName: row.key_name, keyId: row.key_id, sessionIdentityKind: row.session_identity_kind, sessionFingerprint: row.session_fingerprint, userAgent: row.user_agent, apiType: row.api_type, }); - } - if (!userInfoMap.has(row.session_id)) continue; - const requestedIds = userInfoMap.get(row.session_id)?.requestedSessionIds; - if (requestedIds && !requestedIds.includes(row.requested_session_id)) { - requestedIds.push(row.requested_session_id); + } else if (!existing.requestedSessionIds.includes(row.requested_session_id)) { + existing.requestedSessionIds.push(row.requested_session_id); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/repository/message.ts` around lines 1722 - 1741, 在处理 userInfoMap 的循环中，移除紧随初始化逻辑之后针对 userInfoMap.has(row.session_id) 的无效 continue 分支，并将 requestedSessionIds 的获取与追加逻辑改为对应的 else 分支；保留首次创建记录时的初始化行为，以及同一 session_id 合并不同 requested_session_id 的行为。
🤖 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/message.ts`:
- Around line 2295-2335: 在 findAdjacentSessionRequests 的 current 查询和
timelineFilter，以及 findRequestsBySessionIdentity 查询中加入对 isReplay 的过滤，仅保留 isReplay
为 false 的请求。确保运营 Session 时间线不会返回或跳转到 Replay 记录，并保持现有非 Replay 请求的排序与邻接逻辑不变。
---
Nitpick comments:
In `@src/repository/message.ts`:
- Around line 1722-1741: 在处理 userInfoMap 的循环中，移除紧随初始化逻辑之后针对
userInfoMap.has(row.session_id) 的无效 continue 分支，并将 requestedSessionIds
的获取与追加逻辑改为对应的 else 分支；保留首次创建记录时的初始化行为，以及同一 session_id 合并不同 requested_session_id
的行为。
🪄 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: cb309ad0-40f7-4e19-bf5c-f4c0b69708a6
📒 Files selected for processing (33)
src/actions/active-sessions-utils.tssrc/actions/active-sessions.tssrc/app/[locale]/dashboard/logs/_components/usage-logs-table.test.tsxsrc/app/[locale]/dashboard/logs/_components/virtualized-logs-table.test.tsxsrc/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/request-list-sidebar.tsxsrc/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client-actions.test.tsxsrc/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client.tsxsrc/app/api/v1/resources/sessions/handlers.tssrc/app/api/v1/resources/sessions/router.tssrc/app/v1/_lib/proxy/affinity/affinity-store.tssrc/app/v1/_lib/proxy/provider-selector.tssrc/app/v1/_lib/proxy/session-guard.tssrc/app/v1/_lib/proxy/session.tssrc/lib/api-client/v1/actions/active-sessions.tssrc/lib/api-client/v1/openapi-types.gen.tssrc/lib/api/v1/schemas/sessions.tssrc/lib/session-request-locator.tssrc/repository/_shared/usage-log-filters.tssrc/repository/message.tstests/api/v1/sessions/sessions.test.tstests/unit/actions/active-sessions-detail-snapshots.test.tstests/unit/actions/active-sessions-special-settings.test.tstests/unit/actions/active-sessions-termination.test.tstests/unit/api/v1/api-client-actions.test.tstests/unit/lib/cache-effectiveness-gate.test.tstests/unit/lib/session-request-locator.test.tstests/unit/proxy/affinity-recorder.test.tstests/unit/proxy/affinity-store.test.tstests/unit/proxy/provider-selector-affinity-ignore-session.test.tstests/unit/repository/message-aggregate-multiple-session-stats.test.tstests/unit/repository/message-session-request-query.test.tstests/unit/repository/usage-logs-sessionid-filter.test.tstests/unit/terminate-active-sessions-batch.test.ts
🚧 Files skipped from review as they are similar to previous changes (17)
- tests/unit/lib/session-request-locator.test.ts
- src/app/[locale]/dashboard/logs/_components/virtualized-logs-table.test.tsx
- tests/unit/api/v1/api-client-actions.test.ts
- src/app/api/v1/resources/sessions/handlers.ts
- src/app/v1/_lib/proxy/provider-selector.ts
- tests/unit/repository/message-session-request-query.test.ts
- src/lib/api-client/v1/openapi-types.gen.ts
- src/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client-actions.test.tsx
- src/app/v1/_lib/proxy/session.ts
- src/lib/api/v1/schemas/sessions.ts
- src/app/[locale]/dashboard/logs/_components/usage-logs-table.test.tsx
- tests/api/v1/sessions/sessions.test.ts
- tests/unit/proxy/affinity-recorder.test.ts
- src/app/v1/_lib/proxy/session-guard.ts
- tests/unit/proxy/affinity-store.test.ts
- src/app/v1/_lib/proxy/affinity/affinity-store.ts
- tests/unit/actions/active-sessions-detail-snapshots.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f653980221
ℹ️ 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
Author
|
最新 CodeRabbit review body 的可读性建议也已在 |
Contributor
🧪 测试结果
总体结果: ✅ 所有测试通过 |
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 55302773c0
ℹ️ 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".
Comment on lines
+185
to
+189
| const lookup = await getAffinityStore().lookup( | ||
| session.affinity.scopeTag, | ||
| fingerprintsDeepestFirst(chain), | ||
| getEnvConfig().PREFIX_AFFINITY_TTL_SECONDS | ||
| ); |
There was a problem hiding this comment.
When affinityIgnoreClientSessionId is enabled and a duplicate request is served from replay, CHAT_PIPELINE executes this session guard before replayAttach. This call is not read-only: AffinityStore.lookup() runs VALIDATE_LOOKUP_HIT_LUA, which extends the binding TTL and refreshes its descendant registry. Repeated cache-only requests can therefore keep a provider affinity alive indefinitely even though no request is routed upstream, causing a later real request to remain pinned beyond PREFIX_AFFINITY_TTL_SECONDS. Resolve the identity with a non-touch lookup here, or defer the sliding-TTL refresh until the replay guard misses.
Useful? React with 👍 / 👎.
Comment on lines
+339
to
+341
| UPDATE message_request AS replay | ||
| SET | ||
| provider_id = source.provider_id, |
There was a problem hiding this comment.
When a replay survives longer than its prefix-affinity binding, or Redis loses that binding while the PostgreSQL replay payload remains valid, the replay request is initially assigned a new tip-rooted sessionIdentity. This source materialization copies usage fields and provenance but leaves that inferred identity unchanged, so the replay appears under a different public session from the request it actually replays and is omitted when logs are filtered by the source identity. Copy the source row's session_identity, identity kind, scope tag, and affinity fingerprint metadata during this update.
Useful? React with 👍 / 👎.
This was referenced Aug 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
变更概览
TFFT/TTFB缩写，Tooltip 继续显示完整术语。all | replay | non-replay筛选。off调整为enforce，显式off/shadow配置保持有效。数据与接口
0116_gigantic_zombie.sql，包含 Session identity、Replay provenance 与查询索引，并在 migration 前执行重复数据 preflight。sessionId/sourceSessionId用于请求定位；公共 prefix identity 只用于聚合与管理操作。sourceSessionId，Usage Logs API 增加replayFilter，并重新生成 OpenAPI types。验证
bun run lint:fixbun run lintbun run typecheckbun run test: 841 files passed, 8082 tests passed, 2 files / 13 tests skippedbun run buildbun run validate:migrations: 118 migrations passedbun run openapi:generatebun run openapi:checkbun run openapi:lintbun run test:v1: 91 files / 381 tests passed; critical coverage passedgit diff --checkGreptile Summary
The PR unifies prefix-affinity session identity and Replay audit semantics across persistence, APIs, operational aggregates, and dashboard views. The previously reported migration issue is fixed by moving large index installation outside the Drizzle migration transaction.
Confidence Score: 5/5
The PR appears safe to merge.
No blocking failure remains; the previously reported synchronous index rebuild has been replaced with transaction-free concurrent index construction used by both manual and automatic migration paths.
Important Files Changed
Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD A[Acquire migration advisory lock] --> B{Base tables exist and 0116 pending?} B -->|Yes| C[Ensure index prerequisite columns] C --> D[Build and validate temporary indexes concurrently] D --> E[Replace canonical indexes] B -->|No| F[Run Drizzle migrations] E --> F F --> G[Postflight index reconciliation] G --> H[Release advisory lock]Reviews (4): Last reviewed commit: "fix: exclude replay requests from sessio..." | Re-trigger Greptile
Context used: