Conversation
…#11181) Settings → Routing LKGP toggle wrote/persisted but never reached the router: resolveAutoStrategy and applyStrategyOrdering omitted lkgpEnabled on context, so LKGPStrategyImpl's === false guard was dead and pins could not be disabled. Read getSettings().lkgpEnabled (default true) and pass it into selectWithStrategy; skip LKGP reorder in applyStrategyOrdering when false. Unit test covers both toggle states against a seeded pin.
…iegosouzapw#11181) Settings → Routing LKGP toggle wrote/persisted but never reached the router: resolveAutoStrategyOrder built a context literal that omitted lkgpEnabled, so LKGPStrategyImpl's === false guard was dead code. Forward the host-provided settings snapshot (chatCore → targetResolution) into selectWithStrategy. No extra getSettings() round-trip; absent/null keeps prior pin behavior. Explicit-combo applyStrategyOrdering path left untouched (no settings dep today — separate scope). Unit test drives the construction site with deps.settings + a real pin. Changelog fragment for release aggregation.
yaman0786
changed the title
|
@yaman0786 heads up, I think we have independently written the same fix and one of us is going to waste effort. Flagging early rather than letting both sit in the queue. Your #11202 and my #11193 both address #11181, both patch For the record on timing, not as a claim of ownership: #11193 was opened at I am not going to argue for mine over yours. @diegosouzapw, whichever is more useful, please take it and close the other — I genuinely do not mind which, and I would rather one merged fix than two competing ones. One difference worth noting so the decision is informed. The changelog entries differ in a way that matters for traceability:
Looking at the existing entries in @yaman0786 if yours is picked, I am happy to close mine immediately. Just say the word. Separately, and unrelated to which of these wins: both are currently red, and neither is our fault. |
|
Closing as superseded — this exact fix (forwarding lkgpEnabled into the RoutingContext literal in resolveAutoStrategyOrder) shipped today via #11193, including the same deliberate scope boundary (applyStrategyOrdering left untouched). Thank you @yaman0786 — your writeup confirmed the diagnosis independently! |
3 participants
Summary
resolveAutoStrategyOrdernow forwards hostdeps.settings.lkgpEnabledintoRoutingContext(same snapshot chatCore already loaded — no extragetSettings())applyStrategyOrderingleft untouched (no settings dep today; different scope from [BUG] Settings toggle lkgpEnabled never reaches RoutingContext, LKGP cannot be disabled #11181's auto/* path)Test plan
tests/unit/lkgp-enabled-plumbing-11181.test.tssettings=nullkeeps pinlkgpEnabled:false→ rules picks cheap over pinned pricey; log has noLKGP: using last known goodcombo-resolve-auto-strategy-split+combo-apply-strategy-ordering-splitstill passReviewer notes
resolveAutoStrategy.ts)=== false/ optional field so absent/null preserves prior pin behavior