Repository navigation
fix(worker): fall back to thinking field; strip JSON fences before reflection parse (#38) - #39
Open
linhongyu510 wants to merge 1 commit into
Open
linhongyu510 wants to merge 1 commit into
linhongyu510 wants to merge 1 commit into
Conversation
…flection parse (ClaudioDrews#38) Bug 1: ollama_chat only fell back to 'reasoning' when 'response' was empty, but glm-5.x-family models return chain-of-thought under 'thinking' (verified on Ollama Cloud). With a low num_predict every token went to thinking, response stayed empty, and reflection jobs failed even though the model completed fine. Bug 2: reflect_on_memories called json.loads(response) directly. Most current chat models wrap JSON in markdown fences even when asked for raw JSON, so json.loads failed and the reflection was stored as {'raw': ...} - the structured fields (patterns/connections/insights/ actions) were lost and the point degraded to an opaque blob. Extract _extract_response() (response -> reasoning -> thinking) and _strip_json_fences() helpers and use them at both call sites. New pytest module under docker/worker/tests covers both helpers plus a regression test proving a fenced response now lands as structured JSON instead of a raw blob.
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.
Problem
Two related reflection-pipeline bugs reported in #38 (unassigned, no competing PR):
Bug 1 —
thinkingfallback missing.ollama_chat()indocker/worker/services/llm.pyonly fell back to thereasoningresponse key whenresponsewas empty. glm-5.x-family models (e.g.glm-5.3-flashon Ollama Cloud) return their chain-of-thought underthinking. With a reasoning model and a lownum_predict,responsestays empty and every token goes to thinking —ollama_chat()returns""and the reflection job fails even though the model completed fine.Bug 2 — reflection degrades to a raw blob.
reflect_on_memories()indocker/worker/tasks/reflection.pycallsjson.loads(response)directly. Most current chat models wrap JSON in markdown fences even when the prompt asks for raw JSON, sojson.loadsfails on the opening backticks, theJSONDecodeErrorbranch fires, and the point is stored as{"raw": ...}— the structured fields (patterns/connections/insights/actions) are lost.Fix
_extract_response(data)helper:response→reasoning→thinking(each only if non-empty), used byollama_chat()._strip_json_fences(response)helper: strips one markdown code fence (language-tagged or bare, tolerating trailing whitespace) beforejson.loadsinreflect_on_memories().Both are minimal, additive, and match the reporter's suggested fixes.
Tests
New pytest module
docker/worker/tests/test_llm_reflection.py(the worker had no pytest infra — added deliberately small, scoped to the changed code):_extract_response: prefersresponse; falls back toreasoning; falls back tothinking(the llm.py 'thinking' fallback + reflection JSON fence-stripping — reflection points silently degrade to raw blobs #38 case); returns""when nothing useful._strip_json_fences: plain JSON passthrough;```jsonfenced; bare fence + trailing whitespace.reflect_on_memories()with a fenced LLM response (qdrant/embedding mocked) now upserts a point whose payload text containspatterns— before the fix it stored{"raw": ...}.Result: 8 passed (7 unit + 1 regression). Baseline check: with the old code the new tests fail to collect / the regression fails (raw-blob path), confirming red → green.
Notes
python -m py_compile.