Honor the configured provider when a node pins its own model - #151
Open
basil-k-aji-dev wants to merge 1 commit into
Open
basil-k-aji-dev wants to merge 1 commit into
basil-k-aji-dev wants to merge 1 commit into
Conversation
get_google_llm hard-codes ModelProvider.GOOGLE, so the Flash step summarizer and the chunk capsule lens sent an OpenAI-compatible deployment to the Gemini client and failed on an empty GOOGLE_API_KEY. The summarizer is built eagerly at runner init, so the run died before its first step. get_node_model resolves the endpoint the same way get_llm does and overrides only the model name, leaving the operator's provider intact.
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.
Closes #138
Why
get_google_llmhard-codesModelProvider.GOOGLE. Any caller that pins a specific lightweight model therefore ignores the configured provider, and an OpenAI-compatible deployment gets a Gemini client built with an empty key:Two call sites do this, both reached on a normal Flash run:
agents/flash/summarizer.py—VisualStepSummarizeris built eagerly at runner init whenagent.flash.step_summarizer.enabledis on, which is the default. That is why the run dies before the first step rather than at the first summary.memory/chunking.py—StepCapsuleLens._get_llmand_get_fallback_llm, pinningchunking.model.What
get_node_model(ctx, name, *, model_name, is_utils, temperature)inservices/llm.py. It resolves the endpoint through_resolve_endpoint, the same pathget_llmalready uses, and overrides onlymodel_name. The provider, and anything else the operator configured, survive.Two things it deliberately does not do:
RobustChatModelWrapper.VisualStepSummarizer._meter_lens_callskips metering when the model is already wrapped, so returning a wrapped model here would have double-counted every lens call. Matching whatget_google_llmreturned keeps that guard correct.ctxit still builds Google. There is no configuration to read and therefore no provider to honour;StepCapsuleLensis constructed withctx=Nonein places and that path is unchanged.What I left alone
agents/flash/runner.py:287also falls back toget_google_llm("gemini-2.5-flash"), as the issue notes. I did not touch it: it only runs afterget_llm(ctx, "operator")has already raised, so resolving a provider from the same configuration would be circular. The issue argues that path should fail with a clear message instead of building a Gemini client with an empty key, and I agree, but that is a behaviour change on an error path and it is your call rather than mine. Happy to add it here.The Pro-profile judge nodes the issue mentions (
planner_validation,validator_pixel_safety_netvialightweight_judge_default()) are the same class of problem but a different shape, since thedefault+nodesconfig format has no way to override them. Worth its own issue.Testing
tests/unit/test_llm_node_model.py, five cases. The two that matter assert at the real call site rather than on the new helper, because a test that only importedget_node_modelwould fail on an unpatched tree withImportError, which proves the function is new and nothing else.Against the unpatched tree, with the test pointed at
get_google_llm:With the change:
make lintandmake typecheckare clean:The suite, honestly
tests/unit/memoryandtests/unit/agentsreport 80 failed, 535 passed, 4 skipped on this machine, and the identical 80 named tests fail on a clean tree with my three files stashed. I compared the sorted failure lists rather than the totals, because equal counts would also be the signature of breaking one test while an unrelated one happened to pass.They are environmental, not the change:
I have no
GOOGLE_API_KEYhere, so every test that validates providers fails at config time. That also means I could not run the reporter's end-to-end Flash task; the verification above is the targeted unit tests plus the reporter's own confirmation that the same patch shape completed a Flash run against a custom endpoint with usage recorded asopenai:<model>.