Fixture/langfuse record auto mode data error#308
Merged
claude-code-best merged 8 commits intomainfrom Apr 20, 2026
Merged
Conversation
第三方 API（如智谱）在 message_start 中可能不返回完整 usage 数据， 导致 getCurrentUsage 返回全零 usage 对象，使 ctx 显示为 0%。 双重保护： - getCurrentUsage: 跳过全零 usage，继续往前找有真实数据的 message - calculateContextPercentages: totalInputTokens 为 0 时返回 null Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
color-diff-napi、image-processor-napi、audio-capture-napi 声明 "type": "module" 但使用裸 require()，Node.js ESM 中 require 不可用。改用 createRequire(import.meta.url) 或顶层 import。 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
当用户通过 ANTHROPIC_MODEL 或 settings 配置了自定义 provider 支持的模型时， getDefaultSonnetModel/Haiku/Opus 现在会优先使用该配置，而非硬编码 Anthropic 官方模型 ID。 同时改进 Langfuse 可观测性：sideQuery 失败时记录错误信息到 span， optional 模式下标记 WARNING 而非 ERROR。 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
classifyYoloAction 及 classifyYoloActionXml 接收 parentSpan 参数， 透传给 sideQuery 调用，使 auto_mode 的 side-query span 嵌套在主 agent trace 下。 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Poor mode 启用时不执行 findRelevantMemories 的预取调用， 避免额外的 API token 消耗。 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Vite 构建插件的 DEFAULT_BUILD_FEATURES 缺少 BUDDY、TRANSCRIPT_CLASSIFIER、
BRIDGE_MODE、ACP、BG_SESSIONS、TEMPLATES，导致 feature('TRANSCRIPT_CLASSIFIER')
被替换为 false，auto mode 从 Shift+Tab 循环中消失。与 build.ts 对齐。
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
将 DEFAULT_BUILD_FEATURES 列表从 build.ts、dev.ts、vite-plugin-feature-flags.ts 三处内联定义统一到 scripts/defines.ts 单一导出。之前的 Vite 插件缺少 TRANSCRIPT_CLASSIFIER 等 feature flag，导致 auto mode 在 Vite 构建中不可见。 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
Contributor
📝 WalkthroughWalkthroughThis PR centralizes default build feature configurations into Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Memory as Memory Finder
participant Classifier as Classifier
participant SideQuery as Side Query
participant Langfuse
Client->>Memory: findRelevantMemories(query, ..., parentSpan)
Memory->>SideQuery: sideQuery({parentSpan, optional: true})
SideQuery->>Langfuse: createChildSpan(parentSpan)
Langfuse-->>SideQuery: child span instance
SideQuery->>Langfuse: record LLM observation
SideQuery-->>Memory: results
Memory-->>Client: relevant memories
Client->>Classifier: classifyYoloAction(..., parentSpan)
Classifier->>SideQuery: sideQuery({parentSpan})
SideQuery->>Langfuse: createChildSpan(parentSpan)
Langfuse-->>SideQuery: child span instance
SideQuery->>Langfuse: record classification
SideQuery-->>Classifier: classification result
Classifier-->>Client: action classification
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/utils/permissions/yoloClassifier.ts (1)
781-807:⚠️ Potential issue | 🟠 MajorStrip
parentSpanbefore dumping or storing classifier requests.
stage1Opts,stage2Opts, andsideQueryOptsnow contain the liveLangfuseSpan, but the same objects are passed tomaybeDumpAutoMode()andsetLastClassifierRequests(). That risks serializing trace internals or circular SDK state into auto-mode dumps and/sharetranscript data. Keep the span only on the object passed tosideQuery, and store/dump a redacted copy.🛡️ Proposed fix
+function stripTraceContext<T extends { parentSpan?: LangfuseSpan | null }>( + opts: T, +): Omit<T, 'parentSpan'> { + const { parentSpan: _parentSpan, ...rest } = opts + return rest +} + stage1Opts = { model, max_tokens: (mode === 'fast' ? 256 : 64) + thinkingPadding, @@ querySource: 'auto_mode', parentSpan, } const stage1Raw = await sideQuery(stage1Opts) @@ - void maybeDumpAutoMode(stage1Opts, stage1Raw, stage1Start, 'stage1') - setLastClassifierRequests([stage1Opts]) + const stage1DumpOpts = stripTraceContext(stage1Opts) + void maybeDumpAutoMode(stage1DumpOpts, stage1Raw, stage1Start, 'stage1') + setLastClassifierRequests([stage1DumpOpts]) @@ const stage2Opts = { @@ querySource: 'auto_mode' as const, parentSpan, } const stage2Raw = await sideQuery(stage2Opts) @@ - void maybeDumpAutoMode(stage2Opts, stage2Raw, stage2Start, 'stage2') + const stage2DumpOpts = stripTraceContext(stage2Opts) + void maybeDumpAutoMode(stage2DumpOpts, stage2Raw, stage2Start, 'stage2') setLastClassifierRequests( - stage1Opts ? [stage1Opts, stage2Opts] : [stage2Opts], + stage1Opts ? [stripTraceContext(stage1Opts), stage2DumpOpts] : [stage2DumpOpts], ) @@ const sideQueryOpts = { @@ querySource: 'auto_mode' as const, parentSpan, } const result = await sideQuery(sideQueryOpts) - void maybeDumpAutoMode(sideQueryOpts, result, start) - setLastClassifierRequests([sideQueryOpts]) + const dumpOpts = stripTraceContext(sideQueryOpts) + void maybeDumpAutoMode(dumpOpts, result, start) + setLastClassifierRequests([dumpOpts])Also applies to: 869-900, 1140-1169
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/permissions/yoloClassifier.ts` around lines 781 - 807, The objects stage1Opts, stage2Opts, and sideQueryOpts currently include a live LangfuseSpan (parentSpan) and that same object is passed to maybeDumpAutoMode() and setLastClassifierRequests(), risking serialization of trace internals; keep parentSpan only on the object passed to sideQuery (e.g., call sideQuery(stage1Opts) with parentSpan present) but pass a redacted copy (shallow clone with parentSpan removed) to maybeDumpAutoMode() and setLastClassifierRequests(); update the three spots (where stage1Opts/stage2Opts/sideQueryOpts are created and used) to build or clone a copy like { ...opts } then delete or omit parentSpan before calling maybeDumpAutoMode and setLastClassifierRequests so the real span remains for sideQuery but is not stored/dumped.
🧹 Nitpick comments (1)
src/utils/attachments.ts (1)
2197-2226: Type the parent span instead of casting it throughunknown.
parentSpan?: unknownplusparentSpan as Parameters<typeof findRelevantMemories>[5]hides type regressions in the tracing path. Import the span type and keep the signature explicit.♻️ Proposed type tightening
+import type { LangfuseSpan } from '../services/langfuse/index.js' + async function getRelevantMemoryAttachments( input: string, agents: AgentDefinition[], @@ signal: AbortSignal, alreadySurfaced: ReadonlySet<string>, - parentSpan?: unknown, + parentSpan?: LangfuseSpan | null, ): Promise<Attachment[]> { @@ recentTools, alreadySurfaced, - parentSpan as Parameters<typeof findRelevantMemories>[5], + parentSpan, ).catch(() => []),As per coding guidelines, "Use type guards to narrow union types instead of forced type casting."
Also applies to: 2401-2409
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/utils/attachments.ts` around lines 2197 - 2226, The function getRelevantMemoryAttachments uses a loosely typed parentSpan (parentSpan?: unknown) and then casts it when calling findRelevantMemories, which hides type errors; import the proper tracing/span type (e.g., Span or TracingSpan) and change the signature to parentSpan?: Span, remove the cast parentSpan as Parameters<typeof findRelevantMemories>[5] and pass parentSpan directly into findRelevantMemories, and update the other occurrence around lines ~2401-2409 the same way so both call sites use the explicit Span type instead of unknown/casts.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@CLAUDE.md`:
- Around line 61-63: The README line for the "test:all" command is inaccurate:
it claims "typecheck + lint + test" but the actual "test:all" script only runs
typecheck + test; either update the CLAUDE.md description to match the current
behavior or change the "test:all" npm/bun script to include the lint step.
Locate the "test:all" script in package.json (script name "test:all") and either
add the lint invocation into that script (e.g., run the lint script between
typecheck and test) or edit the sentence in CLAUDE.md to state precisely what
"test:all" runs (typecheck + test).
In `@package.json`:
- Line 61: The "test:all" npm script currently only runs typecheck and tests and
should include linting; update the "test:all" entry in package.json (the
"test:all" script) to run lint before typecheck and tests (e.g., run the
existing lint script then typecheck then test) so lint regressions are caught
when developers run test:all; ensure the order is lint -> typecheck -> test and
keep the same execution environment (bun) as other scripts.
In `@packages/audio-capture-napi/src/index.ts`:
- Around line 1-6: The code uses createRequire and a nodeRequire alias to load
the native addon which prevents Bun's static analysis from seeing the require;
remove the import of createRequire and the nodeRequire variable, and replace
uses of nodeRequire(process.env.AUDIO_CAPTURE_NODE_PATH) with a direct
import.meta.require(process.env.AUDIO_CAPTURE_NODE_PATH) call so
AUDIO_CAPTURE_NODE_PATH remains a direct native-addon require; update any
references to createRequire/nodeRequire in this module (and keep the environment
var name AUDIO_CAPTURE_NODE_PATH) and do not introduce node:module imports.
In `@packages/image-processor-napi/src/index.ts`:
- Around line 65-66: The code contains an unused Bun.file(tmpPath) allocation
(const file = Bun.file(tmpPath)) that is dead code because the buffer is read
with readFileSync(tmpPath) instead; remove the unused declaration
(Bun.file(tmpPath)) or, if you intend to use Bun APIs, replace the readFileSync
usage by the corresponding Bun sync read and assign to buffer consistently
(update references to file, tmpPath, and buffer accordingly) so there are no
unused variables.
- Line 1: Remove the unused Bun.file call by deleting the variable assignment
"const file = Bun.file(tmpPath)" (the unused symbol file) and keep the existing
readFileSync(tmpPath) usage; ensure no other code references "file" or
Bun.file(tmpPath) in the surrounding logic (e.g., where tmpPath and buffer are
used) so the code compiles without the unused variable.
In `@scripts/defines.ts`:
- Around line 29-67: DEFAULT_BUILD_FEATURES is reused by scripts/dev.ts but has
UDS_INBOX commented out, which changes bun run dev behavior; restore dev parity
by creating a separate constant DEFAULT_DEV_FEATURES (including UDS_INBOX and
any other dev-only flags) and update scripts/dev.ts to consume
DEFAULT_DEV_FEATURES instead of DEFAULT_BUILD_FEATURES, leaving
DEFAULT_BUILD_FEATURES unchanged for production builds.
In `@src/utils/attachments.ts`:
- Around line 2375-2379: Replace the inline CommonJS require with a static ESM
import: add "import { isPoorModeActive } from '../commands/poor/poorMode.js'" at
the module top, remove the inline require line and its type assertion, and keep
the synchronous call to isPoorModeActive() as-is; ensure no other code relies on
the removed require.
In `@src/utils/model/model.ts`:
- Around line 129-134: The user-specified model parsing can recurse because
getUserSpecifiedModelSetting() may return alias tokens (e.g., "sonnet", "haiku",
"opus", "best", "opusplan") that parseUserSpecifiedModel() itself resolves by
calling the same default-model helpers; to fix, add a guard before calling
parseUserSpecifiedModel(): detect alias tokens (implement or reuse an
isModelAlias(value) check) and if the value is an alias return it or resolve to
a concrete model without invoking the default-model helpers again; update every
call site that directly calls parseUserSpecifiedModel() on
getUserSpecifiedModelSetting() (including the occurrences around
parseUserSpecifiedModel at the other noted call sites) to use the alias guard so
recursion/stacks are prevented.
---
Outside diff comments:
In `@src/utils/permissions/yoloClassifier.ts`:
- Around line 781-807: The objects stage1Opts, stage2Opts, and sideQueryOpts
currently include a live LangfuseSpan (parentSpan) and that same object is
passed to maybeDumpAutoMode() and setLastClassifierRequests(), risking
serialization of trace internals; keep parentSpan only on the object passed to
sideQuery (e.g., call sideQuery(stage1Opts) with parentSpan present) but pass a
redacted copy (shallow clone with parentSpan removed) to maybeDumpAutoMode() and
setLastClassifierRequests(); update the three spots (where
stage1Opts/stage2Opts/sideQueryOpts are created and used) to build or clone a
copy like { ...opts } then delete or omit parentSpan before calling
maybeDumpAutoMode and setLastClassifierRequests so the real span remains for
sideQuery but is not stored/dumped.
---
Nitpick comments:
In `@src/utils/attachments.ts`:
- Around line 2197-2226: The function getRelevantMemoryAttachments uses a
loosely typed parentSpan (parentSpan?: unknown) and then casts it when calling
findRelevantMemories, which hides type errors; import the proper tracing/span
type (e.g., Span or TracingSpan) and change the signature to parentSpan?: Span,
remove the cast parentSpan as Parameters<typeof findRelevantMemories>[5] and
pass parentSpan directly into findRelevantMemories, and update the other
occurrence around lines ~2401-2409 the same way so both call sites use the
explicit Span type instead of unknown/casts.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7fd62a02-d906-4ccf-ada6-314dfd976e09
📒 Files selected for processing (19)
CLAUDE.mdbuild.tspackage.jsonpackages/audio-capture-napi/src/index.tspackages/color-diff-napi/src/index.tspackages/image-processor-napi/src/index.tsscripts/defines.tsscripts/dev.tsscripts/vite-plugin-feature-flags.tssrc/memdir/findRelevantMemories.tssrc/services/langfuse/index.tssrc/services/langfuse/tracing.tssrc/utils/attachments.tssrc/utils/context.tssrc/utils/model/model.tssrc/utils/permissions/permissions.tssrc/utils/permissions/yoloClassifier.tssrc/utils/sideQuery.tssrc/utils/tokens.ts
Comment on lines
+61
to
+63
| # Full check (typecheck + lint + test) — run after completing any task | ||
| bun run test:all | ||
|
|
Contributor
There was a problem hiding this comment.
Fix command description mismatch with actual script behavior.
Line 61 says test:all includes lint, but bun run test:all currently runs only typecheck + test. Please either update the description or include lint in the script to avoid misleading contributors.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@CLAUDE.md` around lines 61 - 63, The README line for the "test:all" command
is inaccurate: it claims "typecheck + lint + test" but the actual "test:all"
script only runs typecheck + test; either update the CLAUDE.md description to
match the current behavior or change the "test:all" npm/bun script to include
the lint step. Locate the "test:all" script in package.json (script name
"test:all") and either add the lint invocation into that script (e.g., run the
lint script between typecheck and test) or edit the sentence in CLAUDE.md to
state precisely what "test:all" runs (typecheck + test).
| "postinstall": "node scripts/run-parallel.mjs scripts/postinstall.cjs scripts/setup-chrome-mcp.mjs", | ||
| "docs:dev": "npx mintlify dev", | ||
| "typecheck": "tsc --noEmit", | ||
| "test:all": "bun run typecheck && bun test", |
Contributor
There was a problem hiding this comment.
test:all is not actually “all” checks right now.
The script omits lint, while docs/CI messaging treat it as full validation. This can let lint regressions pass when developers run only test:all.
Suggested fix
- "test:all": "bun run typecheck && bun test",
+ "test:all": "bun run typecheck && bun run lint && bun test",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 61, The "test:all" npm script currently only runs
typecheck and tests and should include linting; update the "test:all" entry in
package.json (the "test:all" script) to run lint before typecheck and tests
(e.g., run the existing lint script then typecheck then test) so lint
regressions are caught when developers run test:all; ensure the order is lint ->
typecheck -> test and keep the same execution environment (bun) as other
scripts.
Comment on lines
+1
to
+6
| import { createRequire } from 'node:module' | ||
|
|
||
| // createRequire works in both Bun and Node.js ESM contexts. | ||
| // Needed because this package is "type": "module" but uses require() for | ||
| // loading native .node addons — bare require is not available in Node.js ESM. | ||
| const nodeRequire = createRequire(import.meta.url) |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect native addon require handling and build-time defines.
# Expectation: build scripts/comments indicate import.meta.require/direct require
# is the supported analyzable form; createRequire/nodeRequire aliases should be
# explicitly handled before keeping this change.
rg -n -C4 'AUDIO_CAPTURE_NODE_PATH|audio-capture\.node|import\.meta\.require|createRequire|nodeRequire|require\(process\.env' \
--glob '*.ts' \
--glob '*.js'Repository: claude-code-best/claude-code
Length of output: 13385
Bun static analysis requires a direct native addon require call, not one hidden behind a createRequire alias.
The native addon path at line 42-46 documents that AUDIO_CAPTURE_NODE_PATH must remain a direct require() call for Bun to analyze it statically. Using nodeRequire(process.env.AUDIO_CAPTURE_NODE_PATH) breaks this because Bun cannot trace the call through an alias. Additionally, importing createRequire from node:module violates the guideline to use only Bun runtime APIs.
Use import.meta.require instead, which the build system automatically post-processes to Node.js-compatible versions:
-import { createRequire } from 'node:module'
-
-// createRequire works in both Bun and Node.js ESM contexts.
-// Needed because this package is "type": "module" but uses require() for
-// loading native .node addons — bare require is not available in Node.js ESM.
-const nodeRequire = createRequire(import.meta.url)
+// Bun API; build.ts post-processes import.meta.require to Node.js compatibility.
+const nodeRequire = import.meta.require🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/audio-capture-napi/src/index.ts` around lines 1 - 6, The code uses
createRequire and a nodeRequire alias to load the native addon which prevents
Bun's static analysis from seeing the require; remove the import of
createRequire and the nodeRequire variable, and replace uses of
nodeRequire(process.env.AUDIO_CAPTURE_NODE_PATH) with a direct
import.meta.require(process.env.AUDIO_CAPTURE_NODE_PATH) call so
AUDIO_CAPTURE_NODE_PATH remains a direct native-addon require; update any
references to createRequire/nodeRequire in this module (and keep the environment
var name AUDIO_CAPTURE_NODE_PATH) and do not introduce node:module imports.
| @@ -1,3 +1,4 @@ | |||
| import { readFileSync, unlinkSync } from 'node:fs' | |||
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check for node:fs usage patterns and verify if Bun APIs are available
# Search for other node:fs imports in the codebase
echo "=== Checking for node:fs imports ==="
rg -n "from ['\"]node:fs['\"]" --type=ts
# Search for Bun.file usage patterns
echo -e "\n=== Checking Bun.file usage patterns ==="
rg -n "Bun\.file\(" --type=ts -A 2
# Check for readFileSync usage
echo -e "\n=== Checking readFileSync usage ==="
rg -n "readFileSync" --type=tsRepository: claude-code-best/claude-code
Length of output: 17530
Remove the unused Bun.file() call on line 65.
Line 65 creates a Bun.file(tmpPath) variable that is never used. The immediately following line uses readFileSync(tmpPath) instead. Remove the unused variable assignment.
Relevant code context
const file = Bun.file(tmpPath)
const buffer: Buffer = readFileSync(tmpPath)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/image-processor-napi/src/index.ts` at line 1, Remove the unused
Bun.file call by deleting the variable assignment "const file =
Bun.file(tmpPath)" (the unused symbol file) and keep the existing
readFileSync(tmpPath) usage; ensure no other code references "file" or
Bun.file(tmpPath) in the surrounding logic (e.g., where tmpPath and buffer are
used) so the code compiles without the unused variable.
Comment on lines
65
to
+66
| const file = Bun.file(tmpPath) | ||
| // Use synchronous read via Node compat | ||
| const fs = require('fs') | ||
| const buffer: Buffer = fs.readFileSync(tmpPath) | ||
| const buffer: Buffer = readFileSync(tmpPath) |
Contributor
There was a problem hiding this comment.
Remove unused Bun.file() call - dead code detected.
Line 65 declares const file = Bun.file(tmpPath) but this variable is never used. Line 66 immediately uses readFileSync(tmpPath) instead. This suggests an incomplete refactor.
🧹 Proposed fix to remove dead code
Option 1: Remove the unused line (recommended for this synchronous context)
- const file = Bun.file(tmpPath)
const buffer: Buffer = readFileSync(tmpPath)Option 2: Use Bun APIs consistently (if Bun has sync file reading)
If Bun provides synchronous file reading APIs, consider using them instead to align with coding guidelines:
const file = Bun.file(tmpPath)
const buffer = Buffer.from(/* appropriate Bun sync API */)📝 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.
Suggested change
| const file = Bun.file(tmpPath) | |
| // Use synchronous read via Node compat | |
| const fs = require('fs') | |
| const buffer: Buffer = fs.readFileSync(tmpPath) | |
| const buffer: Buffer = readFileSync(tmpPath) | |
| const buffer: Buffer = readFileSync(tmpPath) |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/image-processor-napi/src/index.ts` around lines 65 - 66, The code
contains an unused Bun.file(tmpPath) allocation (const file = Bun.file(tmpPath))
that is dead code because the buffer is read with readFileSync(tmpPath) instead;
remove the unused declaration (Bun.file(tmpPath)) or, if you intend to use Bun
APIs, replace the readFileSync usage by the corresponding Bun sync read and
assign to buffer consistently (update references to file, tmpPath, and buffer
accordingly) so there are no unused variables.
Comment on lines
+29
to
+67
| export const DEFAULT_BUILD_FEATURES = [ | ||
| 'BUDDY', 'TRANSCRIPT_CLASSIFIER', 'BRIDGE_MODE', | ||
| 'AGENT_TRIGGERS_REMOTE', | ||
| 'CHICAGO_MCP', | ||
| 'VOICE_MODE', | ||
| 'SHOT_STATS', | ||
| 'PROMPT_CACHE_BREAK_DETECTION', | ||
| 'TOKEN_BUDGET', | ||
| // P0: local features | ||
| 'AGENT_TRIGGERS', | ||
| 'ULTRATHINK', | ||
| 'BUILTIN_EXPLORE_PLAN_AGENTS', | ||
| 'LODESTONE', | ||
| // P1: API-dependent features | ||
| 'EXTRACT_MEMORIES', | ||
| 'VERIFICATION_AGENT', | ||
| 'KAIROS_BRIEF', | ||
| 'AWAY_SUMMARY', | ||
| 'ULTRAPLAN', | ||
| // P2: daemon + remote control server | ||
| 'DAEMON', | ||
| // ACP (Agent Client Protocol) agent mode | ||
| 'ACP', | ||
| // PR-package restored features | ||
| 'WORKFLOW_SCRIPTS', | ||
| 'HISTORY_SNIP', | ||
| 'CONTEXT_COLLAPSE', | ||
| 'MONITOR_TOOL', | ||
| 'FORK_SUBAGENT', | ||
| // 'UDS_INBOX', | ||
| 'KAIROS', | ||
| 'COORDINATOR_MODE', | ||
| 'LAN_PIPES', | ||
| 'BG_SESSIONS', | ||
| 'TEMPLATES', | ||
| // 'REVIEW_ARTIFACT', // API 请求无响应，需进一步排查 schema 兼容性 | ||
| // P3: poor mode (disable extract_memories + prompt_suggestion) | ||
| 'POOR', | ||
| ] as const; |
Contributor
There was a problem hiding this comment.
Preserve the previous dev-only feature defaults.
DEFAULT_BUILD_FEATURES is now reused by scripts/dev.ts, but Line 58 comments out UDS_INBOX, which the previous dev default list enabled. This silently changes bun run dev behavior. Consider adding a separate dev default list, or re-enable UDS_INBOX if this shared list is intended to preserve dev parity.
Suggested direction
export const DEFAULT_BUILD_FEATURES = [
'BUDDY', 'TRANSCRIPT_CLASSIFIER', 'BRIDGE_MODE',
@@
'POOR',
] as const;
+
+export const DEFAULT_DEV_FEATURES = [
+ ...DEFAULT_BUILD_FEATURES,
+ 'UDS_INBOX',
+] as const;Then have scripts/dev.ts consume DEFAULT_DEV_FEATURES instead of DEFAULT_BUILD_FEATURES.
Based on learnings: Dev mode enables all feature flags by default via -d flag injection through scripts/dev.ts.
📝 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.
Suggested change
| export const DEFAULT_BUILD_FEATURES = [ | |
| 'BUDDY', 'TRANSCRIPT_CLASSIFIER', 'BRIDGE_MODE', | |
| 'AGENT_TRIGGERS_REMOTE', | |
| 'CHICAGO_MCP', | |
| 'VOICE_MODE', | |
| 'SHOT_STATS', | |
| 'PROMPT_CACHE_BREAK_DETECTION', | |
| 'TOKEN_BUDGET', | |
| // P0: local features | |
| 'AGENT_TRIGGERS', | |
| 'ULTRATHINK', | |
| 'BUILTIN_EXPLORE_PLAN_AGENTS', | |
| 'LODESTONE', | |
| // P1: API-dependent features | |
| 'EXTRACT_MEMORIES', | |
| 'VERIFICATION_AGENT', | |
| 'KAIROS_BRIEF', | |
| 'AWAY_SUMMARY', | |
| 'ULTRAPLAN', | |
| // P2: daemon + remote control server | |
| 'DAEMON', | |
| // ACP (Agent Client Protocol) agent mode | |
| 'ACP', | |
| // PR-package restored features | |
| 'WORKFLOW_SCRIPTS', | |
| 'HISTORY_SNIP', | |
| 'CONTEXT_COLLAPSE', | |
| 'MONITOR_TOOL', | |
| 'FORK_SUBAGENT', | |
| // 'UDS_INBOX', | |
| 'KAIROS', | |
| 'COORDINATOR_MODE', | |
| 'LAN_PIPES', | |
| 'BG_SESSIONS', | |
| 'TEMPLATES', | |
| // 'REVIEW_ARTIFACT', // API 请求无响应，需进一步排查 schema 兼容性 | |
| // P3: poor mode (disable extract_memories + prompt_suggestion) | |
| 'POOR', | |
| ] as const; | |
| export const DEFAULT_BUILD_FEATURES = [ | |
| 'BUDDY', 'TRANSCRIPT_CLASSIFIER', 'BRIDGE_MODE', | |
| 'AGENT_TRIGGERS_REMOTE', | |
| 'CHICAGO_MCP', | |
| 'VOICE_MODE', | |
| 'SHOT_STATS', | |
| 'PROMPT_CACHE_BREAK_DETECTION', | |
| 'TOKEN_BUDGET', | |
| // P0: local features | |
| 'AGENT_TRIGGERS', | |
| 'ULTRATHINK', | |
| 'BUILTIN_EXPLORE_PLAN_AGENTS', | |
| 'LODESTONE', | |
| // P1: API-dependent features | |
| 'EXTRACT_MEMORIES', | |
| 'VERIFICATION_AGENT', | |
| 'KAIROS_BRIEF', | |
| 'AWAY_SUMMARY', | |
| 'ULTRAPLAN', | |
| // P2: daemon + remote control server | |
| 'DAEMON', | |
| // ACP (Agent Client Protocol) agent mode | |
| 'ACP', | |
| // PR-package restored features | |
| 'WORKFLOW_SCRIPTS', | |
| 'HISTORY_SNIP', | |
| 'CONTEXT_COLLAPSE', | |
| 'MONITOR_TOOL', | |
| 'FORK_SUBAGENT', | |
| // 'UDS_INBOX', | |
| 'KAIROS', | |
| 'COORDINATOR_MODE', | |
| 'LAN_PIPES', | |
| 'BG_SESSIONS', | |
| 'TEMPLATES', | |
| // 'REVIEW_ARTIFACT', // API 请求无响应，需进一步排查 schema 兼容性 | |
| // P3: poor mode (disable extract_memories + prompt_suggestion) | |
| 'POOR', | |
| ] as const; | |
| export const DEFAULT_DEV_FEATURES = [ | |
| ...DEFAULT_BUILD_FEATURES, | |
| 'UDS_INBOX', | |
| ] as const; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/defines.ts` around lines 29 - 67, DEFAULT_BUILD_FEATURES is reused by
scripts/dev.ts but has UDS_INBOX commented out, which changes bun run dev
behavior; restore dev parity by creating a separate constant
DEFAULT_DEV_FEATURES (including UDS_INBOX and any other dev-only flags) and
update scripts/dev.ts to consume DEFAULT_DEV_FEATURES instead of
DEFAULT_BUILD_FEATURES, leaving DEFAULT_BUILD_FEATURES unchanged for production
builds.
Comment on lines
+2375
to
+2379
| // Poor mode: skip the side-query to save tokens | ||
| const { isPoorModeActive } = require('../commands/poor/poorMode.js') as typeof import('../commands/poor/poorMode.js') | ||
| if (isPoorModeActive()) { | ||
| return undefined | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check the new poor-mode import site and surrounding no-require lint suppression patterns.
rg -n -C3 "require\\('../commands/poor/poorMode\\.js'\\)|isPoorModeActive" --type ts
rg -n -C2 "@typescript-eslint/no-require-imports|no-require-imports" --type tsRepository: claude-code-best/claude-code
Length of output: 50387
Convert to ESM import for isPoorModeActive.
Line 2376 uses bare require() outside any ESM suppression block. Since isPoorModeActive() is called synchronously, use a static import:
import { isPoorModeActive } from '../commands/poor/poorMode.js'Then remove the inline require statement. This aligns with the ESM-only project standard and matches how isPoorModeActive is imported elsewhere in the codebase.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils/attachments.ts` around lines 2375 - 2379, Replace the inline
CommonJS require with a static ESM import: add "import { isPoorModeActive } from
'../commands/poor/poorMode.js'" at the module top, remove the inline require
line and its type assertion, and keep the synchronous call to isPoorModeActive()
as-is; ensure no other code relies on the removed require.
Comment on lines
+129
to
+134
| // Fall back to user's configured model — custom providers may not | ||
| // recognize hardcoded Anthropic model IDs. | ||
| const userSpecifiedOpus = getUserSpecifiedModelSetting() | ||
| if (userSpecifiedOpus) { | ||
| return parseUserSpecifiedModel(userSpecifiedOpus) | ||
| } |
Contributor
There was a problem hiding this comment.
Guard aliases before parsing user defaults.
getUserSpecifiedModelSetting() can return aliases, but parseUserSpecifiedModel() resolves aliases by calling these same default-model helpers. Settings like ANTHROPIC_MODEL=sonnet, haiku, opus, best, or opusplan can now recurse until stack overflow.
🐛 Proposed fix
+function getUserSpecifiedExplicitModelFallback(): ModelName | undefined {
+ const userSpecified = getUserSpecifiedModelSetting()
+ if (!userSpecified) {
+ return undefined
+ }
+
+ const normalized = userSpecified
+ .trim()
+ .toLowerCase()
+ .replace(/\[1m]$/i, '')
+ .trim()
+
+ if (isModelAlias(normalized)) {
+ return undefined
+ }
+
+ if (
+ getAPIProvider() === 'firstParty' &&
+ isLegacyOpusFirstParty(normalized) &&
+ isLegacyModelRemapEnabled()
+ ) {
+ return undefined
+ }
+
+ return parseUserSpecifiedModel(userSpecified)
+}
+
// @[MODEL LAUNCH]: Update the default Opus model (3P providers may lag so keep defaults unchanged).
export function getDefaultOpusModel(): ModelName {
@@
- const userSpecifiedOpus = getUserSpecifiedModelSetting()
- if (userSpecifiedOpus) {
- return parseUserSpecifiedModel(userSpecifiedOpus)
+ const userSpecifiedOpus = getUserSpecifiedExplicitModelFallback()
+ if (userSpecifiedOpus) {
+ return userSpecifiedOpus
}
@@
- const userSpecified = getUserSpecifiedModelSetting()
+ const userSpecified = getUserSpecifiedExplicitModelFallback()
if (userSpecified) {
- return parseUserSpecifiedModel(userSpecified)
+ return userSpecified
}
@@
- const userSpecifiedHaiku = getUserSpecifiedModelSetting()
+ const userSpecifiedHaiku = getUserSpecifiedExplicitModelFallback()
if (userSpecifiedHaiku) {
- return parseUserSpecifiedModel(userSpecifiedHaiku)
+ return userSpecifiedHaiku
}Also applies to: 162-168, 191-196
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/utils/model/model.ts` around lines 129 - 134, The user-specified model
parsing can recurse because getUserSpecifiedModelSetting() may return alias
tokens (e.g., "sonnet", "haiku", "opus", "best", "opusplan") that
parseUserSpecifiedModel() itself resolves by calling the same default-model
helpers; to fix, add a guard before calling parseUserSpecifiedModel(): detect
alias tokens (implement or reuse an isModelAlias(value) check) and if the value
is an alias return it or resolve to a concrete model without invoking the
default-model helpers again; update every call site that directly calls
parseUserSpecifiedModel() on getUserSpecifiedModelSetting() (including the
occurrences around parseUserSpecifiedModel at the other noted call sites) to use
the alias guard so recursion/stacks are prevented.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Chores