diff --git a/.planning/phases/07-redo-and-optimize-llm-integration/07-03-SUMMARY.md b/.planning/phases/07-redo-and-optimize-llm-integration/07-03-SUMMARY.md new file mode 100644 index 0000000..9bb8801 --- /dev/null +++ b/.planning/phases/07-redo-and-optimize-llm-integration/07-03-SUMMARY.md @@ -0,0 +1,168 @@ +--- +phase: 07-redo-and-optimize-llm-integration +plan: "03" +subsystem: backend/ai-providers +tags: + - ai + - anthropic + - output_config + - structured-output + - singleton-client + - classifier-refactor + - db-driven-config + - wave-3 +dependency_graph: + requires: + - "07-02 (ProviderConfig + GenericOpenAIProvider + registry — get_provider accepts ProviderConfig)" + - "07-01 (system_settings table + HKDF helpers + load_provider_config stub)" + provides: + - "AnthropicProvider singleton _client + output_config + _truncate (D-03/D-07/D-12/D-13)" + - "classifier.classify_document driven by load_provider_config with per-user/env fallback (D-04/D-06)" + - "ai_config.py stub removed — real ProviderConfig used everywhere" + - "3 previously-xfailed tests promoted to passing (test_anthropic_structured_output, test_api_key_encrypt_decrypt, test_anthropic_stop_reason_fallback)" + affects: + - "07-04 (Celery retry — classifier is now provider-agnostic; truncation inside providers)" + - "07-05 (Admin AI panel — ProviderConfig is the single config representation)" +tech_stack: + added: [] + patterns: + - "output_config={\"format\": {\"type\": \"json_schema\", \"schema\": _SCHEMA}} for Anthropic constrained decoding (D-03)" + - "stop_reason guard: raw='' when stop_reason != 'end_turn' → graceful degradation (T-07-08)" + - "Uniform ctor signature: __init__(api_key, model, context_chars, base_url) across all providers" + - "load_provider_config(session) → ProviderConfig | None — DB authoritative, env fallback (D-04/D-15)" + - "Per-user override path: ProviderConfig from PROVIDER_DEFAULTS, empty api_key (T-07-06)" +key_files: + created: [] + modified: + - backend/ai/anthropic_provider.py + - backend/ai/__init__.py + - backend/services/ai_config.py + - backend/services/classifier.py + - backend/tests/test_ai_providers.py + - backend/tests/test_ai_config.py + - backend/tests/test_classifier.py +decisions: + - "output_config={'format': {'type': 'json_schema', 'schema': ...}} chosen over tool_use — semantically correct for classification, no extra parsing layer needed" + - "base_url accepted in AnthropicProvider.__init__ for uniform factory signature but unused — SDK manages endpoint" + - "Per-user override path uses empty api_key — admin configures per-provider keys in system_settings; T-07-06 mitigated" + - "test_classifier.py tests updated to assert ProviderConfig properties instead of dict fields — D-06 contract enforced in tests" + - "test_anthropic_stop_reason_fallback added as extra regression guard for T-07-08" +metrics: + duration: "~25 minutes" + completed: "2026-06-04" + tasks_completed: 2 + tasks_total: 2 + files_created: 0 + files_modified: 7 +--- + +# Phase 7 Plan 03: Anthropic output_config + Classifier ProviderConfig Refactor Summary + +AnthropicProvider refactored with singleton _client, output_config constrained-decoding structured output, and _truncate; classifier.py wired to load_provider_config(session) replacing inline dict construction; ai_config.py stub removed; 3 xfailed tests promoted. + +## Tasks Completed + +| Task | Description | Commit | Files | +|------|-------------|--------|-------| +| 1 | AnthropicProvider singleton + output_config + truncation + no MAX_AI_CHARS | efc177a | anthropic_provider.py, ai/__init__.py, tests/test_ai_providers.py | +| 2 | Classifier wired to load_provider_config + ai_config stub removed | 95c386f | services/ai_config.py, services/classifier.py, tests/test_ai_config.py, tests/test_classifier.py | + +## What Was Built + +### Task 1: AnthropicProvider Refactor + +**backend/ai/anthropic_provider.py** rewritten: +- `MAX_AI_CHARS = 8_000` module constant deleted +- `def _client(self)` property method deleted +- `__init__` signature widened to `(api_key, model, context_chars, base_url=None)` — uniform factory contract +- `self._client = anthropic.AsyncAnthropic(api_key=self._api_key)` stored as singleton in `__init__` (D-07) +- `def _truncate(self, text)` added — identical 60/40 split pattern to OpenAIProvider (D-13) +- `_CLASSIFICATION_SCHEMA` and `_SUGGESTIONS_SCHEMA` module-level dicts added (required + additionalProperties=False) +- `classify()` passes `output_config={"format": {"type": "json_schema", "schema": _CLASSIFICATION_SCHEMA}}` (D-03) +- `suggest_topics()` passes `output_config={"format": {"type": "json_schema", "schema": _SUGGESTIONS_SCHEMA}}` (D-03) +- Graceful degradation: `stop_reason != "end_turn"` → `raw = ""` → `parse_classification("")` returns empty ClassificationResult (T-07-08) +- `health_check()` does NOT pass `output_config` — only verifies connectivity/auth + +**backend/ai/__init__.py** updated: +- Anthropic branch now passes `context_chars=effective_context_chars, base_url=effective_base_url` to AnthropicProvider +- Comment updated to reflect widened ctor (Plan 03) + +**backend/tests/test_ai_providers.py** updated: +- `test_anthropic_structured_output` promoted from xfail — patches `ai.anthropic_provider.anthropic.AsyncAnthropic`, asserts `output_config` kwarg present and matches `_CLASSIFICATION_SCHEMA`, asserts AsyncAnthropic constructed once (singleton) +- `test_anthropic_stop_reason_fallback` added — simulates `stop_reason="max_tokens"`, asserts empty ClassificationResult returned without crash (T-07-08 regression guard) + +### Task 2: Classifier ProviderConfig Refactor + ai_config Stub Removal + +**backend/services/ai_config.py** rewritten: +- `_ProviderConfigStub` class deleted entirely +- Module-level import: `from ai.provider_config import ProviderConfig, PROVIDER_DEFAULTS` (no more lazy import inside function body) +- `load_provider_config(session)` return type changed to `Optional[ProviderConfig]` +- `seed_system_settings_from_env` now uses `PROVIDER_DEFAULTS.get(provider_id, {}).get("context_chars", 8000)` for accurate default context_chars per provider + +**backend/services/classifier.py** refactored: +- Imports: `from services.ai_config import load_provider_config` and `from ai.provider_config import ProviderConfig, PROVIDER_DEFAULTS` +- `classify_document`: inline `_settings = {...}` dict construction replaced with: + 1. Per-user override path: `ProviderConfig(provider_id=ai_provider, ...)` from PROVIDER_DEFAULTS (api_key="" — T-07-06 mitigation documented) + 2. System path: `await load_provider_config(session)` → None fallback to env-var defaults + 3. `provider = get_provider(config)` — unchanged call signature +- `suggest_topics_for_document`: same load/override/fallback pattern applied +- No `text[:N]` slices remain — truncation fully delegated to provider `_truncate()` + +**backend/tests/test_ai_config.py** promoted: +- `test_api_key_encrypt_decrypt`: round-trip smoke test (encrypt → decrypt → assert equality), domain salt isolation (different provider_id → different ciphertext), cross-domain decrypt raises `InvalidToken` +- `test_load_provider_config`: DB integration test skipped without `INTEGRATION=1` (psycopg guard) + +**backend/tests/test_classifier.py** updated: +- `test_per_user_provider`: captures `ProviderConfig` instead of dict; asserts `config.provider_id == "openai"` and `config.model == "gpt-4o"` (D-06 contract enforced in tests) +- `test_default_provider_fallback`: patches `load_provider_config` to return None; captures ProviderConfig; asserts `config.provider_id == "ollama"` (env fallback path) + +## Verification Results + +- `grep -v '^#' backend/ai/anthropic_provider.py | grep -c 'MAX_AI_CHARS'` → 0 +- `grep -c 'output_config=' backend/ai/anthropic_provider.py` → 3 (classify, suggest_topics, and schema comment) +- `grep -c '_ProviderConfigStub' backend/services/ai_config.py` → 0 +- `grep -c 'await load_provider_config(' backend/services/classifier.py` → 2 (one per function) +- Full test suite: **1 failed** (pre-existing test_extract_docx ModuleNotFoundError), **366 passed**, **12 xfailed**, **6 skipped** — no new failures; xfailed count down by 3 from wave 2 + +## Deviations from Plan + +### Auto-fixed Issues + +**1. [Rule 1 - Bug] Classifier tests used dict-based interface** +- **Found during:** Task 2 (test_per_user_provider and test_default_provider_fallback asserted dict fields) +- **Issue:** After the refactor, `get_provider` receives a `ProviderConfig` object. The existing tests captured the dict argument and asserted `settings.get("active_provider")` etc. These would fail with `AttributeError` or wrong assertions on a ProviderConfig object. +- **Fix:** Updated `test_per_user_provider` to assert `config.provider_id == "openai"` and `config.model == "gpt-4o"`. Updated `test_default_provider_fallback` to patch `load_provider_config` returning None and assert `config.provider_id == "ollama"`. +- **Files modified:** backend/tests/test_classifier.py +- **Commit:** 95c386f + +**2. [Rule 2 - Missing functionality] `seed_system_settings_from_env` used hardcoded context_chars=8000** +- **Found during:** Task 2 code review +- **Issue:** The original seed function always inserted `context_chars=8000` regardless of provider, which would insert the wrong default for providers like Anthropic (180,000) or Groq (128,000). +- **Fix:** Updated seed to use `PROVIDER_DEFAULTS.get(provider_id, {}).get("context_chars", 8000)` so the seeded row reflects the correct default for each provider. +- **Files modified:** backend/services/ai_config.py +- **Commit:** 95c386f + +## Known Stubs + +None — all plan goals implemented; no placeholders. + +## Threat Flags + +No new threat surface introduced. Changes are internal service-layer refactoring: +- T-07-06 (per-user api_key isolation) mitigated: classifier per-user override path uses `api_key=""` — documented in comment +- T-07-08 (Anthropic stop_reason degradation) mitigated: classify() falls back to `parse_classification("")` when `stop_reason != "end_turn"` + +## Self-Check: PASSED + +Files modified: +- [x] backend/ai/anthropic_provider.py — FOUND (singleton _client, output_config, _truncate, no MAX_AI_CHARS) +- [x] backend/ai/__init__.py — FOUND (context_chars + base_url passed to AnthropicProvider) +- [x] backend/services/ai_config.py — FOUND (ProviderConfig import at module level, no _ProviderConfigStub) +- [x] backend/services/classifier.py — FOUND (load_provider_config + ProviderConfig construction) +- [x] backend/tests/test_ai_providers.py — FOUND (test_anthropic_structured_output promoted) +- [x] backend/tests/test_ai_config.py — FOUND (test_api_key_encrypt_decrypt promoted) +- [x] backend/tests/test_classifier.py — FOUND (per_user_provider + default_fallback tests updated) + +Commits: +- [x] efc177a — feat(07-03): AnthropicProvider singleton + output_config + truncation — D-03/D-07/D-12/D-13 +- [x] 95c386f — feat(07-03): classifier wired to load_provider_config + ai_config stub removed — D-04/D-06