The tools stop being part of the conversation loop - #162
Open
WaylandYang wants to merge 1 commit into
Open
Conversation
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
Pure refactor, no behaviour change. Groundwork for the MCP server: today the seven tools are seven
matcharms inside the chat SSE handler, 20–70 lines each, closing over the streaming loop's locals. That was fine while chat was the only caller. MCP is the second one, and two implementations ofentity_factsthat drift apart is exactly the failure worth spending a refactor to avoid.chat.rs1244 → 700 lines. Newapi/tools.rsholds the execution; the JSON schemas stay inchat.rs, because those are part of the prompt and belong with the conversation strategy.Shape
The sink exists because citation numbering is stateful: the 3 in
[3]depends on how many sources were already cited this turn, so tools cannot each number independently and merge afterwards — the same chunk would get two numbers. The de-dup-and-number logic, previously copy-pasted betweensearch_chunksandsearch_docs, is now onecite()helper.Three accumulators in the handler (
source_ids,sources,resolved_acc) collapse into oneToolSink.Verifying that nothing changed
There is no LLM configured on the dev deployment, so a live chat round-trip was not available. Two checks instead:
Every string literal, compared. These tools are almost entirely string assembly, so the literals and format templates are the behaviour. Extracted all double-quoted literals from the old match arms and from the new functions, sorted and diffed: 72 on each side, zero added, zero lost.
The one genuine structural change, read line by line.
cite()is the only place where inline code became a helper. It pushes the key, then usessource_ids.len()— same order as the original, so the number assigned is identical.Plus: the seven arms and both guard conditions (
query_dataneeds mounted sources,rememberneeds write) survive indispatch; the seven tests that coveredchanges_windowandchange_linemoved with those functions and pass asapi::tools::tests; fmt, clippy--all-targets -D warnings, and the workspace test suite are green.One thing worth flagging:
truncatewas initially rewritten from memory rather than copied, dropping the leadingtrim(). Caught by comparing against the original before it reached a commit — but it is the reason the literal-by-literal check above exists rather than a glance at the diff.Next
Personal tokens per ADR 0014, then Streamable HTTP as a route on this server.
🤖 Generated with Claude Code