feat: per-lane admission model affinity with an explicit model override - #942
Conversation
b7de18d to
b0a7d08
Compare
parseBatchUtilityResponse invalidated the whole chunk's response on any single missing/misindexed entry, falling the entire chunk back to one standalone evaluate() call per candidate. That silently multiplies LLM calls for a chunk that was mostly fine, defeating the call-count guarantee batch mode exists to provide — and is the same evaluateBatch path a future reflection-lane mapped-row admission gate (composing at assembly with the lane/model-affinity work) would hoist onto, so the guarantee needs to hold generically, not just for the extraction lane's own shapes. Now a missing/malformed entry defaults just that row to a neutral 0.5 utility (matching scoreUtility's own failure default) while every well-formed row in the same response keeps its real batch-scored value. The chunk's call count stays exactly one either way; only a call-level failure (completeJson itself throwing) still falls back to per-candidate standalone calls, since that's a materially different failure mode (no response at all, not a partially-malformed one). Added a controller-layer composition test proving evaluateBatch calls stay isolated per AdmissionController instance, so a lane-scoped LLM client (e.g. a future reflection-lane controller under model affinity) never crosses into another lane's batch — the composition point CortexReach#942 will need already holds today with no further plumbing in this file. Red-proved: both new/changed malformed-entry tests failed against the prior code (3 and 5 unwanted standalone calls respectively) before the fix, and pass after.
parseBatchUtilityResponse invalidated the whole chunk's response on any single missing/misindexed entry, falling the entire chunk back to one standalone evaluate() call per candidate. That silently multiplies LLM calls for a chunk that was mostly fine, defeating the call-count guarantee batch mode exists to provide — and is the same evaluateBatch path a future reflection-lane mapped-row admission gate (composing at assembly with the lane/model-affinity work) would hoist onto, so the guarantee needs to hold generically, not just for the extraction lane's own shapes. Now a missing/malformed entry defaults just that row to a neutral 0.5 utility (matching scoreUtility's own failure default) while every well-formed row in the same response keeps its real batch-scored value. The chunk's call count stays exactly one either way; only a call-level failure (completeJson itself throwing) still falls back to per-candidate standalone calls, since that's a materially different failure mode (no response at all, not a partially-malformed one). Added a controller-layer composition test proving evaluateBatch calls stay isolated per AdmissionController instance, so a lane-scoped LLM client (e.g. a future reflection-lane controller under model affinity) never crosses into another lane's batch — the composition point CortexReach#942 will need already holds today with no further plumbing in this file. Red-proved: both new/changed malformed-entry tests failed against the prior code (3 and 5 unwanted standalone calls respectively) before the fix, and pass after.
b0a7d08 to
34ac0cf
Compare
parseBatchUtilityResponse invalidated the whole chunk's response on any single missing/misindexed entry, falling the entire chunk back to one standalone evaluate() call per candidate. That silently multiplies LLM calls for a chunk that was mostly fine, defeating the call-count guarantee batch mode exists to provide — and is the same evaluateBatch path a future reflection-lane mapped-row admission gate (composing at assembly with the lane/model-affinity work) would hoist onto, so the guarantee needs to hold generically, not just for the extraction lane's own shapes. Now a missing/malformed entry defaults just that row to a neutral 0.5 utility (matching scoreUtility's own failure default) while every well-formed row in the same response keeps its real batch-scored value. The chunk's call count stays exactly one either way; only a call-level failure (completeJson itself throwing) still falls back to per-candidate standalone calls, since that's a materially different failure mode (no response at all, not a partially-malformed one). Added a controller-layer composition test proving evaluateBatch calls stay isolated per AdmissionController instance, so a lane-scoped LLM client (e.g. a future reflection-lane controller under model affinity) never crosses into another lane's batch — the composition point CortexReach#942 will need already holds today with no further plumbing in this file. Red-proved: both new/changed malformed-entry tests failed against the prior code (3 and 5 unwanted standalone calls respectively) before the fix, and pass after.
5fee282 to
9ec99d4
Compare
|
Scope restored in db661f7: llm.thinkLevel now ships with this PR. The reasoning-effort knob rides the direct client (OpenRouter-compatible reasoning effort field, sent only when explicitly configured; unset leaves the provider default), and lane affinity carries the full pair: modelAffinity "lane" runs the reflection pipeline on the reflection lane's model AND thinkLevel, "global" keeps every stage on the plugin llm and its thinkLevel. Wire-level regressions cover configured, unconfigured, and blank-value resolution. The host-transport half of the original delta remains in the transport PR (#947), which is where the reasoning field also needs forwarding through the host runtime. |
… direct-client model normalization Recomposed against current master, folding in the standalone-construction seam: one AdmissionController is built independently of SmartExtractor (injected, never constructed inside it) and shared by extraction, the regex fallback, and mapped-reflection gating, preserving the exact availability and audit semantics of the previous fallback path. modelAffinity "lane" routes the mapped-reflection admission judge onto the memoryReflection model (and, with affinity on, its thinkLevel) via a dedicated lane controller; "global" (default, or absent) keeps every lane on the plugin llm model, sharing one controller. An explicit admissionControl.model override governs every admission lane. Every admission-lane model passes through normalizeDirectModelRef so a core-style catalog ref (openrouter/vendor/model) reaches the plugin's direct OpenRouter client provider-stripped; a raw catalog ref 400s at the API. llm.thinkLevel requests reasoning effort on memory LLM calls, sent only when explicitly configured.
87f1417 to
71cce8f
Compare
rwmjhb
left a comment
There was a problem hiding this comment.
Reviewed head 71cce8f. The opt-in reflection-lane model affinity and absolute override preserve the default global behavior, the primary plugin path injects the intended controller, and the targeted tests, full local suite, and all GitHub CI checks pass.
Non-blocking follow-ups: translate memoryReflection.thinkLevel: "off" to the direct API's "none" value and validate wire values; document or support the OAuth transport behavior; preserve llm.thinkLevel through auth login/logout; normalize the primary global direct-client model reference as well as admission clients; keep direct SmartExtractor construction from silently ignoring enabled admission; and move test/llm-thinklevel.test.mjs from CI_TEST_GROUPS into CI_TEST_MANIFEST.
Approved.
What
Adds a per-lane model affinity option to admission control, plus an explicit absolute override, and normalizes model references on the direct-client path so both knobs resolve provider-prefixed and bare model names identically.
admissionControl.modelAffinity: "global" | "lane"(default"global", no behavior change when unset). With"lane", the reflection-mapped admission gate resolves thememoryReflectionmodel instead of the plugin's global LLM model, so the judge auditing reflection-authored rows is never weaker than the author that wrote them. Extraction and fallback admission stay on the global model.admissionControl.model: absolute override. When set, every admission LLM call uses this model regardless of lane.normalizeDirectModelRef(): shared normalization for direct-client model references, used by both the admission model resolution and the host-to-direct fallback path, so one definition decides what a direct client accepts.llm.thinkLevelis threaded through the per-model client factory so lane-resolved clients keep the configured reasoning effort.Why
Reflection-mapped rows are distilled by the (typically stronger)
memoryReflectionmodel, but their admission verdicts previously came from the global memory model, which on cost-tiered setups is the weakest model in the plugin. A weak judge auditing a strong author rejects well-formed durable rows on shallow criteria. Lane affinity fixes the mismatch without forcing a global model upgrade.Notes
modelAffinityunset or"global"resolves exactly as today, pinned by tests (override wins on every lane, one shared controller).