Skip to content

Commit b00b4f4

Browse files
committed
fix(07): WR-05 document _apply_section dict/skip filters + lock contract
The original "skip nested tables (hook/eval/cache/mcp) — handled by _apply_nested" comment in _apply_section was misleading: today none of the _NESTED_TABLES first segments collide with a non-dict ResolvedConfig attribute, so the inner skip_first_segments branch is unreachable. Document it as defense-in-depth for future scalar fields whose names collide with a nested table prefix. Add regression test test_flat_classifier_rooms_under_supamem_is_ignored to lock the documented behavior: a flat "classifier_rooms = { ... }" under [supamem] is intentionally dropped by the dict-typed filter, forcing users onto the canonical [supamem.classifier.rooms] nested-table shape.
1 parent ebfa402 commit b00b4f4

2 files changed

Lines changed: 48 additions & 4 deletions

File tree

src/supamem/config.py

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -183,13 +183,31 @@ def _apply_section(
183183
section: dict[str, Any],
184184
source: Source,
185185
) -> None:
186-
"""Apply flat field overrides from a [supamem] / [tool.supamem] section."""
187-
# For dotted nested keys (e.g. "mcp.caps") only the FIRST segment
188-
# ("mcp") ever appears as a top-level key in the TOML data — skip that.
186+
"""Apply flat field overrides from a [supamem] / [tool.supamem] section.
187+
188+
Notes on the two filters below:
189+
190+
* ``isinstance(getattr(cfg, key), dict)`` — already excludes dict-typed
191+
fields (e.g. ``classifier_rooms``). A user who writes a flat
192+
``classifier_rooms = { ... }`` under ``[supamem]`` (instead of the
193+
canonical nested ``[supamem.classifier.rooms]`` table) has their
194+
value INTENTIONALLY ignored here; the only supported shape is the
195+
nested table consumed by ``_apply_nested``. See
196+
``test_flat_classifier_rooms_under_supamem_is_ignored``.
197+
198+
* ``key in skip_first_segments`` — defense-in-depth for a future
199+
scalar field whose name happens to collide with a nested table's
200+
first segment (e.g. someone adds a flat ``mcp`` attribute on
201+
``ResolvedConfig`` while ``[supamem.mcp.caps]`` already exists).
202+
Today none of the entries in ``_NESTED_TABLES`` ("hook", "eval",
203+
"cache", "mcp", "transcript", "classifier") match a non-dict
204+
attribute on ``ResolvedConfig``, so this branch is unreachable,
205+
but it stays as a guard against silent setattr accidents on
206+
future schema additions.
207+
"""
189208
skip_first_segments = {sub.split(".", 1)[0] for sub, _ in _NESTED_TABLES}
190209
for key, value in section.items():
191210
if hasattr(cfg, key) and not isinstance(getattr(cfg, key), dict):
192-
# skip nested tables (hook/eval/cache/mcp) — handled by _apply_nested
193211
if key in skip_first_segments:
194212
continue
195213
setattr(cfg, key, value)

tests/test_config.py

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,3 +197,29 @@ def test_classifier_rooms_provenance_default(
197197
assert chain.classifier_rooms == "default"
198198
assert list(cfg.classifier_rooms.keys())[0] == "tests"
199199
assert list(cfg.classifier_rooms.keys())[-1] == "backend"
200+
201+
202+
def test_flat_classifier_rooms_under_supamem_is_ignored(
203+
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
204+
) -> None:
205+
"""A flat ``classifier_rooms = { ... }`` under ``[supamem]`` is intentionally
206+
ignored — only the canonical ``[supamem.classifier.rooms]`` nested table is
207+
honored. Locks WR-05 contract: dict-typed fields are excluded by
208+
``_apply_section`` (``isinstance(..., dict)`` filter), so the user's
209+
flat-form value is silently dropped and defaults remain in effect.
210+
"""
211+
monkeypatch.delenv("SUPAMEM_CONFIG", raising=False)
212+
cfg_dir = tmp_path / ".supamem"
213+
cfg_dir.mkdir()
214+
(cfg_dir / "config.toml").write_text(
215+
"[supamem]\n"
216+
'classifier_rooms = { backend = ["src"], tests = ["tests"] }\n',
217+
encoding="utf-8",
218+
)
219+
cfg, chain = load_config(tmp_path)
220+
# User's flat-form override is dropped — defaults still in effect.
221+
assert chain.classifier_rooms == "default"
222+
assert list(cfg.classifier_rooms.keys())[0] == "tests"
223+
assert "backend" in cfg.classifier_rooms
224+
# Default backend prefix list is the shipped one, not the user's ["src"].
225+
assert cfg.classifier_rooms["backend"] != ["src"]

0 commit comments

Comments
 (0)