From 41133274490ceae5d81825472d3b297d3614f4be Mon Sep 17 00:00:00 2001 From: rinaldofesta Date: Thu, 3 Sep 2026 17:40:35 +0200 Subject: [PATCH] ds4-agent: fix every tool call failing on DeepSeek without --vision Since 4771329 the agent builds every tool observation with ds4_chat_append_multimodal_message, whose guard rejects any model that is not GLM unless a DeepSeek vision encoder is loaded (fc8bf3c relaxed only that case). On a DeepSeek text model the observation never builds; agent_tool_observation_fits reports the failure as "tool result would exceed context", compaction runs, and the turn ends with "context full after compaction" at 1.9k of 262k tokens. A message with zero images is an ordinary chat message: delegate it to ds4_chat_append_message, which keeps the per-family rendering the agent used before 4771329 (DeepSeek: user role plus ; GLM: same tokens as the multimodal branch). The vision guard still applies to messages that carry images. Test: --chat-multimodal-text-only asserts token-identical output between the two entry points for the tool and user roles. It fails on the previous engine (ok == 1) and passes with this change. Validation on a MacBook Pro M5 Max 128 GB, Metal, DeepSeek V4 Flash 0731 IQ2_XXS/Q2_K: ./ds4_test --chat-multimodal-text-only (before: 4 assertion failures, after: OK), ./ds4_test --server OK, ./ds4_test --tool-call-quality OK, ./ds4_agent_test OK, a scripted ds4-agent --non-interactive list tool call OK, make ds4_cpu.o clean. Not run: CUDA, ROCm, GLM, --logprob-vectors. Co-Authored-By: Claude Fable 5.1 --- ds4.c | 8 ++++++++ tests/ds4_test.c | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/ds4.c b/ds4.c index 91ab214ab7..bb50607111 100644 --- a/ds4.c +++ b/ds4.c @@ -64144,6 +64144,14 @@ int ds4_chat_append_multimodal_message( } const bool tool = !strcmp(role, "tool") || !strcmp(role, "function"); const bool user = !strcmp(role, "user"); + /* A message without images is an ordinary chat message. Keep the native + * per-family rendering so a text-only tool observation never depends on + * vision support: DeepSeek text models render under the + * user role, not the observation tags used for image messages below. */ + if (image_count == 0 && (tool || user)) { + ds4_chat_append_message(e, tokens, role, text_parts[0]); + return 1; + } if ((DS4_MODEL_FAMILY != DS4_MODEL_FAMILY_GLM_DSA && e->vision_kind != DS4_VISION_DEEPSEEK4) || (!tool && !user)) { if (error && error_cap) diff --git a/tests/ds4_test.c b/tests/ds4_test.c index cf8bca2c52..c86a2c4fea 100644 --- a/tests/ds4_test.c +++ b/tests/ds4_test.c @@ -145,6 +145,38 @@ static void test_close_engine(bool quality) { *slot = NULL; } +/* A tool observation without images must render exactly like the plain chat + * path: the shortcut in ds4_chat_append_multimodal_message never inspects + * model family or vision_kind, so the guarantee holds regardless of build + * configuration, but this test only exercises it on the default (no vision + * loaded) engine. The agent sends every tool result through the multimodal + * entry point, so a text-only rejection breaks all tool calls on DeepSeek + * text models. */ +static void test_chat_append_multimodal_text_only(void) { + ds4_engine *engine = test_get_engine(false); + if (!engine) return; + + static const char *const roles[] = {"tool", "user"}; + const char *text = ".:\nd 96 src/\n- 1204 package.json\n"; + const char *const parts[] = {text}; + for (size_t r = 0; r < sizeof(roles) / sizeof(roles[0]); r++) { + ds4_tokens plain = {0}; + ds4_tokens multimodal = {0}; + char err[160] = {0}; + + ds4_chat_append_message(engine, &plain, roles[r], text); + int ok = ds4_chat_append_multimodal_message(engine, &multimodal, + roles[r], parts, NULL, 0, + NULL, err, sizeof(err)); + TEST_ASSERT(ok == 1); + TEST_ASSERT(plain.len > 0 && multimodal.len == plain.len && + memcmp(multimodal.v, plain.v, + (size_t)plain.len * sizeof(plain.v[0])) == 0); + ds4_tokens_free(&plain); + ds4_tokens_free(&multimodal); + } +} + static void test_session_snapshot_roundtrip(void) { ds4_engine *engine = test_get_engine(false); if (!engine) return; @@ -6796,6 +6828,7 @@ typedef struct { static const ds4_test_entry test_entries[] = { #ifndef DS4_NO_GPU {"--session-snapshot", "session-snapshot", "session snapshot and recurrent-state round trip", test_session_snapshot_roundtrip}, + {"--chat-multimodal-text-only", "chat-multimodal-text-only", "text-only multimodal message renders like the plain chat path", test_chat_append_multimodal_text_only}, {"--long-context", "long-context", "long-context story fact-recall regression", test_long_story_fact_recall}, {"--tool-call-quality", "tool-call-quality", "model tool call and post-result stop regression", test_tool_call_quality}, {"--think-tool-recovery", "think-tool-recovery", "recover a complete tool call emitted inside unclosed reasoning", test_think_tool_recovery},