Give the build.toml reader a well-defined API

[tchristiansen-aquaveo]

The various tools all read the build.toml into a plain dict, then do their own validation and extraction on it. Some of them hand-roll their own readers and/or forget to validate. Validation is piecemeal and some tools will accept things others won't, even though they should all work on the same file. The expected content of the build.toml is also spread out all over the place.

This centralizes reading build.toml in one place and converts it into a dataclass before handing it off to anything else. That ensures everything reads and validates it the same way. It also gives all the tools a well-defined API to the file, with all its contents and defaults explicitly spelled out so they know exactly what is (and isn't) there.

Only basic validation is centralized at this point. The reader checks for low level problems like unknown keys, but doesn't handle high-level things like making sure there's at least one valid build.

[tchristiansen-aquaveo]

Review — toml-refactor vs master

Branch: toml-refactor at 0b084e3 (26 commits, 26 ahead / 0 behind origin/master)

Commit
c1d192c	Read build.toml through load_toml in publish and docker_run
d5402f1	Validate top-level keys everywhere build.toml is read
36099a1	Rename toml_utils to build_toml
727a506	Add the BuildToml dataclasses
6e5c95a	Add toml_to_dataclass for build.toml
10cfc10	Add a BuildToml factory for tests.
ad293ee	Read the [filter] inputs from BuildToml
6bed15e	Resolve the Windows wheel repair switch from BuildToml
4a1503e	Resolve coverage settings from BuildToml
0e7185b	Render the generated build files from BuildToml
7c08ae5	Annotate the xms_dependencies parameter with its element type
94d6cb0	Generate CI files from BuildToml
d527eaa	Generate profiles from BuildToml
73cbf22	Run coverage from BuildToml
1f1f59e	Pick the Docker image from BuildToml
f9ebaac	Read build.toml once in publish
b5ed1c7	Read each library's build.toml as BuildToml in the VS2019 driver
f2d4cea	Add read_build_toml
83981d8	Read build.toml through read_build_toml everywhere
c284693	Make the build.toml reader's building blocks private
c6e28b4	Document the build.toml checks every reader now makes
4811277	Tidy the build.toml reader after review
60a38e6	Keep the [ci] schema with its dataclass
c367ab3	Name intermediate results instead of nesting calls
5b40308	Name the joined key lists in error messages
0b084e3	Name test fixtures before use and accept Path arguments

 docs/USAGE.md                                    |   6 +-
 tests/test_build_file_generator.py               |  20 +-
 tests/test_build_filter.py                       | 101 ++++---
 tests/test_build_toml.py                         | 239 ++++++++++++++++
 tests/test_ci_options.py                         |  90 +------
 tests/test_coverage_generator.py                 |  56 ++--
 tests/test_docker_run.py                         |   8 +
 tests/test_publish.py                            |  44 +--
 tests/test_utils.py                              |   9 +-
 tests/test_vs2019_build.py                       |  16 +-
 tests/utils.py                                   |  14 +
 xmsconan/build_toml.py                           | 329 +++++++++++++++++++++++
 xmsconan/build_tools/vs2019_build.py             |  40 ++-
 xmsconan/ci_options.py                           | 104 ++-----
 xmsconan/ci_tools/docker_run.py                  |  16 +-
 xmsconan/ci_tools/publish.py                     |  44 ++-
 xmsconan/coverage_tools/coverage_generator.py    |  20 +-
 xmsconan/generator_tools/build_file_generator.py | 107 +++-----
 xmsconan/generator_tools/build_filter.py         |  31 ++-
 xmsconan/generator_tools/ci_file_generator.py    | 126 ++++-----
 xmsconan/generator_tools/profile_generator.py    |  20 +-
 xmsconan/toml_utils.py                           | 102 -------
 22 files changed, 919 insertions(+), 623 deletions(-)

Review: origin/master..HEAD — introduce xmsconan/build_toml.py and read build.toml through one typed reader

What changed. 26 commits, 22 files, +919/-623. The branch replaces xmsconan/toml_utils.py with xmsconan/build_toml.py, which parses build.toml into frozen BuildToml / CiTable / CoverageTable / XmsDependency dataclasses, validates top-level and [ci] keys, and is now the only path every generator, CI tool, coverage runner, and the VS2019 driver uses to read the file. ci_options.py shrinks to one function, the [ci] schema moves next to its dataclass, and a make_build_toml test factory plus a new tests/test_build_toml.py (239 lines) are added. docs/USAGE.md is updated. This is a wide but mechanical refactor with no security-sensitive surface.

Notably, xmsconan/build_toml.py:229-231 guards the [ci] integer keys against TOML booleans with a comment explaining the non-obvious reason (bool subclasses int), so test_shards = true fails loudly instead of becoming 1.

Verdict: BLOCK (C=0 M=0 m=15)

The BLOCK is carried solely by writer-reviewer's discipline verdict: no DESIGN: or EVIDENCE: marker anywhere in the range. The validator dropped both marker findings, reasoning that no tracked or untracked file in this repository defines that convention, and it also dropped both test-smell MAJORs as pre-existing style or test-quality preference. No Critical or Major code finding survives. On code findings alone the state would be APPROVE WITH MINOR ITEMS; if the discipline markers are not a requirement for this project, treat the verdict as that.

Minor items

• tests/test_build_filter.py:176-184, 189-192 and tests/test_build_toml.py:21-22 — two exercise-verify cycles per test (raise then accept; default then override), so the name describes half of what can fail; split into one test per cycle.
• tests/test_build_toml.py:25-40, 49-65 — 13 and 15 bare assertions enumerate every default; lines 52-53 and-join two checks and line 54 slips CiTable(linux=False) into a defaults test; compare against an expected-object literal, split line 54.
• tests/test_build_toml.py:210-211 (also test_coverage_generator.py:1769-1770, test_docker_run.py:293-294, test_publish.py:47-48, test_vs2019_build.py:1361-1363) — the same library_name = "xmscore"\nhas_test_files = true write and unknown-key regex copy-pasted into five modules; add a shared write_build_toml(tmp_path, ...) helper in tests/utils.py.
• tests/test_coverage_generator.py:1767-1774 — nothing doubles subprocess; if the reader ever stops raising, run_coverage shells out from tmp_path instead of failing cleanly; patch subprocess.run as a guard.
• tests/utils.py:5,44 (also tests/test_build_toml.py:7) — shared factory and reader tests import private _toml_to_dataclass, _load_toml, _CI_KEY_TYPES, undercutting the "make building blocks private" commit; and because the factory skips _validate_top_level_keys, make_build_toml(bogus=1) raises a dataclass TypeError rather than the reader's ValueError; expose one public dict-to-BuildToml function or route the factory through read_build_toml. (writer-reviewer, test-smell-reviewer)
• xmsconan/build_toml.py:172 — _validate_top_level_keys (182-186) duplicates _reject_unknown_keys (215-219) with a slightly different message; call _reject_unknown_keys(toml_data, _KNOWN_KEYS, ...) instead.
• xmsconan/build_toml.py:245-263 — _coverage_table checks key names but not the types of filters/excludes; a scalar filters = "xmscore/" passes and list(...) at ci_file_generator.py:193-194 splits it into one-character gcovr filters; require list for both.
• xmsconan/build_toml.py:251-258 — float(values[key]) accepts cpp_threshold = true as 1.0, while the [ci] validator at 229-231 deliberately rejects bool-for-int; reject bool before calling float. (writer-reviewer, silent-failure-reviewer)
• xmsconan/build_toml.py:259-262 — str(values["python_version"]) turns unquoted python_version = 3.10 (TOML float 3.1) into "3.1", which ci_file_generator.py:145-153 then rejects as a version the user never wrote; reject non-str with an error that names the quoting problem.
• xmsconan/build_toml.py:266-274 — _xms_dependency checks key presence but not types; no_python = "false" is a truthy string that pyproject.toml.jinja:19 reads as "exclude", silently dropping a runtime requirement from the wheel; type-check name/version as str and no_python as bool. (writer-reviewer, silent-failure-reviewer)
• xmsconan/ci_options.py:19-41 — module now holds one 3-line function whose only inputs live on BuildToml; consider a BuildToml property and delete the module.
• xmsconan/ci_tools/docker_run.py:46 — the resolve_docker_image branch returning config.ci.docker_image has no test; add one that writes [ci] docker_image = "..." to build.toml.
• xmsconan/ci_tools/publish.py:118 — publish() takes 10 parameters; url/username/password are one credentials concept (pre-existing; range only annotated toml_path); group credentials into a parameter object.
• xmsconan/coverage_tools/coverage_generator.py:617 — if config.ci.xvfb: _reexec_under_xvfb() is never reached by a run_coverage test; add one with [ci] xvfb = true and _reexec_under_xvfb patched.
• xmsconan/generator_tools/ci_file_generator.py:242 — generate_ci body runs ~195 lines (257-452) mixing validation, version resolution, a 44-line context dict, and rendering (pre-existing; range shortened it by ~14 lines); extract _validate_ci_config(config) and _ci_context(...).

Dropped in validation (not counted above): the two discipline-marker MAJORs, the test_build_filter.py:125-128 for-loop finding (loop is a pre-existing context line), and the test_publish.py:283-324 _check_xvfb temp-file setup finding (correct tests; style preference).

[tchristiansen-aquaveo]

Response — review of toml-refactor

Addressed in two commits on top of the reviewed head 0b084e3:

• 4ce5659 — Type-check the [coverage] and xms_dependencies values
• 92e8b08 — Build test configurations directly instead of through the reader

docs/USAGE.md                    |  4 +--
tests/test_build_filter.py       | 60 ++++++++++++++++++++--------------------
tests/test_build_toml.py         | 28 ++++++++++++++++++-
tests/test_ci_options.py         |  8 +++---
tests/test_coverage_generator.py | 39 ++++++++++++++------------
tests/test_utils.py              |  9 +-----
tests/utils.py                   | 14 ----------
xmsconan/build_toml.py           | 54 ++++++++++++++++++++++++------------
8 files changed, 122 insertions(+), 94 deletions(-)

Seven fixed (m1 in part, m4–m10), eight declined (m1 in part, m2, m3, m11–m15).
The rule applied throughout: fix what this branch introduced; leave what it
merely touched. Every "pre-existing" call below was checked against
origin/master, not assumed. The four findings the validator dropped are
listed at the end so nothing from the review goes unaccounted for.

	Finding	Outcome
m1	Two exercise-verify cycles per test	Fixed for test_build_toml.py; declined for test_build_filter.py (pre-existing)
m2	Bare-assert defaults tests	Declined
m3	Unknown-key test copy-pasted into five modules	Declined
m4	Coverage rejection test could shell out	Fixed
m5	Test factory imports the private converter	Fixed, by removing the factory
m6	_validate_top_level_keys duplicates _reject_unknown_keys	Fixed
m7	[coverage].filters/excludes not type-checked	Fixed
m8	Boolean threshold read as 1.0	Fixed
m9	Unquoted python_version coerced to a wrong version	Fixed
m10	xms_dependencies values not type-checked	Fixed
m11	ci_options.py is one function	Declined
m12	docker_image branch untested	Declined (pre-existing)
m13	publish() has 10 parameters	Declined (pre-existing)
m14	xvfb dispatch in run_coverage untested	Declined (pre-existing)
m15	generate_ci is ~195 lines	Declined (pre-existing)

---

Fixed

m1 (part) — test_build_toml.py:21-22 split in two

test_build_toml_derives_python_namespaced_dir now checks only the derived
default; test_build_toml_keeps_an_explicit_python_namespaced_dir checks the
override. The two test_build_filter.py sites in the same finding are declined
below.

m4 — the coverage rejection test fails loudly if the run shells out

test_run_coverage_rejects_an_unknown_top_level_key patches
coverage_generator._run with side_effect=AssertionError(...). If the
reader ever stops raising on an unknown key, the test now fails with
"run_coverage shelled out before rejecting build.toml" instead of trying to run
xmsconan gen from a temp directory.

m5 — tests build BuildToml directly; the factory is gone

The finding was right about the coupling but the suggested fix (a public
dict-to-BuildToml function) would have widened the reader's surface to serve
tests. Instead, make_build_toml was deleted from tests/utils.py along with
its _toml_to_dataclass import, and the 28 call sites in
test_build_filter.py, test_ci_options.py, and test_coverage_generator.py
construct the value they need:

config = BuildToml(
    library_name="xmscore",
    ci=CiTable(python_versions=["3.10", "3.13"]),
    filter={"options": {"python_version": "3.10"}},
)

tests/test_build_toml.py still imports _load_toml, _toml_to_dataclass,
and _CI_KEY_TYPES on purpose — it is the converter's own test module. No
other test file reaches past the underscore.

m6 — one unknown-key check

_validate_top_level_keys now calls
_reject_unknown_keys(toml_data, _KNOWN_KEYS, str(toml_path), kind="top-level key").
The kind parameter keeps the message the seven existing tests match on
(... has unknown top-level key(s) ...).

m7, m8, m9, m10 — the sub-table values are checked, not just the keys

All four are in xmsconan/build_toml.py, which is new on this branch, so all
four are this branch's to fix. Each rejection names the file and key.

	Input	Before	Now
m7	filters = "xmscore/"	passed through; iterated into one-character gcovr patterns	[coverage].filters must be a list, got str
m8	cpp_threshold = true	float(True) → 1.0	[coverage].cpp_threshold must be a number, got a boolean
m9	python_version = 3.10 (unquoted)	str(3.1) → "3.1", rejected downstream as a version nobody wrote	[coverage].python_version must be a quoted string like "3.13", got 3.1
m10	no_python = "false"	truthy string; the template dropped the dependency from the wheel	no_python must be true or false, got 'false'; name/version must be strings

The threshold logic moved into _coverage_threshold so _coverage_table stays
flat. The python_version coercion is gone rather than tightened — the
converter no longer guesses at what an unquoted number meant.

New tests: test_toml_to_dataclass_rejects_a_boolean_threshold,
test_toml_to_dataclass_rejects_a_scalar_coverage_pattern_list (parametrized
over filters/excludes),
test_toml_to_dataclass_rejects_an_unquoted_coverage_python_version, and two
new cases in test_toml_to_dataclass_rejects_malformed_xms_dependencies
(numeric-version, quoted-boolean). docs/USAGE.md §5.3 and §5.7 gained one
sentence each describing the new checks.

---

Declined

m1 (part) — test_build_filter.py:176-184, 189-192

Both test shapes exist on origin/master
(test_load_build_filter_checks_the_filter_against_the_narrowed_matrix was
already raise-then-accept; test_ci_python_versions_defaults_to_none already
had two asserts). This branch only swapped their dict literals for dataclass
construction.

m2 — bare asserts in the defaults tests

An expected-object literal would restate the same 13 and 15 values in a
different shape without making a failure easier to read. Line 54
(CiTable(linux=False).linux_enabled is False) is the one non-default case in
that test and belongs there.

m3 — the repeated unknown-key test

Five three-line tests share a write_text call and a regex. The repetition is
small enough that a shared helper would cost more indirection than it saves.

m11 — ci_options.py

The module was already this thin before the branch; the branch removed the
[ci] schema from it, it did not shrink a larger module down. Turning
repairs_windows_wheel into a BuildToml property would put a CI-publishing
decision on the configuration dataclass, which is the wrong home for it.

m12, m14 — untested branches in docker_run.py:46 and coverage_generator.py:617

Both branches and both test gaps exist on origin/master (toml_image = ci_config.get("docker_image"), if ci_config.get("xvfb"):). The branch
changed how the value is read, not whether it is tested. Worth tests, but not
this PR's.

m13 — publish()'s parameter list

Pre-existing. The branch only annotated toml_path: str | Path.

m15 — generate_ci length

Pre-existing, and the branch shortened it by ~14 lines. The extraction the
review suggests is a reasonable follow-up on its own.

---

Dropped in validation

Listed so the review reads as fully answered.

• DESIGN: / EVIDENCE: markers absent from the range. These are
  conventions of the review tooling, not of this repository. The design
  artifact is docs/superpowers/plans/2026-08-27-build-toml-reader.md; the
  evidence is the Gates section below.
• test_build_filter.py:125-128 for loop around exercise and assertion.
  The loop is a context line in the diff; this branch rewrote only its body.
• test_publish.py _check_xvfb tests still write a temp file and call
  read_build_toml. Left as-is. The tests are correct; if the direct-
  construction style from m5 should extend here, it is a four-line change per
  test (BuildToml(library_name="xmscore", ci=CiTable(xvfb=True))).

---

Gates

At 92e8b08:

$ .venv/Scripts/python.exe -m pytest tests -q -p no:cacheprovider
2153 passed, 2 skipped in 31.17s

$ .venv/Scripts/python.exe -m flake8 xmsconan tests
0

The count is six higher than at 0b084e3 (2147 → 2154 after 4ce5659's seven
new tests → 2153 after 92e8b08 removed the factory's smoke test).