Feature/llm runtime wiring - #20
Merged
Merged
Conversation
…time The LLM layer was implemented and tested but never wired: get_twilio_service() built the orchestrator without an interpreter, so every live WhatsApp message was answered by the deterministic parser, and no TracerProvider was ever installed so nothing reached Langfuse. - Settings now declares the LLM and tracing configuration (llm_provider, model, temperature, tokens, timeout, confidence threshold, prices, provider credentials, Langfuse keys) and derives llm_enabled, traces_endpoint, tracing_enabled and traces_auth_header. It stays the only reader of the environment; the legacy os.getenv helper is gone. - New app/agent/factory.py builds the Strands model per provider (openai default, anthropic, bedrock; OpenAI-compatible gateways via OPENAI_BASE_URL) and build_interpreter() fails closed: disabled provider, missing credential or missing SDK returns None plus one llm_disabled warning, so a rescue is never blocked by LLM configuration. - configure_tracing() installs an idempotent OTLP/HTTP exporter to Langfuse Cloud with Basic auth and injectable exporter/provider for hermetic tests; the FastAPI lifespan configures it and flushes on shutdown. - get_twilio_service() injects the interpreter and logs the active provider; the eval runner uses the same factory, so evals and production resolve the provider identically. - Adds openai and the opentelemetry api/sdk/otlp-http dependencies.
… record - ADR-004 records why OpenAI is the first provider, why OpenAI-compatible gateways need no separate code path, and why failure is fail-closed to the deterministic parser instead of crashing at boot. - Runbook gains a configuration section: how to set and verify the provider and the Langfuse keys, what the boot log must say, and the incident rows for a parser-like answer, unexpected cost and missing traces. - assumptions.md adds A5 (OpenAI as the first provider) and closes A4 with the fact that Langfuse Cloud is now wired code rather than intent.
… log - scripts/verify_langfuse.py sends one real span through the configured OTLP exporter and reads it back from Langfuse's v2 observations API, so key problems are diagnosable without deploying (the legacy /api/public/traces endpoint answers 410 for organizations created after 2026-09-16). - The startup log field is 'detail' instead of a 'provider' key holding a full 'provider=... model=...' string.
Test counts, fail-closed behaviour, the live startup log, the confirmed Langfuse Cloud round trip and the .env correction, so the feature record shows what was observed rather than asserted.
Both were invisible to the unit suite because the doubles were more forgiving than the SDK. - interpreter: the real client returns the full Interpretation dump, which already carries prompt_version, so passing it again raised TypeError and every message degraded to the parser through ProviderUnavailableError. Merge the payload instead, with a regression test that feeds a full dump. - llm metering: Strands 1.56 reports EventLoopMetrics.accumulated_usage in camelCase and latency in accumulated_metrics['latencyMs'], so the client was recording 0 tokens and $0 for every call. Extract tolerantly across versions, fall back to wall-clock latency, and keep cached-input tokens visible. Adds scripts/verify_llm.py (real provider smoke check) and records the evidence including the latency observation against the advertised p95 target.
The 1.2 s figure in the Ops screen was a mockup number that was never measured and that the real provider exceeds (1.0-2.8 s per call, including the Strands agent cycle). The target moves to 2.5 s and the demo p95 shown next to it becomes 1980 ms, consistent with what the live path reports; the eval harness already allowed 5 s. The system prompt stays untouched because ~1024 of its ~1150 input tokens are prompt-cache reads, so trimming it would cost interpretation quality for latency we no longer need. ADR-004 records the decision and the open deviation found while measuring: the inbound webhook now awaits the interpretation call, so it takes 1-3 s instead of the < 200 ms the spec requires for an enqueue-only handler.
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.
No description provided.