Skip to content

perf: 为 tool_result 建立惰性索引,避免 /artifacts 二次方扫描 - #1349

Open
ai-yang wants to merge 1 commit into
claude-code-best:mainfrom
ai-yang:audit/perf-artifact-scan
Open

perf: 为 tool_result 建立惰性索引,避免 /artifacts 二次方扫描#1349
ai-yang wants to merge 1 commit into
claude-code-best:mainfrom
ai-yang:audit/perf-artifact-scan

Conversation

@ai-yang

@ai-yang ai-yang commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Summary

  • 首个 artifact 保留原有直接查找路径。
  • 从第 2 个 artifact 开始,一次扫描建立 first-result-wins 的 tool_result 索引。
  • 后续 artifact 通过 tool_use_id 常数时间查找,避免对 transcript 反复全表扫描。
  • 增加重复 result ID、多个 artifact 和 result-before-use 等价性测试。

Benchmark

每场景 60 对预热,3 trial × 120 次随机平衡 AB/BA,共 360 样本/实现;所有输出严格等价。

场景 median p95
1k / 25 artifacts 0.20054 → 0.05370 ms(3.73×) 0.20179 → 0.05491 ms(3.68×)
2k / 200 2.70905 → 0.28186 ms(9.61×) 2.71983 → 0.29307 ms(9.28×)
10k / 500 31.96035 → 0.91182 ms(35.05×) 32.04902 → 0.95202 ms(33.66×)

低基数边界:1k / 1 artifact 的 median 为 0.02630 → 0.02901 ms(+2.7 μs),p95 为 0.04004 → 0.04104 ms;未宣称所有场景都加速。

Test plan

  • bun test src/commands/artifacts/__tests__/scanner.test.ts(8 pass)
  • bun run typecheck
  • Biome check(改动文件)
  • git diff --check
  • duplicate / unmatched / array content / error / malformed fixture 严格等价

关联 issue

Closes #1346

Summary by CodeRabbit

  • Bug Fixes

    • Improved artifact detection when multiple artifacts are indexed.
    • Correctly pairs tool results that appear before their corresponding artifact actions.
    • Preserves the first matching result when duplicate results are present.
  • Performance

    • Improved efficiency when processing conversations containing repeated artifact actions.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The artifact scanner now builds a cached tool-result index after the first artifact. The index preserves first-result-wins behavior. Tests cover multiple artifacts, result ordering, duplicate IDs, and results that precede tool-use messages.

Changes

Artifact scanning

Layer / File(s) Summary
Cached tool-result lookup
src/commands/artifacts/scanner.ts
The scanner uses a shared ArtifactToolResult type, performs direct lookup for the first artifact, and builds a first-result-wins index for later artifacts.
Artifact pairing regression coverage
src/commands/artifacts/__tests__/scanner.test.ts
Tests validate multiple-artifact indexing, newest-first results, duplicate IDs, and result-before-use pairing.

Estimated code review effort: 3 (Moderate) | ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 标题准确概括了为 tool_result 建立惰性索引并避免 /artifacts 二次方扫描这一主要变更。
Linked Issues check ✅ Passed 实现符合 #1346 的性能优化、first-result-wins 语义、行为等价性和回归测试要求。
Out of Scope Changes check ✅ Passed 代码和测试变更均直接支持 #1346 的索引优化与行为保持目标,未发现无关改动。
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@ai-yang
ai-yang marked this pull request as ready for review August 11, 2026 11:47

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/commands/artifacts/scanner.ts`:
- Around line 128-140: The tool_result scan must ignore malformed entries with
non-string tool_use_id values. In the loop processing blocks in scanner.ts,
validate b.tool_use_id with typeof === 'string' and continue before checking
results or calling results.set; retain the existing first-result-wins behavior
for valid string IDs.
🪄 Autofix

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 Plus

Run ID: bef1e453-b237-4ea9-a7b8-b965d3beb793

📥 Commits

Reviewing files that changed from the base of the PR and between 3bb6b57 and e64e174.

📒 Files selected for processing (2)
  • src/commands/artifacts/__tests__/scanner.test.ts
  • src/commands/artifacts/scanner.ts

Comment on lines +128 to +140
for (const block of content) {
if (typeof block !== 'object' || block === null) continue
if (!('type' in block)) continue
const b = block as unknown as Record<string, unknown>
if (b.type !== 'tool_result') continue
const toolUseId = b.tool_use_id as string
if (results.has(toolUseId)) continue

// Match findToolResult's first-result-wins behavior for duplicate IDs.
results.set(toolUseId, {
content: b.content,
is_error: b.is_error as boolean | undefined,
})

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

ast-grep outline src/types/message.ts --items all
rg -n -C 4 '\btool_use_id\b|\bid\b' \
  src/types/message.ts \
  src/commands/artifacts/scanner.ts

Repository: claude-code-best/claude-code

Length of output: 2464


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- scanner.ts ---'
sed -n '1,180p' src/commands/artifacts/scanner.ts

printf '%s\n' '--- message type definitions and relevant references ---'
rg -n -C 5 'ContentBlock|ToolResult|tool_use_id|tool_use' src/types src/commands/artifacts src --glob '*.{ts,tsx}' | head -n 500

printf '%s\n' '--- scanner tests and call sites ---'
rg -n -C 4 'findToolResult|build.*Index|ArtifactToolResult|scanArtifacts|tool_use_id' src --glob '*.{ts,tsx}'

Repository: claude-code-best/claude-code

Length of output: 50385


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- scanner.ts ---'
cat -n src/commands/artifacts/scanner.ts | sed -n '1,175p'

printf '%s\n' '--- scanner symbols and local tests ---'
rg -n -C 8 'ArtifactToolResult|findToolResult|results\.|scanArtifacts|scanner' \
  src/commands/artifacts --glob '*.{ts,tsx}'

printf '%s\n' '--- message.ts relevant declarations ---'
rg -n -C 8 'ContentBlock|ToolResultBlock|tool_use_id|Message' src/types/message.ts

printf '%s\n' '--- package/dependency declarations for SDK block types ---'
rg -n -C 4 'ToolResultBlock|tool_use_id' package.json bun.lockb bun.lock yarn.lock pnpm-lock.yaml node_modules/@anthropic-ai 2>/dev/null || true

Repository: claude-code-best/claude-code

Length of output: 18996


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- scanner tests ---'
cat -n src/commands/artifacts/__tests__/scanner.test.ts | sed -n '1,260p'

printf '%s\n' '--- model-provider type sources in the repository ---'
git ls-files | rg '(^|/)(model-provider|.*message.*|.*content.*)\.(ts|tsx|d\.ts)$' | head -n 200

printf '%s\n' '--- all scanner casts and transcript construction ---'
rg -n -C 3 'as string|as unknown as|tool_use_id' src/commands/artifacts src/types src/utils/sessionStorage.ts src/utils/conversationRecovery.ts --glob '*.{ts,tsx}' | head -n 400

Repository: claude-code-best/claude-code

Length of output: 21571


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- model-provider message types ---'
rg -n -C 12 'ToolResult|tool_result|ToolUse|tool_use|ContentBlock' packages/@ant/model-provider/src/types/message.ts

printf '%s\n' '--- Message input boundary and artifact command callers ---'
rg -n -C 8 'extractArtifacts\(|context\.messages|messages\?:|messages:' \
  src/commands/artifacts packages/@ant/model-provider/src --glob '*.{ts,tsx}'

printf '%s\n' '--- malformed-block test feasibility ---'
rg -n -C 5 'as unknown as Message|tool_use_id: [^'\''"]|tool_use_id: null|tool_use_id: [0-9]' \
  src/commands/artifacts src --glob '*.{test.ts,test.tsx}' | head -n 300

Repository: claude-code-best/claude-code

Length of output: 50034


🏁 Script executed:

#!/bin/bash
set -euo pipefail

node - <<'JS'
const results = new Map()
const malformedToolResult = { type: 'tool_result', tool_use_id: 42, content: 'payload' }
const assertedToolUseId = malformedToolResult.tool_use_id
results.set(assertedToolUseId, { content: malformedToolResult.content })

console.log({
  storedKeyType: typeof [...results.keys()][0],
  lookupWithStringId: results.get('42') ?? null,
  lookupWithNumberId: results.get(42) ?? null,
})
JS

Repository: claude-code-best/claude-code

Length of output: 270


Narrow tool_use_id before indexing.

If a malformed tool_result contains a non-string tool_use_id, the assertion stores a non-string key and string lookups fail. Skip the block unless typeof toolUseId === 'string'.

🤖 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/commands/artifacts/scanner.ts` around lines 128 - 140, The tool_result
scan must ignore malformed entries with non-string tool_use_id values. In the
loop processing blocks in scanner.ts, validate b.tool_use_id with typeof ===
'string' and continue before checking results or calling results.set; retain the
existing first-result-wins behavior for valid string IDs.

Source: Coding guidelines

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant