Router/2383 policy parser hot reload

[eddierichter-amd]

Summary

Closes #2383.

Implements the Lemonade Router M9 policy-loading slice for collection.router.

This adds a backend-free parser that converts validated collection.router JSON into RoutePolicy, a directory-backed hot-reload store that compiles policies into RoutingPolicyEngine instances and atomically swaps snapshots on file changes, and import/export plumbing so router routing blocks survive
round trips.

What Changed

• Added routing_policy_parser:

  • parses collection.router JSON into RoutePolicy
  • resolves candidates, default_model, route_to, and classifier model references through an injectable component resolver
  • validates dangling classifier refs, unknown route/default components, malformed score bands, unknown keys, and unsupported schema versions
  • adds parser key registries for schema/parser parity tests
  • explicitly rejects routing.router with a clear message, leaving L0a desugaring for [Router] llm router classifier + routing.router desugaring (L0a) #2405

• Added routing_policy_store:

  • loads router policy JSON files from a directory
  • compiles each valid policy into a RoutingPolicyEngine
  • tracks per-file load errors without crashing
  • uses DirectoryWatcher and atomic shared_ptr snapshot swaps for hot reload
  • documents that live server dispatch attachment is owned by [Router] collection.router recipe + dispatch #2385

• Added collection.router metadata support:

  • introduces COLLECTION_ROUTER_MODEL_RECIPE
  • adds generic collection-recipe handling where registration/cache/export needs to treat both collection.omni and collection.router as collection metadata
  • preserves routing in user model registration
  • surfaces routing through /models/{id} for router collections
  • adds routing to CLI and desktop export/import allowlists

• Added focused tests and CI coverage:

  • parser behavior and schema/parser key parity
  • directory watcher reload and engine snapshot swap
  • /pull-style ModelManager::validate_collection_request() checks for router policies
  • registration preserving routing in ModelInfo::extras
  • app-side allowlist/export regression coverage
  • CI builds and runs the new C++ routing policy tests

Notes

routing.router is intentionally not implemented in this PR. The schema recognizes it, but executing it requires the LLM router classifier and desugaring work tracked separately in #2405. For now, parser validation fails early with a clear message instead of accepting a policy that cannot execute.

This PR also does not attach the policy store to live request dispatch. The store is the module-level hot-reload implementation for M9; #2385 owns wiring it into the server request path.

Validation

Ran locally:

cmake --preset default
cmake --build --preset default --target test_routing_policy_parser
cmake --build --preset default --target test_routing_policy_store
cmake --build --preset default --target test_model_manager_collection_validation
ctest --test-dir build --output-on-failure -R "RoutingPolicy(Parser|Store)Test"
ctest --test-dir build --output-on-failure -R "RoutingPolicy(Contract|Evaluator|Registry|Semantic|Deterministic|Engine|Parser|Store)Test|ModelManagerCollectionValidationTest"
cmake --build --preset default --target lemond
python test/test_routing_fixtures.py
python test/test_schema_lock.py
git diff --check

[ramkrishna2910]

Overall: strong, well-scoped, ready to merge after one substantive fix. Clean separation (parser = JSON→RoutePolicy, store = directory→compiled engines), the deferral seams to #2385/#2405 are explicit and correctly fenced, and validation is thorough with path-prefixed error messages. Notably the CI wiring is correct: both the build targets and the ctest filter are updated in both workflow sections. The schema/parser key-parity registries are a nice guard against silent drift.

🔴 /pull validation and store loading disagree on what's a valid policy

ModelManager::validate_collection_request (the /pull path) validates a router policy by calling only parse_route_policy_collection. But reject_catastrophic_regex and the other build-time leaf checks run in the engine constructor (compile_match_expr → build_regex), not in the parser.

So a policy with regex: "(a+)+" passes /pull as valid, then fails to load in the store (caught at make_shared<RoutingPolicyEngine>, recorded as a per-file error → the collection silently never routes). Two code paths, two definitions of "valid."

Cheapest robust fix — in the router branch of validate_collection_request, after parsing, also force a compile:

RoutePolicy policy = parse_route_policy_collection(model_data, options);
RoutingPolicyEngine{std::move(policy), {}};  // throws on catastrophic regex / bad leaf

This makes both paths agree by construction and catches any future build-time leaf rejection, not just regex. Neither the parser test nor the validation test currently covers a catastrophic-regex policy — worth adding alongside.

🟡 Naming trap: is_collection_recipe() now silently means "omni only"

After this PR the predicate reads like "is this any collection" but returns false for collection.router. Eight bare callers remain — some correctly omni-only (server.cpp:2179 chat interception is right to stay omni-only), others are exactly the router-load seams #2385 must revisit (server.cpp:1814 / 3946, hf_variants.cpp:326). A future contributor adding a call will misread it.

Recommend renaming the omni-only predicate to is_omni_collection_recipe so there's no bare "collection" predicate that means omni — symmetric with is_router_collection_recipe, and it turns each remaining call site into a deliberate "yes, omni-only" statement.

🟢 Minor / confirmations

• Store thread-safety is sound: std::atomic_load/store on the shared_ptr member; snapshot swap is atomic; concurrent reloads both produce valid snapshots (last-writer-wins, no corruption). The C++11 free-function atomics are deprecated in C++20 but fine at C++17.
• start_watching has a tiny race: a file change landing between the initial reload() and watcher_->start() is missed until the next change. Dev-convenience path, acceptable — maybe a one-line comment.
• routing.router and classifier type: "llm" are both rejected with clear "reserved for #2405" messages. 👍
• Store test covers reload swap, watcher-triggered swap, and invalid-policy-records-error; parser test builds a real engine and routes it, proving the parse→compile→route chain end-to-end.

Handoff to #2385

This gives exactly the seam #2385 needs: parse_route_policy_collection(json, options) is the RoutePolicy boundary, routing now survives registration in ModelInfo::extras and surfaces via /models/{id}, and the store deliberately stops short of dispatch. Clean handoff — the omni-only load-path sites above are the integration points.

[eddierichter-amd]

After this PR the predicate reads like "is this any collection" but returns false for collection.router. Eight bare callers remain — some correctly omni-only (server.cpp:2179 chat interception is right to stay omni-only), others are exactly the router-load seams #2385 must revisit (server.cpp:1814 / 3946, hf_variants.cpp:326). A future contributor adding a call will misread it.

Recommend renaming the omni-only predicate to is_omni_collection_recipe so there's no bare "collection" predicate that means omni — symmetric with is_router_collection_recipe, and it turns each remaining call site into a deliberate "yes, omni-only" statement.

Thanks, this was spot on.

I pushed a follow-up commit that addresses both points:

• /pull validation now matches store loading semantics. In ModelManager::validate_collection_request(), the router branch now parses the policy and immediately constructs a RoutingPolicyEngine with empty ClassifierServices, so build-time leaf validation runs there too. That catches catastrophic
  regex / future compile-time leaf rejection before registration instead of letting the store fail later.

• Added coverage for that path: ModelManagerCollectionValidationTest now includes a catastrophic-regex policy and verifies it is rejected by /pull-style validation.

• Renamed the C++ omni-only predicate from is_collection_recipe() to is_omni_collection_recipe(), and kept is_model_collection_recipe() as the generic “any collection recipe” helper. I updated the remaining call sites so omni-only paths are explicit, including the chat/MCP/HF-manifest paths you called
  out.

Validation after the follow-up:

ctest --test-dir build --output-on-failure -R "ModelManagerCollectionValidationTest|RoutingPolicy(Parser|Store)Test"
cmake --build --preset default --target lemond
git diff --check

All passed.

[fl0rianr]

Very nice, right before approving.

One small non-blocking hardening suggestion you might want to consider:
RoutingPolicyStore currently keys engines by model_name / file stem and assigns directly into next->engines[model_name]. If two policy JSON files resolve to the same model_name, the later visited file silently wins, and recursive directory iteration order should not define routing behavior. I would consider detecting duplicate policy model names, recording a per-file error, and adding a store regression test.

Also worth noting for the #2385 handoff: since live router dispatch is intentionally out of scope here, an explicit “collection.router dispatch is not wired yet” response in /load / request paths would make the current failure mode clearer than falling through like a regular backend model. I would not block this PR on that, but it is a useful integration seam to keep visible.

[ramkrishna2910]

Looks good to me! I will wait for @SlawomirNowaczyk approval.

[eddierichter-amd]

Very nice, right before approving.

One small non-blocking hardening suggestion you might want to consider: RoutingPolicyStore currently keys engines by model_name / file stem and assigns directly into next->engines[model_name]. If two policy JSON files resolve to the same model_name, the later visited file silently wins, and recursive directory iteration order should not define routing behavior. I would consider detecting duplicate policy model names, recording a per-file error, and adding a store regression test.

Also worth noting for the #2385 handoff: since live router dispatch is intentionally out of scope here, an explicit “collection.router dispatch is not wired yet” response in /load / request paths would make the current failure mode clearer than falling through like a regular backend model. I would not block this PR on that, but it is a useful integration seam to keep visible.

Thanks for the comments. Addressed in aba47b8.

[fl0rianr]

Looks good to me. Thanks for the follow-up fixes.

I re-checked the current head after the earlier review: the /pull validation path now parses the router policy and immediately constructs a RoutingPolicyEngine, so parser validation and engine compile-time leaf validation agree before registration. The C++ collection predicate split also makes the omni-only vs any-collection paths much clearer.

One non-blocking hardening note for later: RoutingPolicyStore currently lets duplicate policy model_names silently overwrite based on directory iteration order. It may be worth detecting that and recording an error in a follow-up. Also, the #2385 dispatch/load wiring should probably make the temporary collection.router runtime failure mode explicit.

No blocker for this PR.

[SlawomirNowaczyk]

Looks good, I just have two nitpicky suggestions