180 lines
8.3 KiB
Markdown
180 lines
8.3 KiB
Markdown
# Phase 7: Redo and Optimize LLM Integration - Discussion Log
|
|
|
|
> **Audit trail only.** Do not use as input to planning, research, or execution agents.
|
|
> Decisions are captured in CONTEXT.md — this log preserves the alternatives considered.
|
|
|
|
**Date:** 2026-06-02
|
|
**Phase:** 07-redo-and-optimize-llm-integration
|
|
**Areas discussed:** Structured Output, Provider Factory Refactor, Retry & Resilience, Context Window Strategy, LMStudio Fix, New Providers
|
|
|
|
---
|
|
|
|
## Structured Output
|
|
|
|
| Option | Description | Selected |
|
|
|--------|-------------|----------|
|
|
| JSON mode everywhere | All OpenAI-compat providers use `response_format={"type":"json_object"}`. Anthropic uses native JSON output. Keep regex parser as fallback only. | ✓ |
|
|
| JSON schema / tool_use | Define strict Pydantic schema, use OpenAI/Anthropic tool_use. Guaranteed conformance but local models hit-or-miss. | |
|
|
| Keep current regex parsing | Improve error handling but keep current fragile approach. | |
|
|
|
|
**User's choice:** JSON mode everywhere
|
|
**Notes:** None.
|
|
|
|
### Anthropic strategy sub-question
|
|
|
|
| Option | Description | Selected |
|
|
|--------|-------------|----------|
|
|
| Native tool_use schema | Define ClassificationResult as Anthropic tool. Schema-guaranteed, no parsing needed. | |
|
|
| Prompt-enforced JSON mode | Explicit system prompt + strip fences. Simpler but not schema-enforced. | |
|
|
| You decide | Leave Anthropic strategy to researcher/planner. | ✓ |
|
|
|
|
**User's choice:** You decide
|
|
**Notes:** Researcher should pick the most reliable option consistent with the JSON-mode-everywhere principle.
|
|
|
|
---
|
|
|
|
## Provider Factory Refactor
|
|
|
|
### Client lifecycle sub-question
|
|
|
|
**User's choice:** Free text — "I want you to pick the best method for each provider. Especially when adding new providers which can happen after you research it but even later down the road, new providers can be added and the best method for each provider needs to be elaborated."
|
|
**Notes:** Researcher must investigate and document optimal client lifecycle per provider type so that the pattern is clear for future additions. This is captured as a Claude's Discretion item (D-07).
|
|
|
|
### Configuration storage sub-question
|
|
|
|
| Option | Description | Selected |
|
|
|--------|-------------|----------|
|
|
| Pydantic ProviderConfig models | Typed config models; `get_provider()` accepts ProviderConfig not raw dict. | ✓ (implied) |
|
|
| Keep raw dict with validation | Add Pydantic step before passing dict. Less refactoring. | |
|
|
| Move config into Settings | All provider config in `config.py` Settings env-var driven. | |
|
|
|
|
**User's choice:** Free text — "I want all settings inside the admin settings page and only default values inside any .env or other files. I want the settings to be as interactive as possible."
|
|
**Notes:** This means a new `system_settings` DB table storing all AI provider configuration. Admin panel gets an AI Providers section. Env vars serve as startup defaults only.
|
|
|
|
### DB storage sub-question
|
|
|
|
| Option | Description | Selected |
|
|
|--------|-------------|----------|
|
|
| New system_settings table in DB | Key-value/JSON config table, admin writes via API. API keys encrypted. Multi-instance safe. | ✓ |
|
|
| Extend existing users table / per-user only | Keep settings purely per-user. No global defaults. | |
|
|
| JSON config file on disk | Admin writes JSON file mounted into container. Breaks multi-instance. | |
|
|
|
|
**User's choice:** New system_settings table in DB
|
|
**Notes:** None.
|
|
|
|
---
|
|
|
|
## Retry & Resilience
|
|
|
|
| Option | Description | Selected |
|
|
|--------|-------------|----------|
|
|
| Celery task auto-retry with backoff | Raise retryable exception; 3x retry at 30s/90s/270s. Uses existing Celery infra. | ✓ |
|
|
| tenacity in-process retry | Wrap provider calls in tenacity. Blocks worker thread during wait. | |
|
|
| No retry — better error logging | Fail-fast but structured logs. User manually re-triggers. | |
|
|
|
|
**User's choice:** Celery task auto-retry with backoff
|
|
**Notes:** None.
|
|
|
|
### User visibility sub-question
|
|
|
|
| Option | Description | Selected |
|
|
|--------|-------------|----------|
|
|
| Show status badge + manual re-classify button | Document card shows 'Classification failed' badge + Re-analyze button calls POST /api/documents/{id}/classify | ✓ |
|
|
| Silent auto-retry only | Only Celery retries. No UI feedback if all fail. | |
|
|
| Already exists | Re-analyze button already in DocumentView. | |
|
|
|
|
**User's choice:** Show status badge + manual re-classify button
|
|
**Notes:** None.
|
|
|
|
---
|
|
|
|
## Context Window Strategy
|
|
|
|
| Option | Description | Selected |
|
|
|--------|-------------|----------|
|
|
| Per-provider token limit + smart truncation | Each provider declares context size. Truncation: first 60% + last 40% of limit. MAX_AI_CHARS becomes per-provider. | ✓ |
|
|
| Chunk + merge (multi-call) | Split doc into N chunks, classify each, merge. More accurate but 2-5x API calls. | |
|
|
| Keep 8,000 char limit globally | Keep current behavior. Simple but inaccurate for long docs. | |
|
|
|
|
**User's choice:** Per-provider token limit + smart truncation
|
|
**Notes:** None.
|
|
|
|
---
|
|
|
|
## LMStudio Fix
|
|
|
|
### Failure mode sub-question
|
|
|
|
| Option | Description | Selected |
|
|
|--------|-------------|----------|
|
|
| Classification returns empty/wrong topics | API call succeeds but model returns non-JSON. Fixed by JSON mode. | |
|
|
| Connection refused / not reachable | Docker can't reach host.docker.internal:1234. | |
|
|
| Model name mismatch | Hardcoded 'gemma-4-e4b-it' doesn't match loaded model. | |
|
|
| Other / not sure | Leave to researcher. | |
|
|
|
|
**User's choice:** "Classification failed: Connection error."
|
|
**Notes:** Connection error is the reported symptom.
|
|
|
|
### OS sub-question
|
|
|
|
| Option | Description | Selected |
|
|
|--------|-------------|----------|
|
|
| macOS or Windows | host.docker.internal works automatically. | |
|
|
| Linux | Requires extra_hosts: ["host.docker.internal:host-gateway"] — almost certainly root cause. | ✓ |
|
|
| Both / uncertain | Fix for all OSes. | |
|
|
|
|
**User's choice:** Linux
|
|
**Notes:** Root cause confirmed. Fix: add `extra_hosts: ["host.docker.internal:host-gateway"]` to backend and celery-worker in docker-compose.yml.
|
|
|
|
---
|
|
|
|
## New Providers
|
|
|
|
### Which providers to add
|
|
|
|
| Option | Description | Selected |
|
|
|--------|-------------|----------|
|
|
| Google Gemini | OpenAI-compat endpoint, zero new deps. | ✓ |
|
|
| Mistral | Native SDK or OpenAI-compat. | ✓ |
|
|
| Groq | OpenAI-compat, fastest inference. | ✓ |
|
|
| xAI/Grok + DeepSeek + OpenRouter | All OpenAI-compat, just base_url + api_key. | ✓ |
|
|
|
|
**User's choice:** All four selected.
|
|
**Notes:** None.
|
|
|
|
### Implementation approach
|
|
|
|
| Option | Description | Selected |
|
|
|--------|-------------|----------|
|
|
| One GenericOpenAIProvider class | Single `GenericOpenAIProvider(base_url, api_key, model, context_chars)`. Named presets are factory shortcuts. | ✓ |
|
|
| Separate class per provider | Individual files for each. Explicit but repetitive. | |
|
|
| You decide | Leave to planner. | |
|
|
|
|
**User's choice:** One GenericOpenAIProvider class
|
|
**Notes:** All OpenAI-compat providers (Groq, Grok, DeepSeek, OpenRouter, Gemini-compat, Mistral-compat) use this one class.
|
|
|
|
### Mistral SDK vs compat
|
|
|
|
| Option | Description | Selected |
|
|
|--------|-------------|----------|
|
|
| OpenAI-compat (simpler) | Use GenericOpenAIProvider with Mistral base_url. Zero new dependencies. | ✓ |
|
|
| Native mistralai SDK | Full Mistral features (embeddings, FIM) but adds dependency. | |
|
|
| You decide | Leave to researcher. | |
|
|
|
|
**User's choice:** OpenAI-compat (simpler)
|
|
**Notes:** Native SDK can be added later if Mistral-specific features are needed.
|
|
|
|
---
|
|
|
|
## Claude's Discretion
|
|
|
|
- **Anthropic structured output strategy** — researcher to determine tool_use schema vs response_format vs prompt-enforcement. Must be consistent with JSON-mode-everywhere principle.
|
|
- **Client lifecycle per provider** — researcher to investigate and document optimal instantiation pattern (singleton, pooled, per-request) per provider type, such that future providers can follow the documented pattern without re-researching.
|
|
|
|
## Deferred Ideas
|
|
|
|
- Chunk + merge multi-call classification — 2-5x API cost; revisit when provider layer is stable.
|
|
- Native `mistralai` SDK — not needed for classification; revisit when embedding-based retrieval is added.
|
|
- Token counting (tiktoken or provider tokenizers) — per-provider `context_chars` approximation sufficient for now.
|
|
- Streaming classification — Celery background task makes streaming non-trivial without websockets.
|