Repository navigation
fix(providers): keep model metadata lookups offline when requested - #909
Conversation
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
Reviewed the full github/main...HEAD diff and the surrounding resolver, candidate-alias, OpenRouter-cache, model-picker, binding, and ACP call paths, plus the relevant history. The new policy gate prevents litellm.get_model_info from running when allow_fetch=False while preserving already-loaded static rows, cached OpenRouter rows, and the existing fetch-enabled default. The optional parameters preserve compatibility, and the tests add coverage without weakening existing assertions.
I also checked the repository rules in AGENTS.md/CLAUDE.md, the domain-term routing in CONTEXT-MAP.md, test naming, source language/comments, and the provider/RPC architecture constraints; I found no violations.
Verification: uv run pytest -n 0 tests/test_provider_rates.py tests/test_rpc_model.py tests/test_acp_server.py tests/test_acp_methods.py tests/test_acp_config_options.py tests/test_agent_loop_lazy_provider.py tests/test_lazy_provider.py tests/test_token_wise_pricing.py tests/test_rpc_session_init_bundle.py -q --tb=short passed (602 tests); uv run ty check raven/providers/rates.py and git diff --check github/main...HEAD also passed. An initial run in the newly created base environment failed during autouse setup because the optional workspace package raven_everos was absent; after the declared dev extra was synced, the three modified suites passed 358/358 and the broader run above passed.
|
No blockers in the reviewed scope. Scope: I reviewed the full PR diff (4 changed files), including tests and documentation, and traced affected callers and contracts at Checked both context-window and output-ceiling entry points, loaded-table aliases, SDK lookup refusal when allow_fetch=False, cached OpenRouter fallback, fetch-enabled compatibility, model-picker and binding callers, lazy providers, and ACP session/config-option paths. The no-fetch flag reaches both LiteLLM import policy and SDK metadata policy. Verification (native Windows, Python 3.12, isolated source snapshots; local JUnit-output options omitted below):
The local combined tree with #908 also passed
|
|
Windows compatibility review: no new issue found in the inspected scope. Reviewed head Native Windows / Python 3.13.12, detached temporary checkout with uv run pytest tests/test_provider_rates.py tests/test_rpc_model.py tests/test_acp_server.py -n 0 -x --tb=short --basetemp "$reviewRunDir/pr909/tests"Result: 358 passed. The ACP frame-loop test uses in-memory streams; it does not validate native inherited stdin or the separate Windows stdin repair. No live provider/Ollama service or full suite was run. This scoped result does not establish full Windows compatibility. |
4173494 to
78ed94e
Compare
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
This revision is a pure rebase onto fa29b2744999: git range-diff maps the prior commit to 78ed94ee153e with no patch change, and the provider final comparison remains the same four files. I rechecked the current final diff and the new-base interactions against the repository rules, affected callers/history, compatibility, test-strength, and architecture constraints; the rebase introduces no new issue.
Exact-head CI is fully green, including all four Python unit shards, coverage gates, pre-commit, Python lint/type/import contracts, UI RPC, and platform jobs. git diff --check github/main...HEAD also passes.
Patch-risk assessment: merge / human_review_required; impact if wrong is moderate, regression likelihood low, protection strong, recovery easy, and confidence high. The assessed diff is base fa29b274499968fbad02c74d31f3eee85c5dd2ef to head 78ed94ee153e9346f2803d69dc4288e9f911b3b4, SHA-256 c7fe3a6fe9e06543f95d0fffe1de4b6ba90bc9cf9061382cef45a6092da9dd9d.
|
Windows compatibility re-review: no issue found in the inspected scope. Reviewed the complete 4-file diff at Code inspection: both context-window and output-ceiling lookups propagate Native Windows / Python 3.13.12, isolated detached checkout after uv run pytest tests/test_provider_rates.py tests/test_rpc_model.py tests/test_acp_server.py -n 0 -x --tb=short --basetemp 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\b89bdffc4a4845fb988b4088f488acef/pr909/unit-pytest'358 passed, including the uncached-model/EOF response regression. Limits: this PR's ACP frame-loop test uses in-memory streams; native inherited-stdin and launcher behavior belong to the separately reviewed stacked #908 revision. No live provider/Ollama service, LLM task or full repository suite was run. |
78ed94e to
38ebfd8
Compare
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
This revision rebases the same four-file code patch onto 829eb28e1db8. git range-diff shows no source or test change from the previously reviewed revision; the only commit-level delta is removal of the Codex co-author trailer, which does not conflict with the repository's Claude-specific attribution rule. I rechecked the current final diff, new-base context, repository rules, callers/history, compatibility, test strength, and architecture constraints.
Exact-head verification passed locally: the focused provider/RPC/ACP regression set now has 603 passing tests, uv run ty check raven/providers/rates.py passes, and git diff --check github/main...HEAD passes. Current exact-head CI is also green for Python lint, commit/repository checks, UI RPC, page/bridge, and Windows jobs; remaining Linux jobs were queued or running when reviewed.
|
Windows compatibility re-review: no new issue found in the inspected scope. Reviewed the complete 4-file PR diff at Git checks: source tree identical to previously reviewed Native Windows / Python 3.13.12, detached checkout, PR head: uv run pytest 'tests/test_provider_rates.py' 'tests/test_rpc_model.py' 'tests/test_acp_server.py' -n 0 -x --tb=short --basetemp 'C:\Users\livxu\AppData\Local\Temp\raven-windows-review\fa39f19b3fcb4d7daa4ad3b9472340f8\pr909\unit-pytest'
# 358 passed in 28.53s; exit 0Limits: ACP frame-loop coverage here uses in-memory streams. No live provider/Ollama service or full repository suite was run; the real Windows stdin/launcher path is tested separately on stacked #908. This scoped result does not establish full Windows compatibility. |
Honor allow_fetch=False for context-window and output-ceiling metadata, including a loaded LiteLLM whose SDK can query provider endpoints. Keep cached rows and explicitly allowed SDK lookups available. Cover cache misses, aliases, model options, and ACP session creation at EOF.
…kups allow_fetch=False skipped get_model_info for every candidate. LiteLLM's table files most direct vendors under the bare model name, so an id such as openai/gpt-4o-mini or vertex_ai/gemini-2.5-pro is found only once get_model_info strips the routing prefix, which it does offline. The cache-only lookup fell back to the default window or to the openrouter/ alias row and its OpenRouter numbers: 203 of 413 measured ids changed from the base. Only the Ollama drivers answer a table miss by asking the serving daemon (POST /api/show), which is the request that delayed session/new. Skip get_model_info for those under allow_fetch=False and keep it for every other driver. huggingface stays skipped under both policies.
38ebfd8 to
4ba33b8
Compare
|
One correction for the record. My summary above (#909 (comment)) said there were no blockers, and it missed one. 4ba33b8 keeps that lookup and skips |
gloryfromca
left a comment
gloryfromca
left a comment
There was a problem hiding this comment.
No blockers; this can merge as far as I am concerned.
The new revision addresses the reported regression: cache-only lookups still use LiteLLM's offline prefix normalization for direct-vendor Model Refs, while table misses for Ollama and rewritten Ollama candidates do not reach the daemon. I also checked LiteLLM's installed provider implementations; Ollama is the model-info path that performs an endpoint request on a table miss.
Review scope: the complete four-file diff and the new fix delta, repository rules and domain constraints, affected context-window/output-ceiling callers, commit history, backward compatibility, test strength, and the offline metadata architecture. The new tests replace the overbroad "never call LiteLLM" assertion with direct-vendor parity and real Ollama-driver controls, so coverage was strengthened rather than weakened.
Verification: uv run pytest -n 0 tests/test_provider_rates.py tests/test_rpc_model.py tests/test_acp_server.py tests/test_acp_methods.py tests/test_acp_config_options.py tests/test_agent_loop_lazy_provider.py tests/test_lazy_provider.py tests/test_token_wise_pricing.py tests/test_rpc_session_init_bundle.py -q --tb=short (609 passed); uv run ty check raven/providers/rates.py (passed); git diff --check github/main...HEAD (passed). GitHub CI was otherwise green with two unit shards still running when this review was posted.
|
Windows compatibility re-review: no new issue found in the inspected scope. Scope: the complete 4-file PR diff at head Native Windows, Python 3.13.12, isolated detached checkout and user/Raven/temp directories. Dependencies were synced with uv run pytest tests/test_provider_rates.py tests/test_rpc_model.py tests/test_acp_server.py tests/test_acp_methods.py tests/test_acp_config_options.py tests/test_agent_loop_lazy_provider.py tests/test_lazy_provider.py tests/test_token_wise_pricing.py tests/test_rpc_session_init_bundle.py -n 0 -x --tb=short --basetemp "$R/checks/909-metadata/pytest" --junitxml "$R/checks/909-metadata/junit.xml"
# 609 passed; exit 0.The direct-vendor parity and Ollama-driver regressions pass, including the in-memory EOF/session-new regression. No live provider/Ollama daemon or full repository suite was exercised. Native inherited stdin and launcher behavior were checked separately on stacked #908; those results do not make this PR alone a full Windows compatibility pass. |
|
Not a blocker: re-acceptance at head 4ba33b8 merged onto main 182daca found no blocking issue, and the earlier regression is fixed. Hosted ids give the same window and output ceiling as main under Severity of this one finding, not a verdict on the pull request. The blocking findings from this pass are review threads on the changed files; GitHub renders those collapsed, as a file name with no text. Two narrower regressions remain. Both are non-blocking: they hit Ollama ids spelled in ways Raven's own writers do not produce. 1. The any-segment gate also skips ids that LiteLLM answers from its table.
2. Legacy-spelled Ollama ids no longer get the daemon's window inside a turn. I ran a fake daemon in-process on 127.0.0.1 that answers
That request is the one this PR sets out to stop: The spelling Raven writes, How I measured: every socket blocked except the fake daemon, LiteLLM 1.85.0 imported, an empty RAVEN_HOME, the same probe on both trees. |
|
Both items from the review are fixed in #940. 1. The cache-only gate skipped ids LiteLLM answers from its table. Fixed in 2. The window the daemon reported was thrown away inside the turn. Fixed in Measured against a stand-in daemon on a loopback port: one Two shapes the first draft of that fix got wrong, both caught in the pre-push sweep:
The output ceiling is deliberately not stored beside the window: LiteLLM's driver writes one number into both fields, so a stored ceiling always equals the stored window and tests/test_provider_rates.py: 110 passed. |
## Summary Bundled ACP agents could fail to start on Windows when rendering configuration, reading stdin, or replacing the launcher process. Closing an ACP connection could also fail before releasing its process and transport because Windows has no os.killpg. - Keep all five bundled Windows launchers alive until their ACP child exits, forward the exit status, and apply the same behavior to new scaffolded launchers. POSIX retains exec. - Preserve live rendered configurations with the cross-platform PID probe, retain POSIX file permissions, and preserve seed-file line endings. - Read Windows stdin pipes through a daemon feeder with PeekNamedPipe and reads bounded by the available bytes. - End the live launcher's Windows process tree with bounded taskkill before releasing the transport. Leave an already-exited launcher's PID alone. - Preserve a completed session/new reply when EOF shutdown cancels its optional model-menu lookup. The first LiteLLM import can exceed the five-second grace even after #909 removed the metadata-network delay. Active-connection cancellation and cancellation during required session setup still propagate. - Run real stdio handshakes, all five launchers, command registration, and native teardown cases in Windows CI. The handshake now also runs with a fresh bytecode cache. ## Type - [x] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other ## Verification Native Windows build 10.0.26300, Python 3.13.12, PYTHONUTF8=1. The final tree is based on the fetched main revision; all eight patches replayed unchanged. The last upstream update adds bug-report upload/receiver code and has no file overlap with this PR. The 19-case selection below was rerun on the final tree; the broader selection and controls predate that last unrelated update. Tests used isolated user/Raven homes, frozen dependencies, and verified imports from the tested checkout. Run-owned --basetemp and --junitxml output paths are omitted from the selections below. - `uv run --frozen --no-sync pytest tests/test_acp_server.py::TestFrameLoop::test_session_creation_answers_when_eof_cancels_the_catalogue tests/test_acp_methods.py::TestConfigOptions::test_session_creation_preserves_cancellation_outside_shutdown_metadata tests/integration/test_acp_stdio_smoke.py tests/test_cli_agents_commands.py::test_register_pins_the_resolved_row_into_the_host_roster tests/test_subagent_acp.py::test_close_without_killpg_ends_the_tree_through_taskkill tests/test_subagent_acp.py::test_close_without_killpg_kills_nothing_by_the_pid_of_an_exited_launcher -m 'integration or not integration' -n 0 --tb=short`: **19 passed, no skips** after the final rebase. This includes both EOF timings, cancellation controls, both bytecode-cache states, all five real launchers, and real taskkill. One non-failing idle-ceiling warning was reported. - `uv run --frozen --no-sync pytest tests/test_acp_server.py tests/test_acp_methods.py tests/test_acp_updates.py tests/test_acp_config_options.py tests/test_cli_acp_commands.py tests/test_config_product_render.py tests/test_utils_pid.py tests/test_agents_code_launcher.py tests/test_agents_design_launcher.py tests/test_agents_oncall_launcher.py tests/test_agents_ppt_launcher.py tests/test_agents_research_launcher.py tests/test_cli_agents_commands.py tests/test_subagent_acp.py tests/test_rpc_bootstrap.py tests/test_subagent_host_env.py tests/test_subagent_probe.py -n 0 --tb=short`: **1284 passed, 58 failed, 10 skipped, 4 errors**. The same selection on the rebased PR immediately before the EOF fix returned **1280 passed, the identical 58 failing and 4 error test IDs, and 10 skipped**. No new failure/error IDs from the EOF fix. - All 62 failure/error IDs were also selected on main: 56 failed, 4 errored, and 2 passed. Those two passed in isolation on this PR too, and failed in the complete selection both before and after the EOF fix. The remaining failures include Windows path, shell/command, and symlink-privilege cases. The attempted complete main selection stalled in the pre-existing Windows stdin tests and was terminated; no complete main-suite result is claimed. - Before the EOF fix, `uv run --frozen --no-sync pytest tests/integration/test_acp_stdio_smoke.py::test_a_whole_handshake_runs_in_one_process[cold_imports] -m integration -n 0 --tb=short` failed with response IDs [1, 3]. A separate stack-capture probe measured 6.76 seconds in the first LiteLLM import; the handler awaited model.options while its worker imported LiteLLM and its dependencies. - `uv run --frozen --no-sync pytest tests/test_acp_server.py::TestFrameLoop::test_session_creation_answers_when_eof_cancels_the_catalogue tests/test_acp_methods.py::TestConfigOptions::test_session_creation_preserves_cancellation_outside_shutdown_metadata -n 0 --tb=short`: before the fix, 2 failed and 2 passed. Both EOF cases failed with the missing session reply. They pass with the fix and are included in the broader post-rebase selection above. Warm/cold real handshakes plus these four cases also passed as a six-case selection. - A mutant that treats every connection as closing makes the ordinary-cancellation control fail. Cancellation during required session setup still propagates. - `uv run --frozen --no-sync ruff check raven evolver agents plugins-dist tests scripts docs-site` and the corresponding `ruff format --check`: passed; 2266 files already formatted. - `uv run --frozen --no-sync lint-imports`: 10 contracts kept. `uv run --frozen --no-sync ty check raven/acp/methods.py raven/acp/updates.py raven/acp_client/client.py raven/cli/acp_commands.py raven/config/product_render.py`: passed. - `uv run --frozen --no-sync pre-commit run --from-ref origin/main --to-ref HEAD`: every applicable hook passed. The two final docstring edits also passed their file-scoped pre-commit run. - `node node_modules/@commitlint/cli/cli.js --from origin/main --to HEAD --config commitlint.config.cjs`, `uv run --frozen --no-sync python scripts/check_commit_messages.py origin/main..HEAD`, `uv run --frozen --no-sync python scripts/check_large_files.py origin/main...HEAD`, `uv run --frozen --no-sync python scripts/check_source_language.py origin/main...HEAD`, and `git diff --check origin/main...HEAD`: passed. - The PR body passed the repository body checker and an explicit ASCII scan. The broader Windows selection remains red, so the relevant-tests checkbox is unchecked. This revision has no new Linux or full repository run, live LLM workload, rendering workload, or ACL audit. POSIX-only skips are not counted as Windows passes. - [ ] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [x] User-facing docs or screenshots are updated when needed ## Risk - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes At EOF, a completed session/new can omit optional configOptions when shutdown cancels a slow lookup; the sessionId and session modes are retained. Active connections keep their existing model-menu behavior. The handler grace remains five seconds. Windows keeps the launcher present while its server runs, and connection close waits up to five seconds for taskkill. Idle Windows pipe input polls every 10 ms. Redirected files and text streams retain their existing reader path. POSIX retains exec and 0600 permissions; Windows rendered files inherit directory ACLs. There is no configuration-format change. Previously generated agent folders retain their existing launchers; the scaffold change applies to new folders. Revert this PR to roll back the launcher, stdin, process-cleanup and EOF-response changes. ## Related Issues #909
## Summary Follow-ups to the review threads left on four merged pull requests: #909, #912, #915 and #923. Each is a defect the review named, reproduced before any change, and repaired with a test that fails against the old code. This also addresses the Ollama spelling issue raised in this PR's review. **A cache-only window lookup no longer skips a model LiteLLM answers from its table** (from #909). The gate that keeps metadata lookups off a model's own server skipped `get_model_info` for any id carrying an Ollama segment, including ids LiteLLM answers offline: `ollama/Llama3`, `litellm_proxy/ollama/llama3` and `github/ollama/llama3` were sized on the 200000-token fallback where main read 8192. The gate now puts LiteLLM's own precondition to LiteLLM, keyed on the caller's id, and answers cautiously only for an id with an Ollama segment when the installed release does not ship that check. **A window the model's own server reported is reused inside the turn** (the second item of #909). A caller allowed to fetch resolved a local model's window by asking its server and threw the answer away; the next caller, which may not fetch, answered the fallback for the same model on every iteration, for the life of the process. The answer is now kept under the id the request goes out under, so `ollama/`, `ollama_chat/` and the stored `ollama-chat/` spelling share one entry. The table is consulted before the memo, so a row LiteLLM ships still outranks a previous ask; only a real figure is stored, so a server that was down is asked again. **A measurement taken while the boot verify ran is no longer overwritten** (from #915). The boot read a row before its handshake and wrote its verdict from that read, so a Test recorded during the handshake was replaced by a late timeout, refusal or pass. It reads the row again immediately before the write and leaves a newer measurement in place. The comparison is on `measured_at_ms`, so a modes relearn does not stop the write. Four docstrings that described the replaced rule are corrected. **The Windows environment capture's memo is pinned, and its docstring names the second failure mode** (from #912). **A key is written under the spelling the loaders read** (from #923). Below the top level pydantic reads the camelCase alias whenever it is present, null or not, so preferring a non-null spelling sent a write to a key the schema ignores, and `load_config` and the live readers then disagreed. Only the top level drops a null block; below it the first spelling present is the one read, and every caller now states which case it is in. **A write aimed at a wildcard map entry reaches that entry** (the follow-up #923's review asked for). Below the top level the spelling helper fell back to another spelling of a key, which is right for a schema field and wrong for a map entry: with only `tools.mcpServers.fooBar` present, a write to `tools.mcpServers.foo_bar.enabled` selected `fooBar`, disabling a server the caller never named. The catalog knows which segments a path binds, so a bound segment is now matched as written. `providers` keeps the tolerant rule, because its runtime reader folds those spellings through `names_same_provider`. **Configured Ollama models reach the metadata driver under its accepted spelling.** The wizard and model picker store Ollama ids as `ollama-chat/<model>`, while LiteLLM expects `ollama_chat/`. The window and output-ceiling lookups now translate that endpoint prefix using the provider registry before consulting the candidate list. The memo and no-fetch prefilters use the same registry normalization. The `ollama/` completion prefix is preserved because LiteLLM keys its offline table rows under it; non-Ollama prefixes are unchanged. ## Type - [x] Fix - [ ] Feature - [ ] Docs - [ ] CI / tooling - [ ] Refactor - [ ] Other ## Verification - [x] Relevant tests pass locally - [x] Relevant lint / type checks pass locally - [ ] User-facing docs or screenshots are updated when needed Latest review fix (Linux, Python 3.12.13, LiteLLM 1.85.0; proxy variables unset; consumer tests rechecked after rebase onto current main): - New and strengthened regression cases ran before the fix: 6 failed / 108 passed, covering the memo spelling, both no-fetch metadata lookups, the conservative guard fallback, and an id produced by `stored_model_id` that could not reach the daemon. - `uv run --no-sync pytest tests/test_provider_rates.py -q -n 0 --tb=short`: 114 passed after the fix. - `uv run --no-sync pytest -q --tb=short <consumer-test-files> --cov=raven.providers.rates --cov-branch --cov-report=json:<coverage-json>`: 1312 passed, 1 skipped. The selection contains 16 test modules importing `raven.providers.rates`, plus `test_provider_registry_lookup.py` and `test_provider_wire_model.py`. The skip is the permission test that cannot deny a root process with `chmod 000`; existing Pydantic warnings remain. - Applying the repository's `parse_changed_lines` and `calculate_diff_coverage` to the fix commit after rebase yields 100% (13/13 executable changed lines). This is coverage of this fix, not a new full-PR coverage or full-suite ratchet result. - Six independent in-memory mutants were caught: raw memo prefilter, raw endpoint guard, raw shared predicate, omitted window spelling conversion, omitted ceiling spelling conversion, and replacing only the endpoint conversion with `wire_model` (which loses the `ollama/` table rows). The worktree was not rewritten by the mutation checks. - `UV_NO_SYNC=1 make lint-python lint-imports lint-deps lint-types`: passed after rebase; 2268 Python files formatted, 10 import contracts kept, no dependency issues. Both commit-message validators and the source-language and large-file gates also passed. `uv run --no-sync ty check raven/providers/rates.py tests/test_provider_rates.py` also passed. - Against a loopback stand-in daemon, the actual stored `ollama-chat/qwen2.5` id makes no request in cache-only mode before a fetch. One allowed `/api/show` request learns 8192, after which the cache-only lookup and `ModelBinding.context_window` both return 8192 without another request, and the output limit is 4096. - The daemon-path parameter cases clear LiteLLM's own id-keyed cache so alias cases cannot inherit a previous case's successful response. Earlier author verification, before this review fix: Commands recorded at the original PR revision and main baseline, with proxy variables unset: ``` uv run --frozen --python 3.12 --all-extras pytest -n 4 -q ``` 3 failed, 29796 passed, 158 skipped, 2 xfailed. A same-day run of the original main control gives the same 3 failed with 29771 passed, so the failure set is identical and the 25 extra passes are the tests added here. The three are environment-local: `test_resolve_node_dir_answers_each_case_it_exists_for` (npm on PATH), `test_a_legacy_workbook_is_converted_and_read` (LibreOffice), and `test_what_cannot_be_read_names_nothing` (load-dependent). Focused runs on the changed files: - `tests/test_provider_rates.py` 110 passed - `tests/test_config_self_surface.py` `tests/test_rpc_settings.py` `tests/test_config_update.py` `tests/test_raven_config_tool.py` `tests/test_rpc_console.py` 805 passed - `tests/test_subagent_host_env.py` 30 passed Gates: `ruff check`, `ruff format --check`, `ty check`, `lint-imports`, `deptry`, `scripts/check_commit_messages.py origin/main..HEAD`, `scripts/check_source_language.py origin/main..HEAD` and `scripts/check_large_files.py origin/main..HEAD` all pass. `pre-commit run --all-files` fails two hooks on three files this branch does not touch; the same three fail the same way at the base, so they are pre-existing. Every new test was checked by reverting the fix: six single-construct mutants over the two repaired rules, each restored and compared after running. Two survived the first pass and both were findings rather than tests to tighten. The guard that refused a zeroed window was unreachable (the one caller already has a window), so the parameter is required instead of optional. And the memo originally ranked above LiteLLM's own table, so a previous ask could have overridden a row a caller could have read offline; the table now comes first. Reproduced end to end against a stand-in server on a loopback port: one `/api/show` request answers 8192, `ollama/qwen2.5` read with `allow_fetch=False` answers 8192 with no second request, and `ModelBinding.context_window` on that model answers 8192 where it read the fallback before. ## Risk - [x] Security impact considered - [x] Backward compatibility considered - [x] Rollback path is clear for risky changes No stored config changes shape or meaning. The alias-spelling fix changes where a write lands when a file carries both spellings of one key, which is the file that did not load correctly before. The wildcard fix changes where a write lands when two map entries differ only by spelling; both are left as written, where the old code silently edited the neighbour. The window memo is process-wide and holds context-window and optional output-limit counts per model, cleared by the same `reset_openrouter_cache` the catalogue uses. Stored `ollama-chat/` ids can now make a metadata request to the configured daemon when fetch is allowed; cache-only callers continue to reuse local data. Other hyphenated provider prefixes are outside this fix. Roll back by reverting the merged change; the memo is not persisted. ## Related Issues #909, #912, #915, #923
3 participants
Summary
Creating an ACP session or opening the model picker could query LiteLLM's provider metadata API even with
allow_fetch=False. Unknown Ollama models could delaysession/newbeyond the EOF cleanup grace period and drop its reply.Both context-window and output-ceiling queries now preserve the caller's lookup policy. A cache-only lookup skips LiteLLM's
get_model_infoonly for the Ollama drivers (ollama,ollama_chat): on a table miss they ask the serving daemon (POST /api/show), which is the request that delayedsession/new. Every other driver answers offline, by stripping the routing prefix and reading the table, and that lookup stays on cache-only paths: the table files most direct vendors under the bare model name, so skipping it changed 203 of 413 measured Model Refs, for exampleopenai/gpt-4o-minifell from its 128000-token window to the 200000 fallback, andopenai/gpt-4oread OpenRouter's 4096-token output ceiling instead of the vendor's 16384. huggingface stays skipped under both policies, and fetch-enabled callers retain SDK lookup behavior.Merge this PR before #908. The Windows stdin and launcher changes in #908 are stacked on this metadata fix, and its Windows ACP stdio job validates the complete combination.
Type
Verification
At this head, rebased onto main, Linux, Python 3.12:
uv run --frozen --python 3.12 --all-extras pytest -qwith the proxy variables unset: 3 failed, 29662 passed, 158 skipped. The same 3 fail on main at the merge base on this machine (an npm on PATH, a root-only permission check, a LibreOffice conversion); 0 introduced.uv run --frozen --python 3.12 --all-extras pytest -n 0 tests/test_provider_rates.py tests/test_rpc_model.py tests/test_acp_server.py tests/test_acp_methods.py tests/test_acp_config_options.py tests/test_agent_loop_lazy_provider.py tests/test_lazy_provider.py tests/test_token_wise_pricing.py tests/test_rpc_session_init_bundle.py -q: 609 passed.resolve_context_windowanddeclared_max_output_tokensover 413 Model Refs (every provider-prefixed spelling of a bare-keyed chat row in the bundled LiteLLM table, plus common ones), each tree in its own empty home: this head and main answer identically for all 413, in under 0.2 s per tree, so no lookup left the process.ruff check,ruff format --check,lint-imports,ty checkanddeptryover the Makefile targets,commitlint,scripts/check_commit_messages.py,scripts/check_large_files.py,scripts/check_source_language.pyandpre-commit run --from-ref origin/main --to-ref HEAD: passed.uv run --frozen --python 3.12 --all-extras pytest tests/integration/test_acp_stdio_smoke.py -m integration -q -n 0: 11 passed on Linux. fix: restore bundled ACP agent startup on Windows #908's Windows ACP stdio job runs the same file on windows-latest.The metadata regressions cover uncached Ollama spellings, cached aliases, offline normalization for direct vendors, both lookup policies, real model.options handling, and all three ACP handshake replies after EOF.
Configuration and protocol contracts are unchanged; the resolver docstrings describe the lookup policy.
Risk
Cache-only lookups return what main returns for every Model Ref except an uncached Ollama model, whose window and ceiling stay unknown rather than being asked of the daemon; such a model keeps the configured default, and a self-hosted model can set agents.defaults.contextWindowTokens explicitly. Fetch-enabled lookup behavior is preserved. The Ollama driver set mirrors LiteLLM 1.85.0's
_get_model_info_helper, whose only other branch that leaves the process is huggingface's; a LiteLLM upgrade that adds one would need it added there. Revert this change to roll back.Related Issues
#908