test(config): cover the non-string-key guard in the unknown-key walk (#627)

[Srinivasan8888]

Covers the one uncovered statement in config/unknown_keys.py — the non-string-key guard I shipped in #628. No source change; the guard is correct, only its coverage was missing.

Claimed on #627 before writing anything. Refs #627.

How it was found

Not by eye. A branch-coverage run over the full suite:

$ .venv/bin/python -m pytest tests/ -q -o addopts= --cov=soup_cli --cov-branch --cov-report=term-missing
19935 passed, 108 skipped ... TOTAL 74022 12672 28968 4607 81%   (25m27s)

src/soup_cli/config/unknown_keys.py    ...  miss=1  BrPart=1  97%   missing: 118

Line 118 is the continue in:

for key, value in raw.items():
    if not isinstance(key, str):
        continue

Nothing in tests/test_issue627_unknown_config_keys.py had ever fed the walk a non-string key.

Why it matters

YAML permits non-string mapping keys. Without the guard the key reaches difflib.get_close_matches, which iterates it:

difflib.get_close_matches(1,    [...])  -> TypeError: 'int' object is not iterable
difflib.get_close_matches(True, [...])  -> TypeError: 'bool' object is not iterable

So a config carrying 1: or true: under any section would kill soup train --config with a bare TypeError raised from inside the unknown-key reporter — the code whose entire purpose is to turn a confusing failure into an actionable one. That is the #628 defect wearing different clothes.

Mutation table

Each mutation run against the pre-existing classes in the file and against the new one, so "this was uncovered" is measured rather than asserted:

mutation	PRE-EXISTING	NEW class
guard removed entirely	SURVIVED	KILLED
continue -> return (aborts the walk)	SURVIVED	KILLED
isinstance(key, str) -> isinstance(key, object)	SURVIVED	KILLED

Named failures:

guard removed entirely
  test_a_numeric_key_does_not_raise
  test_a_boolean_key_does_not_raise
  test_a_non_string_key_does_not_mask_a_real_unknown_key
  test_a_non_string_key_nested_in_a_subsection_does_not_raise

continue -> return
  test_a_non_string_key_does_not_mask_a_real_unknown_key
  test_a_non_string_key_nested_in_a_subsection_does_not_raise

The third row is reported, not counted. not isinstance(x, object) is always False, so that mutation is behaviourally identical to removing the guard and produces the same four failures. One kill spelled two ways — I would rather say so than present three independent kills.

unknown_keys.py md5-verified byte-identical after every round; __pycache__ cleared each time.

Two details the tests pin deliberately

continue, not return. The mask test puts the typo after the non-string key in insertion order, so training.epocs is only reported if the walk kept going. A guard rewritten as an early return would silently stop reporting every key after the first non-string one — and would have passed a test that only checked "does not crash".

True == 1. {1: ..., true: ...} collapses to a single dict entry, so yaml.safe_load("1: numeric\ntrue: boolean\n") yields [1], not two keys. A fixture written without noticing would claim to cover two non-string key types while covering one. test_python_collapses_true_and_one_into_one_key pins that premise before the other tests rely on it; the boolean case is therefore constructed directly rather than through YAML.

Verification

$ .venv/bin/python -m pytest tests/test_issue627_unknown_config_keys.py tests/test_config.py \
    tests/test_cli_startup_is_light.py -q -o addopts=
91 passed

$ .venv/bin/python -m ruff check src/soup_cli/ scripts/ tests/
All checks passed!

$ git diff --stat -- src/ | wc -l
0
$ git diff -- tests/ | grep -c "^-[^-]"
0

Measured but deliberately not taken

Recorded so they are findable rather than lost in a terminal. Both are the #273 shape — full statement coverage, one branch each:

• monitoring/plugin_callback.py 34->30 — an enabled plugin that exposes no hooks is never exercised.
• utils/moe.py 60->62 — a config object with no __dict__ is never exercised.

Happy for anyone to take either; say the word if you would rather I fold them into this PR.

Limits

• This pins the guard's behaviour, not a new capability. It changes nothing a working config does today.
• The crash it prevents is demonstrated at the difflib call rather than by shipping a broken build: the TypeError above is real output, but the end-to-end soup train path is only reached through find_unknown_config_keys, which is what the tests drive.

https://claude.ai/code/session_014vWWFgXhj9y46pYCyEjfcy

[MakazhanAlpamys]

Merged as 6aa188f. Thank you — and the part worth naming is which tests carry the weight, not the coverage number.

Four of the six tests would have been satisfied by "does not raise". The two that actually decide the merge assert the walk keeps going: a typo'd key placed after a non-string key is still reported, with its suggestion, at the top level and one section down. That is exactly what separates this from a coverage-chasing PR — an early-return rewrite silently stops reporting every key after the first non-string one, which is the same silent-no-op failure class #627 exists to fight, wearing different clothes.

What I verified independently before merging, at 401fa79:

mutation	pre-existing 45 tests	new class
guard removed entirely	45 passed — SURVIVED	KILLED (4 failed)
continue -> return	45 passed — SURVIVED	KILLED (2 failed)

Both anchored on the full executable line with the diff printed, reverted between rounds. The second row is the discriminating one: only the two "keeps going" tests fail under return, exactly as your own docstrings predict. That is real discrimination rather than a blanket kill.

Also checked and confirmed: difflib.get_close_matches(1, [...]) raises TypeError: 'int' object is not iterable, so the consequence you describe — a bare TypeError raised from inside the reporter whose entire job is to turn a confusing failure into an actionable one — is the real failure mode and not a hypothetical.

Two smaller things I want on the record because they are habits, not accidents:

• You pinned the fixture's premise before relying on it (test_python_collapses_true_and_one_into_one_key), then constructed the boolean case directly rather than through YAML, with the reason in the docstring. Most contributors would have written the YAML and been quietly wrong.
• You flagged the third mutation as behaviourally identical to the first (not isinstance(x, object) is always False) and reported it as "not counted" instead of claiming three independent kills. Confirmed — same four failures.

#627 stays open, and I want that visible so a green coverage PR is not read as the issue being done. #628 shipped phase 1 (warn); UNKNOWN_KEY_REJECTION_VERSION = "0.75" and TestTheDeadline hold the rejection phase, and that is what closes it. Your Refs #627 rather than Fixes was correct.

No changelog fragment, matching #647 and #653. CONTRIBUTORS.md updated — this is the second time you have come back to cover a line in code you shipped yourself, which is a rarer habit than writing the code was.