From e4b76fb06ad9f1f025a5b239d0f2069e4501d16b Mon Sep 17 00:00:00 2001 From: oabrivard Date: Tue, 24 Mar 2026 22:35:10 +0100 Subject: [PATCH] docs: add plan for simplifying LLM provider trait --- .../plans/2026-03-24-simplify-llm-trait.md | 189 ++++++++++++++++++ 1 file changed, 189 insertions(+) create mode 100644 docs/superpowers/plans/2026-03-24-simplify-llm-trait.md diff --git a/docs/superpowers/plans/2026-03-24-simplify-llm-trait.md b/docs/superpowers/plans/2026-03-24-simplify-llm-trait.md new file mode 100644 index 0000000..b14666a --- /dev/null +++ b/docs/superpowers/plans/2026-03-24-simplify-llm-trait.md @@ -0,0 +1,189 @@ +# Simplify LLM Provider Trait — Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Collapse the `LlmProvider` trait from 3 methods to a single `call_llm` method, unifying all provider implementations. + +**Architecture:** Replace `generate_search_pass` and `generate_rewrite_pass` with a single `call_llm`. Each provider keeps one API path (OpenAI: Responses API, Gemini: generateContent, Anthropic: Messages). Remove web search tool support from all providers. + +**Tech Stack:** Rust, async_trait + +**Spec:** `docs/superpowers/specs/2026-03-24-simplify-llm-trait-design.md` + +--- + +### Task 1: Rewrite trait + all 3 provider implementations + +**Files:** +- Modify: `backend/src/services/llm/mod.rs` +- Modify: `backend/src/services/llm/openai.rs` +- Modify: `backend/src/services/llm/gemini.rs` +- Modify: `backend/src/services/llm/anthropic.rs` +- Modify: `backend/src/services/llm/factory.rs` + +- [ ] **Step 1: Rewrite `mod.rs` — new trait** + +Replace the entire trait and remove `ProviderCapabilities`: + +```rust +//! LLM provider abstraction layer. +//! +//! Defines the `LlmProvider` trait that all LLM providers implement, +//! along with shared types and the provider factory function. + +pub mod anthropic; +pub mod factory; +pub mod gemini; +pub mod openai; +pub mod schema; + +use async_trait::async_trait; +use serde_json::Value; + +use crate::errors::AppError; + +/// Trait defining the contract for LLM provider implementations. +/// +/// Each provider (Gemini, OpenAI, Anthropic) implements this trait +/// to provide a unified interface for structured LLM calls. +#[async_trait] +pub trait LlmProvider: Send + Sync { + /// Returns the provider identifier (e.g., "gemini", "openai", "anthropic"). + fn provider_id(&self) -> &str; + + /// Call the LLM with a prompt and expected JSON schema. + /// + /// # Arguments + /// * `model` — The model identifier (e.g., "gpt-4o-mini") + /// * `system_prompt` — System-level instructions + /// * `user_prompt` — The user's prompt + /// * `response_schema` — JSON Schema defining the expected response structure + async fn call_llm( + &self, + model: &str, + system_prompt: &str, + user_prompt: &str, + response_schema: &Value, + ) -> Result; +} +``` + +- [ ] **Step 2: Rewrite `openai.rs` — Responses API only** + +Read the current file. Keep the struct, `new()`, and `extract_responses_api_content`. Remove: +- `call_chat_completions_api` method +- `extract_chat_completions_content` function +- The `include_web_search` parameter from what was `call_responses_api` + +The `impl LlmProvider` block becomes: + +```rust +#[async_trait] +impl LlmProvider for OpenAiProvider { + fn provider_id(&self) -> &str { + "openai" + } + + async fn call_llm( + &self, + model: &str, + system_prompt: &str, + user_prompt: &str, + response_schema: &Value, + ) -> Result { + // This is the existing call_responses_api body, without web search + let body = serde_json::json!({ + "model": model, + "instructions": system_prompt, + "input": user_prompt, + "max_output_tokens": 16384, + "text": { + "format": { + "type": "json_schema", + "name": "synthesis", + "strict": true, + "schema": response_schema + } + } + }); + // ... rest of the HTTP call + response parsing via extract_responses_api_content + } +} +``` + +Keep all existing error handling, `map_openai_error`, and `extract_responses_api_content`. + +Remove tests for `supports_web_search` and Chat Completions. Keep/update tests for Responses API parsing and error handling. Some tests may reference `generate_search_pass` or `generate_rewrite_pass` — rename to `call_llm`. + +- [ ] **Step 3: Rewrite `gemini.rs` — no web search** + +Read the current file. The `build_request_body` function has an `include_search` parameter — remove it and never add the `tools` key. + +The `impl LlmProvider` becomes a single `call_llm` that calls the existing API path without web search. + +Remove `generate_search_pass` and `generate_rewrite_pass`. Replace with `call_llm`. + +Update tests: rename method calls, remove `supports_web_search` assertions, remove web search request body tests. + +- [ ] **Step 4: Rewrite `anthropic.rs` — single method** + +Same pattern. The current `generate_search_pass` and `generate_rewrite_pass` likely share most code. Merge into `call_llm`. + +Update tests. + +- [ ] **Step 5: Update `factory.rs` tests** + +Remove all `assert!(provider.supports_web_search())` assertions from factory tests. Change any `generate_search_pass`/`generate_rewrite_pass` references to `call_llm`. + +- [ ] **Step 6: Verify** + +Run: `cd backend && cargo test --lib` +Expected: all tests pass + +- [ ] **Step 7: Commit** + +```bash +git add backend/src/services/llm/mod.rs backend/src/services/llm/openai.rs backend/src/services/llm/gemini.rs backend/src/services/llm/anthropic.rs backend/src/services/llm/factory.rs +git commit -m "feat: simplify LlmProvider trait to single call_llm method" +``` + +--- + +### Task 2: Update all callers + +**Files:** +- Modify: `backend/src/services/synthesis.rs` +- Modify: `backend/src/services/source_scraper.rs` +- Modify: `backend/src/handlers/api_keys.rs` + +- [ ] **Step 1: Update `synthesis.rs`** + +Search and replace all occurrences: +- `provider.generate_search_pass(` → `provider.call_llm(` (1 occurrence, Phase 2 search) +- `provider.generate_rewrite_pass(` → `provider.call_llm(` (4 occurrences: classification×2, rewrite, article extraction) +- `provider.generate_rewrite_pass(` inside `scrape_single_article_with_llm` → `provider.call_llm(` + +- [ ] **Step 2: Update `source_scraper.rs`** + +Search and replace: +- `provider.generate_rewrite_pass(` → `provider.call_llm(` (1 occurrence, LLM link extraction) + +- [ ] **Step 3: Update `api_keys.rs`** + +Search and replace: +- `.generate_rewrite_pass(` → `.call_llm(` (1 occurrence, key test) + +- [ ] **Step 4: Verify** + +Run: `cd backend && cargo test --lib` +Expected: all tests pass + +Run: `cd backend && cargo build` +Expected: clean build, no warnings about unused methods + +- [ ] **Step 5: Commit** + +```bash +git add backend/src/services/synthesis.rs backend/src/services/source_scraper.rs backend/src/handlers/api_keys.rs +git commit -m "refactor: update all callers from generate_search/rewrite_pass to call_llm" +```