Apply remaining improvements from PR #39#49
Apply remaining improvements from PR #39#49MichaelAnders wants to merge 9 commits intoFast-Editor:mainfrom
Conversation
- Add fallback parsing for Ollama models that return tool calls as JSON text in message content instead of using the structured tool_calls field - Return tool results directly to CLI instead of making a follow-up LLM call, reducing latency and preventing hallucinated rewrites of output - Add dedicated Glob tool returning plain text (one path per line) instead of JSON, with workspace_list accepting both 'pattern' and 'patterns' - Clarify why Glob is not aliased to workspace_list (format mismatch)
- Additional logging for tool call parsing and execution - Hard-coded shell commands for reliable tool execution - Deduplication of tool calls within a single response - Collect and return results from all called tools - Ollama uses specified Ollama model - Fix double-serialized JSON parameters from some providers
Tool results now loop back to the model for natural language synthesis instead of being returned raw to the CLI. This fixes the bug where conversational messages (e.g. "hi") triggered tool calls and dumped raw output. Additional improvements: - Context-aware tiered compression that scales with model context window - Empty response detection with retry-then-fallback - _noToolInjection flag to prevent provider-level tool re-injection - Auto-approve external file reads in tool executor - Conversation context search in workspace_search
… responses The heuristic-based stripThinkingBlocks() matched standard markdown bullets (- item, * item) as "thinking block markers" and dropped all subsequent content. Replace with stripThinkTags() that only strips <think>...</think> tags used by models like DeepSeek and Qwen for chain-of-thought reasoning.
The dotenv override (override: NODE_ENV !== "test") requires NODE_ENV=test to prevent .env values from stomping on test-set environment variables.
The Ollama else-if branch was incorrectly nested inside the OpenAI deduplication if-block, causing the OpenAI parsing + dedup code to appear twice. Restructured so Ollama is a peer branch alongside Anthropic and OpenAI/Databricks, each with its own dedup logic. Addresses PR Fast-Editor#39 review feedback from veerareddyvishal144. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add TOPIC_DETECTION_MODEL, MODEL_DEFAULT, Ollama tuning (OLLAMA_TIMEOUT_MS, OLLAMA_KEEP_ALIVE), and LLM Audit logging section to .env.example for discoverability. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@MichaelAnders I am running into a following issue with this pr
|
|
Can you share your env settings? I need to reproduce this locally. |
Rest of it is just default values |
|
That actually seems to be normal. And it may just show up now as - at least for me - Ollama did not work at all with tools before I began with my patches. I added some support to parse GLM and Qwen3, but that is not good yet. And I have been pulling my hair out why it is so difficult to get a unified approach to tool result extraction. Well, I now noticed, I am not the 1st to realize there are issues... vLLM actually has a ton of tool parsers My approach is now to pick one model and then implement code for parsing that models tool response. Once that works I will do the same for the remaining entries vllm already has. No copying of the code but understanding how they did it. If that is not OK due to Apache 2.0 license of vLLM then... ? Therefore, imo, there is no reason not to apply this code change. It's already huge, and I have a lot more in the pipeline already but I will not do any more pushes / PRs until existing ones are in main branch. Tried that before, didn't work, not again, thank you very much ;) Plus there are big PRs from others as well, and adding them one after the other to the main branch would help everyone, imo. And I will also check this closed Ollama issue: "Ollama's native API (/api/chat) has fully supported streaming + tool calling since May 2025" |
|
We can use Apache 2.0 Licensed software all we need to do is to just cite that repository |
I am seeing this with other providers like azure-openai
|
This will be much better once all my 11 (sic!) ready PRs with close to 8,000 (sic!) changed/added code lines waiting to be merged one after the other into main branch - see this as a "wip" situation local models maybe? I will not and cannot not fix this with the issue you see in one small patch - using non-Claude models was, at least for me, not working at all. Every model had some other issues, hopeless. Just a teaser: In the morning I still had "invoke tool(s): grep, grep, bash" responses from Ollama GLM-4.7-cloud. Since 30 minutes ago GLM-4.7 works perfectly: Lynkr just worked for 14 minutes straight with multiple subagents running in parallel:
Other Ollama models are now on the list to be parsed properly, but I am very happy! Therefore - take my PRs, I will supply one after another (prioritized by me), each push applied after you have merged my previous PR and then you'll get a clean, well working one. And I really hope I do not run into many "This branch has conflicts that must be resolved" - especially if I cannot resolve them "cleanly" as I see here. Thank you for your understanding! |
|
Hi — thanks for the detailed explanation and for all the work you’ve put into this. It’s great to hear that things are stabilizing and that the recent results look solid. Given the size and sequencing of the changes (11 PRs / ~8k LOC), let’s integrate this in a controlled way: Please open all related PRs against a single feature branch. I created a feature branch We’ll merge to main once the full sequence is complete and validated. This should help us avoid conflicts, keep main stable, and still respect your intended merge order. Looking forward to reviewing the other PRs. |
|
Makes sense - working on it |
2 participants
Summary
Cherry-picks the 7 remaining commits from PR #39 that weren't already merged separately. This PR contains critical bug fixes and important improvements that were stranded in the original PR after parts were merged as PRs #40, #41, and #42.
Background
PR #39 had 9 commits total, but 3 of them were already merged as:
This PR applies the remaining 7 commits on top of current main, resolving conflicts and ensuring all changes integrate cleanly.
Critical Bug Fixes
1. Fix stripThinkingBlocks() destroying markdown bullet points
src/orchestrator/index.js,src/config/index.js- item,* item) as "thinking markers" and dropped all content after themstripThinkingBlocks()withstripThinkTags()that only strips<think>...</think>tags2. Fix NODE_ENV=test in all test scripts
package.jsonNODE_ENV=test, so.envvalues override test environment variablesNODE_ENV=testprefix to all test scripts3. Fix duplicated OpenAI response parsing block
src/orchestrator/index.jsImportant Features
4. Remove tool_result_direct short-circuit and improve agentic loop
src/providers/context-window.js(144 lines)5. Improve tool calling response handling
src/clients/ollama-utils.jsextractToolCallFromText()for OllamaDocumentation
6. Add missing env vars to .env.example
.env.exampleFiles Changed
17 files changed: **1,266 insertions(+), 153 deletions(-)
New files created:
src/providers/context-window.js(144 lines)src/clients/ollama-utils.js(69 lines)Major changes:
src/orchestrator/index.js(~310 net additions)src/tools/index.js(~150 additions)src/tools/indexer.js(~115 additions)src/context/compression.js(~100 additions)src/api/middleware/logging.js(~80 additions)src/api/router.js(~40 additions)Testing
✅ All 368 unit tests pass
Test verification:
npm run test:unitManual testing:
Compatibility
Next Steps
After this PR is merged, PR #39 can be closed with a reference to this PR.
🤖 Generated with Claude Code