Skip to content

feat(observability): add request-level Discovery routing trace - #1351

Merged
ding113 merged 13 commits into
ding113:integration/discovery-stack-20260723from
Brisbanehuang:codex/discovery-routing-trace
Jul 23, 2026
Merged

feat(observability): add request-level Discovery routing trace#1351
ding113 merged 13 commits into
ding113:integration/discovery-stack-20260723from
Brisbanehuang:codex/discovery-routing-trace

Conversation

@Brisbanehuang

@Brisbanehuang Brisbanehuang commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Summary

This is the observability follow-up for the bounded streaming Discovery work in #1340. It is stacked on #1349 and depends on #1347 and #1348.

  • Add a versioned request-level routing_trace alongside the existing provider_chain.
  • Record Discovery eligibility, bypass reasons, Sticky probes, rounds, parallel attempts, ready/held states, SLA timeouts, fallback promotion, winner commit, terminal outcome, and Sticky binding finalization.
  • Show the trace in the existing Decision Chain surface, grouping providers from the same Discovery round in parallel.
  • Add a compact Discovery summary to the provider popover and localized UI text for all current locales.

User impact

Administrators can now explain how a request was routed instead of inferring Discovery behavior from the legacy serial provider chain.

The trace distinguishes Discovery from legacy routing, Sticky/normal/fallback attempts, first-byte winner selection from the final stream result, late fake-200 or completion failures, and Sticky generation-CAS outcomes.

Legacy requests retain the existing decision chain and additionally show why Discovery was bypassed, including non-streaming, unsupported protocol, Redis capability, and lease conflicts.

Design boundaries

  • Observability-only: provider selection, retry, billing, failover, cancellation, and Sticky semantics remain unchanged.
  • provider_chain remains the existing billing/retry contract.
  • Live traces use a separate Redis key and a coalesced latest-snapshot writer; legacy chain writes cannot overwrite concurrent Discovery events.
  • Terminal traces are persisted after request finalization; a first-byte winner is not shown as completed before stream validation.
  • Trace data is normalized through a versioned allowlist, capped at 512 events, and excludes request bodies, API keys, upstream URLs, and raw upstream error payloads.
  • Unsupported or malformed trace versions fall back to the existing decision-chain UI.

Migration

  • Add nullable message_request.routing_trace JSONB; existing rows require no backfill.
  • Restrict the usage-ledger trigger to accounting-relevant columns so trace-only updates do not rewrite ledger projections.
  • Keep old rows and rolling-upgrade snapshots readable.

Durable final-trace recovery

  • Stage the latest full trace snapshot in the Redis Hash cch:routing-trace-outbox:v1 before the post-terminal database write. Entries do not expire automatically and are deleted only after an exact-payload database acknowledgement.
  • Use a strictly increasing trace revision plus monotonic SQL so delayed or multi-instance replay cannot overwrite a newer trace. Recovery starts after migrations and scans the Hash every 30 seconds.
  • Emit rate-limited backlog warnings at 1,000 entries and errors at 10,000 entries; a backlog of 10,000 requires operator intervention.
  • If both Redis staging and the database writer are unavailable, log the boundary explicitly as recoverable: false; otherwise the staged snapshot remains recoverable across process restarts.

Stack and issue

Validation

  • Focused routing trace, Discovery, Redis storage, repository, ledger, response-handler, and dashboard regression: 304/304 tests passed.
  • Final durable routing-trace/outbox regression: 85/85 tests passed.
  • Full local suite: 7339 passed, 13 skipped, 0 failed (787 files passed, 2 skipped).
  • bun run typecheck
  • bun run lint
  • bun run format:check
  • bun run validate:migrations
  • bun run openapi:check
  • bun run openapi:lint
  • bun run i18n:audit-messages-no-emoji:fail
  • bun run build

The placeholder audit still reports pre-existing locale-equivalence findings outside the new routing trace keys.

Greptile Summary

This PR adds request-level Discovery routing traces from proxy execution through storage and dashboard display. The main changes are:

  • Versioned routing events for eligibility, attempts, winners, outcomes, and Sticky binding.
  • Durable final-trace persistence with Redis staging, monotonic database writes, and recovery.
  • A nullable routing_trace database field and accounting-trigger updates.
  • Decision Chain visualization, provider summaries, and localized labels.
  • Coverage for routing, streaming, persistence, recovery, shutdown, and UI behavior.

Confidence Score: 5/5

This looks safe to merge.

  • The latest changes keep cancelled attempts from returning to active routing state.
  • Valid natural-EOF streams remain successful while completion markers control Sticky eligibility.
  • Final trace snapshots use durable staging, monotonic writes, and exact-payload acknowledgement.
  • No blocking issue was confirmed in the updated code.

Important Files Changed

Filename Overview
src/app/v1/_lib/proxy/forwarder.ts Adds Discovery routing events and updates attempt cancellation and winner handling.
src/app/v1/_lib/proxy/response-handler.ts Finalizes stream outcomes, completion-marker behavior, and Sticky binding trace events.
src/repository/routing-trace-outbox.ts Adds Redis-backed staging, replay, exact-payload acknowledgement, and backlog reporting.
src/repository/routing-trace-persistence.ts Adds revision-aware database updates for routing traces.
src/repository/message-write-buffer.ts Adds durable post-terminal trace updates and coordinated shutdown draining.
src/repository/message.ts Connects message finalization to durable routing-trace persistence.
src/lib/lifecycle/shutdown.ts Coordinates routing-trace replay shutdown with writer, database, and Redis lifecycles.
drizzle/0111_happy_mauler.sql Adds routing-trace storage and limits ledger-trigger updates to accounting fields.

Reviews (12): Last reviewed commit: "test(discovery): preserve nested Respons..." | Re-trigger Greptile

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

该变更引入有界流式 Discovery,包括配置、协议解析、候选竞速、版本化 session binding、Redis 租约与 CAS、routingTrace 持久化、outbox 回放、终态结算、管理界面展示及相应迁移和测试覆盖。

Changes

Bounded Streaming Discovery 与路由追踪

Layer / File(s) Summary
配置、契约与数据库
.env.example, docs/*, drizzle/*, messages/*, src/types/*, src/lib/validation/*, src/actions/*, src/repository/system-config.ts
新增 Discovery 配置、routingTrace 类型、数据库字段、迁移快照、窗口校验、API 错误码、多语言文案及配置降级读取。
版本化会话绑定与 Redis 能力
src/lib/redis/{lua-scripts,session-binding,client}.ts, src/lib/session-manager.ts, src/lib/session-tracker.ts
新增 Redis Lua 原语、能力探测、租约、CAS、冷却、租户级 content-hash 会话绑定及 legacy 安全回退。
Discovery 调度与流式代理接入
src/app/v1/_lib/proxy/{discovery-coordinator,discovery-validity,forwarder}.ts, src/lib/observability/*
新增状态机、协议有效性解析、候选竞速、轮次推进、灰度分桶、取消传播和 Discovery 指标记录。
流式结算与绑定生命周期
src/app/v1/_lib/proxy/{response-handler,session,stream-finalization}.ts
扩展 completion marker 判定、Discovery lease、绑定 CAS/心跳、延迟结算、客户端中断处理及资源释放。
路由追踪持久化与展示
src/repository/*, src/lib/redis/live-chain-store.ts, src/app/[locale]/dashboard/*
实现 routingTrace 归一化、live 存储、单调写入、异步 outbox 回放、日志查询映射、Discovery 详情展示和 Popover 摘要。
验证与回归测试
tests/*, package.json, src/instrumentation.ts, src/lib/lifecycle/shutdown.ts
新增配置、绑定、Discovery、代理结算、路由追踪、outbox 回放、停机顺序及界面测试。

Estimated code review effort: 5 (Critical) | ~120 minutes

Possibly related PRs

Suggested reviewers: ding113

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 13.99% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed 标题准确概括了本次变更的核心:新增请求级 Discovery routing trace 可观测性。
Description check ✅ Passed 描述与本次变更高度一致,覆盖了路由追踪、展示、迁移和持久化等主要内容。
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai
coderabbitai Bot requested a review from ding113 July 21, 2026 09:48
Comment thread src/repository/message.ts Outdated
Comment thread src/app/v1/_lib/proxy/forwarder.ts
Comment thread src/app/v1/_lib/proxy/forwarder.ts Outdated

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces 'Bounded Streaming Discovery', an optional cold-start routing mode for streaming requests designed to reduce duplicate upstream costs while maintaining request reliability. The changes include database schema updates for new system settings, comprehensive configuration management for Discovery parameters, and a new observability routing trace system. Additionally, it implements a versioned session binding mechanism using Redis Lua scripts to ensure atomic updates and concurrency control. No review comments were provided for this pull request.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (1)
tests/unit/usage-ledger/trigger.test.ts (1)

41-44: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

建议收紧断言范围,避免贪婪正则误报。

/UPDATE OF[\s\S]*routing_trace[\s\S]*ON message_request/ 中的 [\s\S]* 会贪婪地跨越整个文件。只要 routing_trace 出现在第一处 UPDATE OF 与最后一处 ON message_request 之间的任意位置(例如 trigger.sql 里解释“为何排除 routing_trace”的注释),该否定断言就会误报失败,即便列白名单里并不含 routing_trace。建议先截取触发器的列清单再断言:

♻️ 建议的精确断言
   it("does not run the accounting projection for routing-trace-only updates", () => {
     expect(sql).toContain("AFTER INSERT OR UPDATE OF");
-    expect(sql).not.toMatch(/UPDATE OF[\s\S]*routing_trace[\s\S]*ON message_request/);
+    const triggerColumns = sql.match(/AFTER INSERT OR UPDATE OF([\s\S]*?)ON message_request/);
+    expect(triggerColumns).not.toBeNull();
+    expect(triggerColumns?.[1]).not.toContain("routing_trace");
   });
🤖 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/usage-ledger/trigger.test.ts` around lines 41 - 44, 收紧测试中针对
routing_trace 的否定断言范围:在测试 “does not run the accounting projection for
routing-trace-only updates” 中先提取对应触发器的 UPDATE OF 列清单,再仅对该列清单断言不包含
routing_trace,避免正则跨越整个 sql 文本匹配注释或其他内容;保留现有 AFTER INSERT OR UPDATE OF 的断言。
🤖 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/v1/_lib/proxy/discovery-validity.ts`:
- Around line 132-144: 在 openai-responses 的就绪判定中补充
response.reasoning_summary_text.delta 事件:当该事件的 object.delta 通过 hasContent 检查时返回
ready: true。保持现有
output_text.delta、function_call_arguments.delta、output_item.added 及终止事件的处理不变。

In `@src/app/v1/_lib/proxy/error-handler.ts`:
- Line 677: 在 error-handler.ts 的 session.closeLiveObservability() 调用处补充 Promise
拒绝处理:保留异步调用的非阻塞行为,并链式添加 catch,按本文件其他 void 调用的既有错误处理约定记录或处理错误,避免产生未处理的 Promise
拒绝。

In `@src/lib/redis/live-chain-store.ts`:
- Around line 33-45: Update the outcome selection in inferDiscoveryPhase to
prefer terminalEvent.outcome over trace.summary?.outcome, matching
getDiscoveryTerminal in provider-chain-popover.tsx. Preserve the existing phase
mapping while ensuring a request_finished failure overrides an earlier summary
success.

In `@src/lib/validation/discovery-settings.ts`:
- Around line 15-17: Update isDiscoverySettingField to replace the in-operator
check with an Object.prototype.hasOwnProperty.call check against
DISCOVERY_FIELD_LIMITS, so only explicitly defined fields are accepted while
preserving the existing string validation.

In `@tests/unit/lib/session-manager-binding-smart.test.ts`:
- Around line 176-184: Remove the duplicate findProviderById negative assertion
in the forceUpdate=true test, and add a positive assertion that the persistence
path, such as bindingMocks.mutateLegacySessionBindingSafely, is called with the
expected arguments.

---

Nitpick comments:
In `@tests/unit/usage-ledger/trigger.test.ts`:
- Around line 41-44: 收紧测试中针对 routing_trace 的否定断言范围:在测试 “does not run the
accounting projection for routing-trace-only updates” 中先提取对应触发器的 UPDATE OF
列清单,再仅对该列清单断言不包含 routing_trace,避免正则跨越整个 sql 文本匹配注释或其他内容;保留现有 AFTER INSERT OR
UPDATE OF 的断言。
🪄 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: 1b25122d-77c6-467b-863e-4571789fcf8c

📥 Commits

Reviewing files that changed from the base of the PR and between 30bdda8 and 295b24f.

📒 Files selected for processing (106)
  • .env.example
  • docs/streaming-discovery.md
  • drizzle/0110_daffy_rawhide_kid.sql
  • drizzle/0111_happy_mauler.sql
  • drizzle/meta/0110_snapshot.json
  • drizzle/meta/0111_snapshot.json
  • drizzle/meta/_journal.json
  • messages/en/dashboard.json
  • messages/en/settings/config.json
  • messages/ja/dashboard.json
  • messages/ja/settings/config.json
  • messages/ru/dashboard.json
  • messages/ru/settings/config.json
  • messages/zh-CN/dashboard.json
  • messages/zh-CN/settings/config.json
  • messages/zh-TW/dashboard.json
  • messages/zh-TW/settings/config.json
  • package.json
  • src/actions/system-config.ts
  • src/actions/usage-logs.ts
  • src/app/[locale]/dashboard/logs/_components/error-details-dialog.test.tsx
  • src/app/[locale]/dashboard/logs/_components/error-details-dialog/components/DiscoveryTraceView.tsx
  • src/app/[locale]/dashboard/logs/_components/error-details-dialog/components/LogicTraceTab.tsx
  • src/app/[locale]/dashboard/logs/_components/error-details-dialog/components/index.ts
  • src/app/[locale]/dashboard/logs/_components/error-details-dialog/index.tsx
  • src/app/[locale]/dashboard/logs/_components/error-details-dialog/types.ts
  • src/app/[locale]/dashboard/logs/_components/provider-chain-popover.test.tsx
  • src/app/[locale]/dashboard/logs/_components/provider-chain-popover.tsx
  • src/app/[locale]/dashboard/logs/_components/usage-logs-table.tsx
  • src/app/[locale]/dashboard/logs/_components/virtualized-logs-table.tsx
  • src/app/[locale]/settings/config/_components/system-settings-form.tsx
  • src/app/[locale]/settings/config/page.tsx
  • src/app/api/admin/system-config/route.ts
  • src/app/api/v1/resources/system/handlers.ts
  • src/app/api/v1/resources/system/router.ts
  • src/app/v1/_lib/proxy/discovery-coordinator.ts
  • src/app/v1/_lib/proxy/discovery-validity.ts
  • src/app/v1/_lib/proxy/error-handler.ts
  • src/app/v1/_lib/proxy/forwarder.ts
  • src/app/v1/_lib/proxy/provider-selector.ts
  • src/app/v1/_lib/proxy/response-handler.ts
  • src/app/v1/_lib/proxy/session.ts
  • src/app/v1/_lib/proxy/stream-finalization.ts
  • src/drizzle/schema.ts
  • src/lib/api-client/v1/openapi-types.gen.ts
  • src/lib/api/v1/_shared/error-envelope.ts
  • src/lib/api/v1/_shared/request-body.ts
  • src/lib/api/v1/schemas/system-config.ts
  • src/lib/config/env.schema.ts
  • src/lib/config/system-settings-cache.ts
  • src/lib/ledger-backfill/trigger.sql
  • src/lib/observability/discovery-metrics.ts
  • src/lib/redis/client.ts
  • src/lib/redis/live-chain-store.storage.test.ts
  • src/lib/redis/live-chain-store.test.ts
  • src/lib/redis/live-chain-store.ts
  • src/lib/redis/lua-scripts.ts
  • src/lib/redis/session-binding.ts
  • src/lib/session-manager.ts
  • src/lib/session-tracker.ts
  • src/lib/validation/discovery-settings.ts
  • src/lib/validation/schemas.ts
  • src/repository/_shared/transformers.ts
  • src/repository/message-write-buffer.ts
  • src/repository/message.ts
  • src/repository/system-config.ts
  • src/repository/usage-logs.ts
  • src/types/message.ts
  • src/types/routing-trace.ts
  • src/types/system-config.ts
  • tests/api/v1/system/system-config.test.ts
  • tests/configs/integration.config.ts
  • tests/configs/session-binding.config.ts
  • tests/integration/proxy-hedge-lifecycle.test.ts
  • tests/integration/session-binding-versioning-redis.test.ts
  • tests/unit/actions/system-config-save.test.ts
  • tests/unit/api/admin-system-config-route.test.ts
  • tests/unit/lib/redis/client.test.ts
  • tests/unit/lib/redis/session-binding.test.ts
  • tests/unit/lib/session-manager-binding-smart.test.ts
  • tests/unit/lib/session-manager-content-hash.test.ts
  • tests/unit/lib/session-manager-terminate-provider-sessions.test.ts
  • tests/unit/lib/session-manager-terminate-session.test.ts
  • tests/unit/lib/session-manager-versioned-binding.test.ts
  • tests/unit/lib/session-tracker-cleanup.test.ts
  • tests/unit/proxy/discovery-coordinator.test.ts
  • tests/unit/proxy/discovery-validity.test.ts
  • tests/unit/proxy/hedge-winner-dedup.test.ts
  • tests/unit/proxy/provider-selector-cross-type-model.test.ts
  • tests/unit/proxy/provider-selector-group-priority.test.ts
  • tests/unit/proxy/provider-selector-model-mismatch-binding.test.ts
  • tests/unit/proxy/proxy-forwarder-hedge-first-byte.test.ts
  • tests/unit/proxy/proxy-forwarder-provider-session-release.test.ts
  • tests/unit/proxy/response-handler-abort-listener-cleanup.test.ts
  • tests/unit/proxy/response-handler-client-abort-drain.test.ts
  • tests/unit/proxy/response-handler-endpoint-circuit-isolation.test.ts
  • tests/unit/proxy/response-handler-gemini-stream-passthrough-timeouts.test.ts
  • tests/unit/proxy/routing-trace.test.ts
  • tests/unit/proxy/terminal-outcome-contract.test.ts
  • tests/unit/repository/message-terminal-write-apis.test.ts
  • tests/unit/repository/message-write-buffer.test.ts
  • tests/unit/repository/system-config-degradation-ladder.test.ts
  • tests/unit/repository/system-config-update-missing-columns.test.ts
  • tests/unit/types/routing-trace.test.ts
  • tests/unit/usage-ledger/trigger.test.ts
  • tests/unit/validation/system-settings-discovery.test.ts

Comment thread src/app/v1/_lib/proxy/discovery-validity.ts
Comment thread src/app/v1/_lib/proxy/error-handler.ts Outdated
Comment thread src/lib/redis/live-chain-store.ts
Comment thread src/lib/validation/discovery-settings.ts
Comment thread tests/unit/lib/session-manager-binding-smart.test.ts Outdated
Comment thread src/repository/message.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

622-627: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

归一化失败时静默丢弃 routingTrace,缺少任何日志。

normalizeRoutingTrace(routingTrace) 返回空值时函数直接 return,不记录任何日志;而重试耗尽路径(Line 649)却有告警日志。这会导致某个请求的 routingTrace 从未落库时,运维完全无法定位原因,与本 PR "版本化路由追踪观测性" 的目标相悖。

🔧 建议增加归一化失败日志
 export async function updateMessageRequestRoutingTrace(
   id: number,
   routingTrace: RoutingTraceV1
 ): Promise<void> {
   const normalized = normalizeRoutingTrace(routingTrace);
-  if (!normalized) return;
+  if (!normalized) {
+    logger.warn("[MessageRequest] Skipped patching invalid routing trace", { requestId: id });
+    return;
+  }
🤖 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 622 - 627, 在
updateMessageRequestRoutingTrace 中处理 normalizeRoutingTrace
返回空值的分支时增加告警日志,记录消息标识和归一化失败的 routingTrace 上下文后再返回;保持成功路径及重试耗尽路径现有行为不变。
🤖 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.

Outside diff comments:
In `@src/repository/message.ts`:
- Around line 622-627: 在 updateMessageRequestRoutingTrace 中处理
normalizeRoutingTrace 返回空值的分支时增加告警日志,记录消息标识和归一化失败的 routingTrace
上下文后再返回;保持成功路径及重试耗尽路径现有行为不变。

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 379461de-cb1c-473e-aadf-b11486390a14

📥 Commits

Reviewing files that changed from the base of the PR and between 295b24f and 0e31397.

📒 Files selected for processing (17)
  • src/app/v1/_lib/proxy/discovery-validity.ts
  • src/app/v1/_lib/proxy/error-handler.ts
  • src/app/v1/_lib/proxy/forwarder.ts
  • src/app/v1/_lib/proxy/response-handler.ts
  • src/app/v1/_lib/proxy/stream-finalization.ts
  • src/lib/redis/live-chain-store.ts
  • src/lib/validation/discovery-settings.ts
  • src/repository/message.ts
  • tests/unit/lib/redis/live-chain-store.test.ts
  • tests/unit/lib/session-manager-binding-smart.test.ts
  • tests/unit/proxy/discovery-validity.test.ts
  • tests/unit/proxy/error-handler-durable-persistence.test.ts
  • tests/unit/proxy/proxy-forwarder-hedge-first-byte.test.ts
  • tests/unit/proxy/response-handler-client-abort-drain.test.ts
  • tests/unit/proxy/response-handler-endpoint-circuit-isolation.test.ts
  • tests/unit/repository/message-terminal-write-apis.test.ts
  • tests/unit/validation/system-settings-discovery.test.ts
🚧 Files skipped from review as they are similar to previous changes (10)
  • src/app/v1/_lib/proxy/error-handler.ts
  • tests/unit/validation/system-settings-discovery.test.ts
  • src/app/v1/_lib/proxy/stream-finalization.ts
  • src/lib/validation/discovery-settings.ts
  • src/app/v1/_lib/proxy/discovery-validity.ts
  • tests/unit/lib/session-manager-binding-smart.test.ts
  • src/app/v1/_lib/proxy/forwarder.ts
  • tests/unit/proxy/response-handler-endpoint-circuit-isolation.test.ts
  • tests/unit/proxy/proxy-forwarder-hedge-first-byte.test.ts
  • src/app/v1/_lib/proxy/response-handler.ts

Comment thread src/repository/message-write-buffer.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-write-buffer.ts (1)

329-342: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

normalizeRoutingTrace 返回 null 时会写入 jsonb 'null' 而非 SQL NULL。

value 是已定义但结构非法的 routingTrace 时,normalizeRoutingTrace(value) 返回 nullJSON.stringify(null) 得到字符串 "null",最终以 'null'::jsonb(JSON null)落库,而不是跳过或写入 SQL NULL。这条路径可经 enqueueMessageRequestUpdate(id, { routingTrace }) 到达(该入口不做前置校验,见本仓库测试对原始 trace 的直接入队),导致列中出现 JSON null 的“脏值”。建议在归一化为 null 时写 SQL NULL。

Proposed fix
       if (
         key === "providerChain" ||
         key === "routingTrace" ||
         key === "specialSettings" ||
         key === "costBreakdown"
       ) {
         if (value === null) {
           cases.push(sql`WHEN ${update.id} THEN NULL`);
           continue;
         }
-        const json = JSON.stringify(key === "routingTrace" ? normalizeRoutingTrace(value) : value);
-        cases.push(sql`WHEN ${update.id} THEN ${json}::jsonb`);
+        if (key === "routingTrace") {
+          const normalized = normalizeRoutingTrace(value);
+          if (!normalized) {
+            cases.push(sql`WHEN ${update.id} THEN NULL`);
+            continue;
+          }
+          cases.push(sql`WHEN ${update.id} THEN ${JSON.stringify(normalized)}::jsonb`);
+          continue;
+        }
+        cases.push(sql`WHEN ${update.id} THEN ${JSON.stringify(value)}::jsonb`);
         continue;
       }
🤖 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-write-buffer.ts` around lines 329 - 342, Update the
providerChain/routingTrace/specialSettings/costBreakdown handling so the
normalized result of normalizeRoutingTrace is checked before JSON.stringify;
when routingTrace normalization returns null, enqueue SQL NULL via the existing
null branch instead of serializing JSON null. Preserve current JSONB
serialization for valid routingTrace values and the other supported keys.
🤖 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.

Outside diff comments:
In `@src/repository/message-write-buffer.ts`:
- Around line 329-342: Update the
providerChain/routingTrace/specialSettings/costBreakdown handling so the
normalized result of normalizeRoutingTrace is checked before JSON.stringify;
when routingTrace normalization returns null, enqueue SQL NULL via the existing
null branch instead of serializing JSON null. Preserve current JSONB
serialization for valid routingTrace values and the other supported keys.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: b25e0cdf-e1f9-477e-899a-609b1aa55116

📥 Commits

Reviewing files that changed from the base of the PR and between 0e31397 and 0c6df07.

📒 Files selected for processing (4)
  • src/repository/message-write-buffer.ts
  • src/repository/message.ts
  • tests/unit/repository/message-terminal-write-apis.test.ts
  • tests/unit/repository/message-write-buffer.test.ts

@Brisbanehuang

Copy link
Copy Markdown
Contributor Author

@coderabbitai Addressed the latest outside-diff finding in 01277fc: buildBatchUpdateSql now checks the normalized routing trace before serialization and emits SQL NULL for an invalid trace instead of JSONB null. Added a regression test covering the generated SQL. The same commit also adds the durable Redis outbox requested by Greptile.

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

@Brisbanehuang Thanks for addressing this. Emitting SQL NULL when normalization fails prevents invalid traces from being persisted as JSONB null, and the regression coverage should protect that behavior.


If you found this review helpful, would you consider giving us a shout-out on X?

Thank you for using CodeRabbit!

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
src/types/routing-trace.ts (1)

210-212: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

可选:避免对 sessionTtlSeconds 重复调用 finiteNumber

当前对 finiteNumber(config.sessionTtlSeconds) 调用了两次,可先计算一次再复用,提高可读性。

♻️ 建议改动
-    ...(finiteNumber(config.sessionTtlSeconds) !== undefined
-      ? { sessionTtlSeconds: finiteNumber(config.sessionTtlSeconds) }
-      : {}),
+    ...(() => {
+      const sessionTtlSeconds = finiteNumber(config.sessionTtlSeconds);
+      return sessionTtlSeconds !== undefined ? { sessionTtlSeconds } : {};
+    })(),
🤖 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/types/routing-trace.ts` around lines 210 - 212, 在构建配置对象的逻辑中,更新
sessionTtlSeconds 处理以仅调用一次
finiteNumber(config.sessionTtlSeconds),保存计算结果后复用于条件判断和属性赋值,保持现有 undefined
时不添加该属性的行为不变。
src/app/[locale]/dashboard/logs/_components/error-details-dialog/components/DiscoveryTraceView.tsx (1)

681-689: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

可选:将 knownCancellationKinds 提升到模块作用域。

该常量集合与组件状态无关,却在每次 AttemptCard 渲染时重新构造。提升为模块级常量可避免重复分配。

🤖 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/error-details-dialog/components/DiscoveryTraceView.tsx
around lines 681 - 689, 将 AttemptCard 中的 knownCancellationKinds
常量集合提升到模块作用域,保持其内容不变,并让组件直接复用该模块级集合,避免每次渲染重复创建。
🤖 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
`@src/app/`[locale]/dashboard/logs/_components/error-details-dialog/components/DiscoveryTraceView.tsx:
- Around line 681-689: 将 AttemptCard 中的 knownCancellationKinds
常量集合提升到模块作用域,保持其内容不变,并让组件直接复用该模块级集合,避免每次渲染重复创建。

In `@src/types/routing-trace.ts`:
- Around line 210-212: 在构建配置对象的逻辑中,更新 sessionTtlSeconds 处理以仅调用一次
finiteNumber(config.sessionTtlSeconds),保存计算结果后复用于条件判断和属性赋值,保持现有 undefined
时不添加该属性的行为不变。

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6ae8cc5c-888d-4622-99ee-afacf65cdc6a

📥 Commits

Reviewing files that changed from the base of the PR and between 01277fc and 5f14973.

📒 Files selected for processing (19)
  • messages/en/dashboard.json
  • messages/en/settings/config.json
  • messages/ja/dashboard.json
  • messages/ja/settings/config.json
  • messages/ru/dashboard.json
  • messages/ru/settings/config.json
  • messages/zh-CN/dashboard.json
  • messages/zh-CN/settings/config.json
  • messages/zh-TW/dashboard.json
  • messages/zh-TW/settings/config.json
  • src/app/[locale]/dashboard/logs/_components/error-details-dialog.test.tsx
  • src/app/[locale]/dashboard/logs/_components/error-details-dialog/components/DiscoveryTraceView.tsx
  • src/app/[locale]/dashboard/logs/_components/error-details-dialog/components/LogicTraceTab.tsx
  • src/app/[locale]/dashboard/logs/_components/provider-chain-popover.test.tsx
  • src/app/[locale]/dashboard/logs/_components/provider-chain-popover.tsx
  • src/app/[locale]/settings/config/_components/system-settings-form.tsx
  • src/app/[locale]/settings/config/page.tsx
  • src/app/v1/_lib/proxy/forwarder.ts
  • src/types/routing-trace.ts
🚧 Files skipped from review as they are similar to previous changes (12)
  • messages/zh-TW/settings/config.json
  • messages/ru/settings/config.json
  • messages/zh-CN/settings/config.json
  • messages/ru/dashboard.json
  • messages/ja/dashboard.json
  • messages/zh-TW/dashboard.json
  • messages/ja/settings/config.json
  • messages/en/dashboard.json
  • src/app/[locale]/dashboard/logs/_components/error-details-dialog/components/LogicTraceTab.tsx
  • messages/zh-CN/dashboard.json
  • src/app/[locale]/dashboard/logs/_components/provider-chain-popover.tsx
  • src/app/v1/_lib/proxy/forwarder.ts

@Brisbanehuang

Copy link
Copy Markdown
Contributor Author

@coderabbitai Addressed both latest nitpicks in c7a1dc3: sessionTtlSeconds is normalized once and the known cancellation-kind set is now module-scoped. The same commit fixes the Unit Tests failure by replacing the new full-width Japanese parentheses with the repository-required half-width form. Focused regression: 62/62 tests passed; typecheck, format check, and lint passed.

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

@Brisbanehuang Thanks for addressing these. The single normalization and module-scoped cancellation-kind set resolve the nits, and the locale punctuation fix restores the repository’s test convention. Great to see the focused tests, typecheck, formatting, and lint all passing.

🐇 ✅

@Brisbanehuang

Copy link
Copy Markdown
Contributor Author

Follow-up UI clarification pushed in 34f82f38:

  • Show the actual per-attempt role transition as Candidate -> Fallback or Sticky -> Fallback only when the source role is present in the trace. Truncated traces remain Fallback without inventing history.
  • Humanize the exact fallback-winner binding result as Not written · Fallback winner. The raw fallback_winner reason remains available in the title, while all other diagnostic reason codes remain visible.
  • Restrict the humanized binding result to summary.winnerOrigin=fallback, so normal winners are not mislabeled.

Validation:

  • request details component tests: 42/42
  • focused i18n tests: 13/13
  • typecheck and format check passed
  • lint passed with repository baseline warnings only
  • no-emoji i18n audit passed

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown

@Brisbanehuang Thanks for the clear follow-up. I’ll review the trace-derived role transitions, fallback-only binding-result humanization, and localization behavior.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@Brisbanehuang

Brisbanehuang commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up UI routing summary pushed in 31b83990:

  • Replaced the compact rounds / attempts table badge with the request entry route: Sticky, Cold start, or Rediscovery.
  • Kept rounds and attempts in the provider popover, and added the full final provider, terminal HTTP result, and winner source (Sticky, normal candidate, or fallback takeover).
  • Kept the final Discovery winner multiplier visible after multiple attempts, while preserving legacy retry/Hedge behavior.
  • Derived route labels from the authoritative Discovery trace, with bounded fallbacks for live and truncated traces.
  • Made long localized route/winner labels shrink or wrap so they do not hide the provider name or multiplier in narrow table cells.
  • Added regression coverage for Sticky failure, Sticky-to-Discovery transitions, fallback takeover, live winners without a summary, stale legacy session_reuse, truncated traces, and lease-vs-binding conflicts.

Validation:

  • focused UI regression: 79/79
  • typecheck passed
  • format check passed
  • lint passed with repository baseline warnings only
  • no-emoji i18n audit passed

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

@Brisbanehuang I’ll review the updated routing-summary UI, trace fallback behavior, winner/multiplier presentation, responsive localization, and the added regression coverage.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown

@Brisbanehuang I’ll review the updated routing-summary UI, authoritative-trace fallbacks, winner and multiplier display, responsive localized labels, and the regression coverage.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/lib/utils/provider-chain-display.ts`:
- Around line 16-20: 在 shouldShowCostBadgeInCell 的多行参数声明中,为最后一个参数 routingTrace
添加尾逗号,使其符合 Biome 的 trailing comma 格式配置。
🪄 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: 7a5eb930-90c6-417a-b524-d06dc4f83a33

📥 Commits

Reviewing files that changed from the base of the PR and between 34f82f3 and 31b8399.

📒 Files selected for processing (13)
  • messages/en/dashboard.json
  • messages/ja/dashboard.json
  • messages/ru/dashboard.json
  • messages/zh-CN/dashboard.json
  • messages/zh-TW/dashboard.json
  • src/app/[locale]/dashboard/logs/_components/provider-chain-popover.test.tsx
  • src/app/[locale]/dashboard/logs/_components/provider-chain-popover.tsx
  • src/app/[locale]/dashboard/logs/_components/usage-logs-table.test.tsx
  • src/app/[locale]/dashboard/logs/_components/usage-logs-table.tsx
  • src/app/[locale]/dashboard/logs/_components/virtualized-logs-table.test.tsx
  • src/app/[locale]/dashboard/logs/_components/virtualized-logs-table.tsx
  • src/lib/utils/provider-chain-display.test.ts
  • src/lib/utils/provider-chain-display.ts
🚧 Files skipped from review as they are similar to previous changes (6)
  • messages/ja/dashboard.json
  • messages/ru/dashboard.json
  • messages/zh-TW/dashboard.json
  • messages/en/dashboard.json
  • messages/zh-CN/dashboard.json
  • src/app/[locale]/dashboard/logs/_components/provider-chain-popover.tsx

Comment thread src/lib/utils/provider-chain-display.ts
@Brisbanehuang

Copy link
Copy Markdown
Contributor Author

Follow-up compact route badge palette pushed in f1ee7eea:

  • Cold start + normal winner: blue
  • Cold start + fallback winner: amber
  • Sticky winner: violet
  • Rediscovery + normal winner: teal
  • Rediscovery + fallback winner: slate
  • Kept lease-conflict single-route protection unchanged (leading shield + amber badge)
  • Kept winner-less/incomplete traces neutral and added full route/winner semantics to title and aria-label

Validation:

  • focused provider-chain popover tests: 26/26
  • typecheck passed
  • Prettier and focused Biome checks passed
  • full lint passed with repository baseline warnings only

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 22, 2026

Copy link
Copy Markdown
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

git show --stat --oneline --decorate --no-renames f1ee7eea
printf '\n--- changed files ---\n'
git diff-tree --no-commit-id --name-only -r f1ee7eea

printf '\n--- component outline ---\n'
ast-grep outline 'src/app/[locale]/dashboard/logs/_components/provider-chain-popover.tsx' --items all --view expanded || true

printf '\n--- follow-up diff: component and tests ---\n'
git show --format=fuller --no-ext-diff --unified=80 f1ee7eea -- \
  'src/app/[locale]/dashboard/logs/_components/provider-chain-popover.tsx' \
  'src/app/[locale]/dashboard/logs/_components/provider-chain-popover.test.tsx'

printf '\n--- palette/accessibility references ---\n'
rg -n -C 5 -e 'Rediscovery|Cold start|Sticky|lease_conflict|aria-label|title=|badge|winnerOrigin|routeMode' \
  'src/app/[locale]/dashboard/logs/_components/provider-chain-popover.tsx' \
  'src/app/[locale]/dashboard/logs/_components/provider-chain-popover.test.tsx'

Length of output: 129037


@Brisbanehuang Reviewed f1ee7eea — no issues found.

The badge mapping is trace-derived and correctly keeps winner-less/incomplete combinations neutral. The compact badge title and trigger aria-label include the localized route and winner semantics, and lease-conflict protection remains isolated with the shield and amber treatment. The added regression coverage exercises the primary palettes, neutral fallback, accessibility text, and lease-conflict styling.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@ding113
ding113 changed the base branch from dev to integration/discovery-stack-20260723 July 22, 2026 23:28
ding113 added 2 commits July 23, 2026 08:42
…0723' into agent/discovery-1351-on-integration

# Conflicts:
#	messages/en/dashboard.json
#	messages/ja/dashboard.json
#	messages/ru/dashboard.json
#	messages/zh-CN/dashboard.json
#	messages/zh-TW/dashboard.json
#	src/app/[locale]/dashboard/logs/_components/usage-logs-table.test.tsx
#	src/app/v1/_lib/proxy/forwarder.ts
#	tests/unit/proxy/response-handler-endpoint-circuit-isolation.test.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants