fix: warn when [[providers.local]] discards its fields

A malformed local entry was the one skip that said nothing, on the
reasoning that local still ends up working. It does, but the merge
rebuilds it from the bare base_url, so an api_key, filter or ctx_size
set on that entry is dropped without a word. The app then comes up
looking healthy, which is the strongest possible signal that nothing is
wrong, making this the case that most needs saying, not least.

Local surviving at all depends on DEFAULTS supplying base_url, since
that is what the rebuild reads. Noted in config.py so the key is not
removed as redundant, and pinned by a test.

The not-a-table wording now follows what was written: a list gets the
single-bracket fix by name, while a scalar entry does not, since telling
someone to fix a [[...]] they never typed points at the wrong line.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0176tzAW6H1i2Kz8vm2XXVGV

4 files changed, 125 insertions, 5 deletions

diff --git a/docs/superpowers/plans/2026-08-09-external-providers.md b/docs/superpowers/plans/2026-08-09-external-providers.md
index 4a428a1..3fd1e2f 100644
--- a/docs/superpowers/plans/2026-08-09-external-providers.md
+++ b/docs/superpowers/plans/2026-08-09-external-providers.md
@@ -177,6 +177,12 @@ Expected: FAIL with `ModuleNotFoundError: No module named 'llamachat.providers'`
 Create `llamachat/providers.py`. Copy the 14-line GPL header verbatim from the top
 of `llamachat/config.py`, then:
 
+> **This block is the state at the end of Task 1, not the final module.**
+> Tasks 2, 3 and 4b all amend `parse()` in place. In particular, `parse()` as
+> written here crashes on several malformed config shapes; Task 4b replaces
+> its coercion and loop guards. Read top-down and implement in order, but do
+> not copy this block as the finished function.
+
 ```python
 """Provider definitions, model-id namespacing and API key resolution."""
 
@@ -905,6 +911,10 @@ merge before the loop is reached (`[[providers.local]]`). The bracket slip is
 the one that matters most: `[[providers.together]]` for `[providers.together]`
 is an easy miscount and reads as a list of tables.
 
+`[[providers.local]]` is the worst of the eight, and the easiest to
+under-rate. It is the only one where the app comes up looking healthy, so it
+needs the loudest warning rather than the quietest. See Step 3.
+
 Note the first group raises two different exception types, `ValueError` for a
 string or list and `TypeError` for a number. That is why the guard is an
 isinstance test rather than a try/except.
@@ -921,6 +931,12 @@ asserting that every shape above leaves a working local provider, that a
 `None` entry is skipped, and that each skip names the provider on stderr
 (capture it with `contextlib.redirect_stderr`).
 
+For `[[providers.local]]`, pin the loss and not only the survival: assert the
+warning is emitted, that `base_url` falls back to the bare one, and that
+`api_key`, `filter` and `ctx_size` set on that entry are discarded. Pin the
+message wording both ways too, that a list case says `not [[providers.x]]`
+and a scalar case does not mention brackets at all.
+
 Add `test_unusable_config_exits()`, which runs the real entry point in a
 child process. It must be a subprocess: `CONFIG_PATH` is read from the
 environment at import time and bound into `load()`'s default argument, so
@@ -985,15 +1001,35 @@ Replace the table coercion and the local merge at the top of `parse()`:
 
 and, in the merge below it, read the local entry through `isinstance` rather
 than truthiness, because `[[providers.local]]` is a truthy list that would
-raise here before the loop could skip it:
+raise here before the loop could skip it. Warn at this site too: the loop
+never sees this entry, because the merge replaces it:
 
 ```python
     bare = values.get("base_url")
-    local = table.get(LOCAL) if isinstance(table.get(LOCAL), dict) else {}
+    raw_local = table.get(LOCAL)
+    if raw_local is not None and not isinstance(raw_local, dict):
+        # Warned about here rather than left to the loop, which never sees it:
+        # the merge below replaces it with a synthesized entry, so every field
+        # the user set on it is dropped. That makes this the case where saying
+        # something matters most, not least. The app comes up working, local
+        # answers, and an api_key or ctx_size they set is simply gone, with a
+        # healthy-looking window as the only feedback.
+        _skipped(LOCAL, _not_a_table(LOCAL, raw_local))
+    local = raw_local if isinstance(raw_local, dict) else {}
     if bare and not local.get("base_url"):
         table[LOCAL] = {**local, "base_url": bare}
 ```
 
+This one is easy to talk yourself out of, since local still works afterwards.
+It is the case that most needs the warning: a working app is the strongest
+possible signal that nothing is wrong, so the discarded `api_key` or
+`ctx_size` has nothing else to announce it.
+
+Note also that local only survives because `config.DEFAULTS` always supplies
+`base_url` for the rebuild. Without a bare URL, `[[providers.local]]` yields
+no local provider at all. Add a comment in `config.py` saying that key is
+load-bearing, so nobody removes it as redundant.
+
 In the loop, replace the `entry = entry or {}` guard, which caught falsy
 values but not a truthy non-dict. The isinstance test subsumes the `None`
 case, so nothing is lost by dropping it:
@@ -1005,7 +1041,7 @@ case, so nothing is lost by dropping it:
             # Most likely [[providers.x]] written for [providers.x], which
             # TOML reads as a list of tables. Skipping keeps every other
             # provider working, including local.
-            _skipped(name, "it is not a [providers.<name>] table")
+            _skipped(name, _not_a_table(name, entry))
             continue
 ```
 
@@ -1013,6 +1049,18 @@ Give the two existing skips the same voice, naming what was wrong:
 `_skipped(name, "a provider name cannot be empty or contain ':'")` and
 `_skipped(name, "it has no base_url")`.
 
+The wording of the not-a-table message depends on what was written, so it
+lives in its own helper. A list is the double-bracket slip and should name the
+single-bracket fix; anything else was written as `x = 5`, where pointing at a
+bracket the user never typed would send them to fix the wrong line:
+
+```python
+def _not_a_table(name: str, entry) -> str:
+    if isinstance(entry, list):
+        return f"use [providers.{name}], not [[providers.{name}]]"
+    return f"a provider must be a [providers.{name}] table"
+```
+
 - [ ] **Step 4: Fix `__main__.py`**
 
 Wrap the `config.load()` call in `main()`:
diff --git a/llamachat/config.py b/llamachat/config.py
index ac53823..8fa4933 100644
--- a/llamachat/config.py
+++ b/llamachat/config.py
@@ -126,6 +126,13 @@ def load(path: Path = CONFIG_PATH) -> Config:
     # Providers are built from the raw values so a bare base_url still
     # synthesizes the local entry. DEFAULTS supplies base_url when the file
     # names neither, which keeps a config with no network settings working.
+    #
+    # That default is load-bearing, not a convenience: it is the only reason
+    # a malformed [providers.local] still leaves a working local provider,
+    # since parse() rebuilds local from the bare URL after skipping it.
+    # Dropping base_url from DEFAULTS, or letting a caller reach parse()
+    # without one, would turn every such config into an app with no local
+    # provider at all. Keep the key.
     provider_table = providers_mod.parse(values)
 
     return Config(
diff --git a/llamachat/providers.py b/llamachat/providers.py
index 52a1132..c31e82e 100644
--- a/llamachat/providers.py
+++ b/llamachat/providers.py
@@ -59,6 +59,20 @@ def _number(raw, cast):
         return None
 
 
+def _not_a_table(name: str, entry) -> str:
+    """Why this entry is not a provider table, phrased for what was written.
+
+    A list is almost always the [[providers.x]] double-bracket slip, so the
+    message names the single-bracket fix directly: "is not a table" alone does
+    not tell the user which character to change. Any other scalar was written
+    as `x = 5`, where suggesting a bracket fix would point at a line the user
+    never wrote, so that case just describes the shape.
+    """
+    if isinstance(entry, list):
+        return f"use [providers.{name}], not [[providers.{name}]]"
+    return f"a provider must be a [providers.{name}] table"
+
+
 def _skipped(name: str, reason: str) -> None:
     """Say that a provider was dropped, so the loss is not silent.
 
@@ -98,7 +112,16 @@ def parse(values: dict) -> dict[str, Provider]:
     # [[providers.local]] is a truthy list, and merging into it would raise
     # here, before the loop below ever gets a chance to skip it.
     bare = values.get("base_url")
-    local = table.get(LOCAL) if isinstance(table.get(LOCAL), dict) else {}
+    raw_local = table.get(LOCAL)
+    if raw_local is not None and not isinstance(raw_local, dict):
+        # Warned about here rather than left to the loop, which never sees it:
+        # the merge below replaces it with a synthesized entry, so every field
+        # the user set on it is dropped. That makes this the case where saying
+        # something matters most, not least. The app comes up working, local
+        # answers, and an api_key or ctx_size they set is simply gone, with a
+        # healthy-looking window as the only feedback.
+        _skipped(LOCAL, _not_a_table(LOCAL, raw_local))
+    local = raw_local if isinstance(raw_local, dict) else {}
     if bare and not local.get("base_url"):
         table[LOCAL] = {**local, "base_url": bare}
 
@@ -109,7 +132,7 @@ def parse(values: dict) -> dict[str, Provider]:
             # Most likely [[providers.x]] written for [providers.x], which
             # TOML reads as a list of tables. Skipping keeps every other
             # provider working, including local.
-            _skipped(name, "it is not a [providers.<name>] table")
+            _skipped(name, _not_a_table(name, entry))
             continue
         if not name or ":" in name:
             # The id scheme splits on the first colon, so a name containing
diff --git a/test_llamachat.py b/test_llamachat.py
index ed50995..29f305f 100755
--- a/test_llamachat.py
+++ b/test_llamachat.py
@@ -1054,6 +1054,48 @@ def test_provider_malformed_shapes():
     messages = err.getvalue()
     assert "listy" in messages and "urlless" in messages and "a:b" in messages
     assert "base_url" in messages  # the missing-URL case names what is missing
+    # A list is the double-bracket slip, so the message names the fix. Any
+    # other scalar was not written that way, and must not be told to change a
+    # bracket it never had.
+    assert "not [[providers.listy]]" in messages
+    scalar = io.StringIO()
+    with redirect_stderr(scalar):
+        providers.parse({"providers": {"n": 5}})
+    assert "[[" not in scalar.getvalue(), scalar.getvalue()
+
+    # A malformed [[providers.local]] is the loudest case that needs saying,
+    # not the quietest: the merge below it rebuilds local from the bare URL,
+    # so the app comes up working and every field the user set is discarded
+    # silently. The skip is reported at the merge site because the loop never
+    # sees this entry.
+    err = io.StringIO()
+    with redirect_stderr(err):
+        clobbered = providers.parse(
+            {
+                "base_url": "http://localhost:8181",
+                "providers": {"local": [{"api_key": "env:SOME_VAR",
+                                         "filter": ["qwen"],
+                                         "ctx_size": 32768}]},
+            }
+        )
+    assert "local" in err.getvalue()
+    assert "not [[providers.local]]" in err.getvalue()
+    # The local provider survives, which is the non-negotiable.
+    assert clobbered["local"].base_url == "http://localhost:8181"
+    # Pinning the loss rather than only the survival: these fields are gone,
+    # and the warning above is the only thing that tells the user so.
+    assert clobbered["local"].api_key == ""
+    assert clobbered["local"].filter == []
+    assert clobbered["local"].ctx_size is None
+
+    # Without a bare base_url there is nothing to rebuild local from, so it
+    # vanishes entirely. config.DEFAULTS always supplies one, which is what
+    # keeps the real app safe; this pins that the safety net is that default
+    # and not something parse() does on its own.
+    assert providers.parse(
+        {"providers": {"local": [{"base_url": "http://y.example.org"}]}}
+    ) == {}
+    assert "base_url" in config.DEFAULTS
     print("ok  malformed provider shapes are skipped, not raised")