# Claude Review: PR #1398

- PR: https://github.com/KestrelSovereignAI/kestrel-sovereign/pull/1398
- Reviewed: 2026-05-26T16:45:12Z

## PR #1398 Review: LLM Provider Capabilities

### No blocking findings.

This is a clean, additive PR. The SDK contract usage is correct, adapter declarations are consistent, and the test matrix covers all in-tree adapters. A few residual items:

---

### Residual risks (non-blocking)

**1. SDK floor bump to 0.17.0 with no PyPI release yet** — `pyproject.toml:47`. The PR body acknowledges SDK 0.17.0 isn't published. Anyone installing from PyPI (CI, fresh clones) will fail until SDK PR #24 lands. This is fine for stacking but the merge order matters: SDK must release first.

**2. OpenRouter inherits `supports_vision=True` from OpenAI parent but marks vision as model-dependent** — `openrouter_adapter.py:63-71`. The `model_dependent=("tools", "vision", "structured_output")` tuple correctly signals this is conditional, but the boolean `supports_vision` stays `True` (inherited). Consumers that only check the boolean without consulting `model_dependent` will overestimate. This is a design trade-off documented in the architecture doc, not a bug.

**3. Test matrix tuple positional fragility** — `tests/unit/test_llm_provider_capabilities.py:108-170`. The 6-element tuples rely on positional unpacking. A field addition silently shifts assertions. Low risk (test-only, currently correct), but a named structure would be more resilient.

**4. `_convert_providers_format` re-calls `adapter.provider_capabilities()` as fallback** — `service.py:813`. If `provider.capabilities` is `None` (shouldn't happen for in-tree routes after this PR, but possible for stale plugin `ProviderInfo` objects), it falls back to calling the method again. Safe since returns are static/frozen, but worth knowing if you later cache or measure.

**5. Entrypoint discovery equality check** — `provider_registry.py:435`:
```python
if getattr(info, "capabilities", None) == ProviderCapabilities():
```
This only backfills capabilities when the existing value equals the default instance. If a plugin sets *any* non-default field, this skips the backfill — correct behavior, but subtle. A comment explaining the intent would help future readers.

**6. `notes` field is a tuple of strings** — All adapters use `notes=(...)` tuples. The `to_dict()` serializes to `list`. Consistent, but callers consuming the raw dataclass get tuples while API consumers get lists. Fine for JSON serialization, just a shape asymmetry to be aware of.

### Verdict

Ship it. The design is sound — SDK owns the contract, core adapters declare capabilities, the dict bridge handles plugins, and the test matrix is comprehensive. Merge after SDK 0.17.0 hits PyPI.
