aboutsummaryrefslogtreecommitdiffstats
path: root/docs/superpowers/plans
diff options
context:
space:
mode:
Diffstat (limited to 'docs/superpowers/plans')
-rw-r--r--docs/superpowers/plans/2026-08-09-external-providers.md66
1 files changed, 65 insertions, 1 deletions
diff --git a/docs/superpowers/plans/2026-08-09-external-providers.md b/docs/superpowers/plans/2026-08-09-external-providers.md
index 0c8fc19..72db023 100644
--- a/docs/superpowers/plans/2026-08-09-external-providers.md
+++ b/docs/superpowers/plans/2026-08-09-external-providers.md
@@ -332,6 +332,8 @@ def test_model_ids_and_filtering():
assert providers.split("weird:name", table) == ("local", "weird:name")
# Only the first colon splits.
assert providers.split("together:a:b", table) == ("together", "a:b")
+ # Task 9 addresses a provider itself with an empty model name.
+ assert providers.split("together:", table) == ("together", "")
# Filtering is case-insensitive substring, any match wins.
listed = [
@@ -353,11 +355,41 @@ def test_model_ids_and_filtering():
# A filter matching nothing yields nothing, it does not fall back to all.
table["together"].filter = ["zzz"]
assert providers.apply_filter(table["together"], listed) == []
+ # An empty needle is a typo rather than a request to hide everything, so
+ # it means no filter. The opposite of the "zzz" case above, deliberately.
+ table["together"].filter = [""]
+ assert providers.apply_filter(table["together"], listed) == listed
print("ok model ids and filtering")
```
Register it after `test_provider_parsing()` in the `__main__` block.
+Task 2 also closes three malformed-config shapes found reviewing Task 1, by
+adding to `test_provider_parsing()`:
+
+```python
+ # Any other non-list shape is wrapped too, rather than iterated: a number
+ # would raise, and a dict would silently degrade into its keys.
+ odd = providers.parse(
+ {"providers": {"n": {"base_url": "http://x.example.org", "filter": 5},
+ "d": {"base_url": "http://y.example.org",
+ "filter": {"a": 1}}}}
+ )
+ assert odd["n"].filter == ["5"]
+ # The invariant is that the dict was wrapped whole, not iterated into its
+ # keys. Asserting that rather than its repr, which is not ours to pin.
+ assert odd["d"].filter != ["a"] and len(odd["d"].filter) == 1
+
+ # A colon in a provider name would make every id built from it ambiguous,
+ # so such a provider is skipped rather than silently routed to local.
+ colonic = providers.parse(
+ {"providers": {"local": {"base_url": "http://x.example.org"},
+ "a:b": {"base_url": "http://y.example.org"},
+ "": {"base_url": "http://z.example.org"}}}
+ )
+ assert set(colonic) == {"local"}
+```
+
- [ ] **Step 2: Run the suite to verify it fails**
Run: `./test_llamachat.py`
@@ -365,7 +397,26 @@ Expected: FAIL with `AttributeError: module 'llamachat.providers' has no attribu
- [ ] **Step 3: Write the implementation**
-Append to `llamachat/providers.py`:
+In `parse()`, broaden the filter guard so any non-list shape is wrapped as one
+needle rather than iterated, and skip a provider whose name is unusable:
+
+```python
+ name = str(name)
+ if not name or ":" in name:
+ # The id scheme splits on the first colon, so a name containing
+ # one builds ids that split back to something else entirely, and
+ # an empty name builds ":model". Both route to local under a
+ # nonsense name, silently. Skipping is the only honest option.
+ continue
+```
+
+```python
+ needles = entry.get("filter") or []
+ if not isinstance(needles, (list, tuple)):
+ needles = [needles]
+```
+
+Then append to `llamachat/providers.py`:
```python
def qualify(provider: str, model: str) -> str:
@@ -381,6 +432,14 @@ def split(model_id: str, table: dict[str, Provider]) -> tuple[str, str]:
A prefix that is not a configured provider is treated as part of the
model name, which keeps a bare local model containing a colon working.
"""
+ # ponytail: this is not injective, and cannot be while local ids stay
+ # bare. A local model actually named "together:x" is indistinguishable
+ # from together's model "x", so it resolves to local only until the user
+ # configures a provider called "together", at which point the stored
+ # session id rebinds to the cloud model and starts billing without a
+ # word. Unlikely, since local names are usually filenames, and the bare
+ # local id is the feature's premise. The upgrade path is to qualify local
+ # as "local:<model>" too and migrate the sessions.model column.
prefix, sep, rest = model_id.partition(":")
if sep and prefix in table and prefix != LOCAL:
return prefix, rest
@@ -393,8 +452,13 @@ def apply_filter(provider: Provider, listed: list[str]) -> list[str]:
Case-insensitive, because provider ids capitalise inconsistently:
"qwen" has to match "Qwen/Qwen2.5-72B-Instruct-Turbo".
"""
+ # Every path returns a fresh list on purpose: callers hold onto the
+ # result, and handing back "listed" itself would let them mutate the
+ # caller's own list. Not a redundant copy, do not simplify it away.
if provider.is_local or not provider.filter:
return list(listed)
+ # An empty needle is a typo, not a request to hide every model, so a
+ # filter of only empty strings means no filter.
needles = [f.lower() for f in provider.filter if f]
if not needles:
return list(listed)