Probe current model's reasoning ladder first in ACP-native discovery - #3008
Open
bradhallett wants to merge 1 commit into
Open
Probe current model's reasoning ladder first in ACP-native discovery#3008bradhallett wants to merge 1 commit into
bradhallett wants to merge 1 commit into
Conversation
…very deadline env-overridable Reasoning discovery probed ACP models serially under a fixed 5s budget, so with large catalogs (e.g. omp's 374 models) the active model was never reached and fell back to the medium-only effort list. Seed the probe order with the model config option's currentValue, ahead of reasoningProbePriorityModelIds, and read the deadline from ACP_NATIVE_REASONING_DISCOVERY_TIMEOUT_MS (default 5s).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When an ACP agent exposes a large model catalog, most models show only a Medium reasoning effort in the model picker, even though the agent advertises a full ladder (low/medium/high/…) per model.
Root cause:
discoverAcpNativeReasoningByModelprobes models serially for theirthought_leveloptions under a fixed 5s budget (ACP_NATIVE_REASONING_DISCOVERY_TIMEOUT_MS = 5_000). With a 374-model catalog (current omp), the probe reaches only the first ~10–50 models before the deadline; every unprobed model falls back to the medium-onlyACP_NATIVE_REASONING_EFFORTSentry. The session's currently selected model is frequently deep in the list (e.g.zai/*sorts last), so the model the user is actually on is the least likely to have real reasoning levels.Change
modelsToProbeis now seeded with the model config option'scurrentValue(already captured atsession/new) before thereasoningProbePriorityModelIdsordering. The active model always gets its real ladder within budget.acpNativeReasoningDiscoveryTimeoutMs()readsACP_NATIVE_REASONING_DISCOVERY_TIMEOUT_MS(default5_000unchanged), mirroring thesessionBusyRetryTimeoutMs()pattern. Operators with small catalogs can raise it; tests can shrink it.Non-current model ordering is unchanged. No behavior change for clients that don't hit the deadline.
Tests
[low, medium, high]ladder while an unprobed neighbor keeps the medium-only fallback.currentValue("default") first — the seeding change legitimately reorders it.FAKE_ACP_INITIAL_MODEL(set the model option'scurrentValue) andFAKE_ACP_SET_CONFIG_MODEL_DELAY_MS(per-probe latency), documented in the fixture header.A follow-up PR will make probing fully lazy (capture reasoning levels from
session/set_config_optionresponses on model switch) so every model a user touches gets real levels without eager discovery.