feat(bench): --llm-backend ollama|llama-server for TurboQuant runs - #62
Merged
Conversation
Phase 1 of docs/specs/2026-04-29-qwen36-turboquant-benchmark-design.md.
The qwen3.6-35B-A3B model fits on a 12 GB card only with TurboQuant
KV-cache offload, served by the llama-cpp-turboquant fork's llama-server
on its OpenAI-compatible /v1/chat/completions endpoint. This wires
locomo_runner so a single --llm-backend flag swaps the generator over.
What changed:
- New `_llama_server_generate()` POSTs to `{url}/v1/chat/completions`
with `messages=[{"role":"user","content":prompt}]` and reads
`choices[0].message.content`. Honours `temperature`, `thinking_mode`,
and `no_think_prefix` (the same `/no_think` prefix trick works inside
the llama-server chat template).
- New `_generate(backend, ...)` dispatcher routes to either
`_ollama_generate` (default) or `_llama_server_generate`.
- `_judge` now also goes through the dispatcher, defaulting to ollama
for back-compat. The in-runner self-judge stays on the same backend
as the generator so a llama-server-only model isn't required to also
serve via Ollama just for self-judging.
- `_process_qa` gains `llm_backend` and `llama_server_url` params;
`gen_url` is resolved once at the top of the function (covering both
the retrieval and full-context branches — the original draft put it
inside the else and would have NameError'd on --full-context).
- HyDE call (line ~599) and main answer call (line ~649) now go
through `_generate(llm_backend, client, gen_url, ...)`.
- New CLI args: `--llm-backend {ollama,llama-server}` (default
ollama) and `--llm-server-url` (default http://localhost:8085).
- JSON meta now records `llm_backend` and `llm_server_url` for
reproducibility.
What does NOT change:
- Default behaviour: every existing chain script that doesn't set
`--llm-backend` keeps hitting Ollama exactly as before.
- `_decompose_query` still calls `_ollama_generate` directly — it
uses `gemma4:e2b` as a small utility model, kept on Ollama
regardless of the main generator's backend.
- The external rescore (`locomo_rescore_streaming.py`) is untouched;
the qwen3:4b external judge always runs on Ollama.
Test plan:
- [x] `python3 -c "import ast; ast.parse(open('benchmarks/locomo_runner.py').read())"` — syntax OK
- [x] `pytest tests/ -q` — 128 passing (no regression on the
retrieval/api/rescore test surface)
- [x] `--help` shows the new flags
- [ ] Smoke test: `--limit 5 --llm-backend llama-server --model
Qwen3.6-35B-A3B-UD-Q4_K_M --llm-server-url http://localhost:8085`
once the llama-server build finishes and is running. Will run
on the Fedora host before firing the full bench.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds llama-server as an OpenAI-compatible generation backend and generalizes generation and self-judging to dispatch to either Ollama or llama-server; extends _process_qa, CLI, and run metadata to accept and propagate backend, server URL, and expansion-model options. ChangesLLM Backend Support
Sequence Diagram(s)sequenceDiagram
participant Runner as Locomo Runner
participant Client as httpx.AsyncClient
participant Ollama as Ollama Server
participant LlamaSrv as llama-server
Note over Runner: Request generation (HyDE / final / judge)
Runner->>Client: prepare request
alt llm_backend == "llama-server"
Client->>LlamaSrv: POST {url}/v1/chat/completions (OpenAI format)
LlamaSrv-->>Client: choices[0].message.content
else
Client->>Ollama: Ollama API request
Ollama-->>Client: response
end
Client-->>Runner: generated text
Runner->>Runner: judge uses same dispatch (backend) to score prediction
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Review rate limit: 9/10 reviews remaining, refill in 6 minutes. Comment |
Code Review SummaryStatus: 1 Issues Found | Recommendation: Address before merge Overview
Issue Details (click to expand)SUGGESTION
Files Reviewed (1 files)
Reviewed by grok-code-fast-1:optimized:free · 157,312 tokens |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
benchmarks/locomo_runner.py (1)
647-679:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBackend selection is bypassed for query expansion and multihop decomposition
Line 647 still hardcodes
llm_url=ollama_url, and Line 679 always calls_decompose_query(..., ollama_url, ...). With--llm-backend llama-server, these paths can silently degrade via broadexceptblocks instead of using the selected backend.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@benchmarks/locomo_runner.py` around lines 647 - 679, The code is hardcoding ollama_url for query expansion and multihop decomposition, bypassing the selected backend and silently swallowing failures; update the calls to use the chosen backend URL/identifier (use the existing llm_url or llm_backend variable instead of ollama_url) for expand_query_llm(...) and _decompose_query(...), and change the broad except Exception: pass handlers to at minimum log the exception (e.g., logger.exception or process_logger.error with the exception) so backend failures are visible rather than silently degraded.
🧹 Nitpick comments (1)
benchmarks/locomo_runner.py (1)
217-224: Consider archive instrumentation for the OpenAI-compatible API call (line 217)Per coding guidelines, OpenAI-compatible API calls should wrap archive recording to capture request/response pairs. However, since
benchmarks/locomo_runner.pyis a pure benchmark utility (not production code) and intentionally avoids Archive initialization to isolate test data, this may be optional. If this benchmark will be used to measure production-like scenarios or long-term evaluation runs, archive wrapping would be valuable for audit and replay; otherwise, it can remain uninstrumented.If you decide to add it: initialize Archive alongside VectorMemory in the main setup, pass it through to
_process_qa(), and wrap theclient.post()call at line 217 withawait archive.record()to capture the prompt and model response.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@benchmarks/locomo_runner.py` around lines 217 - 224, The OpenAI-compatible POST at client.post(...) in _process_qa should be wrapped with archive recording: initialize an Archive instance alongside VectorMemory in the main setup, thread that Archive into _process_qa (add an archive parameter), and replace the direct await client.post(...) with an await archive.record(...) call that executes the POST and captures the request payload and response body; ensure the archive.record invocation returns the same resp object/response body so the subsequent resp.raise_for_status(), body parsing, and message extraction (choices/msg/content) keep working unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@benchmarks/locomo_runner.py`:
- Around line 196-224: In _llama_server_generate, the thinking_mode parameter is
ignored; update the payload_prompt construction so thinking_mode overrides
no_think_prefix (i.e., when thinking_mode is True use the raw prompt so the
model can emit CoT, otherwise apply the existing no_think prefix logic), then
continue to build payload and post as before using payload_prompt, message role
handling, and temperature/stream fields.
---
Outside diff comments:
In `@benchmarks/locomo_runner.py`:
- Around line 647-679: The code is hardcoding ollama_url for query expansion and
multihop decomposition, bypassing the selected backend and silently swallowing
failures; update the calls to use the chosen backend URL/identifier (use the
existing llm_url or llm_backend variable instead of ollama_url) for
expand_query_llm(...) and _decompose_query(...), and change the broad except
Exception: pass handlers to at minimum log the exception (e.g., logger.exception
or process_logger.error with the exception) so backend failures are visible
rather than silently degraded.
---
Nitpick comments:
In `@benchmarks/locomo_runner.py`:
- Around line 217-224: The OpenAI-compatible POST at client.post(...) in
_process_qa should be wrapped with archive recording: initialize an Archive
instance alongside VectorMemory in the main setup, thread that Archive into
_process_qa (add an archive parameter), and replace the direct await
client.post(...) with an await archive.record(...) call that executes the POST
and captures the request payload and response body; ensure the archive.record
invocation returns the same resp object/response body so the subsequent
resp.raise_for_status(), body parsing, and message extraction
(choices/msg/content) keep working unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 498eb30a-6845-401c-9e5f-cc104c4effab
📒 Files selected for processing (1)
benchmarks/locomo_runner.py
Comment on lines
+196
to
+224
| async def _llama_server_generate(client: httpx.AsyncClient, url: str, model: str, | ||
| prompt: str, temperature: float = 0.2, | ||
| thinking_mode: bool = False, | ||
| no_think_prefix: bool = False) -> str: | ||
| """OpenAI-compatible generator for llama-server (TurboQuant fork). | ||
|
|
||
| Used when ``--llm-backend llama-server`` is set so Qwen3.6-35B-A3B can | ||
| serve answers via llama.cpp's chat completions endpoint (the model | ||
| doesn't fit in Ollama at this quant on a 12 GB card without TurboQuant | ||
| KV-cache offload). ``thinking_mode`` and ``no_think_prefix`` keep their | ||
| semantics: with ``no_think_prefix`` we prepend ``/no_think`` so the | ||
| chat template can suppress reasoning tokens; with ``thinking_mode`` we | ||
| leave both off and let the generator emit CoT. | ||
| """ | ||
| payload_prompt = ("/no_think\n\n" + prompt) if no_think_prefix else prompt | ||
| payload = { | ||
| "model": model, | ||
| "messages": [{"role": "user", "content": payload_prompt}], | ||
| "temperature": temperature, | ||
| "stream": False, | ||
| } | ||
| resp = await client.post(f"{url}/v1/chat/completions", json=payload) | ||
| resp.raise_for_status() | ||
| body = resp.json() | ||
| choices = body.get("choices") or [] | ||
| if not choices: | ||
| return "" | ||
| msg = choices[0].get("message") or {} | ||
| return (msg.get("content") or "").strip() |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
--thinking-mode has no effect on the llama-server path
thinking_mode is accepted but never used in _llama_server_generate, so behavior does not change between enabled/disabled states for this backend.
Suggested minimal fix
async def _llama_server_generate(client: httpx.AsyncClient, url: str, model: str,
prompt: str, temperature: float = 0.2,
thinking_mode: bool = False,
no_think_prefix: bool = False) -> str:
@@
- payload_prompt = ("/no_think\n\n" + prompt) if no_think_prefix else prompt
+ use_no_think = no_think_prefix or not thinking_mode
+ payload_prompt = ("/no_think\n\n" + prompt) if use_no_think else prompt🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@benchmarks/locomo_runner.py` around lines 196 - 224, In
_llama_server_generate, the thinking_mode parameter is ignored; update the
payload_prompt construction so thinking_mode overrides no_think_prefix (i.e.,
when thinking_mode is True use the raw prompt so the model can emit CoT,
otherwise apply the existing no_think prefix logic), then continue to build
payload and post as before using payload_prompt, message role handling, and
temperature/stream fields.
log failures instead of swallowing them CodeRabbit caught a real issue (rated 🟠 Major) on PR #62: with --llm-backend=llama-server + --llm-query-expansion, the expansion call would hit Ollama with a GGUF-named model the Ollama daemon doesn't have, fail inside `expand_query_llm`, and silently fall through the bare `except: pass` block. The bench would run with no expansion at all and the user would never know — exactly the silent-failure pattern the project's policy memory warns against. Same shape on `_decompose_query`: small utility-model call on Ollama, also silently swallowed exceptions. What changed: - New `--expansion-model` arg defaulting to "" (uses --model for back-compat). Required when --llm-backend=llama-server because the generator's GGUF name is meaningless to Ollama in that mode. - Plumbed `expansion_model` through `_process_qa` and the `_guarded` callsite. Recorded in the JSON meta block. - Replaced both `except Exception: pass` handlers with explicit prints to stderr. The expansion failure log includes the model and URL so a misconfigured backend is obvious. The decompose failure log includes the URL. - expand_query_llm always uses `ollama_url` (because that's the only endpoint shape it speaks); the model is now configurable. What does NOT change: - Default behaviour for existing chains: --expansion-model unset → uses --model → identical Ollama call shape as before. - 128 tests still pass.
Owner
Author
|
Pushed `a53c47c` addressing the major finding. Real bug — confirmed:
Fixes:
Pre-existing chains that don't set the new flag get identical Ollama call shape as before. 128 tests still pass. |
3 tasks
4 tasks
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
1 participant
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.
Summary
Phase 1 of the TurboQuant benchmark spec. Wires `locomo_runner.py` so a single `--llm-backend` flag swaps the generator between Ollama (default, today) and llama-server (TurboQuant fork, for Qwen3.6-35B-A3B with CPU-MoE offload).
The qwen3.6-35B-A3B model only fits on the 3060 12 GB card with TurboQuant KV-cache offload, which lives in the `TheTom/llama-cpp-turboquant` fork — not in Ollama. That fork serves OpenAI-compatible `/v1/chat/completions`; this PR teaches the runner to speak it.
What changed
What does NOT change
Status of the surrounding TurboQuant work
Test plan
Summary by CodeRabbit
New Features
Improvements
Bug Fixes