Conversation
Co-authored-by: tesgth032 <tesgth032@users.noreply.github.com>
#1223) * feat(anthropic): detect model from thinking signature in stream Use the thinking signature protobuf embedded in Anthropic stream content_block_delta events to extract the actual model name. This is more accurate than the message_start plain text when thinking is on. Three-state detection: - signature found → use model from signature - no signature with thinking on → fall back to plain model, warn via UI - no signature without thinking → plain model (normal) Add a UI badge and i18n labels for five locales when thinking was enabled but no signature arrived. * fix(proxy): drop claude- prefix gate and validate thinking signature model Remove the requestedModel prefix check so the detection works with model-redirect, Bedrock, and Vertex providers. Validate signature-decoded model names for length (≤128 chars) and presence of "claude" to reject spoofed payloads. Also fix several related issues: - Accept both standard and URL-safe base64 alphabets, and correct the length validation to only reject remainder 1. - Show the no-signature badge in the UI even when no model field is present. - Return fallback_no_thinking for empty streams to avoid false no-signature alerts. - Record the truthful thinkingEnabled value in audit logs instead of a source-derived heuristic. - Fix SSE chunk joining in tests to ensure proper event boundary blank lines. - Correct full-width punctuation in zh-CN and zh-TW tooltip strings. * fix(proxy): strip base64 padding and use @/ imports - Strip trailing padding from base64 input before checking invalid length, so that padded strings like "xxxxx=" are correctly rejected. - Convert relative imports to @/ path aliases for consistency. - Update test fixture to encode protobuf length fields with varint. * fix(proxy): skip thinking-signature detection for non-Anthropic models Restores a guard on the requested model family (claude- or anthropic/ prefix) so that providers using the Anthropic-compatible API path but serving non-Anthropic models (e.g. GLM) are not incorrectly classified. Without this gate, those providers could produce a fallback_no_signature_with_thinking source even though they never emit thinking signatures. Drops the requirement that the signature-decoded model name contain "claude", leaving only the length check (<= 128 chars). This allows future Anthropic model families that may drop the "claude-" prefix. Fixes import order in response-handler.ts after lint:fix.
* fix(proxy): 规范化 Responses 输出空值 * fix(proxy): 完善 Responses 输出归一化边界
* fix: return full self user list shape * fix: target self user display loading * chore: remove unreachable self user guard
* 优化公开状态页 Redis 聚合与轮询性能 * 修复公开状态页 rollup 边界问题 * 完善公开状态页 rollup 可靠性 * 完善公开状态页轮询测试清理 * 完善公开状态页轮询测试清理 --------- Co-authored-by: tesgth032 <tesgth032@users.noreply.github.com>
github-actions
Bot
added
enhancement
📝 WalkthroughWalkthrough统一将默认/示例模型更新为 gpt-5.5；引入 Anthropic 流“thinking signature”实际模型检测与“无签名”UI提示；新增 Responses 输出归一化；Public Status 升级至 v2 rollup：新键空间、读取/重建/聚合与视图更新；用户 self 接口改为 getCurrentUserDisplay；全面测试更新。 ChangesGPT-5.5 与 Public Status v2 综合更新
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120+ minutes Possibly related PRs
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request upgrades default models from GPT-5.4 to GPT-5.5, introduces Anthropic streaming thinking signature model detection, and implements a response output normalizer for Responses API payloads. It also optimizes public status aggregation by transitioning to a 5-minute rollup store and refactors user display logic to load only the current session user. The review feedback highlights a few areas for improvement: using loose equality (== null) to safely handle potential undefined values when formatting metrics, safely casting event.relatedTarget to Node to avoid TypeScript compilation errors, and applying optional chaining to previousModel?.timeline?.length to prevent runtime errors if the timeline is null or undefined.
| availability: | ||
| activeBucket.availabilityPct === null ? "—" : `${activeBucket.availabilityPct.toFixed(2)}%`, | ||
| ttfb: formatTtfb(activeBucket.ttfbMs), | ||
| tps: activeBucket.tps === null ? "—" : activeBucket.tps.toFixed(1), |
There was a problem hiding this comment.
Using strict equality === null does not guard against undefined values. If availabilityPct or tps is undefined (which is common for optional or missing API payload fields), the strict check will evaluate to false, and calling .toFixed() on undefined will throw a runtime TypeError and crash the page. Using loose equality == null safely handles both null and undefined.
| availability: | |
| activeBucket.availabilityPct === null ? "—" : `${activeBucket.availabilityPct.toFixed(2)}%`, | |
| ttfb: formatTtfb(activeBucket.ttfbMs), | |
| tps: activeBucket.tps === null ? "—" : activeBucket.tps.toFixed(1), | |
| availability: | |
| activeBucket.availabilityPct == null ? "—" : `${activeBucket.availabilityPct.toFixed(2)}%`, | |
| ttfb: formatTtfb(activeBucket.ttfbMs), | |
| tps: activeBucket.tps == null ? "—" : activeBucket.tps.toFixed(1), |
| onBlur={(event) => { | ||
| if (!event.currentTarget.contains(event.relatedTarget)) { | ||
| setActiveIndex(null); | ||
| } | ||
| }} |
There was a problem hiding this comment.
In TypeScript, event.relatedTarget is of type EventTarget | null. Since EventTarget is not assignable to Node | null, passing it directly to contains will cause a compilation error under strict type checking. Additionally, if event.relatedTarget is null (e.g., when focus leaves the window), we should still clear the active index. Checking for relatedTarget nullability and casting/checking type is safer.
| onBlur={(event) => { | |
| if (!event.currentTarget.contains(event.relatedTarget)) { | |
| setActiveIndex(null); | |
| } | |
| }} | |
| onBlur={(event) => { | |
| if (!event.relatedTarget || !event.currentTarget.contains(event.relatedTarget as Node)) { | |
| setActiveIndex(null); | |
| } | |
| }} |
| return { | ||
| ...model, | ||
| timeline: previousModel?.timeline ?? [], | ||
| timelineReusedFromPrevious: Boolean(previousModel?.timeline.length), |
There was a problem hiding this comment.
To ensure robust defensive programming, use optional chaining when accessing the length property of timeline. If timeline is ever null or undefined, previousModel?.timeline.length will throw a runtime error. Using previousModel?.timeline?.length is safer.
| timelineReusedFromPrevious: Boolean(previousModel?.timeline.length), | |
| timelineReusedFromPrevious: Boolean(previousModel?.timeline?.length), |
🧪 测试结果
总体结果: ✅ 所有测试通过 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/lib/public-status/rollup-store.ts (1)
103-108: 💤 Low value模块级缓存需注意内存与并发
cachedConfiguredGroups是模块级可变状态。在长时间运行的 Node.js 进程中：
- 缓存过期逻辑正确（基于
expiresAt时间戳）- 但在高并发场景下，多个请求可能同时触发缓存刷新
当前实现可接受，但如果未来出现竞态问题，可考虑使用
Promise缓存模式避免重复读取。🤖 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/public-status/rollup-store.ts` around lines 103 - 108, cachedConfiguredGroups is a mutable module-level cache that can cause duplicate concurrent refreshes under high load; change the refresh logic to use a Promise-based in-flight marker (e.g., store a Promise in a companion variable like cachedConfiguredGroupsFetch or convert cachedConfiguredGroups to hold a pending Promise) so concurrent callers await the same fetch instead of triggering multiple reads, and ensure the resolved value replaces cachedConfiguredGroups with the final object (including configVersion, groups, retryable, expiresAt) and that errors clear the in-flight marker so subsequent attempts can retry.src/lib/public-status/read-store.ts (1)
346-354: 💤 Low value多次调用
triggerRebuildHint可考虑合并当 legacy 读取成功时，连续调用了三次
triggerRebuildHint（rollup-coverage-incomplete、legacy-generation、config-version-mismatch）。如果triggerRebuildHint内部有 Redis 写入或其他开销，可考虑批量传递 reason 数组以减少 I/O 次数。不过若当前实现已有去重或短路逻辑（如
rebuild-hints.ts中的 TTL 检查），则当前写法可接受。🤖 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/public-status/read-store.ts` around lines 346 - 354, When legacyRead.ok is true the code calls input.triggerRebuildHint three times (rollup-coverage-incomplete, legacy-generation, and conditionally config-version-mismatch) which may incur repeated I/O; refactor the block around legacyRead.ok to collect reasons into an array (always push "rollup-coverage-incomplete" and "legacy-generation", and push "config-version-mismatch" only when input.configVersion !== legacyRead.projection.selectedManifest.configVersion) and call input.triggerRebuildHint once with that array (or introduce a helper like batchTriggerRebuildHints that accepts string[]), keeping the existing conditional logic but reducing redundant calls to triggerRebuildHint.
🤖 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/users.ts`:
- Around line 526-529: The current code logs the full error via logger.error but
returns error.message to the client; change it to always return a generic public
message while keeping the full error detail in logs. Specifically, in the block
that calls logger.error(...) and builds the message variable, remove returning
error.message and instead set the returned error string to a generic text like
"Internal server error" (or the module's standard message) while leaving the log
call unchanged (or augment it with context). Ensure the return still uses
ERROR_CODES.INTERNAL_ERROR and the same shape { ok: false, error: <generic>,
errorCode: ERROR_CODES.INTERNAL_ERROR } so the internal exception details are
not exposed to clients.
In `@src/app/v1/_lib/proxy/response-output-normalizer.ts`:
- Line 2: 将相对导入改为仓库约定的 `@/` 别名导入：在文件 response-output-normalizer.ts 中把 import type
{ ProxySession } from "./session"; 替换为使用 "`@/`..." 路径别名导入对应的 session 模块（例如 import
type { ProxySession } from "`@/app/v1/_lib/proxy/session`";），确保与 tsconfig/paths
映射一致并运行构建/类型检查以验证路径正确性。
In `@src/lib/model-vendor-icons.test.ts`:
- Line 137: Update the dynamic import in the test so it uses the project path
alias instead of a relative path: replace the import("./model-vendor-icons")
call in model-vendor-icons.test.ts with the alias-based import (e.g.
import("`@/lib/model-vendor-icons`")) so getModelVendor is loaded via the '`@/`...'
mapping; ensure the symbol name getModelVendor remains the same and the dynamic
import target matches your tsconfig/webpack alias configuration.
---
Nitpick comments:
In `@src/lib/public-status/read-store.ts`:
- Around line 346-354: When legacyRead.ok is true the code calls
input.triggerRebuildHint three times (rollup-coverage-incomplete,
legacy-generation, and conditionally config-version-mismatch) which may incur
repeated I/O; refactor the block around legacyRead.ok to collect reasons into an
array (always push "rollup-coverage-incomplete" and "legacy-generation", and
push "config-version-mismatch" only when input.configVersion !==
legacyRead.projection.selectedManifest.configVersion) and call
input.triggerRebuildHint once with that array (or introduce a helper like
batchTriggerRebuildHints that accepts string[]), keeping the existing
conditional logic but reducing redundant calls to triggerRebuildHint.
In `@src/lib/public-status/rollup-store.ts`:
- Around line 103-108: cachedConfiguredGroups is a mutable module-level cache
that can cause duplicate concurrent refreshes under high load; change the
refresh logic to use a Promise-based in-flight marker (e.g., store a Promise in
a companion variable like cachedConfiguredGroupsFetch or convert
cachedConfiguredGroups to hold a pending Promise) so concurrent callers await
the same fetch instead of triggering multiple reads, and ensure the resolved
value replaces cachedConfiguredGroups with the final object (including
configVersion, groups, retryable, expiresAt) and that errors clear the in-flight
marker so subsequent attempts can retry.
🪄 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: c4e0d4e0-828d-4ead-bfc4-9780ba4d9b1f
📒 Files selected for processing (110)
CHANGELOG.mdmessages/en/dashboard.jsonmessages/en/settings/prices.jsonmessages/en/settings/providers/form/modelSelect.jsonmessages/en/settings/providers/form/strings.jsonmessages/en/usage.jsonmessages/ja/dashboard.jsonmessages/ja/settings/prices.jsonmessages/ja/settings/providers/form/modelSelect.jsonmessages/ja/settings/providers/form/strings.jsonmessages/ja/usage.jsonmessages/providers-i18n-additions.jsonmessages/ru/dashboard.jsonmessages/ru/settings/prices.jsonmessages/ru/settings/providers/form/modelSelect.jsonmessages/ru/settings/providers/form/strings.jsonmessages/ru/usage.jsonmessages/zh-CN/dashboard.jsonmessages/zh-CN/settings/prices.jsonmessages/zh-CN/settings/providers/form/modelSelect.jsonmessages/zh-CN/settings/providers/form/strings.jsonmessages/zh-CN/usage.jsonmessages/zh-TW/dashboard.jsonmessages/zh-TW/settings/prices.jsonmessages/zh-TW/settings/providers/form/modelSelect.jsonmessages/zh-TW/settings/providers/form/strings.jsonmessages/zh-TW/usage.jsonsrc/actions/providers.tssrc/actions/users.tssrc/app/[locale]/dashboard/logs/_components/error-details-dialog/components/SummaryTab.tsxsrc/app/[locale]/dashboard/logs/_components/usage-logs-table.test.tsxsrc/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client-actions.test.tsxsrc/app/[locale]/dashboard/sessions/[sessionId]/messages/_components/session-messages-client.test.tsxsrc/app/[locale]/settings/providers/_components/forms/api-test-button.tsxsrc/app/[locale]/status/[slug]/page.tsxsrc/app/[locale]/status/_components/public-status-timeline.tsxsrc/app/[locale]/status/_components/public-status-view.tsxsrc/app/[locale]/usage-doc/page.tsxsrc/app/api/v1/resources/users/handlers.tssrc/app/v1/_lib/proxy/actual-response-model.tssrc/app/v1/_lib/proxy/anthropic-actual-response-model.tssrc/app/v1/_lib/proxy/response-fixer/index.tssrc/app/v1/_lib/proxy/response-fixer/response-fixer.test.tssrc/app/v1/_lib/proxy/response-handler.tssrc/app/v1/_lib/proxy/response-output-normalizer.tssrc/app/v1/_lib/proxy/thinking-signature-model.tssrc/app/v1/_lib/responses-ws/__tests__/server-helpers.test.tssrc/app/v1/_lib/responses-ws/__tests__/upstream-adapter.test.tssrc/lib/model-vendor-icons.test.tssrc/lib/model-vendor-icons.tsxsrc/lib/model-vendor-rules.tssrc/lib/provider-testing/data/cx_base.jsonsrc/lib/provider-testing/data/cx_codex_basic.jsonsrc/lib/provider-testing/data/cx_gpt_basic.jsonsrc/lib/provider-testing/presets.tssrc/lib/provider-testing/test-service.test.tssrc/lib/provider-testing/utils/test-prompts.tssrc/lib/public-status/aggregation-core.tssrc/lib/public-status/aggregation.tssrc/lib/public-status/config-publisher.tssrc/lib/public-status/config-snapshot.tssrc/lib/public-status/read-store.tssrc/lib/public-status/rebuild-hints.tssrc/lib/public-status/rebuild-worker.tssrc/lib/public-status/redis-contract.tssrc/lib/public-status/rollup-store.tssrc/lib/public-status/scheduler.tssrc/lib/public-status/vendor-icon-key.tssrc/lib/session-manager-detail-snapshots.test.tssrc/lib/utils/special-settings.tssrc/repository/message.tssrc/types/model-price.tssrc/types/special-settings.tstests/api/v1/model-prices/model-prices.test.tstests/api/v1/providers/providers.read.test.tstests/api/v1/users/users.test.tstests/e2e/responses-ws-codex-cli-transport.test.tstests/integration/billing-model-source.test.tstests/integration/non-chat-endpoint-fallback-observability.test.tstests/unit/actions/active-sessions-detail-snapshots.test.tstests/unit/actions/model-prices.test.tstests/unit/api/v1/api-client-actions.test.tstests/unit/codex/session-completer.test.tstests/unit/lib/utils/pricing-resolution.test.tstests/unit/proxy/actual-response-model.test.tstests/unit/proxy/anthropic-actual-response-model.test.tstests/unit/proxy/codex-provider-overrides.test.tstests/unit/proxy/non-chat-endpoint-fallback.test.tstests/unit/proxy/non-chat-endpoint-session-context.test.tstests/unit/proxy/proxy-forwarder-large-chunked-response.test.tstests/unit/proxy/proxy-forwarder-nonok-body-hang.test.tstests/unit/proxy/proxy-forwarder-raw-passthrough-regression.test.tstests/unit/proxy/response-handler-abort-listener-cleanup.test.tstests/unit/proxy/response-output-normalizer.test.tstests/unit/proxy/thinking-signature-model.test.tstests/unit/public-status/aggregation.test.tstests/unit/public-status/config-publisher.test.tstests/unit/public-status/config-snapshot.test.tstests/unit/public-status/no-db-import-guard.test.tstests/unit/public-status/public-status-view.test.tsxtests/unit/public-status/read-store.test.tstests/unit/public-status/rebuild-worker.test.tstests/unit/public-status/redis-contract.test.tstests/unit/public-status/rollup-store.test.tstests/unit/repository/message-public-status-rollup.test.tstests/unit/server-ws-close-handshake.test.tstests/unit/settings/prices/price-list-multi-provider-ui.test.tsxtests/unit/settings/providers/api-test-button.test.tsxtests/unit/usage-doc/opencode-usage-doc.test.tsxtests/unit/users-action-get-users-compat.test.ts
| logger.error("Failed to fetch current user display data:", error); | ||
| const message = | ||
| error instanceof Error ? error.message : "Failed to fetch current user display data"; | ||
| return { ok: false, error: message, errorCode: ERROR_CODES.INTERNAL_ERROR }; |
There was a problem hiding this comment.
避免向客户端返回原始异常信息。
这里直接返回 error.message，会把内部错误细节暴露给前端调用方。建议只记录详细日志，对外统一返回通用错误文案与错误码。
建议修改
} catch (error) {
logger.error("Failed to fetch current user display data:", error);
- const message =
- error instanceof Error ? error.message : "Failed to fetch current user display data";
- return { ok: false, error: message, errorCode: ERROR_CODES.INTERNAL_ERROR };
+ const tError = await getTranslations("errors");
+ return {
+ ok: false,
+ error: tError("OPERATION_FAILED"),
+ errorCode: ERROR_CODES.INTERNAL_ERROR,
+ };
}
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/actions/users.ts` around lines 526 - 529, The current code logs the full
error via logger.error but returns error.message to the client; change it to
always return a generic public message while keeping the full error detail in
logs. Specifically, in the block that calls logger.error(...) and builds the
message variable, remove returning error.message and instead set the returned
error string to a generic text like "Internal server error" (or the module's
standard message) while leaving the log call unchanged (or augment it with
context). Ensure the return still uses ERROR_CODES.INTERNAL_ERROR and the same
shape { ok: false, error: <generic>, errorCode: ERROR_CODES.INTERNAL_ERROR } so
the internal exception details are not exposed to clients.
| @@ -0,0 +1,189 @@ | |||
| import { logger } from "@/lib/logger"; | |||
| import type { ProxySession } from "./session"; | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
请改为 @/ 路径别名导入
这里使用了相对导入，和仓库 TS 导入规范不一致，建议改为 @/ 别名路径。
建议修改
-import type { ProxySession } from "./session";
+import type { ProxySession } from "`@/app/v1/_lib/proxy/session`";As per coding guidelines **/*.{ts,tsx,js,jsx}: Use path alias @/ to map to ./src/ for imports.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import type { ProxySession } from "./session"; | |
| import type { ProxySession } from "`@/app/v1/_lib/proxy/session`"; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/app/v1/_lib/proxy/response-output-normalizer.ts` at line 2, 将相对导入改为仓库约定的
`@/` 别名导入：在文件 response-output-normalizer.ts 中把 import type { ProxySession } from
"./session"; 替换为使用 "`@/`..." 路径别名导入对应的 session 模块（例如 import type { ProxySession }
from "`@/app/v1/_lib/proxy/session`";），确保与 tsconfig/paths 映射一致并运行构建/类型检查以验证路径正确性。
| vi.stubEnv("NODE_ENV", "development"); | ||
|
|
||
| try { | ||
| const { getModelVendor: getMockedModelVendor } = await import("./model-vendor-icons"); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
动态导入请统一改为 @/ 别名路径
这里是新增导入点，建议按仓库规则改为 @/ 别名，避免相对路径继续扩散。
建议修改
- const { getModelVendor: getMockedModelVendor } = await import("./model-vendor-icons");
+ const { getModelVendor: getMockedModelVendor } = await import("`@/lib/model-vendor-icons`");As per coding guidelines **/*.{ts,tsx,js,jsx}: Use path alias @/ to map to ./src/ for imports.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { getModelVendor: getMockedModelVendor } = await import("./model-vendor-icons"); | |
| const { getModelVendor: getMockedModelVendor } = await import("`@/lib/model-vendor-icons`"); |
🤖 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/model-vendor-icons.test.ts` at line 137, Update the dynamic import in
the test so it uses the project path alias instead of a relative path: replace
the import("./model-vendor-icons") call in model-vendor-icons.test.ts with the
alias-based import (e.g. import("`@/lib/model-vendor-icons`")) so getModelVendor
is loaded via the '`@/`...' mapping; ensure the symbol name getModelVendor remains
the same and the dynamic import target matches your tsconfig/webpack alias
configuration.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR #1225 Code Review Summary (release v0.8.3)
Reviewer: Automated Multi-Perspective Code Review
Confidence Threshold: 80
Files Reviewed: 110 | +5,621 / -838 lines
Size Label Applied: size/XL
Issues by Category
| # | Category | Severity | Confidence | File | Description |
|---|---|---|---|---|---|
| 1 | ERROR-SILENT | Medium | 85 | response-output-normalizer.ts:171 |
Silent JSON.parse failure returns original response with no logging |
| 2 | ERROR-SILENT | Low-Medium | 80 | rebuild-worker.ts:308 |
Empty catch block on config bootstrap failure (intentional but unlogged) |
Critical Issues
None identified.
High Issues
None identified.
Medium Issues
#1 - Silent JSON.parse failure in response-output-normalizer.ts (Confidence: 85)
In normalizeResponseOutput(), the catch block after JSON.parse(rawText) silently returns the original response without any logging. If upstream returns malformed JSON for a Responses API call, this error is swallowed entirely. The normalizer already has a logger used for successful normalization - it should also log parse failures.
Suggested fix:
} catch (error) {
logger.warn("[ResponseOutputNormalizer] Failed to parse response JSON", {
error: error instanceof Error ? error.message : String(error),
sessionId: session.sessionId ?? null,
requestSequence: session.requestSequence ?? null,
});
return response;
}Low-Medium Issues
#2 - Empty catch block in rebuild-worker.ts bootstrap (Confidence: 80)
The catch block when publishCurrentPublicStatusConfigProjection fails during config bootstrap contains only a comment. While the behavior is intentional (fall through to missing-config semantics), the lack of logging means persistent bootstrap failures are invisible. Adding a warn-level log would help ops detect repeated failures without changing the control flow.
Suggested fix:
} catch (error) {
logger.warn("[RebuildWorker] Bootstrap config publish failed", {
reason: error instanceof Error ? error.message : String(error),
});
// Intentional: let caller continue with missing-config semantics.
}Review Coverage Checklist
| Perspective | Status | Details |
|---|---|---|
| Comment Analyzer | Done | No misleading comments found; comments are accurate and purposeful |
| Test Analyzer | Done | 4 new test files with good coverage (thinking-signature, anthropic-model, response-normalizer, rollup-store). Edge cases covered. Mock boundaries appropriate. |
| Silent Failure Hunter | Done | 2 issues found (see above). All other catch blocks are either intentional null-return patterns (protobuf decoder) or have proper logging. |
| Type Design Auditor | Done | Discriminated unions used correctly (ActionResult, rebuild status types). No stringly-typed state where enums would be better. |
| General Code Reviewer | Done | No security issues. Base64 validation is thorough (standard + URL-safe). Redis pipeline usage is correct. Distributed locking uses proper Lua compare-and-delete. |
| Code Simplifier | Done | buildUserDisplays() extraction avoids duplication. No significant over-engineering detected. Fire-and-forget rollup pattern is appropriate for the use case. |
Key Sub-PRs Reviewed
- Anthropic thinking signature detection - Hand-rolled protobuf decoder, appropriate null-return-on-error design, well-tested.
- OpenAI Responses API normalization - Null-to-default conversion is correct and scoped properly. Silent parse failure noted above.
- User list crash fix - Defense-in-depth with Date instance check and self-lookup guard.
buildUserDisplays()extraction is clean. - Public status Redis rollup v2 - HINCRBYFLOAT bucket design is sound. FIFO cache eviction is correct. Dual-namespace fallback (v2/v1) is well-structured.
- Model reference updates - Straightforward, no concerns.
Verdict: The PR is well-structured with good test coverage. Two medium/low silent failure issues should be addressed before or shortly after merge. No blocking issues found.
| let parsed: unknown; | ||
| try { | ||
| parsed = JSON.parse(rawText); | ||
| } catch { |
There was a problem hiding this comment.
[ERROR-SILENT] Silent JSON.parse failure (Confidence: 85)
The catch after JSON.parse(rawText) returns response with no logging. If upstream returns malformed JSON for a Responses API call, this silently swallows the error and returns the original (potentially broken) response.
Suggestion:
} catch (error) {
logger.warn("[ResponseOutputNormalizer] Failed to parse response JSON", {
error: error instanceof Error ? error.message : String(error),
sessionId: session.sessionId ?? null,
requestSequence: session.requestSequence ?? null,
});
return response;
}| @@ -280,19 +308,25 @@ export async function rebuildPublicStatusProjection(input: { | |||
| | { status: "skipped"; reason: "distributed-lock-held"; sourceGeneration: string } | |||
There was a problem hiding this comment.
[ERROR-SILENT] Empty catch block on bootstrap failure (Confidence: 80)
The catch when publishCurrentPublicStatusConfigProjection fails is empty (only a comment). While the behavior is intentional (fall through to missing-config), the lack of logging means persistent bootstrap failure is invisible.
Suggestion:
} catch (error) {
logger.warn("[RebuildWorker] Bootstrap config publish failed", {
reason: error instanceof Error ? error.message : String(error),
});
// Intentional: let caller continue with missing-config semantics.
}
| let parsed: unknown; | ||
| try { | ||
| parsed = JSON.parse(rawText); | ||
| } catch { |
There was a problem hiding this comment.
[ERROR-SILENT] Silent JSON.parse failure (Confidence: 85)
The catch after JSON.parse(rawText) returns response with no logging. If upstream returns malformed JSON for a Responses API call, this silently swallows the error and returns the original (potentially broken) response.
Suggestion:
} catch (error) {
logger.warn("[ResponseOutputNormalizer] Failed to parse response JSON", {
error: error instanceof Error ? error.message : String(error),
sessionId: session.sessionId ?? null,
requestSequence: session.requestSequence ?? null,
});
return response;
}| @@ -280,19 +308,25 @@ export async function rebuildPublicStatusProjection(input: { | |||
| | { status: "skipped"; reason: "distributed-lock-held"; sourceGeneration: string } | |||
There was a problem hiding this comment.
[ERROR-SILENT] Empty catch block on bootstrap failure (Confidence: 80)
The catch when publishCurrentPublicStatusConfigProjection fails is empty (only a comment). While the behavior is intentional (fall through to missing-config), the lack of logging means persistent bootstrap failure is invisible.
Suggestion:
} catch (error) {
logger.warn("[RebuildWorker] Bootstrap config publish failed", {
reason: error instanceof Error ? error.message : String(error),
});
// Intentional: let caller continue with missing-config semantics.
}![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const [displayUser] = await buildUserDisplays([user], session, session.user.role === "admin"); | ||
| return { ok: true, data: displayUser }; |
There was a problem hiding this comment.
buildUserDisplays somehow returns an empty array (the guard at the top of the function prevents it, but nothing enforces it at the type level without noUncheckedIndexedAccess), displayUser is undefined at runtime while typed as UserDisplay. Adding an explicit check makes the failure branch explicit and prevents a silent { ok: true, data: undefined } response.
| const [displayUser] = await buildUserDisplays([user], session, session.user.role === "admin"); | |
| return { ok: true, data: displayUser }; | |
| const [displayUser] = await buildUserDisplays([user], session, session.user.role === "admin"); | |
| if (!displayUser) { | |
| return { ok: false, error: tError("USER_NOT_FOUND"), errorCode: ERROR_CODES.NOT_FOUND }; | |
| } | |
| return { ok: true, data: displayUser }; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/actions/users.ts
Line: 523-524
Comment:
Array destructuring without a null guard — if `buildUserDisplays` somehow returns an empty array (the guard at the top of the function prevents it, but nothing enforces it at the type level without `noUncheckedIndexedAccess`), `displayUser` is `undefined` at runtime while typed as `UserDisplay`. Adding an explicit check makes the failure branch explicit and prevents a silent `{ ok: true, data: undefined }` response.
```suggestion
const [displayUser] = await buildUserDisplays([user], session, session.user.role === "admin");
if (!displayUser) {
return { ok: false, error: tError("USER_NOT_FOUND"), errorCode: ERROR_CODES.NOT_FOUND };
}
return { ok: true, data: displayUser };
```
How can I resolve this? If you propose a fix, please make it concise.| let rawText: string; | ||
| try { | ||
| rawText = await response.clone().text(); | ||
| } catch (error) { |
There was a problem hiding this comment.
isJsonContentType but does not guard against a already-consumed body before calling response.clone().text(). If ResponseFixer or any earlier middleware has drained the body (response.bodyUsed === true), clone() on a consumed body throws in some runtimes. Adding a bodyUsed guard mirrors the defensive pattern used elsewhere in the proxy and keeps the fallback path clean.
| let rawText: string; | |
| try { | |
| rawText = await response.clone().text(); | |
| } catch (error) { | |
| if (response.bodyUsed) return response; | |
| let rawText: string; | |
| try { | |
| rawText = await response.clone().text(); | |
| } catch (error) { |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/v1/_lib/proxy/response-output-normalizer.ts
Line: 156-159
Comment:
The function bypasses SSE/streaming via `isJsonContentType` but does not guard against a already-consumed body before calling `response.clone().text()`. If `ResponseFixer` or any earlier middleware has drained the body (`response.bodyUsed === true`), `clone()` on a consumed body throws in some runtimes. Adding a `bodyUsed` guard mirrors the defensive pattern used elsewhere in the proxy and keeps the fallback path clean.
```suggestion
if (response.bodyUsed) return response;
let rawText: string;
try {
rawText = await response.clone().text();
} catch (error) {
```
How can I resolve this? If you propose a fix, please make it concise.
github-project-automation
Bot
moved this from Backlog
to Done
in Claude Code Hub Roadmap
4 participants
Summary
Release v0.8.3 - merges the
devbranch intomain. This release includes 5 PRs covering Anthropic stream model detection, OpenAI Responses API compatibility fixes, dashboard user list crash fix, public status page performance overhaul, and model reference updates.Included Changes
feat(anthropic): detect actual model from thinking signature in stream (#1223)
Adds a new three-state model detection mechanism for Anthropic streaming responses that extracts the actual model name from the
signature_deltaprotobuf payload embedded in SSE events. This is more accurate than themessage_startplain textmodelfield when thinking is enabled.thinking-signature-model.ts(hand-rolled protobuf decoder) andanthropic-actual-response-model.ts(three-way decision logic)ThinkingSignatureModelDetectionSpecialSettingtype for audit persistenceproviderType in {claude, claude-auth}withrequestedModelstarting withclaude-oranthropic/fix(proxy): normalize nullable Responses output fields (#1220)
Fixes the
TypeError: 'NoneType' object is not iterablecrash in the officialopenai-pythonSDK when using CCH as an OpenAI-compatible proxy for/v1/responses. Some upstream providers returnnullfor fields where the SDK expects arrays, strings, or objects.response-output-normalizer.ts-- convertsnullfields to schema-compatible defaults (output: null -> [],text: null -> "",function_call.arguments: null -> "{}", etc.)originalFormat === "response", 2xx JSON responses onlyfix: return full self user list shape (#1213)
Fixes a dashboard crash for non-admin users who open the users page. The
/api/v1/users:selfendpoint previously returned a user detail shape without akeysarray, but the dashboard unconditionally readsuser.keys.length, causing the global error page.buildUserDisplays()fromgetUsers()as a shared helpergetCurrentUserDisplay()that returns the sameUserDisplaylist-item shape (includingkeys) without triggering a full admin user-list scanDateobject corruption inredactUserKeysso Hono JSON serialization emits ISO stringsOptimize public status page Redis aggregation and polling performance (#1211)
Major redesign of the public status page data layer to reduce Redis memory growth and DB back-scan costs. Replaces per-request DB back-scanning with an incremental v2 Redis rollup architecture.
public-status:v2-- 5-minuteHINCRBYFLOATrollup buckets written at request finalizationinclude=meta,defaults,groups(no timeline); timeline is reused client-side; single shared hover summary replaces per-cell Radix Tooltip instanceschore: upgrade Codex test models to GPT-5.5 (#1221)
Bulk-update all Codex/OpenAI model references from older GPT-5.x versions (5.2, 5.3, non-mini 5.4) to GPT-5.5 across all 5 locales, test fixtures, provider-testing defaults, and documentation.
gpt-5.4-minireferences are preserved as the current latest mini model.Purely cosmetic/reference update -- no runtime behavior or API changes.
Related Issues
/v1/responsesreturns 200 butopenai-pythonparser fails withNoneTypeobject is not iterableactualResponseModelinfrastructure with thinking signature detection/v1/responses(request-side counterpart)Breaking Changes
None. The public status page metric change (TTFB/TPS from median to mean) is a behavioral change but not a breaking API change.
Testing
All included PRs were individually verified:
bun run typecheck-- passesbun run lint-- passesbun run test-- passesbun run build-- passesbun run test:v1-- passesVerification
bun run typecheck bun run lint bun run test bun run buildDescription enhanced by Claude AI
Greptile Summary
Release v0.8.3 merges five independently reviewed PRs: Anthropic thinking-signature model detection (hand-rolled protobuf decoder + three-state decision logic), OpenAI Responses API null-field normalizer, a
/api/v1/users:selfcrash fix for non-admin users, a major public status data-layer overhaul from DB back-scans to a v2 Redis HINCRBYFLOAT rollup, and a bulk GPT-5.5 model reference update.signature_deltapayloads; falls back tomessage_startplain-text model; audited viaThinkingSignatureModelDetectionSpecialSetting.nulloutput/content/arguments fields to schema-compatible defaults, scoped to non-SSE 2xxoriginalFormat === \"response\"responses only.buildUserDisplaysextracted as a shared helper;getCurrentUserDisplayreturns the fullUserDisplayshape (includingkeys);redactUserKeysnow correctly passesDateobjects through.HINCRBYFLOATbucket writes replace per-rebuild DB back-scans; dual-namespace read path (v2 primary, v1 legacy fallback) handles warm-up coverage gaps.Confidence Score: 4/5
Safe to merge; changes are well-scoped and tested, with no regressions expected in existing behaviour.
The five sub-PRs are each independently tested and touch distinct subsystems. The protobuf decoder fails gracefully to null on every bad-input path. The response normalizer only activates for Responses-API non-streaming 2xx JSON. The users fix properly restores the full UserDisplay shape. The public-status rollup architecture includes a dual-namespace warm-up fallback. The only open questions are minor defensive-programming gaps (array-destructure null guard, bodyUsed check) that have no realistic failure path in production.
src/actions/users.ts (getCurrentUserDisplay array destructure) and src/app/v1/_lib/proxy/response-output-normalizer.ts (bodyUsed guard) would benefit from small hardening touches noted in the inline comments.
Important Files Changed
Sequence Diagram
sequenceDiagram participant RH as ResponseHandler participant AARM as AnthropicActualResponseModel participant TSM as ThinkingSignatureModel participant EAR as extractActualResponseModel RH->>AARM: resolveAnthropicStreamActualResponseModel alt not claude provider OR non-Anthropic model prefix AARM-->>RH: source null RH->>EAR: extractActualResponseModelForProvider EAR-->>RH: model from message_start else Anthropic model family AARM->>TSM: extractThinkingSignatureModelFromStream TSM->>TSM: find signature_delta in SSE chunks TSM->>TSM: decodeBase64Strict TSM->>TSM: walkLengthDelimitedPath buf [2,1,6] TSM-->>AARM: model or null alt signature valid AARM-->>RH: source signature else fallback AARM->>EAR: extractActualResponseModelForProvider EAR-->>AARM: fallbackModel alt thinkingEnabled and fallbackModel AARM-->>RH: source fallback_no_signature_with_thinking else AARM-->>RH: source fallback_no_thinking end end RH->>RH: addSpecialSetting + persist finalActualResponseModel endPrompt To Fix All With AI
Reviews (1): Last reviewed commit: "优化公开状态页 Redis 聚合与轮询性能 (#1211)" | Re-trigger Greptile