Skip to content

fix(claude): retain ToolSearch references in translated results - #6660

Merged
lidge-jun merged 3 commits into
lidge-jun:devfrom
Ingwannu:ingw/fix-6647-tool-reference
Oct 6, 2026
Merged

lidge-jun merged 3 commits into
lidge-jun:devfrom
Ingwannu:ingw/fix-6647-tool-reference

Conversation

@Ingwannu

@Ingwannu Ingwannu commented Oct 6, 2026 •

Copy link
Copy Markdown
Owner

Summary

Closes #6647.

  • Preserve nonempty string tool_reference.tool_name values as Tool loaded: <name> text in the paired translated function_call_output, instead of silently turning reference-only ToolSearch results into an empty string.
  • Preserve call IDs, chronological slots, mixed text/image order, error markers and caller input. Missing/non-string/empty names do not fabricate loaded-tool evidence. This does not add tool declarations, widen tool-call authorization, enable translated server-side deferral or change native Anthropic passthrough.
  • Add three schema/parser-backed regression cases, register the test in both layout inventories, and update the owning contract plus English/Korean mapping references. Reporter and minimal projection suggestion credited to @rhomat27.

Verification

Base: f92482065217bae891a88e27fd6c00012f212042.
Head: 502b6e38fb6a0b46f51b84ab8e8f0a36b13f0a8f.
Linux, repository-pinned Bun 1.4.0 (34cbb9a40). No live accounts/providers or installed runtime operations.

  • Red control on unchanged production source plus the new fixture: bun test tests/claude-integration/claude-inbound-tool-reference.test.ts --timeout 20000: 1 pass / 2 fail. The reference-only result was empty; the mixed result omitted the loaded-name block.
  • Green: bun scripts/test.ts --parallel=1 tests/claude-integration/claude-inbound-tool-reference.test.ts tests/claude-integration/claude-inbound.test.ts tests/claude-integration/claude-inbound-cache-stabilize.test.ts tests/test-layout.test.ts tests/test-layout-tooling.test.ts tests/ci-workflows/file-size-ratchet.test.ts --timeout 20000: 140 pass / 0 fail, 997 assertions across six files.
  • bun scripts/structure-ssot.ts and git diff --cached --check: passed.
  • bun node_modules/typescript/bin/tsc --noEmit: did not complete within its finite 180-second cgroup deadline (terminated on timeout). Typecheck is unverified, not passing; the following chained structure command did not run, so structure was executed separately as reported above. No memory/CPU cap increase or unbounded retry.
  • One earlier wrapper launch stopped before running tests because GUI dependencies were not linked and its attempted bun install executable was absent from the isolated PATH. No install executed. Existing local root/GUI dependencies were then linked; the green command above is the completed run.

Each workload used fresh private HOME/CODEX_HOME/OPENCODEX_HOME/TMPDIR. Tests/typecheck used CPUQuota=75%, MemoryHigh=1280M, MemoryMax=1536M, MemorySwapMax=0, TasksMax=64, IOWeight=20, nice=10 and finite deadlines; the separate structure gate used the smaller 50%/384M/512M/32 envelope. Effective cgroup settings were checked before execution; workloads ran serially. Protected Codex/OpenCodex/Paseo files were unchanged.

Full backend/import-connected suite and docs build were not run: the large repository-wide workloads are disproportionate to this small projection under the user's strict shared-host limits, and typecheck already reached its bound. The six focused files cover the changed mapping, existing translation behavior and indirect layout/source-as-data gates. Native Windows/macOS and real Claude Code/ToolSearch round trips remain unverified. No local security/privacy scan was run under the user's explicit prohibition.

Keep Draft pending successful current-head typecheck/required hosted CI, documentation-build validation and independent review. Fixture success does not establish live model loop resolution, performance or a released fix.

Checklist

  • Scope stays focused and avoids unrelated cleanup.
  • Docs or release notes were updated when needed.
  • Security-sensitive changes were reviewed for secrets, auth, and unsafe defaults.

Review readiness checklist

  • Required local validation passed with commands, scope and exceptions recorded.
  • Branch is based on the recorded current dev commit.
  • All correct Codex and CodeRabbit findings are resolved.
  • Ready for review.

Co-authored-by: rhomat27 8294456+rhomat27@users.noreply.github.com

Maintainer lane verification (sweep lane A, head c17f053)

  • Merged origin/dev 38455fb into the branch (no force push).
  • Packed the new layout.json entry onto the existing claude-inbound.test.ts line: dev's layout.json is at 1999 lines and a 2000th line trips file-size ratchet: repository (NEW_OVERSIZED).
  • Folded an independent gpt-6.1-sol review finding: text-only tool output is joined without separators in src/responses/parser-content.ts, so adjacent markers fused into Tool loaded: ReadTool loaded: Write. Markers are now line-delimited, with a parser-level regression test; re-review PASS.
  • Red control: the new test file fails 3 of 4 on the pre-fold inbound.ts.
  • bun test tests/claude-integration/claude-inbound-tool-reference.test.ts tests/claude-integration/claude-inbound.test.ts tests/claude-integration/claude-inbound-cache-stabilize.test.ts tests/test-layout.test.ts tests/test-layout-tooling.test.ts tests/ci-workflows/file-size-ratchet.test.ts: 141 pass / 0 fail.
  • bun run typecheck: exit 0. bun run structure:check: passed.
  • The full local suite was not run because parallel lanes share this machine; remaining coverage is left to exact-head hosted CI.

Summary by CodeRabbit

  • New Features
    • Translated Claude tool results now retain named tool references as “Tool loaded: [name]” lines in the paired result, preserving their order alongside other text and error markers. References without a nonempty tool name do not produce a loaded-tool line.
    • These lines do not add executable tool declarations or enable server-side deferral.
  • Documentation
    • Updated the Claude integration guides to describe how tool references appear in translated results.

Preserve client-reported loaded names in paired tool output instead of
silently discarding reference-only ToolSearch results. This does not add
tool declarations or enable server-side deferral on translated routes.

Reported and minimal text projection suggested in lidge-jun#6647 by rhomat27.

Co-authored-by: rhomat27 <8294456+rhomat27@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Repository: lidge-jun/opencodex/.coderabbit.yaml
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 8b8a9886-3f6f-4bb3-9039-a217293c75c3
📥 Commits

Reviewing files that changed from the base of the PR and between 38455fb and c17f053.

📒 Files selected for processing (7)
  • docs-site/src/content/docs/guides/claude-code.md
  • docs-site/src/content/docs/ko/guides/claude-code.md
  • scripts/test-layout/layout.json
  • src/claude/inbound.ts
  • structure/clients/claude-desktop.md
  • tests/claude-integration/claude-inbound-tool-reference.test.ts
  • tests/fixtures/test-layout-expected.json

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

The Claude inbound translation now represents named tool_reference blocks as newline-separated Tool loaded: <tool_name> text in the paired tool result. Integration tests and documentation cover the translation and its boundaries.

Changes

Tool reference translation

Layer / File(s) Summary
Translate and validate tool references
src/claude/inbound.ts, tests/claude-integration/claude-inbound-tool-reference.test.ts, scripts/test-layout/layout.json, tests/fixtures/test-layout-expected.json, docs-site/src/content/docs/guides/claude-code.md, docs-site/src/content/docs/ko/guides/claude-code.md, structure/clients/claude-desktop.md
toolResultOutput converts references with nonempty string names to ordered Tool loaded: <name> text, separated from adjacent content by newlines. Tests cover mixed content, error markers, invalid names, input immutability, and the absence of additional tool declarations. Documentation describes the translated behavior and notes that it does not enable server-side deferral or change native passthrough.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Merge Risk: ⚪ Minimal · up to c17f0

No actionable issue remains from this review. Complete the outstanding hosted CI and documentation-build checks before merging.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (5 skipped: 5 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly describes the main change: preserving ToolSearch references in translated results.
Linked Issues check ✅ Passed Issue #6647 asks the translated Messages-to-Responses route to preserve loaded tool names in the paired tool result. In src/claude/inbound.ts, toolResultOutput maps nonempty string `tool_reference…
Out of Scope Changes check ✅ Passed The reported changes are limited to the #6647 translation behavior, its regression coverage, test-layout registration, and related English/Korean and contract documentation. The marker newline handlin…
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (5 skipped: 5 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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.

@github-actions github-actions Bot added the bug Something isn't working label Oct 6, 2026
@github-actions

github-actions Bot commented Oct 6, 2026

Copy link
Copy Markdown
Contributor

✅ Deterministic PR hygiene checks passed.

Text-only tool output is joined without separators downstream, so adjacent
Tool loaded markers fused ("Tool loaded: ReadTool loaded: Write"). Give each
marker its own line breaks and assert the parsed toolResult content.

Pack the new layout entry onto the existing claude-inbound line: dev's
layout.json sits at 1999 lines and a 2000th trips the file-size ratchet.
@lidge-jun
lidge-jun marked this pull request as ready for review October 6, 2026 12:29
@lidge-jun
lidge-jun self-requested a review as a code owner October 6, 2026 12:29
@lidge-jun

Copy link
Copy Markdown
Owner

Maintainer integration into dev (MAINTAINERS.md dev-only maintainer integration; not a self-approval or an independent review).

Co-authored-by credit is retained: this is the contributor's own PR branch.

@lidge-jun
lidge-jun merged commit aed4f1c into lidge-jun:dev Oct 6, 2026
36 of 41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants