diff --git a/.claude/skills/adapter-ops/SKILL.md b/.claude/skills/adapter-ops/SKILL.md index 521111db32..840d8741dc 100644 --- a/.claude/skills/adapter-ops/SKILL.md +++ b/.claude/skills/adapter-ops/SKILL.md @@ -56,7 +56,7 @@ LiteLLM requires provider prefixes on model names: 1. **Run initialization script**: ```bash - python .claude/skills/unstract-adapter-extension/scripts/init_llm_adapter.py \ + python .claude/skills/adapter-ops/scripts/init_llm_adapter.py \ --provider newprovider \ --name "New Provider" \ --description "New Provider LLM adapter" \ @@ -122,7 +122,7 @@ LiteLLM requires provider prefixes on model names: 1. **Run initialization script**: ```bash - python .claude/skills/unstract-adapter-extension/scripts/init_embedding_adapter.py \ + python .claude/skills/adapter-ops/scripts/init_embedding_adapter.py \ --provider newprovider \ --name "New Provider" \ --description "New Provider embedding adapter" \ @@ -190,7 +190,7 @@ LiteLLM requires provider prefixes on model names: 3. **Run management script** for automated updates: ```bash - python .claude/skills/unstract-adapter-extension/scripts/manage_models.py \ + python .claude/skills/adapter-ops/scripts/manage_models.py \ --adapter llm \ --provider openai \ --action add \ @@ -238,17 +238,17 @@ Compare existing adapter schemas against known LiteLLM features to identify pote 1. **Run the update checker**: ```bash # Check all adapters - python .claude/skills/unstract-adapter-extension/scripts/check_adapter_updates.py + python .claude/skills/adapter-ops/scripts/check_adapter_updates.py # Check specific adapter type - python .claude/skills/unstract-adapter-extension/scripts/check_adapter_updates.py --adapter llm - python .claude/skills/unstract-adapter-extension/scripts/check_adapter_updates.py --adapter embedding + python .claude/skills/adapter-ops/scripts/check_adapter_updates.py --adapter llm + python .claude/skills/adapter-ops/scripts/check_adapter_updates.py --adapter embedding # Check specific provider - python .claude/skills/unstract-adapter-extension/scripts/check_adapter_updates.py --provider openai + python .claude/skills/adapter-ops/scripts/check_adapter_updates.py --provider openai # Output as JSON - python .claude/skills/unstract-adapter-extension/scripts/check_adapter_updates.py --json + python .claude/skills/adapter-ops/scripts/check_adapter_updates.py --json ``` 2. **Review the report**: @@ -278,7 +278,8 @@ Before submitting adapter changes: - [ ] Adapter class inherits from correct parameter class AND `BaseAdapter` - [ ] `get_id()` returns unique `{provider}|{uuid}` format - [ ] `get_metadata()` returns dict with `name`, `version`, `adapter`, `description`, `is_active` -- [ ] **CRITICAL: `get_provider()` returns value matching `litellm_provider` in LiteLLM pricing data** (see below) +- [ ] `get_provider()` matches the static JSON filename (`static/{get_provider()}.json`) +- [ ] **CRITICAL: the model string produced by `validate_model()` resolves in LiteLLM's cost map** (see below) - [ ] `get_adapter_type()` returns correct `AdapterTypes.LLM` or `AdapterTypes.EMBEDDING` - [ ] JSON schema has `adapter_name` as required field - [ ] `validate()` method adds correct model prefix @@ -286,43 +287,85 @@ Before submitting adapter changes: - [ ] All static methods decorated with `@staticmethod` - [ ] Icon path follows pattern `/icons/adapter-icons/{Name}.png` -### Provider Name Verification (MANDATORY) +### Model Prefix Verification (MANDATORY) -The `get_provider()` return value is used for **cost calculation**. It MUST match the `litellm_provider` field in LiteLLM's pricing data, otherwise costs will show as $0. +Cost is looked up from the **validated model string**, not from `get_provider()`. The string +that `validate_model()` produces (e.g. `mistral/mistral-embed`) is passed straight to +`litellm.cost_per_token()`. If LiteLLM's cost map has no entry for it, the lookup raises, the +exception is swallowed, and usage records **$0**. -**Before implementing any adapter, verify the provider name:** +The lookup sites: -```bash -# Fetch LiteLLM pricing data and check provider name -curl -s https://raw.githubusercontent.com/BerriAI/litellm/main/model_prices_and_context_window.json | \ - jq 'to_entries | map(select(.value.litellm_provider != null)) | - map({key: .key, provider: .value.litellm_provider}) | - unique_by(.provider) | - sort_by(.provider) | - .[].provider' | sort -u -``` +| Path | Site | +|------|------| +| LLM | `unstract/sdk1/src/unstract/sdk1/audit.py` — `cost_per_token(model=model_name)` | +| Embedding | `unstract/sdk1/src/unstract/sdk1/usage_handler.py` — `litellm.cost_per_token(...)` | + +`model_name` is `self._cost_model or self.kwargs["model"]` — the prefixed string. Both call +sites catch every exception and fall back to `0.0`, so a miss is **silent**. It will not fail a +test, a build, or a run. The only symptom is revenue-affecting: zero-cost usage rows. + +**Before implementing any adapter, verify the prefix resolves.** Use the pinned LiteLLM in the +sdk1 venv rather than curling upstream JSON — the pinned version is what actually runs: -**Common provider name mappings:** -| Display Name | `get_provider()` Value | LiteLLM Provider | -|--------------|------------------------|------------------| -| OpenAI | `openai` | `openai` | -| Anthropic | `anthropic` | `anthropic` | -| Azure OpenAI | `azure` | `azure` | -| Azure AI Foundry | `azure_ai` | `azure_ai` | -| AWS Bedrock | `bedrock` | `bedrock` | -| Google VertexAI | `vertex_ai` | `vertex_ai` | -| Mistral | `mistral` | `mistral` | -| Ollama | `ollama` | `ollama` | - -**Example verification for Azure AI Foundry:** ```bash -curl -s https://raw.githubusercontent.com/BerriAI/litellm/main/model_prices_and_context_window.json | \ - jq 'to_entries | map(select(.key | startswith("azure_ai"))) | .[0].value.litellm_provider' -# Output: "azure_ai" (NOT "azure_ai_foundry") +cd unstract/sdk1 && uv run python -c " +import litellm +from litellm import cost_per_token + +# 1. Does LiteLLM have a native provider for this vendor? +print([p for p in litellm.provider_list if 'YOUR_VENDOR' in str(p).lower()]) + +# 2. Which of its models are priced? +print([k for k in litellm.model_cost if k.lower().startswith('YOUR_PROVIDER/')]) + +# 3. Does the string your validate_model() emits actually resolve? +for m in ['YOUR_PROVIDER/some-model', 'custom_openai/some-model']: + try: + print(m, cost_per_token(model=m, prompt_tokens=1_000_000, completion_tokens=1_000_000)) + except Exception as e: + print(m, 'RAISES', type(e).__name__, '=> cost silently recorded as 0.0') +" ``` -**Why this matters:** -The cost calculation in `platform-service/src/unstract/platform_service/helper/cost_calculation.py` filters models by checking if the provider string is contained in `litellm_provider`. A mismatch (e.g., returning `"azure_ai_foundry"` when LiteLLM uses `"azure_ai"`) causes the cost lookup to fail silently, returning $0. +**Branded OpenAI-compatible adapters: pick the right base class.** + +Many vendors speak the OpenAI wire protocol, which tempts you to subclass +`OpenAICompatibleLLMParameters` and just pin `api_base`. But its `validate_model()` +unconditionally prepends `custom_openai/`, and **nothing** in LiteLLM's cost map is keyed that +way. That silently forfeits cost tracking. + +| If… | Then | Cost tracking | +|-----|------|---------------| +| LiteLLM has a native provider for the vendor | Extend `BaseChatCompletionParameters`, emit `{provider}/{model}` — follow `OpenRouterLLMParameters` | ✅ resolves | +| LiteLLM has **no** priced models for the vendor | Extend `OpenAICompatibleLLMParameters`, pin `api_base` — follow `NvidiaBuildLLMParameters` | ⚠️ $0 either way, nothing forfeited | + +`NvidiaBuildLLMParameters` is only a safe template because LiteLLM prices zero `nvidia_nim/` +chat models — there is no cost to lose. Do not copy that shape for a vendor LiteLLM *does* +price. Check first with the snippet above. + +**What `get_provider()` is actually for:** + +1. Resolving the static schema path — `{type}1/static/{get_provider()}.json` (case-sensitive `open()`). +2. The `provider` column on the usage row (`audit.py`) — metadata only, not used for pricing. + +Keeping it equal to LiteLLM's `litellm_provider` value is still good hygiene, and matters when +you route natively (the prefix and the provider name coincide). But a correct `get_provider()` +does **not** on its own guarantee cost resolution — a branded adapter can return `"minimax"`, +match `litellm_provider` exactly, and still bill $0 because its model string carries the +`custom_openai/` prefix. + +**Common provider names:** +| Display Name | `get_provider()` Value | +|--------------|------------------------| +| OpenAI | `openai` | +| Anthropic | `anthropic` | +| Azure OpenAI | `azure` | +| Azure AI Foundry | `azure_ai` | +| AWS Bedrock | `bedrock` | +| Google VertexAI | `vertex_ai` | +| Mistral | `mistral` | +| Ollama | `ollama` | ## Maintenance Workflow @@ -332,7 +375,7 @@ Periodic maintenance to keep adapters current with LiteLLM features: 1. **Run the update checker**: ```bash - python .claude/skills/unstract-adapter-extension/scripts/check_adapter_updates.py + python .claude/skills/adapter-ops/scripts/check_adapter_updates.py ``` 2. **Review LiteLLM changelog** for new provider features: diff --git a/.claude/skills/adapter-ops/references/adapter_patterns.md b/.claude/skills/adapter-ops/references/adapter_patterns.md index a3b200335b..9bab1d5ea1 100644 --- a/.claude/skills/adapter-ops/references/adapter_patterns.md +++ b/.claude/skills/adapter-ops/references/adapter_patterns.md @@ -568,48 +568,87 @@ print(schema) 4. **Missing `is_active: True`** - Adapter won't be registered without it 5. **Wrong inheritance order** - Parameter class must come before BaseAdapter 6. **Incorrect provider in get_provider()** - Must match filename and schema path -7. **Provider name not matching LiteLLM pricing data** - Causes $0 cost calculation (see below) +7. **Model prefix not resolving in LiteLLM's cost map** - Causes silent $0 cost calculation (see below) -## Provider Name Verification (CRITICAL) +## Model Prefix Verification (CRITICAL) -The `get_provider()` return value MUST match the `litellm_provider` field in LiteLLM's pricing JSON. A mismatch causes cost calculation to silently fail, returning $0. +Cost is derived from the **validated model string**, not from `get_provider()`. Whatever +`validate_model()` emits is handed to `litellm.cost_per_token()`. No cost-map entry means the +call raises, the exception is swallowed, and the usage row records $0. + +### Cost Calculation Flow + +1. **Usage recording**: `LLM._record_usage()` logs tokens (no cost math here). +2. **Cost lookup**: + - LLM → `Audit.push_usage_data()` → `cost_per_token(model=model_name)` in `audit.py` + - Embedding → `UsageHandler` → `litellm.cost_per_token(...)` in `usage_handler.py` + - `model_name` is `self._cost_model or self.kwargs["model"]` — the **prefixed** string. +3. **Failure mode**: both call sites wrap the lookup in a bare `except` and fall back to + `cost_in_dollars = 0.0`, logged at `debug`/`warning`. + +The `provider` value travels alongside the usage payload and lands in the `provider` DB column. +It is **not** consulted for pricing. + +Because the failure is swallowed, nothing breaks loudly. Tests pass. Runs succeed. Usage rows +just quietly say $0. ### How to Verify -**Before implementing any new adapter:** +Query the pinned LiteLLM in the sdk1 venv — that's the version that actually runs: ```bash -# Step 1: Identify the correct provider name from LiteLLM pricing data -curl -s https://raw.githubusercontent.com/BerriAI/litellm/main/model_prices_and_context_window.json | \ - jq 'to_entries | map(select(.key | startswith("YOUR_PROVIDER"))) | .[0].value.litellm_provider' - -# Step 2: Use that exact value in get_provider() +cd unstract/sdk1 && uv run python -c " +import litellm +from litellm import cost_per_token + +print([p for p in litellm.provider_list if 'YOUR_VENDOR' in str(p).lower()]) +print([k for k in litellm.model_cost if k.lower().startswith('YOUR_PROVIDER/')]) + +for m in ['YOUR_PROVIDER/some-model', 'custom_openai/some-model']: + try: + print(m, cost_per_token(model=m, prompt_tokens=1_000_000, completion_tokens=1_000_000)) + except Exception: + print(m, 'RAISES => cost silently recorded as 0.0') +" ``` -### Real Example: Azure AI Foundry Bug +### Real Example: the `custom_openai/` trap -**Wrong implementation (caused $0 costs):** -```python -@staticmethod -def get_provider() -> str: - return "azure_ai_foundry" # WRONG - doesn't match litellm_provider -``` +A vendor speaking the OpenAI wire protocol invites subclassing +`OpenAICompatibleLLMParameters` and pinning `api_base`. That class's `validate_model()` +unconditionally prepends `custom_openai/`, and **no** LiteLLM cost-map key uses that prefix. -**Correct implementation:** ```python -@staticmethod -def get_provider() -> str: - return "azure_ai" # CORRECT - matches litellm_provider in pricing data +# Vendor has a native LiteLLM provider AND priced models. +# WRONG - emits "custom_openai/MiniMax-M3", which is not in the cost map -> $0 +class MiniMaxLLMParameters(OpenAICompatibleLLMParameters): + api_base: str = "https://api.minimax.io/v1" + +# CORRECT - emits "minimax/MiniMax-M3", priced at $0.30 / $1.20 per 1M tokens +class MiniMaxLLMParameters(BaseChatCompletionParameters): + ... # follow OpenRouterLLMParameters ``` -**How the bug was found:** -```bash -# Check what LiteLLM actually uses for azure_ai models -curl -s https://raw.githubusercontent.com/BerriAI/litellm/main/model_prices_and_context_window.json | \ - jq 'to_entries | map(select(.key | startswith("azure_ai"))) | .[0]' +Note that `get_provider()` returns `"minimax"` in *both* cases and matches `litellm_provider` +exactly. The provider string was never the problem — the model prefix was. -# Output shows: "litellm_provider": "azure_ai" (NOT "azure_ai_foundry") -``` +### Choosing a base class + +| If… | Then | Cost tracking | +|-----|------|---------------| +| LiteLLM has a native provider for the vendor | `BaseChatCompletionParameters`, emit `{provider}/{model}` — see `OpenRouterLLMParameters` | ✅ resolves | +| LiteLLM has **no** priced models for the vendor | `OpenAICompatibleLLMParameters`, pin `api_base` — see `NvidiaBuildLLMParameters` | ⚠️ $0 regardless, nothing forfeited | + +`NvidiaBuildLLMParameters` is a safe template *only because* LiteLLM prices zero `nvidia_nim/` +chat models. Don't copy its shape for a vendor LiteLLM prices — verify first. + +### What `get_provider()` is for + +1. Resolving the static schema path: `{type}1/static/{get_provider()}.json` (case-sensitive). +2. The `provider` column on the usage row — metadata only. + +Match it to LiteLLM's `litellm_provider` for hygiene and because it coincides with the prefix +when routing natively. But it does not, by itself, guarantee cost resolution. ### Provider Name Reference @@ -623,14 +662,3 @@ curl -s https://raw.githubusercontent.com/BerriAI/litellm/main/model_prices_and_ | Google VertexAI | `vertex_ai` | | Mistral | `mistral` | | Ollama | `ollama` | - -### Cost Calculation Flow - -Understanding why this matters: - -1. **Usage Recording**: `LLM._record_usage()` → `Audit.push_usage_data(provider=get_provider())` -2. **Platform Service**: Receives provider name with usage data -3. **Cost Lookup**: `CostCalculationHelper` checks `provider in model_info.get("litellm_provider", "")` -4. **Result**: If check fails, returns `cost = 0` - -The check `"azure_ai_foundry" in "azure_ai"` returns `False`, causing silent failure. diff --git a/.claude/skills/adapter-ops/references/provider_capabilities.md b/.claude/skills/adapter-ops/references/provider_capabilities.md index 4a004abae4..45ea99ee1b 100644 --- a/.claude/skills/adapter-ops/references/provider_capabilities.md +++ b/.claude/skills/adapter-ops/references/provider_capabilities.md @@ -148,14 +148,21 @@ curl -s https://raw.githubusercontent.com/BerriAI/litellm/main/model_prices_and_ ### Why This Matters The cost calculation flow: -1. `LLM._record_usage()` calls `Audit.push_usage_data()` with provider from `get_provider()` -2. Platform service receives usage data with provider name -3. `CostCalculationHelper.calculate_cost()` filters models where `provider in litellm_provider` -4. If no match found, cost = $0 - -**Example bug**: If `get_provider()` returns `"azure_ai_foundry"` but LiteLLM uses `"azure_ai"`: -- Check: `"azure_ai_foundry" in "azure_ai"` = `False` -- Result: Cost calculation returns $0 +1. `LLM._record_usage()` logs tokens — no cost math. +2. `Audit.push_usage_data()` (LLM) / `UsageHandler` (embedding) calls + `litellm.cost_per_token(model=model_name)`, where `model_name` is the **prefixed** model + string emitted by `validate_model()`. +3. No cost-map entry for that string → the call raises → the bare `except` records $0. + +The `provider` value from `get_provider()` rides along into the usage row's `provider` column +but is **not** used for pricing. + +**Example bug**: a branded OpenAI-compatible adapter emits `custom_openai/MiniMax-M3`. LiteLLM +prices `minimax/MiniMax-M3` but has no `custom_openai/` keys, so cost silently resolves to $0 — +even though `get_provider()` correctly returns `"minimax"`. + +See `references/adapter_patterns.md` → *Model Prefix Verification* for how to choose a base +class so the prefix resolves. ## Models Supporting Advanced Features diff --git a/.claude/skills/adapter-ops/scripts/init_embedding_adapter.py b/.claude/skills/adapter-ops/scripts/init_embedding_adapter.py index 0122c39ed7..9835d79233 100755 --- a/.claude/skills/adapter-ops/scripts/init_embedding_adapter.py +++ b/.claude/skills/adapter-ops/scripts/init_embedding_adapter.py @@ -25,9 +25,7 @@ SCRIPT_DIR = Path(__file__).parent SKILL_DIR = SCRIPT_DIR.parent # Find the sdk1 adapters directory relative to the repo root -REPO_ROOT = ( - SKILL_DIR.parent.parent.parent -) # .claude/skills/unstract-adapter-extension -> repo root +REPO_ROOT = SKILL_DIR.parent.parent.parent # .claude/skills/adapter-ops -> repo root SDK1_ADAPTERS = REPO_ROOT / "unstract" / "sdk1" / "src" / "unstract" / "sdk1" / "adapters" ICONS_DIR = REPO_ROOT / "frontend" / "public" / "icons" / "adapter-icons" diff --git a/.claude/skills/adapter-ops/scripts/init_llm_adapter.py b/.claude/skills/adapter-ops/scripts/init_llm_adapter.py index 8226f69eeb..828e04c3dc 100755 --- a/.claude/skills/adapter-ops/scripts/init_llm_adapter.py +++ b/.claude/skills/adapter-ops/scripts/init_llm_adapter.py @@ -26,9 +26,7 @@ SCRIPT_DIR = Path(__file__).parent SKILL_DIR = SCRIPT_DIR.parent # Find the sdk1 adapters directory relative to the repo root -REPO_ROOT = ( - SKILL_DIR.parent.parent.parent -) # .claude/skills/unstract-adapter-extension -> repo root +REPO_ROOT = SKILL_DIR.parent.parent.parent # .claude/skills/adapter-ops -> repo root SDK1_ADAPTERS = REPO_ROOT / "unstract" / "sdk1" / "src" / "unstract" / "sdk1" / "adapters" ICONS_DIR = REPO_ROOT / "frontend" / "public" / "icons" / "adapter-icons"