Conversation
📝 WalkthroughWalkthrough添加“思考签名整流器”功能:数据库迁移、新设置字段、请求修复模块及测试、代理转发器集成、UI 与多语言文案、相关类型/验证与测试配置变更。 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Summary of ChangesHello @ding113, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a robust error-handling feature for Anthropic API requests. It automatically detects and attempts to resolve specific 400 errors by intelligently modifying the request payload and retrying, thereby improving the reliability of interactions with Anthropic models, especially in complex session or channel-switching scenarios. The feature is configurable via system settings and provides detailed audit trails for transparency. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
Code Review
这个 PR 引入了一个 "thinking signature 整流器",用于自动处理 Anthropic 供应商因会话切换等原因返回的特定 400 错误。这是一个很好的稳定性改进,能够提高长会话场景下的用户体验。
主要改动包括:
- 在代理转发逻辑中增加了错误检测和单次重试机制。
- 新增了
thinking-signature-rectifier.ts模块,用于检测和修正请求体。 - 在系统设置中添加了
enableThinkingSignatureRectifier开关,并更新了多语言界面。 - 为新功能添加了专门的单元测试和覆盖率配置,确保了代码质量。
- 数据库迁移和相关类型、配置的更新都已完成。
代码整体结构清晰,考虑周全,特别是对重试逻辑、日志审计(通过 specialSettings)和测试覆盖都做了很好的处理。
我在 thinking-signature-rectifier.ts 中发现了一个小问题,并提出了修改建议。除此之外,其他改动看起来都很棒。
| for (const msg of messages) { | ||
| if (!msg || typeof msg !== "object") continue; | ||
| const msgObj = msg as Record<string, unknown>; | ||
| const content = msgObj.content; | ||
| if (!Array.isArray(content)) continue; | ||
|
|
||
| const newContent: unknown[] = []; | ||
|
|
||
| for (const block of content) { | ||
| if (!block || typeof block !== "object") { | ||
| newContent.push(block); | ||
| continue; | ||
| } | ||
|
|
||
| const blockObj = block as Record<string, unknown>; | ||
| const type = blockObj.type; | ||
|
|
||
| if (type === "thinking") { | ||
| removedThinkingBlocks += 1; | ||
| applied = true; | ||
| continue; | ||
| } | ||
|
|
||
| if (type === "redacted_thinking") { | ||
| removedRedactedThinkingBlocks += 1; | ||
| applied = true; | ||
| continue; | ||
| } | ||
|
|
||
| if ("signature" in blockObj) { | ||
| const { signature: _signature, ...rest } = blockObj as any; | ||
| removedSignatureFields += 1; | ||
| applied = true; | ||
| newContent.push(rest); | ||
| continue; | ||
| } | ||
|
|
||
| newContent.push(blockObj); | ||
| } | ||
|
|
||
| if (newContent.length !== content.length) { | ||
| msgObj.content = newContent; | ||
| } else if (removedSignatureFields > 0) { | ||
| // 即使长度不变,只要移除了 signature,也需要落盘替换后的对象 | ||
| msgObj.content = newContent; | ||
| } | ||
| } |
There was a problem hiding this comment.
当前更新 msgObj.content 的逻辑存在一个潜在的 bug。removedSignatureFields 计数器是整个函数范围的,而不是针对单个消息循环。这意味着,如果第一个消息中的 signature 被移除,removedSignatureFields 会大于 0。在处理后续没有变化的消息时,else if (removedSignatureFields > 0) 这个条件仍然会为 true,导致不必要的 msgObj.content 重新赋值。
为了修复这个问题并使逻辑更清晰,建议在每个消息的循环内部使用一个局部标志位来跟踪内容是否被修改。
for (const msg of messages) {
if (!msg || typeof msg !== "object") continue;
const msgObj = msg as Record<string, unknown>;
const content = msgObj.content;
if (!Array.isArray(content)) continue;
const newContent: unknown[] = [];
let contentWasModified = false;
for (const block of content) {
if (!block || typeof block !== "object") {
newContent.push(block);
continue;
}
const blockObj = block as Record<string, unknown>;
const type = blockObj.type;
if (type === "thinking") {
removedThinkingBlocks += 1;
contentWasModified = true;
continue;
}
if (type === "redacted_thinking") {
removedRedactedThinkingBlocks += 1;
contentWasModified = true;
continue;
}
if ("signature" in blockObj) {
const { signature: _signature, ...rest } = blockObj as any;
removedSignatureFields += 1;
contentWasModified = true;
newContent.push(rest);
continue;
}
newContent.push(blockObj);
}
if (contentWasModified) {
applied = true;
msgObj.content = newContent;
}
}There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/actions/my-usage.ts (1)
544-553: UTC 解析日期边界:修复跨时区偏移,方向正确这里用
Date.parse(...Z)统一按 UTC 解析YYYY-MM-DD,并保持endTime作为“次日零点”的排他上界,能避免本地时区导致的日期漂移。小建议:把 getMyUsageLogs 的 startDate/endDate 解析也统一到 UTC,避免同页不同统计口径
目前getMyUsageLogs仍用new Date("${date}T00:00:00")(本地时区语义),可能和这里的 UTC 语义产生不一致。src/app/v1/_lib/proxy/thinking-signature-rectifier.ts (1)
52-124: 整流函数实现正确,有一个小的优化建议。实现正确地处理了三种场景:
- 移除
thinking块- 移除
redacted_thinking块- 移除
signature字段但保留其他内容小优化建议:Line 112 的
else if (removedSignatureFields > 0)使用的是全局累计计数器,如果前面的 message 已移除 signature,后续未修改的 message 也会触发不必要的content重新赋值。这不影响正确性,但可以考虑用局部变量跟踪当前 message 是否有 signature 移除:可选优化
for (const msg of messages) { if (!msg || typeof msg !== "object") continue; const msgObj = msg as Record<string, unknown>; const content = msgObj.content; if (!Array.isArray(content)) continue; const newContent: unknown[] = []; + let signatureRemovedInThisMessage = false; for (const block of content) { // ... if ("signature" in blockObj) { const { signature: _signature, ...rest } = blockObj as any; removedSignatureFields += 1; + signatureRemovedInThisMessage = true; applied = true; newContent.push(rest); continue; } // ... } if (newContent.length !== content.length) { msgObj.content = newContent; - } else if (removedSignatureFields > 0) { + } else if (signatureRemovedInThisMessage) { msgObj.content = newContent; } }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
📒 Files selected for processing (27)
.gitignoredrizzle/0051_silent_maelstrom.sqldrizzle/meta/0051_snapshot.jsondrizzle/meta/_journal.jsonmessages/en/settings.jsonmessages/ja/settings.jsonmessages/ru/settings.jsonmessages/zh-CN/settings.jsonmessages/zh-TW/settings.jsonpackage.jsonsrc/actions/my-usage.tssrc/actions/system-config.tssrc/app/[locale]/settings/config/_components/system-settings-form.tsxsrc/app/[locale]/settings/config/page.tsxsrc/app/v1/_lib/proxy/forwarder.tssrc/app/v1/_lib/proxy/thinking-signature-rectifier.test.tssrc/app/v1/_lib/proxy/thinking-signature-rectifier.tssrc/drizzle/schema.tssrc/lib/config/system-settings-cache.tssrc/lib/utils/special-settings.tssrc/lib/validation/schemas.tssrc/repository/_shared/transformers.tssrc/repository/system-config.tssrc/types/special-settings.tssrc/types/system-config.tstests/unit/proxy/proxy-forwarder-thinking-signature-rectifier.test.tsvitest.thinking-signature-rectifier.config.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
No emoji characters in any code, comments, or string literals
Files:
src/types/system-config.tssrc/repository/_shared/transformers.tssrc/app/[locale]/settings/config/page.tsxsrc/app/v1/_lib/proxy/thinking-signature-rectifier.test.tsvitest.thinking-signature-rectifier.config.tssrc/app/v1/_lib/proxy/thinking-signature-rectifier.tssrc/actions/system-config.tssrc/repository/system-config.tssrc/lib/config/system-settings-cache.tssrc/drizzle/schema.tssrc/lib/utils/special-settings.tssrc/actions/my-usage.tstests/unit/proxy/proxy-forwarder-thinking-signature-rectifier.test.tssrc/lib/validation/schemas.tssrc/app/[locale]/settings/config/_components/system-settings-form.tsxsrc/types/special-settings.tssrc/app/v1/_lib/proxy/forwarder.ts
**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,jsx,js}: All user-facing strings must use i18n (5 languages supported: zh-CN, en, ja, ko, de). Never hardcode display text
Use path alias @/ to map to ./src/
Use Biome for code formatting with configuration: double quotes, trailing commas, 2-space indent, 100 character line width
Prefer named exports over default exports
Use next-intl for internationalization
Use Next.js 16 App Router with Hono for API routes
Files:
src/types/system-config.tssrc/repository/_shared/transformers.tssrc/app/[locale]/settings/config/page.tsxsrc/app/v1/_lib/proxy/thinking-signature-rectifier.test.tsvitest.thinking-signature-rectifier.config.tssrc/app/v1/_lib/proxy/thinking-signature-rectifier.tssrc/actions/system-config.tssrc/repository/system-config.tssrc/lib/config/system-settings-cache.tssrc/drizzle/schema.tssrc/lib/utils/special-settings.tssrc/actions/my-usage.tstests/unit/proxy/proxy-forwarder-thinking-signature-rectifier.test.tssrc/lib/validation/schemas.tssrc/app/[locale]/settings/config/_components/system-settings-form.tsxsrc/types/special-settings.tssrc/app/v1/_lib/proxy/forwarder.ts
**/drizzle/**/*.sql
📄 CodeRabbit inference engine (CLAUDE.md)
Never create SQL migration files manually. Always generate migrations by editing src/drizzle/schema.ts, then run bun run db:generate, review the generated SQL, and run bun run db:migrate
Files:
drizzle/0051_silent_maelstrom.sql
**/*.{tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use React 19, shadcn/ui, Tailwind CSS, and Recharts for the UI layer
Files:
src/app/[locale]/settings/config/page.tsxsrc/app/[locale]/settings/config/_components/system-settings-form.tsx
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place unit tests in tests/unit/, integration tests in tests/integration/, and source-adjacent tests in src/**/*.test.ts
Files:
src/app/v1/_lib/proxy/thinking-signature-rectifier.test.tstests/unit/proxy/proxy-forwarder-thinking-signature-rectifier.test.ts
🧠 Learnings (3)
📚 Learning: 2026-01-05T03:01:39.354Z
Learnt from: NieiR
Repo: ding113/claude-code-hub PR: 539
File: src/types/user.ts:158-170
Timestamp: 2026-01-05T03:01:39.354Z
Learning: In TypeScript interfaces, explicitly document and enforce distinct meanings for null and undefined. Example: for numeric limits like limitTotalUsd, use 'number | null | undefined' when null signifies explicitly unlimited (e.g., matches DB schema or special UI logic) and undefined signifies 'inherit default'. This pattern should be consistently reflected in type definitions across related fields to preserve semantic clarity between database constraints and UI behavior.
Applied to files:
src/types/system-config.tssrc/repository/_shared/transformers.tssrc/app/v1/_lib/proxy/thinking-signature-rectifier.test.tssrc/app/v1/_lib/proxy/thinking-signature-rectifier.tssrc/actions/system-config.tssrc/repository/system-config.tssrc/lib/config/system-settings-cache.tssrc/drizzle/schema.tssrc/lib/utils/special-settings.tssrc/actions/my-usage.tssrc/lib/validation/schemas.tssrc/types/special-settings.tssrc/app/v1/_lib/proxy/forwarder.ts
📚 Learning: 2026-01-05T03:02:06.594Z
Learnt from: NieiR
Repo: ding113/claude-code-hub PR: 539
File: src/app/[locale]/dashboard/_components/user/user-key-table-row.tsx:66-66
Timestamp: 2026-01-05T03:02:06.594Z
Learning: In the claude-code-hub project, the translations.actions.addKey field in UserKeyTableRowProps is defined as optional for backward compatibility, but all actual callers in the codebase provide the complete translations object. The field has been added to all 5 locale files (messages/{locale}/dashboard.json).
Applied to files:
messages/en/settings.json
📚 Learning: 2026-01-07T17:05:36.362Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T17:05:36.362Z
Learning: Applies to **/*.test.{ts,tsx,js,jsx} : Place unit tests in tests/unit/, integration tests in tests/integration/, and source-adjacent tests in src/**/*.test.ts
Applied to files:
package.json
🧬 Code graph analysis (5)
src/app/v1/_lib/proxy/thinking-signature-rectifier.test.ts (1)
src/app/v1/_lib/proxy/thinking-signature-rectifier.ts (2)
detectThinkingSignatureRectifierTrigger(17-41)rectifyAnthropicRequestMessage(52-124)
vitest.thinking-signature-rectifier.config.ts (2)
scripts/sync-settings-keys.js (1)
path(9-9)scripts/validate-migrations.js (1)
__dirname(18-18)
src/repository/system-config.ts (1)
src/drizzle/schema.ts (1)
systemSettings(456-508)
src/app/[locale]/settings/config/_components/system-settings-form.tsx (3)
scripts/sync-settings-keys.js (3)
result(31-31)t(72-72)p(102-102)src/components/ui/label.tsx (1)
Label(21-21)src/components/ui/switch.tsx (1)
Switch(28-28)
src/app/v1/_lib/proxy/forwarder.ts (3)
src/app/v1/_lib/proxy/thinking-signature-rectifier.ts (2)
detectThinkingSignatureRectifierTrigger(17-41)rectifyAnthropicRequestMessage(52-124)src/lib/config/index.ts (1)
getCachedSystemSettings(8-8)src/lib/circuit-breaker.ts (1)
getCircuitState(395-398)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: pr-description
- GitHub Check: 🌐 API Tests
- GitHub Check: check-codex-status
- GitHub Check: pr-review
- GitHub Check: Docker Build Test
- GitHub Check: dev-build-deploy
🔇 Additional comments (37)
.gitignore (1)
18-18: 忽略新增的覆盖率目录:合理与新增的独立 coverage 输出目录匹配,避免污染仓库状态。
messages/ja/settings.json (1)
105-106: 新增开关文案已进入 i18n:OK键名与功能语义匹配,描述也明确了“同一 provider 仅重试一次”和默认开启。
messages/ru/settings.json (1)
105-106: 俄语翻译补齐功能开关:OK文案清晰表达了触发条件(thinking signature/invalid request)与动作(移除 thinking 相关块并对同一提供商重试一次)。
messages/zh-TW/settings.json (1)
105-106: 繁中新增开关与说明:OK位置在
config.form下且表述完整(触发错误类型、移除 thinking 区块、同供应商重试一次、默认开启)。src/types/system-config.ts (2)
43-45: 代码实现正确。新增的
enableThinkingSignatureRectifier字段定义符合以下要求:
- 在 SystemSettings 接口中为必填字段(与数据库 NOT NULL 约束一致)
- 类型为 boolean,与数据库架构匹配
- 注释清晰说明了功能目的和默认行为
84-85: LGTM!在 UpdateSystemSettingsInput 中将字段设为可选是正确的,符合部分更新的语义,与其他系统设置字段的处理方式保持一致。
drizzle/meta/_journal.json (1)
362-367: LGTM!新的迁移日志条目格式正确,索引顺序连续,标签名称与迁移文件名匹配,符合 Drizzle 的元数据规范。
src/app/[locale]/settings/config/page.tsx (1)
44-44: 实现正确,需验证 UI 国际化配置。新增设置项的数据传递逻辑正确,与其他系统设置的处理方式一致。根据 AI 摘要,相关的国际化字符串已在
messages/*/settings.json中配置,但这些文件未包含在本次审查范围内。请确认SystemSettingsForm组件已正确配置该字段的多语言标签和描述文本。基于代码规范:所有用户可见的字符串必须使用 i18n(支持 5 种语言:zh-CN, en, ja, ko, de)。
src/lib/validation/schemas.ts (1)
744-745: LGTM!验证规则定义正确:
- 使用
z.boolean().optional()符合可选布尔字段的验证需求- 与其他系统设置字段(如
enableResponseFixer)的处理方式保持一致- 注释清晰说明了字段用途
drizzle/0051_silent_maelstrom.sql (1)
1-1: 迁移文件已通过正确的流程生成。已确认
enable_thinking_signature_rectifier字段在src/drizzle/schema.ts中正确定义(第 490 行),字段规范与迁移 SQL 完全匹配。此迁移文件遵循了规范要求,通过编辑 schema.ts 后由bun run db:generate生成。src/types/special-settings.ts (1)
12-12: 新增的审计类型结构良好!
ThinkingSignatureRectifierSpecialSetting类型定义清晰,字段完整地覆盖了整流器的关键信息:
- 触发条件(
trigger)- 重试次数追踪(
attemptNumber,retryAttemptNumber)- 清理统计(
removedThinkingBlocks,removedRedactedThinkingBlocks,removedSignatureFields)- 供应商关联(
providerId,providerName,支持 null)类型设计符合现有
SpecialSetting联合类型的模式,便于在日志详情中统一展示。Also applies to: 90-109
messages/zh-CN/settings.json (1)
88-89: i18n 配置清晰明确。新增的两个翻译键准确描述了 thinking 签名整流器的功能与行为:
- 功能开关标签简洁("启用 thinking 签名整流器")
- 描述详细说明了触发条件、处理逻辑和默认状态
文案表达专业,便于用户理解该功能的作用和影响范围。
src/lib/utils/special-settings.ts (1)
58-69: 序列化逻辑正确且完整。新增的
thinking_signature_rectifier分支正确处理了所有字段:
- 使用
providerId ?? null确保序列化一致性- 涵盖全部 9 个字段(类型、命中状态、供应商信息、触发原因、重试次数、清理统计)
- 遵循既有的 JSON 序列化模式
这确保了整流器审计信息能够在日志系统中被正确去重和展示。
package.json (1)
21-21: 测试脚本配置正确。新增的
test:coverage:thinking-signature-rectifier脚本遵循现有模式,引用对应的专项配置文件并启用覆盖率收集。脚本命名清晰,便于在 CI/CD 流程中独立验证该模块的测试覆盖率。vitest.thinking-signature-rectifier.config.ts (1)
1-52: 测试配置结构正确,覆盖率目标清晰合理。针对性的覆盖率配置有助于聚焦核心模块验证,避免受 Next.js/DB/Redis 等重模块影响。覆盖率阈值(80/80/70/80)满足新增功能 80% 的覆盖率要求。测试文件组织方式符合项目规范(源代码相邻测试在 src/ 目录,单元测试在 tests/unit/)。
src/repository/_shared/transformers.ts (1)
173-173: LGTM!默认值设置合理。新增的
enableThinkingSignatureRectifier字段默认值为true,与 PR 描述一致(默认启用)。实现方式与其他布尔配置项保持一致,代码清晰且类型安全。src/app/[locale]/settings/config/_components/system-settings-form.tsx (1)
33-33: LGTM!UI 集成实现完整且一致。新增的
enableThinkingSignatureRectifier功能已正确集成到表单中:
- 表单属性、本地状态、提交负载和响应处理均已完整添加
- UI 开关组件遵循现有模式,与其他开关保持一致
- 正确使用 i18n(
t()函数),无硬编码字符串Also applies to: 60-62, 88-88, 106-106, 236-251
messages/en/settings.json (1)
107-108: LGTM!翻译文本清晰准确。新增的英文翻译键符合命名规范,描述文本清晰地说明了功能用途(自动移除不兼容的 thinking 块并重试)。放置位置合理,与其他请求处理相关配置项组织在一起。
src/app/v1/_lib/proxy/thinking-signature-rectifier.test.ts (1)
1-88: LGTM!测试覆盖全面且合理。单元测试覆盖了关键功能路径:
- detectThinkingSignatureRectifierTrigger:验证了各种错误消息格式的匹配逻辑,包括带/不带反引号、大小写混用等场景,并测试了非匹配情况
- rectifyAnthropicRequestMessage:验证了 thinking/redacted_thinking 块的移除、signature 字段的清理,以及边界情况(messages 缺失或非数组)
测试断言具体且有意义,覆盖了正向、负向和边界场景,符合关键路径测试要求。
src/lib/config/system-settings-cache.ts (1)
29-33: LGTM!缓存集成实现正确。
enableThinkingSignatureRectifier已正确集成到系统设置缓存中:
DEFAULT_SETTINGS类型注解已更新- 默认值设置为
true,与 PR 描述和其他文件保持一致- 故障转移逻辑(fallback)包含了该字段
实现方式与现有设置项完全一致,确保了配置的正确传播。
Also applies to: 37-37, 105-105
src/drizzle/schema.ts (1)
488-493: LGTM!Schema 新增字段结构正确。新增的
enableThinkingSignatureRectifier字段:
- 命名风格与现有字段一致(camelCase 映射到 snake_case)
- 默认值
true符合 PR 描述的"默认启用"行为notNull约束适用于布尔开关类型字段src/actions/system-config.ts (2)
41-41: LGTM!新增字段遵循现有的可选布尔字段模式,与
interceptAnthropicWarmupRequests等其他设置保持一致。
65-65: LGTM!正确地将验证后的值传递给
updateSystemSettings,符合现有字段的处理模式。drizzle/meta/0051_snapshot.json (1)
1923-1929: LGTM!Drizzle 自动生成的 snapshot 文件,
enable_thinking_signature_rectifier列定义与 schema.ts 保持一致。tests/unit/proxy/proxy-forwarder-thinking-signature-rectifier.test.ts (3)
44-95: 测试辅助函数设计合理。
createSession函数正确设置了包含thinking、redacted_thinking和带signature字段的测试数据,覆盖了整流器需要处理的所有内容类型。
114-159: LGTM!测试用例完整覆盖了整流重试成功场景。测试验证了:
- 首次 400 错误触发整流
- 整流后 thinking/redacted_thinking 块被移除
- signature 字段被移除
- 重试成功返回 200
- specialSettings 正确记录审计信息
208-239: LGTM!重试失败场景测试完整。正确验证了:
- 重试失败后不再继续重试(doForward 仅调用 2 次)
- 审计更新只执行一次
- 最终错误正确抛出
src/repository/system-config.ts (4)
151-151: LGTM!降级设置中的默认值
true与 schema 定义保持一致。
184-184: LGTM!正确添加到查询字段列表中。
317-320: LGTM!更新逻辑遵循现有模式:仅在字段提供时(非 undefined)才更新。
352-352: LGTM!正确添加到 returning 子句中。
src/app/v1/_lib/proxy/forwarder.ts (4)
44-47: LGTM!正确引入整流器模块的类型和函数。
208-212: LGTM!
maxAttemptsPerProvider改为let以支持整流器扩展重试次数,thinkingSignatureRectifierRetried标志用于防止无限重试循环。
419-543: 整流器核心逻辑实现完整且健壮。实现要点:
- 仅对 Anthropic 类型供应商 (
claude/claude-auth) 生效- 检测到触发条件后获取系统设置判断是否启用
- 已重试过时将错误分类为
NON_RETRYABLE_CLIENT_ERROR,避免污染熔断器- 详细的审计记录(provider chain、special settings)
- 使用
Math.max确保至少有一次额外重试机会一个小建议:两处持久化操作(Lines 504-515 和 517-528)都使用了 fire-and-forget 模式,虽然有错误日志,但如果两处都失败,审计数据会丢失。考虑到这是非关键路径且已有日志,当前设计是可接受的。
382-382: LGTM!
errorCategory改为let是整流器覆写错误分类的必要修改。src/app/v1/_lib/proxy/thinking-signature-rectifier.ts (2)
1-10: LGTM!类型定义清晰,结果类型包含详细的计数字段便于审计。
17-41: LGTM!触发检测逻辑实现合理。
- 使用宽松的字符串匹配(lowercase)兼容不同格式的错误消息
- 正则表达式支持中英文错误提示
- 与 Issue #432 / Rule 6 保持一致
src/app/v1/_lib/proxy/forwarder.ts
Outdated
|
|
||
| // 整流请求体(原地修改 session.request.message) | ||
| const rectified = rectifyAnthropicRequestMessage( | ||
| session.request.message as Record<string, unknown> | ||
| ); | ||
|
|
||
| // 写入审计字段(specialSettings) | ||
| session.addSpecialSetting({ | ||
| type: "thinking_signature_rectifier", | ||
| scope: "request", | ||
| hit: rectified.applied, | ||
| providerId: currentProvider.id, | ||
| providerName: currentProvider.name, | ||
| trigger: rectifierTrigger, | ||
| attemptNumber: attemptCount, | ||
| retryAttemptNumber: attemptCount + 1, | ||
| removedThinkingBlocks: rectified.removedThinkingBlocks, | ||
| removedRedactedThinkingBlocks: rectified.removedRedactedThinkingBlocks, | ||
| removedSignatureFields: rectified.removedSignatureFields, | ||
| }); | ||
|
|
||
| const specialSettings = session.getSpecialSettings(); | ||
| if (specialSettings && session.sessionId) { | ||
| try { | ||
| await SessionManager.storeSessionSpecialSettings( | ||
| session.sessionId, | ||
| specialSettings, | ||
| session.requestSequence | ||
| ); | ||
| } catch (persistError) { | ||
| logger.error("[ProxyForwarder] Failed to store special settings", { | ||
| error: persistError, | ||
| sessionId: session.sessionId, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| if (specialSettings && session.messageContext?.id) { | ||
| try { | ||
| await updateMessageRequestDetails(session.messageContext.id, { | ||
| specialSettings, | ||
| }); | ||
| } catch (persistError) { | ||
| logger.error("[ProxyForwarder] Failed to persist special settings", { | ||
| error: persistError, | ||
| messageRequestId: session.messageContext.id, | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| logger.info("ProxyForwarder: Thinking signature rectifier applied, retrying", { | ||
| providerId: currentProvider.id, | ||
| providerName: currentProvider.name, | ||
| trigger: rectifierTrigger, |
There was a problem hiding this comment.
The code retries even when the rectifier doesn't actually remove anything (rectified.applied = false).
Issue: When an error message matches the rectifier trigger pattern but the request has no thinking blocks or signature fields to remove, rectifyAnthropicRequestMessage() returns applied: false. However, the code still:
- Records special settings with
hit: false - Ensures
maxAttemptsPerProvider >= attemptCount + 1(lines 531-532) - Continues to retry with the unchanged request (line 533:
continue)
This means the retry will hit the exact same error again, creating a wasted retry cycle.
Expected behavior: If rectified.applied === false, the error should be treated as non-retryable (since no actual rectification occurred), not retried.
Suggested fix:
const rectified = rectifyAnthropicRequestMessage(
session.request.message as Record<string, unknown>
);
// Only retry if rectification actually changed something
if (!rectified.applied) {
// Treat as non-retryable error instead of retrying
errorCategory = ErrorCategory.NON_RETRYABLE_CLIENT_ERROR;
} else {
// ... existing retry logic (lines 498-533)
}Prompt To Fix With AI
This is a comment left during a code review.
Path: src/app/v1/_lib/proxy/forwarder.ts
Line: 480:533
Comment:
The code retries even when the rectifier doesn't actually remove anything (`rectified.applied = false`).
**Issue**: When an error message matches the rectifier trigger pattern but the request has no thinking blocks or signature fields to remove, `rectifyAnthropicRequestMessage()` returns `applied: false`. However, the code still:
1. Records special settings with `hit: false`
2. Ensures `maxAttemptsPerProvider >= attemptCount + 1` (lines 531-532)
3. Continues to retry with the unchanged request (line 533: `continue`)
This means the retry will hit the exact same error again, creating a wasted retry cycle.
**Expected behavior**: If `rectified.applied === false`, the error should be treated as non-retryable (since no actual rectification occurred), not retried.
**Suggested fix**:
```typescript
const rectified = rectifyAnthropicRequestMessage(
session.request.message as Record<string, unknown>
);
// Only retry if rectification actually changed something
if (!rectified.applied) {
// Treat as non-retryable error instead of retrying
errorCategory = ErrorCategory.NON_RETRYABLE_CLIENT_ERROR;
} else {
// ... existing retry logic (lines 498-533)
}
```
How can I resolve this? If you propose a fix, please make it concise.| } | ||
|
|
||
| // ⭐ 2.5 Thinking signature 整流器:命中后对同供应商“整流 + 重试一次” | ||
| // 目标:解决 Anthropic 与非 Anthropic 渠道切换导致的 thinking 签名不兼容问题 |
There was a problem hiding this comment.
[MEDIUM] [STANDARD-VIOLATION] Emoji in code comment violates repo rule
Evidence: src/app/v1/_lib/proxy/forwarder.ts:420
// ⭐ 2.5 Thinking signature 整流器:命中后对同供应商“整流 + 重试一次”Why this is a problem: CLAUDE.md says "No Emoji in Code - Never use emoji characters in any code, comments, or string literals".
Suggested fix:
// 2.5 Thinking signature rectifier: trigger -> rectify + retry onceThere was a problem hiding this comment.
Code Review Summary
This PR implements a "thinking signature rectifier" to handle Anthropic API errors related to thinking block signature incompatibility. The feature automatically removes incompatible thinking blocks and retries once against the same provider. The implementation is well-structured with proper test coverage, i18n support, and database migration.
PR Size: XL
- Lines changed: 3120 (3112 additions, 8 deletions)
- Files changed: 27
Note: For XL PRs, consider splitting into smaller, more focused PRs in the future. This PR could potentially be split into:
- Core rectifier logic + tests
- Database migration + repository changes
- UI/i18n changes
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 0 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Validation Notes
Re: Greptile's concern about applied=false case (forwarder.ts)
After thorough analysis, this is not a bug but a deliberate design choice:
-
The trigger detection (
detectThinkingSignatureRectifierTrigger) identifies error messages indicating signature-related issues -
The
rectified.appliedfield is used for auditing purposes (recorded inspecialSettings) to track whether content was actually modified -
The retry happens regardless because:
- The error pattern match already indicates a signature-related issue worth retrying
- Even if no content was removed, the retry might succeed if the error was transient
- The
thinkingSignatureRectifierRetriedflag prevents infinite retry loops (max 1 extra retry)
-
The audit trail correctly records
hit: rectified.applied(true/false) for debugging -
If the retry fails again, the error is reclassified as
NON_RETRYABLE_CLIENT_ERRORand thrown immediately
The log message "Thinking signature rectifier applied, retrying" could be slightly misleading when applied=false, but this is a minor documentation concern, not a functional bug. The code safely handles all edge cases.
Review Coverage
- Logic and correctness - Clean (no bugs found, edge cases handled)
- Security (OWASP Top 10) - Clean (no injection points, proper input validation)
- Error handling - Clean (errors logged, proper categorization)
- Type safety - Clean (proper TypeScript typing throughout)
- Documentation accuracy - Clean (comments match behavior)
- Test coverage - Adequate (unit + integration tests with 80% threshold)
- Code clarity - Good (well-structured, follows existing patterns)
Automated review by Claude AI
There was a problem hiding this comment.
Code Review Summary
PR adds a thinking-signature rectifier to retry Anthropic requests after specific 400 errors, plus a system setting toggle, i18n strings, unit tests, and a Drizzle migration.
PR Size: XL
- Lines changed: 3120
- Files changed: 27
- Split suggestions: (1) core rectifier + forwarder integration, (2) system settings UI + i18n, (3) Drizzle migration/meta snapshot
Issues Found
| Category | Critical | High | Medium | Low |
|---|---|---|---|---|
| Logic/Bugs | 0 | 0 | 0 | 0 |
| Security | 0 | 0 | 0 | 0 |
| Error Handling | 0 | 0 | 0 | 0 |
| Types | 0 | 0 | 0 | 0 |
| Comments/Docs | 0 | 0 | 1 | 0 |
| Tests | 0 | 0 | 0 | 0 |
| Simplification | 0 | 0 | 0 | 0 |
Critical Issues (Must Fix)
None.
High Priority Issues (Should Fix)
- [MEDIUM] [STANDARD-VIOLATION]
src/app/v1/_lib/proxy/forwarder.ts:420adds an emoji in a code comment:// ⭐ 2.5 ..., violatingCLAUDE.md: "No Emoji in Code - Never use emoji characters in any code, comments, or string literals".
Review Coverage
- Logic and correctness
- Security (OWASP Top 10)
- Error handling
- Type safety
- Documentation accuracy
- Test coverage
- Code clarity
Automated review by Codex AI
There was a problem hiding this comment.
- Identified PR
#576and classified it asXL(3120 lines changed across 27 files); applied the labelsize/XL. - Posted 1 inline review comment on
src/app/v1/_lib/proxy/forwarder.ts:420for aCLAUDE.mdrule violation (emoji⭐in a code comment) with a concrete replacement suggestion. - Submitted the required PR review summary via
gh pr review --comment(includes the XL split suggestions and the issues table).
|
已根据 review 反馈补齐边界处理并更新测试:
commit: c1ff977 |
🧪 测试结果
总体结果: ✅ 所有测试通过 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/unit/proxy/proxy-forwarder-thinking-signature-rectifier.test.ts (1)
44-95:createSession辅助函数实现良好,但有一个小问题。
messageContext.key设置为空对象{},这可能与实际类型不匹配。虽然在测试中可能不会造成问题,但建议添加必要的字段以更准确地模拟真实场景。此外,Line 94 的
as any类型断言在测试辅助函数中是可接受的做法。
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to Reviews > Disable Cache setting
📒 Files selected for processing (3)
src/app/v1/_lib/proxy/forwarder.tssrc/app/v1/_lib/proxy/thinking-signature-rectifier.tstests/unit/proxy/proxy-forwarder-thinking-signature-rectifier.test.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
No emoji characters in any code, comments, or string literals
Files:
src/app/v1/_lib/proxy/forwarder.tstests/unit/proxy/proxy-forwarder-thinking-signature-rectifier.test.tssrc/app/v1/_lib/proxy/thinking-signature-rectifier.ts
**/*.{ts,tsx,jsx,js}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,jsx,js}: All user-facing strings must use i18n (5 languages supported: zh-CN, en, ja, ko, de). Never hardcode display text
Use path alias @/ to map to ./src/
Use Biome for code formatting with configuration: double quotes, trailing commas, 2-space indent, 100 character line width
Prefer named exports over default exports
Use next-intl for internationalization
Use Next.js 16 App Router with Hono for API routes
Files:
src/app/v1/_lib/proxy/forwarder.tstests/unit/proxy/proxy-forwarder-thinking-signature-rectifier.test.tssrc/app/v1/_lib/proxy/thinking-signature-rectifier.ts
**/*.test.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place unit tests in tests/unit/, integration tests in tests/integration/, and source-adjacent tests in src/**/*.test.ts
Files:
tests/unit/proxy/proxy-forwarder-thinking-signature-rectifier.test.ts
🧠 Learnings (2)
📚 Learning: 2026-01-07T17:05:36.362Z
Learnt from: CR
Repo: ding113/claude-code-hub PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-07T17:05:36.362Z
Learning: Applies to **/*.{js,ts,tsx,jsx} : No emoji characters in any code, comments, or string literals
Applied to files:
src/app/v1/_lib/proxy/forwarder.ts
📚 Learning: 2026-01-05T03:01:39.354Z
Learnt from: NieiR
Repo: ding113/claude-code-hub PR: 539
File: src/types/user.ts:158-170
Timestamp: 2026-01-05T03:01:39.354Z
Learning: In TypeScript interfaces, explicitly document and enforce distinct meanings for null and undefined. Example: for numeric limits like limitTotalUsd, use 'number | null | undefined' when null signifies explicitly unlimited (e.g., matches DB schema or special UI logic) and undefined signifies 'inherit default'. This pattern should be consistently reflected in type definitions across related fields to preserve semantic clarity between database constraints and UI behavior.
Applied to files:
src/app/v1/_lib/proxy/forwarder.tssrc/app/v1/_lib/proxy/thinking-signature-rectifier.ts
🧬 Code graph analysis (1)
src/app/v1/_lib/proxy/forwarder.ts (4)
src/app/v1/_lib/proxy/thinking-signature-rectifier.ts (2)
detectThinkingSignatureRectifierTrigger(17-41)rectifyAnthropicRequestMessage(52-123)src/lib/config/system-settings-cache.ts (1)
getCachedSystemSettings(56-112)src/repository/message.ts (1)
updateMessageRequestDetails(117-201)src/lib/circuit-breaker.ts (1)
getCircuitState(395-398)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Docker Build Test
- GitHub Check: 🌐 API Tests
- GitHub Check: dev-build-deploy
🔇 Additional comments (12)
tests/unit/proxy/proxy-forwarder-thinking-signature-rectifier.test.ts (4)
114-159: 测试用例实现正确,覆盖了整流成功的核心路径。测试正确验证了:
thinking和redacted_thinking块被移除signature字段被移除doForward被调用两次(首次失败 + 重试成功)specialSettings包含整流器审计记录
161-206: 测试用例正确覆盖了invalid_request触发条件。与第一个测试用例结构相似,正确验证了不同错误消息格式下的整流行为。
208-235: 关键边缘场景测试,验证了 no-op 时跳过重试的逻辑。此测试覆盖了 PR 描述中提到的问题:"触发条件匹配但无可整流内容时不应做无意义重试"。正确验证了
hit: false时只调用一次doForward。
237-268: 测试正确验证了重试失败后的错误传播行为。确保整流重试失败后:
- 不会继续重试或切换供应商
- 正确抛出
ProxyError- 审计记录只写入一次
src/app/v1/_lib/proxy/forwarder.ts (5)
44-47: 导入声明正确引入了整流器模块。从
thinking-signature-rectifier导入detectThinkingSignatureRectifierTrigger和rectifyAnthropicRequestMessage函数,符合模块边界设计。
208-212: 变量声明变更合理。
maxAttemptsPerProvider改为let是必要的,因为整流器需要动态扩展重试次数thinkingSignatureRectifierRetried标志确保每个供应商最多只有一次额外的整流重试标志在每次供应商切换时重置(在外层循环内声明),设计正确。
419-559: 整流器集成逻辑实现正确,覆盖了主要场景。核心逻辑设计合理:
- 仅对
claude/claude-auth类型供应商生效- 通过系统设置开关控制启用/禁用
thinkingSignatureRectifierRetried标志防止无限重试rectified.applied === false时跳过重试(解决了 PR 描述中的 no-op 问题)- 审计数据完整记录了整流前的请求详情
Line 554 的
Math.max(maxAttemptsPerProvider, attemptCount + 1)确保即使maxAttemptsPerProvider=1也能完成额外重试。
463-490: 审计数据持久化采用了合理的容错策略。
try-catch块捕获持久化错误但不重新抛出(fail-open 模式),确保审计操作失败不会阻塞主请求流程。日志记录足够用于调试。
382-382:errorCategory改为可变变量是整流器逻辑的必要调整。允许整流器分支覆盖错误分类,以便在特定条件下强制走
NON_RETRYABLE_CLIENT_ERROR路径。src/app/v1/_lib/proxy/thinking-signature-rectifier.ts (3)
1-10: 类型定义清晰,语义明确。
ThinkingSignatureRectifierTrigger和ThinkingSignatureRectifierResult类型设计合理,为调用方提供了完整的返回信息。
52-123: 整流函数实现正确,边界条件处理完善。核心逻辑:
- 安全地跳过非数组
messages和非数组content- 正确移除
thinking和redacted_thinking块- 通过解构赋值移除
signature字段并保留其他属性- 原地修改已在文档注释中明确说明
Line 101 的
as any类型断言用于解构移除属性,是可接受的做法。
17-41: 无需修改 —— 正则模式的设计是有意为之。
invalid_request正则模式/非法请求|illegal request|invalid request/i直接对应error-rules.ts中的 Rule 6 规则(已标记为默认规则),该设计是为了与系统范围的错误规则分类保持一致。代码注释明确说明该函数独立于错误规则开关,仅通过字符串/正则判断。此外,测试用例已验证此模式应该触发整流器,且函数仅在错误重试场景中调用,使用上下文清晰有界。
背景
signatureinthinkingblock / invalid request 等问题(参考 [BUG] API Error 400 - Thinking Block Modification Error anthropics/claude-code#10199、cline#7620)。改动
数据库
验证
Greptile Overview
Greptile Overview
Greptile Summary
Overview
This PR implements a "thinking signature rectifier" to handle Anthropic API errors related to thinking block signature incompatibility, particularly when switching between different provider channels. The feature automatically removes incompatible thinking blocks and retries once against the same provider.
Changes Made
thinking-signature-rectifier.tswith two main functions:detectThinkingSignatureRectifierTrigger()- identifies error messages indicating thinking signature issuesrectifyAnthropicRequestMessage()- removes thinking/redacted_thinking blocks and signature fieldsenable_thinking_signature_rectifiercolumn (default: true) to system_settingsStrengths
Critical Issues Found
Issue 1: Retry Without Verification (forwarder.ts lines 480-533)
The code retries even when rectification doesn't remove anything. If
rectified.applied === false, it should not retry the same unchanged request. Currently it will attempt a retry that's guaranteed to fail with the same error.Issue 2: Misleading Log Message (forwarder.ts line 530)
Log message always says "applied" regardless of whether anything was actually changed, creating confusion during debugging.
Additional Observations
Confidence Score: 2/5
Important Files Changed
File Analysis
Sequence Diagram
sequenceDiagram participant Client participant Forwarder as ProxyForwarder participant Detector as detectTrigger participant Rectifier as rectifyMessage participant Provider as Anthropic Provider participant ErrorHandler as Error Handler Client->>Forwarder: Send request with thinking blocks Forwarder->>Provider: Forward request Provider-->>Forwarder: 400 Error (signature issue) Forwarder->>Detector: Check error message Detector-->>Forwarder: Match detected (trigger found) alt Rectifier Enabled Forwarder->>Rectifier: Rectify request message Rectifier-->>Forwarder: applied=true (blocks removed) Forwarder->>Forwarder: Record special settings<br/>(hit=true) Forwarder->>Forwarder: maxAttempts++ Forwarder->>Provider: Retry with rectified request Provider-->>Forwarder: 200 Success Forwarder-->>Client: Return response else applied=false (Bug!) Forwarder->>Forwarder: Still retries unchanged<br/>(WASTED RETRY) Forwarder->>Provider: Retry with SAME request Provider-->>Forwarder: 400 Error again end alt Retry Failed ErrorHandler->>ErrorHandler: Set errorCategory to<br/>NON_RETRYABLE_CLIENT_ERROR ErrorHandler-->>Client: Return error endImportant Files Changed
File Analysis
Sequence Diagram
sequenceDiagram participant Client participant Forwarder as ProxyForwarder participant Detector as detectTrigger participant Rectifier as rectifyMessage participant Provider as Anthropic Provider participant ErrorHandler as Error Handler Client->>Forwarder: Send request with thinking blocks Forwarder->>Provider: Forward request Provider-->>Forwarder: 400 Error (signature issue) Forwarder->>Detector: Check error message Detector-->>Forwarder: Match detected (trigger found) alt Rectifier Enabled Forwarder->>Rectifier: Rectify request message Rectifier-->>Forwarder: applied=true (blocks removed) Forwarder->>Forwarder: Record special settings<br/>(hit=true) Forwarder->>Forwarder: maxAttempts++ Forwarder->>Provider: Retry with rectified request Provider-->>Forwarder: 200 Success Forwarder-->>Client: Return response else applied=false (Bug!) Forwarder->>Forwarder: Still retries unchanged<br/>(WASTED RETRY) Forwarder->>Provider: Retry with SAME request Provider-->>Forwarder: 400 Error again end alt Retry Failed ErrorHandler->>ErrorHandler: Set errorCategory to<br/>NON_RETRYABLE_CLIENT_ERROR ErrorHandler-->>Client: Return error end