Guard LocalLLM's provider identity and add its GenAI telemetry - #691
Merged
Conversation
AnthropicLLM.chat already refuses a request whose model_ref.provider disagrees with the adapter serving it, because cost resolves from the Agent's declared (provider, model) while the serving route comes from LLM_PROVIDER configuration. LocalLLM had no such check: with LLM_PROVIDER=local and an Agent declaring anthropic, the call would be served free on the facility GPU and priced at vendor rates, silently misattributing spend and defeating the buy-vs-build comparison the two routes exist to support. The provider identity is a fixed module constant here, not an injected constructor argument like AnthropicLLM's provider_name. AnthropicLLM needs the injection because ArgoLLM composes it and overrides the identity to "argo"; no gateway composes LocalLLM the same way, so there is nothing yet to override it for. Add the injection if that changes. docs/deployments/2-bm/llm_debrief.md already documented this refusal as applying to the local arm; that claim is now true rather than aspirational. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
LocalLLM emitted no OpenTelemetry GenAI signal at all: neither track_in_flight_call nor record_llm_call was imported, unlike AnthropicLLM which wraps every call in both. The in-flight counter is load-bearing beyond observability, since a sustained nonzero reading is the stated trigger for building a heavier budget-enforcement tier, and its absence was a hole exactly where concurrency is most likely: a batching GPU server is the one serving route that answers many calls at once. Mirrors AnthropicLLM.chat's shape: track_in_flight_call and an "llm.chat" span wrap the backend await, and record_llm_call fires on the success path only, fed from the completion the backend returns (response_model_id, usage, stop_reason) and request.max_output_tokens. record_llm_call's returned cost is discarded here exactly as it is in AnthropicLLM: it feeds the telemetry histogram, not the durable spend ledger, which the caller writes from the returned LLMResponse. The telemetry context nests inside the existing GPU occupancy meter's try/finally so that meter keeps opening and closing around every call, success or failure, unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Coverage report
Click to see where and how coverage changed
This report was generated by python-coverage-comment-action |
||||||||||||||||||||||||
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
1 participant
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.
Summary
LocalLLM.chatnow refuses a request whosemodel_ref.providerisn't"local", mirroringAnthropicLLM's existing guard: cost resolves from the Agent's declared(provider, model)while the serving route comes fromLLM_PROVIDERconfiguration, so a mismatch would silently misattribute spend between the bought and built arms.LocalLLM.chatnow emits the same OpenTelemetry GenAI signalsAnthropicLLMdoes:track_in_flight_callaround the backend call (feeds thecora.agent.llm.concurrent_callscounter), anllm.chatspan, andrecord_llm_callon the success path only.The provider identity is a fixed module constant (
_PROVIDER_NAME = "local") rather than an injected constructor argument likeAnthropicLLM.provider_name.AnthropicLLMneeds the injection becauseArgoLLMcomposes it and overrides the identity to"argo"; no gateway composesLocalLLMthe same way today, so there is nothing yet to override it for.docs/deployments/2-bm/llm_debrief.mdalready documented this refusal as applying to the local arm; no doc change was needed since the implementation now matches what it already claimed.Test plan
uv run pytest tests/unit/agent -q --timeout=60 --timeout-method=thread(938 passed)uv run pytest tests/architecture -q --timeout=60 --timeout-method=thread(31181 passed, 629 skipped)uv run ruff format . && uv run ruff check .uv run pyright src/cora/agent/adapters/local_llm.py tests/unit/agent/test_local_llm.pyrecord_llm_callfires only on the success path; the GPU occupancy meter still measures correctly with the new telemetry nesting.🤖 Generated with Claude Code