Skip to content

serve: honour conv_route_batch_after from the config, not only the flag - #16

Merged
rwkeyes merged 1 commit into
dirigo/0.2.12-verifiedfrom
fix/conv-route-batch-after
Nov 15, 2026
Merged

rwkeyes merged 1 commit into
dirigo/0.2.12-verifiedfrom
fix/conv-route-batch-after

Conversation

@rwkeyes

@rwkeyes rwkeyes commented Nov 14, 2026 •

Copy link
Copy Markdown
Owner

Problem and result

conv_route_batch_after in the run config was ignored. serve/server.py read a.conv_route_batch_after — the
--conv-route-batch-after flag, default 4 — while bin/strata-b70-serve.sh writes the key into the config JSON and
starts the server with only --engine, --config and --port. A deployment that set 1 or 2 in the config got 4, and
nothing reported the mismatch.

The fix resolves the value in one place: explicit flag → config key → 4, with malformed values falling back
instead of raising. The flag's argparse default becomes None so "absent" is distinguishable from "explicitly 4".

How it was found

A mixed-traffic sweep set conv_route_batch_after in two configs (4 and 1) and got results identical to 0.02 s on all
eleven steps. Passing --conv-route-batch-after 1 by hand reproduced the 2026-11-09 result immediately — the
conversation's second turn re-prefilled at 2.465 s against 0.587 s. So the config key was inert, not the router.

Validation

Passed:

  • python -m pytest serve/test_conv_router.py -q → 12 passed, including the new
    test_the_config_key_is_honoured_when_the_flag_is_absent, which pins: config 2 → 2, config "3" → 3, flag 1 beats
    config 2 → 1, no flag and no key → 4, and 0 / -1 / True / "off" / [1] / {} → 4 rather than an exception.
  • import serve.server in the fix worktree → ok, and resolve_batch_after(None, {"conv_route_batch_after": 2}) → 2
    at the real import path, so the new name resolves where the server uses it.
  • python -m pytest serve/test_server.py -q → 376 passed, 526 subtests passed in 110 s, on the fix branch in a
    throwaway worktree (the default branch's tree was never touched).
  • Source confirms both halves: default=None on the flag, and batch_after=resolve_batch_after(...) at the call site.

Failed:

  • -k "conv_route or batch_after" on serve/test_server.py deselects all 376 tests — the wiring has no test there,
    which is why the key could rot unnoticed. The new test lives with the router, where the decision does. (An earlier
    attempt at the same run returned pytest exit 4 because --timeout is not installed in this venv, and exit 5 from the
    filtered run; neither was a test failure.)

Merge note — do not merge into dirigo/0.2.12-verified

That branch keeps src/, include/, sycl/, metal/, serve/, setup.py and CMakeLists.txt byte-identical to
the deployed snapshot so a re-clone of v0.2.12 plus the carry still reproduces the tree hash (DIRIGO.md). Merging this
would break that check. The deployment is not affected today — its launcher config says 4, which is what it gets — so
this should land with the next tree bump.

The launcher writes "conv_route_batch_after" into the run config and starts the
server without --conv-route-batch-after, while the server read only the flag
(argparse default 4). Every deployment therefore ran 4 whatever the config said.

Measured on 2026-11-14: two mixed-traffic windows whose configs differed only in
that key (4 against 1) produced identical results on all eleven steps - a
conversation's second turn 0.587 s in one arm and 0.555 s in the other, the
single-shot steps matching to 0.02 s. Passing the flag by hand reproduced the
2026-11-09 finding at once (C2 2.465 s, a re-prefill).

resolve_batch_after() in serve/conv_router.py resolves the value: explicit flag,
then the config key, then 4; malformed values fall back rather than raise. The
flag's argparse default becomes None so an absent flag is distinguishable from
an explicit 4. A test pins all four cases.

Not for dirigo/0.2.12-verified: that branch keeps serve/ byte-identical to the
deployed snapshot by design, and the deployment is unaffected today (its config
says 4). Land this with the next tree bump.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant