fix: read reasoning_content as fallback for empty content in generate#18
fix: read reasoning_content as fallback for empty content in generate#18Oliverbot26 wants to merge 1 commit into
Conversation
Some models (e.g. GLM-4.5-flash with /no_think) return content as empty string and put the actual response in reasoning_content field. This patch reads reasoning_content as fallback when content is empty.
konradre
left a comment
There was a problem hiding this comment.
This is a real interop gap. GLM-4.5-flash with /no_think does put the answer in reasoning_content and leaves content empty, so reading it back is the right instinct, and the content || reasoning_content || "" order is correct for that model ("0" stays truthy, empty falls through as intended).
My hesitation is that the fallback is unconditional, and the remote generate() it lives in feeds a lot more than query expansion. The same method backs A-MEM note synthesis, the observer, consolidation, entity and intent extraction, and the merge guards, around fifteen call sites in all. Several of them parse the text as structured output: intent extraction does JSON.parse(...) as QueryClause[] at src/intent.ts:325, and A-MEM runs the result through extractJsonFromLLM in src/amem.ts. Query expansion splits the same text on lex: / vec: / hyde:.
The case that worries me: someone runs against a thinking model with CLAWMEM_LLM_NO_THINK=false, or against an endpoint that ignores /no_think and returns its chain of thought in reasoning_content with an empty content. This fallback then hands that raw reasoning to a JSON parser or the expansion splitter. Best case it fails the parse and the caller recovers. Worse case it quietly yields junk expansions or a malformed note. Today, with no-think on, that path returns empty and the caller already handles it, so the fallback widens what can reach those parsers.
I think the fix is to scope the fallback to the case it solves. Either only fall back to reasoning_content when no-think mode is on (where it genuinely is the answer), or put it behind an explicit opt-in for endpoints known to behave this way. Both keep the GLM fix without exposing the structured-output callers to reasoning text.
Some tests would pin it down: one where no-think is on and the answer arrives via reasoning_content, and one where a thinking-style response (populated reasoning_content, empty content) does not leak into a consumer that expects clean output.
The instinct is good. I just want it narrowed before it goes in. Thanks for digging into the GLM behavior.
Problem
Some models (e.g. GLM-4.5-flash with /no_think) return an empty
contentfield and put the actual response inreasoning_content. ClawMem only readscontent, resulting in empty generate results.Change
Reads
reasoning_contentas fallback whencontentis empty ingenerateRemote().Testing
Verified: GLM-4.5-flash now returns actual text via reasoning_content instead of empty string.