chore: merge executor worktree (wave3-recovery/07-03)
This commit is contained in:
@@ -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
|
||||
Reference in New Issue
Block a user