apps/dottie: cap Ollama generations, default qwen3:8b, fix file_ops CRLF on Windows - #66
Merged
Merged
Conversation
OllamaPolicy sent no num_predict, and llama-server runs with --context-shift, so a degenerate generation is bounded only by the server. In the 2026-09-27 local-LLM A/B, granite4.2:3b at T0.2 ran one call for 708 s to 81,920 tokens (done_reason=length). Every /api/chat call now sends options.num_predict, default 2048. New env knobs, each explicit argument > env > default: - DOTTIE_OLLAMA_NUM_PREDICT: default 2048; 0 or negative sends no cap. - DOTTIE_OLLAMA_TEMPERATURE: default stays 0.2; a per-call complete(temperature=...) still wins, so the research stages keep their own temperatures. - DOTTIE_OLLAMA_NUM_CTX: unset, 0 or negative sends nothing (today's request). A value that does not parse raises ValueError naming the variable, so a typo cannot silently run uncapped. DEFAULT_OLLAMA_MODEL and the compose default move from qwen3:32b, which is not pulled on the 4080 box, to qwen3:8b, which the research loop already pins through its env. Docs that named 32b as the default follow. Tests use a fake httpx.post (no network) and cover the default cap on both the CodeAct and complete() paths, env and argument precedence, blank and garbage values, and that the compose default matches the policy default. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014LHC4Z5q8HGJxLCSG3h8Db
On Windows, open('report.txt', 'w') stores every "\n" as "\r\n", so a
model that hashes the bytes it really wrote reports the digest of the
CRLF form, and the verifier only knew the LF digest. In the 2026-09-27
local-LLM A/B, file_ops scored 0/10 on all five arms and none of the 50
finals carried the LF digest.
The verifier now also accepts sha256[:12] of the expected content with
CRLF line endings (VerifiedTask.alt_expected, shown in verifier_detail).
That digest is reachable only from the exact expected lines, so no wrong
content scores. Re-scoring the A/B's 50 file_ops finals with this
verifier reproduces its Amendment 1 row for row: 6, 3, 10, 9 and 6 of 10
(granite 3B T0.2/T1.0, granite 8B T0.2/T1.0, qwen3:8b).
Why the verifier and not newline='' in the sandbox: the sandbox is the
shared factory substrate (apps/ava-factory/dottie/rl/codeact_sandbox.py),
a newline shim there would also change what "w+"/"r+" reads return for
every consumer, and the prompts stay byte-identical this way (digest of
all 5 families x seeds 0-999, prompt + LF expected + tool sources,
unchanged: 907531785ca2).
The no-leakage guard and its test now check every accepted token. New
tests: a CRLF unit test over 15 seeds (fails on the old verifier on any
OS) and a text-mode solver through the real sandbox (fails on the old
verifier on Windows; asserts which digest the FINAL carried per OS).
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014LHC4Z5q8HGJxLCSG3h8Db
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
11 tasks
jcdavis131
added a commit
that referenced
this pull request
Sep 28, 2026
…safe herd kill (#68) * scout-cli: harness llm tier default -> qwen3:8b llm_exec.DEFAULT_OLLAMA_MODEL was qwen2.5:7b-instruct, which is not pulled on this box (`ollama list`: only qwen3:8b and agentos-qwen). Without DOTTIE_LLM_MODEL set, the llm tier's "reachable" check still passed (Ollama is running) but `ollama pull qwen2.5:7b-instruct` was never actually run here, so a bare run of this tier failed at completion time -- the harness's own ROUTER_LABELS_RUNBOOK.md example never worked on this box either. Cam's call (2026-09-27, PR #67 review): move the default to qwen3:8b even though it changes which model produces "scout router probe" labels -- a probe run without DOTTIE_LLM_MODEL set now records backend `ollama:qwen3:8b` instead of `ollama:qwen2.5:7b-instruct`. Updates docs/ROUTER_LABELS_RUNBOOK.md and docs/ARCHITECTURE.md, which both named the old default. Test expectations in test_harness_executors.py move to the new default (the router-label change above, not a loosened assertion), plus a pin test so the default can't silently drift again. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014LHC4Z5q8HGJxLCSG3h8Db * scout-cli: cap every Ollama generation (no output limit today) None of scout-cli's five direct /api/chat|/api/generate callers sent a generation cap: bigbang.core.llm.chat_with_metrics ignored its own max_tokens argument on the ollama branch (it already honored it for koboldcpp), and ollama_chat / _ollama_generate sent no options at all. The fallback direct posts in agent/cli.py, ava/cli.py and write/cli.py (grepped for `/api/chat` and `/api/generate`) had the same gap, and so did `scout ollama run`'s own --num-predict, which sent nothing when the flag was omitted. apps/dottie hit the real failure mode this closes (PR #66): with llama-server's --context-shift, an uncapped call does not error, it just keeps generating — one run went 708s to 81,920 tokens. Adds bigbang.core.llm.resolve_num_predict(): explicit argument > DOTTIE_OLLAMA_NUM_PREDICT env > 2048 default; 0 or negative means no cap. This is the SAME env var apps/dottie's OllamaPolicy reads (PR #66), so one setting caps every local Ollama caller on the box, and a malformed value raises rather than silently running uncapped (mirrored from that same PR). Wired into chat_with_metrics/ollama_chat/_ollama_generate directly; the three duplicate fallback code paths (agent/cli.py, ava/cli.py: only reachable when importing bigbang.core.llm itself has already failed; write/cli.py: its own primary /api/chat call) each get their own small env-read rather than an import from a module that, in the agent/ava cases, is by definition unusable there. `scout ollama run`'s --num-predict now resolves the same way instead of sending no cap on an omitted flag; bigbang/core/ollama.py's complete() is untouched — it is documented as pure options passthrough and has its own test asserting an omitted `options=` sends none, which is correct for that layer. scripts/bench_local_runner.py already passes an explicit --max-tokens through chat_with_metrics on both backends; this fixes an existing asymmetry where koboldcpp respected it and ollama silently did not, so a prior ollama-vs-kobold bench comparison ran ollama uncapped against a capped kobold run. Coverage: resolve_num_predict's precedence/parsing, chat_with_metrics's default/explicit/malformed-env cases (including that a malformed cap fails closed rather than running uncapped), ollama_chat, scout-cli's own _options()/--num-predict CLI path (default, override, zero-means-no-cap, malformed-env-fails-actionably), and write/cli.py's _ollama_chat. The agent/cli.py and ava/cli.py fallback branches are fixed but not exercised by an automated test — they only run when importing bigbang.core.llm has already failed, which does not happen in this repo today, and forcing that import failure reliably under pytest is disproportionate to a defensive fallback. Documents DOTTIE_OLLAMA_NUM_PREDICT in docs/ROUTER_LABELS_RUNBOOK.md (added in the prior commit alongside the qwen3:8b default line, ahead of this one landing it — a doc-only forward reference, not a functional dependency). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014LHC4Z5q8HGJxLCSG3h8Db * scout-cli: make herd close --kill work on Windows close_session's kill path called os.killpg(pid, signal.SIGTERM) unconditionally on every platform, catching only ProcessLookupError. Neither os.killpg nor signal.SIGKILL exist on Windows at all (os.killpg is not a module attribute there; signal.SIGKILL is not defined), so the first line raised AttributeError, uncaught. This was latent until PR #67 made _pid_alive correct on Windows -- before that, alive() could not be trusted enough to route here in the first place. Now it can, so the crash is real: `herd close --kill` against a live session cannot complete on Windows at all. Extracts the existing TERM-then-KILL sequence into _kill_session_posix() unchanged (same exception handling, same 0.2s grace sleep), adds _kill_session_win32() using `taskkill /PID <pid> /T /F` via subprocess with an argument list (never shell=True -- scripts/check_shell_true.py gates that), and dispatches on sys.platform in close_session, the same pattern _pid_alive already uses. /T is load-bearing: the pid on record is start_session's supervisor, not the real command, so a plain `taskkill /PID` would leave the actual work running. /F is the closest single-shot equivalent to SIGKILL -- an ordinary Windows console process has no SIGTERM analogue for an external caller, so there is no honest graceful-then-forceful staging to add on that side; one forceful pass is correct, not a shortcut. Tests: both branches run on whichever platform executes the suite (POSIX tests fake os.killpg/os.kill/signal.SIGKILL with raising=False, since none of the three exist on this Windows dev box either -- the same reasoning TestPidAlive already documents for why CI being ubuntu-only isn't enough), a dispatcher test per platform via sys.platform, and one real end-to-end test: start a real long-running child through the CLI, `herd close --kill --force` it, and confirm the pid is actually gone (not just removed from the ledger). That end-to-end test is a genuine regression check on this box -- it fails against the pre-fix code with the exact AttributeError this commit removes. herd/cli.py's --kill help text and a start_session comment referencing "killpg's the new session group" are updated to name both platforms. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014LHC4Z5q8HGJxLCSG3h8Db * Address review findings Three minor findings from PR #68's review, all confirmed: 1. ollama_chat()'s `cap = resolve_num_predict(num_predict)` sat inside its own broad `except Exception: return None`, so a malformed DOTTIE_OLLAMA_NUM_PREDICT degraded to the same generic None a down server or network error returns, instead of the actionable ValueError resolve_num_predict raises. Moved the resolve call before the client opens and before the try, so the raise reaches the caller. The review's evidence claimed _ollama_generate/chat_with_metrics already contrasted with this by propagating the ValueError into meta['error'] — empirically false (confirmed with a live repro before touching any code): _ollama_generate had the IDENTICAL swallow, so chat_with_metrics reported 'backend returned no completion' for a garbage cap too, not the env var's name. Fixed the same way there, and tightened the existing test_ollama_chat_with_metrics_a_garbage_cap_fails_closed_not_uncapped to assert the message now names DOTTIE_OLLAMA_NUM_PREDICT (it failed against pre-fix code, confirming it tests something real). Added test_ollama_chat_a_garbage_cap_raises_not_silently_none for ollama_chat() itself. Left ava/cli.py's _route_with_ollama untouched: it wraps _core_ollama_chat in its own pre-existing `except Exception: raw = None`, so `scout ava` still degrades to keyword routing on a bad env var rather than surfacing the message — a separate, pre-existing resilience design in that plugin, out of this finding's scope. 2. agent/cli.py's and ava/cli.py's fallback ollama_chat/_route_with_ollama duplicates (reachable only when importing bigbang.core.llm has already failed) shipped in PR #67/#68 with no automated coverage — confirmed via `bigbang.plugins.ava.cli._HAS_CORE_LLM` being True in this repo's real environment, so the branch never ran. Added one test per plugin that forces the import failure with `sys.modules["bigbang.core.llm"] = None` (the documented sys.modules sentinel for ImportError, not a monkeypatch of real internals), reloads the plugin so it takes the fallback branch, and drives the real function with a fake httpx client — restoring and reloading again in `finally` so no other test sees the fallback state. 3. bigbang/core/ollama.py's complete() has no default generation cap of its own — deliberate (it's documented as pure options passthrough with its own test), but nothing said so at the call site. Added a one-line comment. Verified: uv run pytest tests -q from apps/scout-cli — 2686 passed, 1 failed, 2 skipped (243s). The one failure is the pre-existing tests/test_loop_plugin.py::test_evaluate_blocked_on_stale_metrics_exits_2 (fcntl import in packages/dottie-loop, Windows-only, unrelated to this PR). 2686 = the PR's prior 2683 + these 3 new tests. check_documented_counts.py --check: 970 reproduces, unchanged. gate_audit.py --check: no new candidates. goat_audit.py --check --min-mean 7.0: no regressions vs baseline. check_declared_capabilities.py / check_resolver_fallbacks.py / check_shell_true.py / check_cli_path_args.py / store_symmetry_audit.py / dag_next.py / check_todos_timestamps.py / check_handoff_fresh.py: all clean. leaks scan .: clean. State-pollution sweep around the new/changed tests: CLEAN. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014LHC4Z5q8HGJxLCSG3h8Db --------- Co-authored-by: camml210 <camdavis131@gmail.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
This branch was successfully deployed
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.


Two fixes in
apps/dottie, both found by the 2026-09-27 local-LLM A/B (dottie-main-evals/llm-ab-20260927, DEVIATIONS.md follow-ups (a) and (b)). Local models only; no paid API, no key.1. Cap every Ollama generation (
dottie/policy.py)OllamaPolicy._chatsent nonum_predict. llama-server runs with--context-shift, so a degenerate generation is bounded only by the server. In the A/B (Amendment 2, 13:07),granite4.2:3bat T0.2 ran 708 s to 81,920 tokens on task 8,done_reason=length, with repeated context shifts (-c 8192). The research loop runs the same code on CPU, where that runaway would hold the call until its 1800 s read timeout.Every
/api/chatcall now sendsoptions.num_predict. Knobs, each explicit argument > env > default:DOTTIE_OLLAMA_NUM_PREDICTDOTTIE_OLLAMA_TEMPERATUREcomplete(temperature=...)still wins, so ideation/implementation keep their ownDOTTIE_OLLAMA_NUM_CTXA value that doesn't parse raises
ValueErrornaming the variable, so a typo can't quietly run uncapped.Does 2048 clip the live research loop? On the 4080 box's ledger (177 experiments) the longest stored implementation
codeis 4,369 characters, with p99 at 2,990. At the 3 to 4 characters per token typical for code, that's roughly 1.1k to 1.5k tokens (an estimate; I had no tokenizer to count them), so the longest completion fits under 2048. All the A/B arms ran with this cap: 603 calls, and the runaways it stopped were 3 to 5 calls per Granite arm, none for qwen3:8b.Caveat: thinking tokens count against
num_predict. I didn't measure this, since every arm and the live loop runTHINK=false. It follows from the mechanism: the<think>block comes out of the same decode as the answer. WithDOTTIE_OLLAMA_THINKunset, a long qwen3 thought can use up the cap and end the turn inside an unclosed<think>. The research loop runsTHINK=falseand so did the A/B. If you need thinking on for the CodeAct path, raiseDOTTIE_OLLAMA_NUM_PREDICT. This PR doesn't detectdone_reason=length; that's a possible follow-up.DEFAULT_OLLAMA_MODELand the compose default change fromqwen3:32btoqwen3:8b.ollama liston the 4080 box showsqwen3:8band noqwen3:32b, and the research loop already pinsqwen3:8bthroughresearch_env.local.ps1. The README, runbooks, crontab example and status note that named 32b as the default now say 8b.2. file_ops scored the host's newlines, not the content (
dottie/tasks.py)On Windows,
open('report.txt', 'w')stores every\nas\r\n. A model that hashes the bytes it really wrote reports the CRLF digest, and the verifier only knew the LF one. In the A/B, the file_ops family scored 0/10 on all five arms (report.json), and none of the 50 file_ops finals carried the LF digest (lf_hitfalse in every row of report_amend1.json's audit).Fix: the verifier also accepts
sha256[:12]of the same expected content with CRLF line endings (VerifiedTask.alt_expected, exposed inverifier_detail). That digest can only come from the exact expected lines, so wrong content still scores 0.expectedstays the LF digest, so API/harness readers are unchanged.Re-scoring the A/B's 50 file_ops finals with the new verifier reproduces Amendment 1 row for row:
The baseline's overall success rate moves 0.6933 → 0.8133 under that scoring.
Why the verifier and not
newline=''in the sandbox. The verifier's own contract is "proves the derived content, not the write", and the CRLF form is that content. The sandbox is the shared factory substrate (apps/ava-factory/dottie/rl/codeact_sandbox.py). A newline shim there would also change whatw+/r+reads return, for every consumer and not just this family. Rewording the prompt to ask for binary mode would change the task, while this change keeps every prompt byte-identical: a digest of all 5 families × seeds 0–999 (prompt + LF expected + tool sources) is907531785ca2…before and after. The no-leakage guard, and its test, now check every accepted token, not onlyexpected.Tests
New tests, written first and seen failing on the old code:
test_tasks.py::test_file_ops_scores_windows_text_mode_crlf_bytes(15 seeds): the CRLF digest scores 1.0 and CRLF digests of wrong content score 0.0. It fails on the old verifier on any OS (assert 0.0 == 1.0).test_verified_engine.py::test_scripted_file_ops_text_mode_write_scores: a text-mode solver runs through the real sandbox. It fails on the old verifier on Windows (r_task 0.0) and asserts which digest the FINAL carried on each OS.test_policy.py: a fakehttpx.post(no network). Covers the default cap on the CodeAct andcomplete()paths, env and argument precedence for all three knobs, blank and garbage values, and the compose default matching the policy default.What I ran (Windows 11, Python 3.11):
apps/dottie/pyproject.toml[dev]plus numpy, pyyaml, torch 2.14 CPU and ruff 0.15.22 on PATH, withAVA_FACTORY_ROOT=<worktree>/apps/ava-factory.pytest tests: 319 passed, 3 skipped. On origin/main with the same env it was 297 passed, 3 skipped; the 22 new tests are the difference. WithoutAVA_FACTORY_ROOT,test_climb::test_missing_prereqs...fails the same way it does on main.ruff checkon apps/dottie, with its own config: 624 findings before and after. The only one in a touched file is the existing N818 in policy.py.ruff format --checkis clean on the touched .py files.uv sync --all-groups --frozen):pytest packages/ava-open-harnessgives 44 passed, 4 skipped.test_dottie_assistant.pyalone runs the engine and file_ops verifier: 10 passed, 1 skipped (no ava checkpoint).gate_audit --check,check_declared_capabilities --check,dag_next --check,store_symmetry_audit --check,check_todos_timestamps,check_handoff_fresh --check,check_resolver_fallbacks --check,check_cli_path_args --check,check_shell_true --check,check_documented_counts --check, the leaks working-tree scan,python -m factory checkandpython -m harness --help.factory/testshas 2 local-only failures, a Windows process-tree cancel and the RAM/disk preflight on this box. Both are infactory/, which this PR doesn't touch.🤖 Generated with Claude Code
https://claude.ai/code/session_014LHC4Z5q8HGJxLCSG3h8Db