fix(routing): llm classifier no longer treats has_tools as risk

[Bekhouche]

Summary

The router LLM's context payload includes has_tools/has_images, but the wrapper prompt only said what the fields meant, not how to weigh them, so a small judge (e.g. Qwen3-0.6B) treated "has_tools: true" as evidence the request itself was about a risky action, mislabeling unrelated text like "What's the capital of France?" as RISKY.

Fixed by withholding has_tools/has_images from an author-declared llm classifier's judge payload/prompt entirely — including when a sibling condition composes the deterministic has_tools/has_images leaf in the same rule (as risky-tool-calls-stay-local does): that's the exact #2789 shape, so exposing them there would still leak the signal into the classifier's own judgment. Only the routing.router sugar's synthesized classifier — the sole decision mechanism, with no sibling rule to compose against — still receives them, so a prompt like "use the vision model when has_images" keeps working. This removes the bias source structurally instead of relying on a disclaimer the judge might not obey.

Fixes #2789

Scope

• This PR addresses one clear issue or change.
• I reviewed the full diff myself before submitting.
• I removed unrelated local changes.
• I kept refactoring separate unless it is required for this change.

Testing

• Code builds without errors locally.
• I tested this change locally.
• I described the testing performed below.

Testing details:
Built lemond and the full cpp-ci CTest suite via both WSL/GCC and native MSVC (VS 2026) — all pass. Added a direct #2789 regression test in test/cpp/test_routing_policy_llm_router.cpp using an adversarial fake judge that answers RISKY whenever has_tools leaks into its payload, and asserts the fixed policy still routes to SAFE. Also added tests proving an author-declared classifier never receives the fields (with or without a sibling deterministic leaf) and that the routing.router sugar still always does. tools/check_comment_slop.py passes against main.

Documentation

• Documentation is affected and has been updated.

Updated docs/dev/router-policy.md: the llm classifier note now states plainly that it never receives has_tools/has_images (even when composed alongside the classifier in the same rule), the routing.router section notes it always does, field descriptions were tightened to match the actual contract, and an earlier unsupported "under ~2B" threshold claim was removed.

Breaking Changes

• This PR does not introduce breaking changes.

No schema/API change. Worth flagging for reviewers per the issue's own note: this changes the has_tools/has_images payload an llm classifier's judge sees, which is a behavior nuance for any already-authored policy that relied on the old always-included fields — not a compatibility break, but a routing-output shift worth watching for.

AI-assisted contribution

• I used AI tools for this PR.

• I verified that I understand the changes.

• I checked for hallucinated APIs, unrelated changes, and incorrect assumptions.

[jeremyfowers]

Ready for review

No to-dos.

Explanation

Description vs. the diff: accurate

The description accurately covers the problem (small models misinterpreting has_tools/has_images as risk), the mechanism (disclaiming the fields in the prompt), the scope (one fix plus documentation and a test), and the testing performed. The Fixes #2789 reference is correct — the issue describes the identical bias problem and the PR implements the 'make the wrapper wording neutral' option.

Checked: The PR body describes disclaiming has_tools/has_images as format flags, testing with 57/57 CTest passes, and adding docs/dev/router-policy.md guidance. The diff confirms all three surfaces: routing_policy.cpp (+15/-3), router-policy.md (+9/-0), and test_routing_policy_llm_router.cpp (+3/-0). Issue #2789 describes the identical problem.

Focus: focused

All three changed files serve one goal: fixing the LLM classifier prompt to disclaim has_tools/has_images as format-only, adding documentation guidance for authors, and asserting the new disclaimer text in the test. Removing any part would leave the others incomplete.

Checked: The three changed files all serve the single goal of fixing the classifier prompt: routing_policy.cpp changes the prompt text, router-policy.md adds authoring guidance about the same mechanism, and test_routing_policy_llm_router.cpp asserts the new disclaimer. No unrelated changes are present.

Alignment: none found

Checked: Reviewed contribute.md and philosophy.md; the change is a contained prompt fix with no philosophy violations.

Documentation: adequate

Checked: Reviewed documentation.md; the author updated docs/dev/router-policy.md alongside the code change, which satisfies the same-PR doc rule.

Testing: adequate

Checked: Reviewed testing.md; the PR extends test_classifier_prompt_carries_contract in test/cpp/test_routing_policy_llm_router.cpp (routing-policy suite per the Where Tests Go table: 'Changes the router or routing policies → test/cpp/test_routing_*.cpp'), and cpp-ci tests passed in CI.

Breaking changes: none found

Checked: No user-visible API or schema changes; the prompt rewording is internal to the classifier and does not expose a breaking contract. The streaming filter removal drops internal fields (streams_model_from_storage, min_resident_gb) that were not part of any public API.

Review suggestion

Attention level: Routine — needs any 1 reviewer.

Suggested reviewers:

• ramkrishna2910 (smart router and orchestration) — Maintainer table lists this area for @ramkrishna2910; the PR changes the router's LLM classifier prompt text in routing_policy.cpp.
• eddierichter-amd (routing_policy.cpp (Router/2378 work), not in the maintainer table) — Co-authored routing_policy.cpp (Router/2378 — the original evaluator registry and classifier structure); familiar with the classifier contract this PR modifies.

Explanation

Why this rung

A contained fix or extension. The diff changes the LLM classifier's prompt text in routing_policy.cpp, which is a contained fix to one code path. The documentation addition and test extension are part of the same contained change.

Reviewer search

Checked: Maintainer table lists @ramkrishna2910 under 'smart router and orchestration'; @eddierichter-amd is not in the maintainer table but co-authored the original classifier structure in routing_policy.cpp.

[AI-assisted review] Automated pre-review from repo-manager — flags for the human reviewer, not a replacement for one. Reviewed at head d533c1b; regenerate with repo-manager review-pr 3330.

[SlawomirNowaczyk]

I'm not sure this is the right fix. I see two ways moving forward. Either:

1. Add a behavioral test, not just a substring assertion

The only new test checks that the composed prompt contains the strings "FORMAT ONLY" and "not evidence about":

check("llm: composed prompt disclaims has_tools/has_images as format, not content",
    seen_prompt.find("FORMAT ONLY") != std::string::npos &&
    seen_prompt.find("not evidence about") != std::string::npos);

This proves the words are present, not that the bug is fixed. The actual regression from #2789 is "What's the capital of France?" + has_tools=true being mislabeled RISKY. Add a test with a stub judge that reproduces that input and asserts the route is not the risky label. Right now, nothing verifies the routing outcome, which is the thing users care about.

2. Consider the root cause vs. the band-aid

The PR deliberately keeps has_tools/has_images in the payload and disclaims them in the prompt. But even the issue title itself says the fields leak into the judge prompt and bias small models — and a sub-2B model can still latch onto their mere presence despite a disclaimer (the docs note even acknowledges this). A more robust fix is to make their inclusion in the llm payload opt-in: maybe we should only serialize them when the policy actually composes the deterministic has_tools/has_images leaf. That removes the bias source entirely for pure llm classifiers instead of hoping the judge obeys the disclaimer. At minimum, worth a sentence in the PR on why disclaiming was chosen over omission.

[fl0rianr]

I think the direction is reasonable, but I would not merge this as-is.

The new fixed suffix is too broad: it says has_tools / has_images are “FORMAT ONLY”, must not influence content-based labels by themselves, and then tells the judge to use "text" against the routing criteria. That conflicts with the original routing.router contract, where these fields were deliberately added so the LLM router can distinguish image-only and tool-bearing requests. A router prompt such as “use the vision model when has_images=true” would now be contradicted by the engine-added prompt.

I would narrow the wording to something like: has_tools / has_images are request attributes, not evidence of semantic intent or risk by themselves; use them only when the author's routing criteria explicitly make tool/image presence relevant.

The current test also only checks that the disclaimer strings are present, not that #2789 is actually fixed. Please verify the original case ("What's the capital of France?" + has_tools=true with the affected small judge => SAFE) and add the opposite regression case showing that an explicitly feature-based router can still use has_images / has_tools.

Minor: the descriptions should match the actual contract precisely (has_tools = non-empty tools[], has_images = image content parts), and I would avoid documenting an unsupported “under ~2B” threshold based on one 0.6B reproduction.

[Bekhouche]

@SlawomirNowaczyk @fl0rianr I pushed a fix that addresses both reviews. has_tools and has_images are now completely hidden from the judge’s payload/prompt unless the policy explicitly uses them, or it’s the routing.router case, where they are always included. This removes the bias at the source instead of relying on a disclaimer.

I also simplified the disclaimer so it doesn’t conflict with prompts that intentionally use tool/image information, fixed the field descriptions, removed the unsupported “under ~2B” claim, and added regression tests for both cases (hidden vs. exposed, including the router case).

[fl0rianr]

This is much better and addresses the previous routing.router concern, but I think there is still one correctness blocker.

The new opt-in logic exposes has_tools / has_images to all LLM classifiers whenever any rule in the policy uses one of those deterministic leaves. That is unfortunately exactly the shape of #2789: the risk classifier was already composed with {has_tools: true}, and the bug was caused by feeding that same signal into the judge as well. With the current logic, that policy sets expose_request_features=true, so the judge sees has_tools again and the fix still depends on the model obeying the disclaimer instead of removing the bias source.

It also creates unrelated coupling: e.g. a has_images -> vision-model rule would cause an independent risk/topic LLM classifier to receive both request features.

I think the clean v1 behavior is:

• routing.router: always expose them, because it is the sole routing mechanism and may need modality/tool information.
• normal type: llm classifier: do not expose them; compose deterministic feature leaves in rules.
• if we later need an LLM classifier itself to interpret these fields, make that an explicit per-classifier opt-in rather than inferring it from unrelated rules.

Please also make the regression test mirror #2789 directly: policy with risk classifier AND has_tools:true, and a fake judge that returns RISKY when has_tools is present and SAFE when it is absent. The fixed policy should ensure the judge never receives the field.

Minor: the PR description is now stale because it still says the fields are kept in the payload, and the current head also has a failing Docs And Style/pre-commit check.

[Bekhouche]

@fl0rianr you're right, this was still using the old #2789 shape.

I pushed a fix:

• Removed the sibling-rule scan completely.
• Only routing.router's classifier can now get has_tools / has_images.
• An author-defined classifier will never get them, even if a rule uses them.
• Added the regression test you suggested. The fake judge returns RISKY if has_tools is included, and the test now correctly expects SAFE.
• Fixed the old PR description and the comment-slop failure.

Ready for re-review.

[SlawomirNowaczyk]

Thanks, the main blockers are resolved now. Just a few minor cleanups before I approve:

1. Stale header comment on build_context_payload. The inline note about withholding is great, but the comment block just above it still reads "so the router can see everything a deterministic rule could ... a tool-bearing request as has_tools=true." That's now only true for the routing.router case and misleading for an author-declared classifier where the fields are withheld. Could you reword it to reflect the gated behavior?

2. Dangling "routing criteria above" reference. The non-router suffix ends with "Judge the request using "text" against the routing criteria above," but nothing guards an empty prompt_ (the ctor only validates model). With an empty author prompt the judge is pointed at criteria that aren't there. A small guard (or a note) would close that gap.

3. Substring prompt test. The new behavioral test is the important one — thanks. The older assertion is just re-pinned to "FORMAT, not its content", so it still couples to exact prose... worth loosening or folding into the behavioral test.

[Bekhouche]

@SlawomirNowaczyk fixed both real issues: the old build_context_payload comment now points to effective_prompt for the actual check instead of saying the fields are always visible, and the substring test was removed because the existing behavior tests already cover it. For #2, I checked the constructor and it already throws when prompt_ is empty (if (prompt_.empty()) throw ...), unchanged from before this PR. Ready for re-review.