Skip to content

[MISC] Fix adapter-ops skill cost-tracking guidance and stale paths #2163

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
chandrasekharan-zipstack wants to merge 1 commit into
base: main
Choose a base branch
from
+166 −92
Open

[MISC] Fix adapter-ops skill cost-tracking guidance and stale paths #2163

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 83 additions & 40 deletions .claude/skills/adapter-ops/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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" \
Expand Down Expand Up @@ -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" \
Expand Down Expand Up @@ -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 \
Expand Down Expand Up @@ -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
Comment on lines +241 to +251

@coderabbitai coderabbitai Bot Jul 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Make --json output valid JSON.

check_adapter_updates.py prints SDK1 Adapters Path: ... before json.dumps(...), so the documented --json command cannot be parsed by jq or other automation. Send the diagnostic to stderr or suppress it in JSON mode.

🧰 Tools
🪛 SkillSpector (2.3.7)

[warning] 74: [PE2] Sudo/Root Execution: Commands invoke sudo or root privileges. Verify this elevated access is necessary and justified.

Remediation: Avoid sudo/root unless strictly required. Prefer least-privilege patterns. If elevation is needed, document the justification and scope.

(Privilege Escalation (PE2))

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.claude/skills/adapter-ops/SKILL.md around lines 241 - 251, Update
check_adapter_updates.py so its --json mode emits only the json.dumps(...)
payload on stdout; route the “SDK1 Adapters Path” diagnostic to stderr or
suppress it when JSON output is enabled, while preserving the diagnostic for
normal output.

```

2. **Review the report**:
Expand Down Expand Up @@ -278,51 +278,94 @@ 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
- [ ] `validate_model()` method handles prefix idempotently (doesn't double-prefix)
- [ ] 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 |

@coderabbitai coderabbitai Bot Jul 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Add the required blank line before the table.

The Common provider names label is immediately followed by the table, triggering markdownlint MD058.

🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 359-359: Tables should be surrounded by blank lines

(MD058, blanks-around-tables)

🪛 SkillSpector (2.3.7)

[warning] 74: [PE2] Sudo/Root Execution: Commands invoke sudo or root privileges. Verify this elevated access is necessary and justified.

Remediation: Avoid sudo/root unless strictly required. Prefer least-privilege patterns. If elevation is needed, document the justification and scope.

(Privilege Escalation (PE2))

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.claude/skills/adapter-ops/SKILL.md at line 359, Insert a blank line between
the “Common provider names” label and the table beginning with “Display Name” to
satisfy markdownlint MD058.

Source: Linters/SAST tools

|--------------|------------------------|
| 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

Expand All @@ -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:
Expand Down
104 changes: 66 additions & 38 deletions .claude/skills/adapter-ops/references/adapter_patterns.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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.
23 changes: 15 additions & 8 deletions .claude/skills/adapter-ops/references/provider_capabilities.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment on lines +151 to +165

@coderabbitai coderabbitai Bot Jul 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Remove the stale upstream pricing verification.

This file still directs readers to curl LiteLLM’s main pricing JSON at Lines [143-145], while the new guidance requires checking the pinned sdk1 environment. That can validate a prefix against data different from the version used in production.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.claude/skills/adapter-ops/references/provider_capabilities.md around lines
151 - 165, Remove the instructions directing readers to curl LiteLLM’s upstream
main pricing JSON near the pricing verification guidance. Replace them with
instructions to validate model-prefix pricing against the pinned sdk1
environment and its installed LiteLLM data, keeping the guidance consistent with
production dependencies.


## Models Supporting Advanced Features

Expand Down
4 changes: 1 addition & 3 deletions .claude/skills/adapter-ops/scripts/init_embedding_adapter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
4 changes: 1 addition & 3 deletions .claude/skills/adapter-ops/scripts/init_llm_adapter.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
Loading