ADR-004: Model the LLM as a Platform Service, Not a Tool
Status
Accepted (supersedes the LLMTool design from Phase 1 revision 1)
Context
Phase 1's original design modeled LLM inference as LLMTool, resolved through the same Tool Registry as NewsTool and SECFilingsTool. Architecture review identified this as a category error: every Skill needs the LLM, none optionally needs a specific external data Tool the same way; the LLM carries cross-cutting concerns (retries, provider failover, cost/token accounting, future prompt versioning and model routing) that don't fit the generic ToolResult/BaseTool contract designed for simple external data fetches.
Decision
Remove LLMTool from the Tools layer entirely. Introduce LLMService (backend/src/services/llm_service.py) as a distinct architectural layer, injected into every Skill as a first-class constructor dependency (not registry-resolved like Tools). LLMService wraps the existing backend/src/llm/ package (factory.py, provider_catalog.py, providers/*), which keeps its current responsibilities unchanged — provider-specific request/response mechanics. LLMService adds the policy layer on top: model/provider selection, retries, failover, token/cost accounting, and a placeholder for prompt versioning.
Alternatives Considered
A. Keep LLMTool as-is (do nothing)
Rejected: the review's core objection stands — the Tool Registry becomes the wrong place to grow retry/failover/cost-accounting logic that only the LLM needs, and every other Tool would eventually be tempted to acquire similar machinery "for consistency," diluting what "Tool" means.
B. Put LLM policy logic inside each Skill
Rejected: would duplicate retry/failover/cost logic across CompanySkill, FinancialSkill, NewsSkill, GenericChatSkill — exactly the kind of cross-cutting duplication a service layer exists to prevent.
C. Put LLM policy logic inside llm/factory.py itself
Rejected, but closest alternative considered: llm/factory.py already resolves which provider class to use based on settings — it would be tempting to add retries/failover there directly. Kept separate because llm/ is provider-mechanics-focused and already has a clear, narrow job; layering policy concerns (cost tracking, future model-routing heuristics, prompt versioning) on top as a distinct LLMService keeps llm/factory.py simple and keeps the policy layer independently testable (mock llm/factory.py entirely when testing LLMService's retry/failover behavior).
Consequences
Positive:
- One obvious home for model routing, cost optimization, and provider failover as they mature — no special-casing inside the Tool contract.
- Skill unit tests get simpler: one mocked
LLMServiceinstead of a Tool-Registry-resolvedLLMToolplus whatever ad hoc retry mocking would otherwise be needed per Skill. - The Tools layer's meaning stays crisp: "external, non-LLM data integrations only."
Negative / accepted tradeoffs:
LLMServiceis a special-cased, always-injected dependency rather than a uniformly registry-resolved one like Tools — a deliberate asymmetry, accepted because the LLM genuinely isn't a peer of other Tools (every Skill needs exactly one LLM Service; Skills need a variable number of Tools, including zero).- This is a breaking change to the Phase 1 v1 Skill constructor signature and the Tool Registry contents — mitigated by the fact that Phase 1 has not yet been implemented in code at the time of this revision, so there is no runtime migration, only a documentation/design update (see
phase1-revision-notes.md).
Interview Talking Point
This mirrors a common real-world lesson in agent system design: not everything an agent "invokes" belongs in the same abstraction. Reasoning (the LLM) and acting-on-the-world (Tools) are different enough in their failure modes, cost profile, and lifecycle that conflating them into one "Tool" concept creates exactly the kind of hidden coupling a clean architecture is supposed to prevent.