5f7716e121
Address the 11 confirmed bugs and 4 cleanups from context-pruning-and-framing-review.md, plus the image-accounting regression it found on a second pass. classifier framing: - _previous_context walks back to the nearest assistant turn only, skipping system/user/tool so raw tool output and the system prompt never contaminate the framing signal; reuses context_prune.extract_text instead of a duplicated block parser. - classify() drops its redundant if/not-context branch. - framing instruction moved out of the static system prompt and into _classifier_user_content so it travels only with content that justifies it (respects context_framing opt-out). context pruning: - extract_text counts image_url bytes so image-bearing tool results are sized (and pruned) correctly. - -- new, follow-up review -- image_url blocks now count toward the size trigger but not toward the replacement text, and are stubbed to a short marker when the message is trimmed, so an image-heavy tool result genuinely shrinks rather than staying full-size while tokens_saved lied and base64 leaked into the text block. - tokens_saved is now the honest whole-list before/after reduction (max(orig - final, 0)), never a per-message estimate that could drift. - max_summarize_chars validated >= 3000 at config load; trim branch guards against negative math and message growth. - recency guard protects the newest tool result even with no user turn. - non-text blocks survive trimming (type preserved) rather than being flattened away. - dropped/stat terminology corrected to summarized; _turn_of() removed; module-level pinch defaults removed in favor of PinchConfig. Dispatch and config: - prune_context runs once before the measured-context routing decision (reused at dispatch, no double-prune, no in-place mutation); no-op when pinch is disabled. - dead context=prev_context arg removed from the measured reroute. - TaskRequest.context dual use documented. 15 new regression tests pin the fixes; 545 tests pass.
291 lines
10 KiB
Python
291 lines
10 KiB
Python
"""The classifier and the local verifier are separate endpoints.
|
|
|
|
They used to be one: the verifier derived its URL by stripping ``/v1`` off
|
|
``classifier.base_url``. That silently coupled two unrelated decisions, and
|
|
the coupling only becomes visible when the classifier is moved off-host —
|
|
pointing classification at a cloud provider would have sent every local
|
|
verification to ``<provider>/api/chat``, which does not exist.
|
|
|
|
The verifier speaks Ollama's NATIVE API (``/api/chat`` with ``think=False``)
|
|
because that is the only way to disable the reasoning trace, so it cannot
|
|
follow the classifier anywhere the classifier can go.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import copy
|
|
|
|
import pytest
|
|
import yaml
|
|
|
|
from config import RouterConfig
|
|
|
|
|
|
@pytest.fixture
|
|
def raw() -> dict:
|
|
with open("config.yaml") as fh:
|
|
return yaml.safe_load(fh)
|
|
|
|
|
|
def test_moving_the_classifier_to_a_cloud_provider_leaves_the_verifier_local(raw):
|
|
cfg = copy.deepcopy(raw)
|
|
cfg["classifier"]["base_url"] = "https://api.neuralwatt.com/v1"
|
|
cfg["classifier"]["api_key_env"] = "NEURALWATT_API_KEY"
|
|
cfg["classifier"]["model"] = "deepseek-v4-flash"
|
|
|
|
loaded = RouterConfig(**cfg)
|
|
|
|
assert loaded.classifier.base_url == "https://api.neuralwatt.com/v1"
|
|
# The whole point: this did NOT follow the line above.
|
|
assert "localhost" in loaded.verification.base_url
|
|
|
|
|
|
def test_the_verifier_defaults_to_a_local_ollama(raw):
|
|
cfg = copy.deepcopy(raw)
|
|
cfg["verification"].pop("base_url", None)
|
|
assert RouterConfig(**cfg).verification.base_url == "http://localhost:11434"
|
|
|
|
|
|
def test_an_absent_api_key_env_means_unauthenticated(raw):
|
|
# The local Ollama case. It ignores the key entirely, but the SDK requires
|
|
# one to be set, so the dispatcher substitutes a placeholder. Asserted on
|
|
# an explicit config rather than on whatever config.yaml currently says,
|
|
# which is a deployment choice and not a property of the code.
|
|
cfg = copy.deepcopy(raw)
|
|
cfg["classifier"]["base_url"] = "http://localhost:11434/v1"
|
|
cfg["classifier"].pop("api_key_env", None)
|
|
assert RouterConfig(**cfg).classifier.api_key_env is None
|
|
|
|
cfg["classifier"]["api_key_env"] = "NEURALWATT_API_KEY"
|
|
assert RouterConfig(**cfg).classifier.api_key_env == "NEURALWATT_API_KEY"
|
|
|
|
|
|
def test_verification_model_may_be_unset_while_both_run_on_one_host(raw):
|
|
cfg = copy.deepcopy(raw)
|
|
cfg["classifier"]["base_url"] = "http://localhost:11434/v1"
|
|
cfg["verification"]["model"] = None
|
|
loaded = RouterConfig(**cfg)
|
|
# Resolved by the dispatcher as `verification.model or classifier.model`,
|
|
# which is right only because the hosts match.
|
|
assert loaded.verification.model is None
|
|
|
|
|
|
def test_an_unset_verifier_model_is_refused_once_the_hosts_differ(raw):
|
|
# The failure this prevents is SILENT, which is why it is a load-time
|
|
# error rather than a documented caveat. Observed directly: with the
|
|
# classifier on NeuralWatt and this left null, the verifier POSTed
|
|
# `deepseek-v4-flash` to localhost:11434, 404d, caught it, logged "local
|
|
# verification unavailable" and recorded no sample. Verification looked
|
|
# enabled while producing nothing.
|
|
cfg = copy.deepcopy(raw)
|
|
cfg["classifier"]["base_url"] = "https://api.neuralwatt.com/v1"
|
|
cfg["classifier"]["model"] = "deepseek-v4-flash"
|
|
cfg["verification"]["model"] = None
|
|
cfg["verification"]["local_llm_enabled"] = True
|
|
|
|
with pytest.raises(ValueError, match="verification.model must be set"):
|
|
RouterConfig(**cfg)
|
|
|
|
|
|
def test_a_split_host_setup_loads_once_the_verifier_model_is_stated(raw):
|
|
cfg = copy.deepcopy(raw)
|
|
cfg["classifier"]["base_url"] = "https://api.neuralwatt.com/v1"
|
|
cfg["classifier"]["api_key_env"] = "NEURALWATT_API_KEY"
|
|
cfg["classifier"]["model"] = "deepseek-v4-flash"
|
|
cfg["verification"]["model"] = "qwen3.5:latest"
|
|
|
|
loaded = RouterConfig(**cfg)
|
|
assert loaded.classifier.model == "deepseek-v4-flash"
|
|
assert loaded.verification.model == "qwen3.5:latest"
|
|
|
|
|
|
def test_disabling_the_local_check_lifts_the_requirement(raw):
|
|
# A host with no local inference at all: nothing to name, nothing to guard.
|
|
cfg = copy.deepcopy(raw)
|
|
cfg["classifier"]["base_url"] = "https://api.neuralwatt.com/v1"
|
|
cfg["verification"]["model"] = None
|
|
cfg["verification"]["local_llm_enabled"] = False
|
|
|
|
loaded = RouterConfig(**cfg)
|
|
assert loaded.verification.local_llm_enabled is False
|
|
|
|
|
|
def test_a_host_with_no_local_inference_can_turn_the_local_check_off(raw):
|
|
# An RPi has no usable local model. Structural verification is pure Python
|
|
# and keeps running; only the LLM check goes away.
|
|
cfg = copy.deepcopy(raw)
|
|
cfg["verification"]["local_llm_enabled"] = False
|
|
loaded = RouterConfig(**cfg)
|
|
assert loaded.verification.local_llm_enabled is False
|
|
|
|
|
|
# --- an unknown key is an error, not a no-op ------------------------------
|
|
|
|
def test_a_key_in_the_wrong_section_is_rejected(raw):
|
|
# Exactly the mistake that shipped: max_input_chars belongs to the
|
|
# classifier and was written into verification, where pydantic's default
|
|
# extra="ignore" accepted it, dropped it, and left the code default in
|
|
# force. It carried the same value, so nothing looked wrong -- but editing
|
|
# it would have done nothing at all.
|
|
cfg = copy.deepcopy(raw)
|
|
cfg["verification"]["max_input_chars"] = 4000
|
|
with pytest.raises(ValueError, match="max_input_chars"):
|
|
RouterConfig(**cfg)
|
|
|
|
|
|
def test_a_misspelled_key_is_rejected(raw):
|
|
cfg = copy.deepcopy(raw)
|
|
cfg["routing"]["min_tool_proficency"] = 0.5 # sic
|
|
with pytest.raises(ValueError, match="min_tool_proficency"):
|
|
RouterConfig(**cfg)
|
|
|
|
|
|
def test_the_shipped_config_has_no_unknown_keys(raw):
|
|
# Guards the whole file, not just the sections a test happens to name.
|
|
RouterConfig(**raw)
|
|
|
|
|
|
def test_the_tool_filter_can_be_turned_off_from_config(raw):
|
|
# It is a knob to experiment with, so null must be a legal value rather
|
|
# than something requiring a code change.
|
|
cfg = copy.deepcopy(raw)
|
|
cfg["routing"]["min_tool_proficiency"] = None
|
|
assert RouterConfig(**cfg).routing.min_tool_proficiency is None
|
|
|
|
|
|
def test_disabling_the_filter_lifts_the_category_name_check(raw):
|
|
# With no filter there is nothing to join, so an unused category name
|
|
# must not block startup.
|
|
cfg = copy.deepcopy(raw)
|
|
cfg["routing"]["min_tool_proficiency"] = None
|
|
cfg["routing"]["tool_use_category"] = "not_a_real_category"
|
|
assert RouterConfig(**cfg).routing.min_tool_proficiency is None
|
|
|
|
|
|
# --- the CLI sanity check -------------------------------------------------
|
|
|
|
def test_the_config_summary_names_fields_that_exist(raw):
|
|
"""`python config.py` is the documented setup step, and it crashed.
|
|
|
|
It printed cfg.weights, which had been replaced by cfg.objective, so the
|
|
one command whose job is to prove the config is fine reported "Config
|
|
loaded OK" and then died with AttributeError. A function plus this test
|
|
means the names cannot rot silently again.
|
|
"""
|
|
from config import summary_lines
|
|
|
|
lines = summary_lines(RouterConfig(**raw))
|
|
|
|
assert lines[0] == "Config loaded OK"
|
|
body = "\n".join(lines[1:])
|
|
assert "quality_tolerance=" in body
|
|
assert "classifier:" in body
|
|
assert "dispatch providers:" in body
|
|
|
|
|
|
def test_a_removed_key_is_rejected_rather_than_ignored(raw):
|
|
"""log_path named a file nothing ever wrote; leaving it valid would lie."""
|
|
import copy
|
|
|
|
import pytest
|
|
|
|
cfg = copy.deepcopy(raw)
|
|
cfg["logging"]["log_path"] = "router.log"
|
|
|
|
with pytest.raises(Exception, match="log_path|Extra inputs"):
|
|
RouterConfig(**cfg)
|
|
|
|
|
|
# --- capability gates and the local vision fallback ------------------------
|
|
|
|
def test_the_shipped_config_loads_with_the_new_keys(raw):
|
|
loaded = RouterConfig(**raw)
|
|
assert loaded.routing.require_vision is True
|
|
assert loaded.routing.require_json_mode is True
|
|
assert loaded.local_vision.model == "qwen3-vl:4b"
|
|
assert loaded.local_vision.enabled is True
|
|
|
|
|
|
def test_a_typo_in_require_vision_is_rejected(raw):
|
|
cfg = copy.deepcopy(raw)
|
|
cfg["routing"]["require_visoin"] = True # sic
|
|
with pytest.raises(ValueError, match="require_visoin"):
|
|
RouterConfig(**cfg)
|
|
|
|
|
|
def test_local_vision_can_be_disabled_from_config(raw):
|
|
cfg = copy.deepcopy(raw)
|
|
cfg["local_vision"]["enabled"] = False
|
|
assert RouterConfig(**cfg).local_vision.enabled is False
|
|
|
|
|
|
def test_local_vision_defaults_when_absent(raw):
|
|
cfg = copy.deepcopy(raw)
|
|
cfg.pop("local_vision")
|
|
assert RouterConfig(**cfg).local_vision.enabled is True
|
|
|
|
|
|
def test_nonpositive_local_timeout_is_rejected(raw):
|
|
cfg = copy.deepcopy(raw)
|
|
cfg["local_vision"]["timeout_seconds"] = 0
|
|
with pytest.raises(ValueError, match="timeout_seconds"):
|
|
RouterConfig(**cfg)
|
|
|
|
|
|
def test_the_shipped_config_loads_the_pinch_section(raw):
|
|
loaded = RouterConfig(**raw)
|
|
assert loaded.pinch.enabled is False
|
|
assert loaded.pinch.budget_tokens > 0
|
|
assert loaded.pinch.keep_last_turns > 0
|
|
|
|
|
|
def test_pinch_defaults_when_absent(raw):
|
|
cfg = copy.deepcopy(raw)
|
|
cfg.pop("pinch")
|
|
loaded = RouterConfig(**cfg)
|
|
assert loaded.pinch.enabled is False
|
|
assert loaded.pinch.budget_tokens == 50000
|
|
assert loaded.pinch.keep_last_turns == 4
|
|
|
|
|
|
def test_nonpositive_pinch_budget_is_rejected(raw):
|
|
cfg = copy.deepcopy(raw)
|
|
cfg["pinch"]["budget_tokens"] = 0
|
|
with pytest.raises(ValueError, match="budget_tokens"):
|
|
RouterConfig(**cfg)
|
|
|
|
|
|
def test_nonpositive_pinch_keep_last_turns_is_rejected(raw):
|
|
cfg = copy.deepcopy(raw)
|
|
cfg["pinch"]["keep_last_turns"] = 0
|
|
with pytest.raises(ValueError, match="keep_last_turns"):
|
|
RouterConfig(**cfg)
|
|
|
|
|
|
def test_classifier_context_framing_loads(raw):
|
|
# Defaults on (see config.yaml); the legacy layout is opt-out.
|
|
assert RouterConfig(**raw).classifier.context_framing is True
|
|
cfg = copy.deepcopy(raw)
|
|
cfg["classifier"]["context_framing"] = False
|
|
assert RouterConfig(**cfg).classifier.context_framing is False
|
|
|
|
|
|
def test_nonminimum_pinch_max_summarize_chars_is_rejected(raw):
|
|
cfg = copy.deepcopy(raw)
|
|
cfg["pinch"]["max_summarize_chars"] = 100
|
|
with pytest.raises(ValueError, match="max_summarize_chars.*>=.*3000"):
|
|
RouterConfig(**cfg)
|
|
|
|
|
|
def test_pinch_max_summarize_chars_at_valid_min_loads(raw):
|
|
cfg = copy.deepcopy(raw)
|
|
cfg["pinch"]["max_summarize_chars"] = 3000
|
|
loaded = RouterConfig(**cfg)
|
|
assert loaded.pinch.max_summarize_chars == 3000
|
|
|
|
|
|
def test_pinch_max_summarize_chars_accepts_default(raw):
|
|
loaded = RouterConfig(**raw)
|
|
assert loaded.pinch.max_summarize_chars == 4000
|
|
|