Repository navigation
fix(generation): generation-path cleanups, prompt quality, Ollama load status, translated history, request ids - #315
Conversation
… constants Delete what nothing in production calls: GenerationResult, the apply_memory_command helper (memory proposals are confirmed through the memories API), the fit_history_to_context and select_history_messages wrappers (fit_history returns both renderings from one walk), _select_history, and _call_with_optional_kwargs (the engine protocol already declares the translate_text keywords). Share what was written twice: one _load_asset with a cache replaces four near-identical loaders; DEFAULT_NUM_CTX names the fallback the service and the engine each spelled as 8192; the llama.cpp-only option keys are exported once and reused for the option log. The flattened transcript is now rendered from answered_exchanges, the same pairing rule the structured turns use. It used to keep a user turn that got no reply and to leave answers unstripped, so the history that sized a turn's attachments could be larger than what the model was sent. ModelSettings.title is kept and documented: stored settings files carry the key and the model rejects unknown ones. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…names its real section The built-in refusal policy told the model to answer hostile messages with a fixed sentence declaring the interaction terminated. A frustrated message reads as hostile to a small model, and once that sentence sits in the history it is pattern-matched on later, harmless turns. Section 2.1 is now decline-and-redirect: illegal acts and serious harm are still declined, briefly and with an offer of what can be done instead, the hostility category and the undefined "false documentation" are gone, and no phrase that ends the chat exists to be repeated. The memory prompt cost about three times what it needed to on every turn with memory on, and described a "[Relevant Memories]" section that nothing builds. It is rewritten to one command spec with one example, one line on clearing, and one line naming the real header and fence. Tests that sized windows around the old prompt sizes now work the windows out from what the prompts cost, so rewording an asset cannot quietly turn a test of trimming into a test of nothing. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The user turn was ordered memory, question, tool observations, documents, images, so the question sat between the memory and up to 32k characters of reference data. Small models attend most to the end of the prompt, which made the instruction to act on the thing most likely to be lost. The reference data now comes first and the question last, under a "## USER QUESTION" header whenever there is data above it to tell it apart from. Alone, the question is still the whole message. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
prompt_eval_count was taken from llama-server's timings.prompt_n, which counts only the tokens evaluated for this request, so a prompt whose start was in the server's cache reported a fraction of its length. That figure is saved with the message and decides whether the "context is full" notice appears, so a cache hit could hide a prompt that had filled the window. It is now usage.prompt_tokens, the length of the whole prompt, with the evaluated count kept as the fallback for a server that sends no usage. prompt_eval_duration still comes from timings.prompt_ms, and the adapter documents that it times the evaluated part alone. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
After Ollama unloads an idle model, the next turn spends ten seconds or more loading it while the UI says "Gathering thoughts...", and Cortex never asked Ollama to keep the model around. OllamaChatClient now takes the status callback the llama.cpp client already had. When a listener is attached it asks Ollama which models are loaded before the request and, if the model is not among them, says "Loading <model> into Ollama..." through the existing loading_model phase. The check is best effort: no listener, no ps, an unreachable Ollama or an unexpected answer reports nothing, and the chat call still runs and reports its own failure. GenerationSettings.keep_alive_minutes (default 5, 0 to leave Ollama's own default alone, -1 to keep the model loaded, at most a day) becomes a keep_alive request field. It rides in the turn's options under one key, is lifted out by the Ollama client, is never seen by llama.cpp, and is carried onto the title and translation calls that run last so they do not reset a longer setting to Ollama's default. There is no control for it in the Settings panel yet. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
A 500 logged only "Cortex API list chats failed (OperationalError)" through the root logger, with no location and nothing to tie it to the response the user saw. Worker-thread job failures were the same. The privacy rule (no prompts in logs) was met by logging almost nothing, which is not the same thing as logging safely. A small ASGI middleware now gives every HTTP request a short id (its own, never the caller's), holds it in a ContextVar for the request's duration and echoes it in X-Request-ID. Repository failures and failed jobs are logged through named loggers with the exception class and the chain of causes as file, line and function frames -- never an exception's text, so a prompt quoted in an error stays out of the log. The frames name locations inside cortex_backend or a dependency and not the directories above them. The id is in the log line and on the record as request_id; the 500 body quotes it so a user can report it; and jobs remember the id of the request that reserved them, so a failure on the worker thread still names the request and the job. The other root-logger calls in routes.py and jobs.py move to the module loggers. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ion is on With translation on, the translation replaced the answer before it was saved, so the next turn showed the model its own earlier answers in the target language. A small model mirrors the language of recent turns and starts answering in it, after which the translation model is asked to translate a language into itself. Titles were made from the translation as well. The chat database gains messages.original_content (schema version 5, an idempotent ALTER in its own transaction like the earlier steps). For an assistant message whose content is a translation, original_content holds the answer as the model wrote it; NULL means content is the model's own text, which is every message that existed before. The generation service returns the untranslated answer as original_response only when a translation replaced it, the API saves both, and the service shows the model original_content wherever a message has one, whether the history came from the API or from the loader. The title is made from the original. The chat response is built from a fixed list of fields, so the client still receives only the translation. Regenerating an answer replaces or clears its original, forks copy it, and a repository is handed original_content only when there is one, so an untranslated turn calls it exactly as before. Tests that pinned the head schema version at 4 now follow DatabaseManager.SCHEMA_VERSION. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
A keep_alive sent on a request overrides the server's OLLAMA_KEEP_ALIVE, so the default of 5 minutes shortened a longer value (30m, -1) that a user had configured, and unloaded their models sooner than before this setting existed. The default is now 0, which sends nothing. 1 to 1440 minutes and -1 are unchanged for anyone who opts in; the README now says so. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…sals An exception no route handled reached Starlette's outermost error handler, which sits outside RequestIdMiddleware, so the caller got a plain-text 500 with no X-Request-ID and nothing to quote. The middleware now catches it first: it logs the class and frames with the request id (never the text), answers a generic JSON 500 that quotes the id, and raises the exception again so the server and the test client see it as before. If the response had already started, only the log line is added. The claim in create_app that the middleware is outermost, so a refusal carries the id, had no test: moved innermost, all the request-id tests still passed. New tests assert the header on a bad-Host 400, an oversize 413 (declared length and chunked) and a CORS preflight; with the middleware innermost those four fail. The README no longer says every response quotes the id: every response the API sends carries the header, and only repository failures and unhandled errors quote it. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…language history Rows written before original_content existed have it NULL, and the model is shown content as stored, so a chat that was already translated keeps showing the model its earlier answers in the target language; only newly translated turns are fed back untranslated. The migration docstring, the history helper and the README said such rows read as untranslated, which is only true of answers that were never translated. Nothing is backfilled. The migration test is renamed to say what it asserts. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…s an answer The HTTP fake had no /api/ps, so a real ollama client asking which models are loaded got a 404 (ResponseError): the model-load message could never appear against it and each turn logged a could-not-ask warning. It now lists the models in FakeOllamaState.loaded_models (none by default, like a freshly started Ollama) in the shape the client parses. Two tests run the real client over a loopback socket against it, with a model loaded and with none; without the endpoint the nothing-loaded case gets no message and logs the warning. The in-process fakes the Playwright and screenshot servers use never call ps, so those runs are unaffected; the fake's /api/chat is not yet accepted by a real client either (its chunks carry no role). Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ps-review # Conflicts: # backend/cortex_backend/api/app.py
|
Review follow-up, pushed as four commits plus merges of 1. keep_alive default (fixed,
|
Main's change #315 already took chat-database schema version 5 (messages.original_content). This branch's threads.revision step is renumbered to version 6 (_migrate_to_v6, SCHEMA_VERSION = 6) and ordered after main's v5. The API error helper keeps main's request-id logging and this branch's 507 mapping for a full disk. The in-memory repository keeps both the empty-means- absent normalisation and original_content. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
1 participant
Problem
Six small reliability and quality defects on the generation path, each verified against current
mainbefore changing anything:Root cause
GenerationResult,apply_memory_command, thefit_history_to_context/select_history_messages/_select_historywrappers,_call_with_optional_kwargs), four copies of an asset loader, and the pairing rule written twice with a deliberate difference (_format_history_messageskept a user turn with no reply and left text unstripped;answered_exchangesdid neither).OllamaChatClienthad no status hook and nokeep_alive._adapt_to_ollama_shapetookprompt_eval_countfromtimings.prompt_n(evaluated tokens only) in preference tousage.prompt_tokens(the whole prompt)._raise_repository_errorand the job runner logged the exception class only, throughloggingrather than a named logger, and nothing carried a request id.Change
Seven commits, one concern each.
refactor(generation)): deletes the dead code above; one_load_asset(filename, *, required)with a dict cache;DEFAULT_NUM_CTX(the settings default) replaces three literal8192fallbacks; the llama.cpp-only option keys are exported once and reused for the option log;fit_historyis the single entry point (the redundant_retained_historywrapper is folded into it). The transcript is now rendered fromanswered_exchanges, so it and the structured turns share one pairing rule;_prepend_history_chunksfollows the same rule.ModelSettings.titleis kept and documented, because stored settings files carry the key and the model rejects unknown ones. Already fixed onmain(not touched): the CLI-only thinking fallback (BE-33) and the memory-proposal flow that madeapply_memory_commanddead (BE-26).fix(prompts)): section 2.1 of the system prompt is decline-and-redirect (hostility category, the undefined "false documentation" and the verbatim phrase are gone; nothing in the prompt announces an ending). The memory prompt is one command spec with one example, one line on clearing and one line naming the real header and fence. By the repo's conservative estimator (3.5 characters per token): system prompt 412 to 241, memory prompt 1366 to 298. Tests that sized windows around the old prompt sizes now compute their windows from what the prompts cost.fix(prompts)): the user turn is memory, observations, documents, images, then the question under## USER QUESTION(bare when there is no data above it).fix(llamacpp)):prompt_eval_countisusage.prompt_tokens, falling back totimings.prompt_nonly when the server sent no usable usage.prompt_eval_durationstill comes fromtimings.prompt_msand the adapter documents that it times the evaluated part only.prompt_token_countis kept (the estimate calibration reads it first because it is never the fallback).feat(ollama)):OllamaChatClient.set_status_callback; before a request, with a listener attached, it asksps()and says "Loading into Ollama..." through the existingloading_modelphase if the model is not loaded.GenerationSettings.keep_alive_minutes(default 0, which sends nothing and leaves Ollama's own keep-alive alone; 1 to 1440 minutes, or -1 to keep the model loaded, for anyone who opts in) becomes akeep_aliverequest field. It travels in the turn's options under one key, is lifted out by the Ollama client, is never read by llama.cpp, and is carried onto the title and translation calls that run last. Contracts regenerated (one optional field).feat(api)):RequestIdMiddleware(outermost) makes a 12-hex id per HTTP request (never the caller's), holds it in aContextVar, and echoes it inX-Request-ID. Repository failures and failed jobs are logged throughcortex_backend.api.routes/cortex_backend.api.jobswith the exception class and the cause chain asfile:line in functionframes (locations trimmed to insidecortex_backendorsite-packages), never the message. The id is in the line and on the record asrequest_id; the 500 body quotes it (Could not list chats. (Request ID: ...));_JobRecord.trace_idis stamped atreserve()so a worker-thread failure names the request and the job. An exception that no route handled is caught by the same middleware: logged with the id (class and frames only), answered with a generic JSON 500 that quotes the id, and raised again, so the server and test client see it as before.fix(chat)): schema version 5 addsmessages.original_content(idempotentALTER, in its own transaction like the earlier steps). For a translated assistant message,contentis what the user reads andoriginal_contentis the answer as the model wrote it.GenerationServiceResult.original_responseis set only when a translation replaced the answer; the API saves both; the service shows the modeloriginal_contentwherever a message has one (API-supplied history and loader history alike); the title is made from the original. The chat response is built from a fixed field list, so the client still receives only the translation. Regeneration replaces or clears the original, forks copy it, and a repository is passedoriginal_contentonly when there is one.Compatibility and rollback
original_contentNULL, and the model is shown such a row as stored. That is right for answers that were never translated. A chat that was translated before this upgrade has no untranslated copy of those answers, so the model keeps seeing them in the target language and only turns translated from now on are fed back untranslated. Nothing is backfilled. The pre-upgrade snapshot (.pre-v4.bak) is taken by the existing ladder machinery; an older build refuses a version-5 file untouched, as designed. Rolling back is the documented snapshot restore.keep_aliveat all, so an existing user's models stay loaded exactly as long as before (Ollama's own default, or theirOLLAMA_KEEP_ALIVE). A value sent on a request overrides that variable, so a non-zero default would have shortened a longer one; that is why it is opt-in. Setting 1 to 1440 or -1 through the settings API makes Cortex send that value on every Ollama request, and it then takes priority overOLLAMA_KEEP_ALIVE.keep_alive_minuteshas a default, so existing settings files load. The settings model rejects unknown keys, so a build older than this one will not accept a settings file this build has saved (it reports the stored settings as invalid). There is no control for the new setting in the Settings panel yet; it is reachable through the settings API.DatabaseManager.SCHEMA_VERSION. Another change that adds a schema step will collide on the version number 5.Checks
Windows. Full runs on Python 3.14.0; the touched test files also on Python 3.12 (that interpreter has no pytest-timeout or gguf installed, so the whole suite was not run on it, and the
timeoutini option warning was ignored). Python 3.10 and 3.13 were not run; CI covers them after merge.main(63cdcf5):python -m pytest -q-> 2531 passed.python -m pytest -q-> 2586 passed (55 new tests).mainat 101cc4b:python -m pytest -q-> 2782 passed, 25 subtests passed, 3 warnings.py -3.12 -m pytest -qon the 13 touched test files (test_prompt_shape, test_fixed_prompt_budget, test_chat_correctness, test_code_prompt, test_services, test_boundary_safety, test_history_selection_equivalence, test_chat_client_routing, test_generation_harness_controls, test_settings_compatibility, test_api_contract, test_chat_groups, test_original_content) -> 786 passed.python -m ruff check backend tests tools main.py app_factory.py scripts-> all checks passed.python -m mypy-> no issues in 97 source files.python tools/generate_contracts.py --check-> up to date (after--write:openapi.json+7 lines,cortex-api.ts+1 line, both the one optionalkeep_alive_minutesfield).python tools/artifact_boundary_review.py --json --strict(persistence was touched) -> passed 12, failed 0, blocked 0.git push: all 12 checks passed in 528.8 s, including pytest, the boundary review, contracts, and the frontendtsc,eslintandvitest.backendandassets, the new tests fail as follows: BE-36 transcript pairing (1 of 1), BE-37 (4 of 5; the fifth, a bare question with no data, is a guard), BE-42 (3 of 3), BE-40 (19 of 20; the llama.cpp-ignores-keep_alive test is a guard), BE-17 (7 of 7), BE-41 (14 of 15; the malformed-original test is a guard). The old tree reproduces the reported log line,ERROR root:routes.py:1428 Cortex API list chats failed (RuntimeError).The rest of BE-36 (dead-code removal, shared constants) is behaviour-preserving, so its tests (asset loader,DEFAULT_NUM_CTXfallback) are guards or cover the new seam, and could not fail before.Security / data-loss / concurrency
RuntimeError("SECRET ...")(and a cause with its own secret) and assert neither text nor any part of it reaches the log, on both the request path and the worker-thread path. The request id is generated in-process; a caller-suppliedX-Request-IDis ignored so a request cannot put text of its choosing into the log.BEGIN IMMEDIATEwith the version bump, like the earlier steps.original_contentis written only for assistant rows.ContextVaris set per request and reset in afinally; sync routes andasyncio.to_threadcopy the context.OllamaChatClientreads its status callback once per call; the callback is cleared at the end of each turn by the existing servicefinally.ps()runs on the shared Ollama client, whose read timeout is 600 s, only when a listener is attached, and Stop is not observed while it is in flight (an Ollama that accepts connections and never answers would also stall the chat call that follows).Limits
X-Request-ID; a response the web server itself sends before a request reaches Cortex (a malformed request) does not. Only repository failures and unhandled errors quote the id in the message; other 500s (a change that did not persist), 404s and 409s carry the header only. An unhandled exception is still re-raised after its 500 is sent, so uvicorn still logs it with its own traceback.keep_alivehas no Settings-panel control;ModelSettings.titleis still in the model (decision recorded above); the Ollama load message is not exercised against a real Ollama here (a stub client stands in). The fake HTTP Ollama now answers/api/psand a realollamaclient is tested against it over loopback, but the fake's/api/chatis still not accepted by a real client (its chunks carry norole). The Playwright and screenshot servers use the in-process fakes, which never callps.contracts/cortex-api.ts); the frontend checks in the pre-push hook passed.tests/test_history_selection_equivalence.pynow callSynthesisAgent._select(whatfit_historyselects with) instead of the removed_select_historywrapper; they compare message selection, not rendering.Review follow-up
Four commits after review (details and exact test names in the PR comment): the
keep_alive_minutesdefault is now 0 so Ollama's own setting is left alone unless opted in; an unhandled exception is answered with a generic 500 that carries the request id, and the README claims about the id are narrowed to what is true; tests now pin that the request id middleware is outermost (bad Host, oversize body, CORS preflight); the BE-41 wording says a chat translated before this upgrade keeps target-language history for its old turns; the fake Ollama app serves/api/psand is tested with a realollamaclient.mainwas merged in (not rebased) three times; the diff againstmainis this pull request's own 33 files.After the last merge (Python 3.14):
python -m pytest -q-> 2929 passed, 25 subtests passed;py -3.12 -m pytest -qon 8 touched files -> 346 passed; ruff clean; mypy no issues in 97 files; contracts up to date; boundary review 12 passed, 0 failed, 0 blocked.Plan item: BE-36, BE-37, BE-40, BE-41, BE-42, BE-17
🤖 Generated with Claude Code