feat: 減少三種多餘的 tool call - #23
Conversation
A tool call costs about three seconds of model turn plus a third of a second of network, against a tenth of a millisecond of local work, so the number of calls is the only thing worth optimising. A logged session showed three kinds of call that returned nothing about the code. Searching for a call site is written `foo(`, which is not a valid regex, and `search` handed that back as an error. The model then repeated the search with fixed_strings and got what it asked for. The search is now retried as a literal when the backend rejects it, inside the same call, and searchPatternNote quotes the failure, because results from a literal search read differently. Retried rather than pre-checked: each backend has its own dialect -- ripgrep searches bytes and accepts expressions the regex crate rejects, grep is POSIX ERE -- so the only reliable test of a pattern is whether the backend that runs it accepted it. The note quotes rather than diagnoses because a backend can also fail to start, which this cannot tell apart from a rejected expression. If the literal fails too, the original error is what comes back. An edit told the model how many operations applied but not what the file now says, so it spent calls asking git. CatDesk already diffs the workspace around every tool call; those changes went to _meta for the widget and never to structuredContent, which is what the model reads. They go to both now, counts always and a diff whole-or-not-at-all against a budget, because half a diff reads like a complete one. The schema states what change tracking actually delivers: at most MAX_DIFF_FILES files, with changedFilesAtCap saying when that limit was reached because a full list is otherwise indistinguishable from a complete one; counts over the first MAX_FILE_CAPTURE_BYTES of each file; everything a background job has changed so far rather than only what arrived in the call that reports it; and nothing at all under ShowDetailMode::Disable, which turns tracking off because its own description promises the user "fastest and uses least memory". Every conversation opened by calling catdesk_instruction. The tool description asked for that -- "must call this tool successfully once after CatDesk starts before calling any other CatDesk tool" -- while the gate behind it was one process-wide flag that only ever enforced anything at startup. A canary placed in the discover instructions was quoted back by the model in a conversation whose log shows server/discover and zero tools/call, so in the observed ChatGPT flow that field reaches the model without a CatDesk round trip. The workspace guidance is in it now as well, and the description and the gate are gone, along with the flag, the CATDESK_INSTRUCTION_REQUIRED response and its constants. A discover response that cannot resolve the workspace guidance says so and names the tool that can retry, rather than quietly shipping the base text: the tool reports that failure, and a caller that never saw it would act on partial guidance with no reason to suspect it. The widget keeps rendering instruction_required panels even though nothing produces them now. It draws from results stored in conversations, the schema string did not change, and a payload from before this commit would otherwise fall through to the panel for whichever tool was blocked and render as invalid. Measured against a baseline build over the same four tasks, four conversations each: the instruction call is one per conversation, so its share depends on the length of the conversation -- 14% of a seven-call conversation and 2% of a fifty-three-call one. The literal fallback saves the first call-site search of a session; after one rejection the model starts passing fixed_strings itself. The changed-files report is not claimed to save anything yet. Verification calls fell from ten to seven, which is inside the noise of four tasks, and most of what the model asks git is either orientation before it has edited anything or an audit across a dozen edits, neither of which a per-call report answers. The risk this accepts: a client that connects without reading discover gets no guidance and is no longer told to ask. Only the ChatGPT dialect can reach this server -- a standard MCP client is rejected at the protocol level -- and that client does read it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013buU6PLouJjZRBDwN2HNeX
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (5)
Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review. 📝 WalkthroughWalkthroughThe change removes mandatory instruction calls, serves workspace guidance during discovery, attaches bounded changed-file data to tool results, and adds literal fallback for rejected search patterns. It also preserves legacy widget rendering and updates related tests. ChangesMCP guidance and changed-file results
Search pattern fallback
Estimated code review effort: 4 (Complex) | ~45 minutes Merge Risk: ⚪ Minimal · up to No material regression was identified in the discovery, search fallback, or change-result updates. This is ready to merge. Sequence Diagram(s)sequenceDiagram
participant Client
participant MCPServer
participant Workspace
participant ToolResult
Client->>MCPServer: request server/discover
MCPServer->>Workspace: resolve guidance and AGENTS.md
Workspace-->>MCPServer: return instructions
MCPServer-->>Client: return instructions
Client->>MCPServer: call local tool
MCPServer->>Workspace: execute tool
Workspace-->>MCPServer: return structured result and changes
MCPServer->>ToolResult: attach bounded changed-file data
ToolResult-->>Client: return enriched result
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 56.60% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 53 functions across 4 files. (1 skipped: 1 unsupported.)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 Clippy (1.97.1)Clippy execution failed 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 |
|
OMG Claude 寫的Commit好長喔 |
確實有點太長了xdd 這是個問題 PR 的 body 我是叫他非常精簡了 xd 也覺得是重點,每項都花很多時間去抓瓶頸和實際測試,重啟 catdesk 和 reconnet 還有輸入測試問題蠻花時間的,因為要自動化比較麻煩哈 在實測時有幾個有趣的點可以分享,之前我也有弄個 PR 被 merge,是 batch read,實測確實挺不錯的,會做 batch read 是因為之前測試瓶頸可能比較複雜會是在 api gateway / replica router / schedule or kv cache 啥的,甚至可能會是 GPU,因為catdesk 的特性(不是那些 ai infra,就 mcp 做 gpt chat & local agent 通訊),所以效能分析就比較簡單,通常可能會是網路、IO、探索等問題,實際上網路和 IO 問題最大,這很直覺 所以其實以優先級來說,這 PR 優先級不咋高 |
|
哇 |
不錯哇,不過不確定 CatDesk -> Codex 會是怎樣哈哈,因為 CatDesk 最主要的優勢是用 gpt chat 替代 codex 的 token 限制, 如果還是用 codex 的話那為啥不直接用 codex?如果是 Codex -> CatDesk 的話可能可以當作 subagent 來減少 token 消耗,感覺是不錯的 |
三個少打 tool call 的改動。
說明改在連線時給
server/discover的instructions帶完整說明,含 AGENTS.md。catdesk_instruction的 gate 和「用任何工具前先呼叫這個」的描述拿掉,工具留著供重讀。在說明裡塞一個特徵字串,模型在零
tools/call的對話裡答得出它存在——ChatGPT 確實會把這欄給模型。設定讀不到時 discover 會講。這個錯原本是工具在報的,吞掉的話呼叫端會拿著殘缺說明繼續做事。
search被拒絕時改用字面搜尋搜呼叫點寫出來是
foo(,不是合法 regex,原本回錯誤,模型再帶fixed_strings送一次。重試而不預檢:rg 搜 bytes,接受
regexcrate 拒絕的東西,grep 是 POSIX ERE,只有要跑的那個 backend 能判斷。searchPatternNote引述錯誤而不下判斷,因為 backend 也可能只是起不來。改檔的工具回傳改了什麼
diff 本來每次呼叫都算,只進
_meta給 widget,沒進模型讀的structuredContent。現在兩邊都有。diff 要嘛整份要嘛不給,半份讀起來跟完整的一樣。schema 寫明四個限制:16 檔上限(
changedFilesAtCap說有沒有碰到)、行數只算前 128 KiB、背景指令回的是整個 job 累積的改動、ShowDetailMode::Disable下沒有。數字
同樣四題、各四個對話,對照沒有這些改動的 build。
說明呼叫每個對話 1 → 0。固定成本,7 通呼叫的對話佔 14%,53 通的佔 2%。
字面退回省每個 session 第一次,之後模型自己會帶
fixed_strings。改動回傳沒有數字。git 驗證 10 → 7,四題規模下是雜訊。
相容
多了幾個 output schema 欄位和
instructions,連線不用重加。widget 只多一行註解,revision 沒動;instruction_required的渲染留著,舊對話存的結果還會拿來渲染。不讀 discover 的 client 拿不到說明,也不再被提示去要。目前只有 ChatGPT 方言連得上,而它會讀。
Summary by CodeRabbit
New Features
Bug Fixes