Repository navigation
docs: variant-dependent documentation without Jinja, the same in Sphinx and ubCode - #4
Merged
Merged
Conversation
Phase 0 of doc/variant-handling-without-jinja-plan.md. None of the variant-data work below is testable while the toolchain contradicts itself: - bootstrap_python.sh installed CPython 3.11 and claimed the project required it, while pyproject requires >=3.12,<3.14. Following the documented bootstrap produced a venv Poetry rejects. 3.12 everywhere now: the bootstrap, the pypeline venv step, bootstrap.json, the devcontainer and the two READMEs. - ubproject.toml extended a hard-coded .venv/lib/python3.13 path that does not exist (the venv is 3.12), so the whole base needs configuration silently failed to resolve for ubCode. Corrected to 3.12; the later phase that makes the file self-contained removes the venv path entirely. - [needs] schema_definitions_from_json named schema.json, which exists nowhere in the tree. Dropped rather than left dangling. - /generated (the current-variant pointer) is now ignored instead of showing up as untracked output. Also documents how to point the venv at a local spl-core checkout, since the next phases need spl-core changes: CMakeLists.txt discovers spl-core by importing it, so one editable install redirects both the Python and the CMake side. The path stays out of pyproject on purpose -- CI must keep resolving the released version. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… content
Phase 1 of doc/variant-handling-without-jinja-plan.md. Takes the three commits
that replace the Jinja constructs in hand-written documents with two declarative
mechanisms both Sphinx and ubCode read:
- feature conditionals became {if} var.features.X fences,
- the {% for %} loop over components became [[source.mounts]] entries with
attach_to onto an empty toctree in doc/components/index.md,
- the report shape became {if} var.build_config.target == "reports" fences,
- CMakeLists.txt mirrors the KConfig JSON to build/autoconf.json with the
variant name added, so ubCode follows whatever CMake Tools configured.
Only {{ timestamp }} in index.md is still Jinja in hand-written content.
Taken as-is; the gaps it leaves -- the two readers still evaluating different
variant maps, the still-global Jinja hook, build/ being indexed wholesale, and
the mount condition that re-encodes parts.cmake -- are the next phases.
uv.lock is kept. The branch deleted it, which is unrelated to variant handling.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Phases 2 and 4 of doc/variant-handling-without-jinja-plan.md. Phase 4 is pulled
ahead of Phase 3 because its only blocker -- spl-core emitting `{% raw %}` into
the generated source listings -- is now gone, and until the hook was gone no
intermediate state could be verified: a bare `sphinx-build` crashed outright on
any brace in any document.
One generation step, one map
----------------------------
tools/variant_data.py writes build/variants/<Variant>/<kit>/<target>.json for
the whole matrix, plus build/autoconf.json as the "current" pointer and a
`generated` link to the current build directory. Each cell carries everything a
condition may name: the complete feature vector (including the promptless
booleans KConfig omits, via the new spl-core accessor), the variant, the kit,
the target, and the component list parsed out of parts.cmake.
That last one matters twice. It is the single source of truth with parts.cmake,
so the integration-test document can be gated by membership instead of by a
hard-coded variant name; and `target` being IN the file is what turns the report
fences from undecidable into a clean false for a reader that is not building
reports.
conf.py no longer synthesizes any of it. It reads one file and adds nothing --
which is the whole point, since ubCode, ubc and a reviewer's editor cannot
execute it. CMake picks the cell per build shape through spl-core's new
SPL_VARIANT_DATA_FILE_DOCS / _REPORTS.
The generator is a standalone script, not CMake code, on purpose: KConfig is
pure Python while the top-level project() call demands a C toolchain, so the
documentation and its gate must not have to go through CMake at all.
Hardened before committing: the parts.cmake parser now raises on any construct
outside the grammar this project uses, instead of treating a guarded body as
unconditional and returning a component list that is quietly wrong for one kit.
`--check` regenerates and diffs for CI. 37 tests cover the grammar, the
completeness of the feature vector and the shape of every cell.
No Jinja
--------
The `source-read` hook and `{{ timestamp }}` are gone. The timestamp is the
theme's last-updated footer instead, which also makes the build reproducible.
`sphinx-build -b html .` now works standalone for any variant -- previously it
rendered the components page empty, without a warning, and crashed on braces.
Verified: Disco (AUTO_OFF off, BLINKING on) builds neither the auto_off nor the
brightness_controller page and does build the integration document; Sleep builds
both component pages and not the integration document.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Phase 3 of doc/variant-handling-without-jinja-plan.md. The configuration was split in a way that could not be noticed from either end. ubproject.toml extended spl-core's base configuration through a venv path with the Python minor baked into it -- a path that did not exist, so for ubCode the base configuration silently resolved to nothing. conf.py resolved the same file a second time through importlib, so the Sphinx build was fine. Two mechanisms for one file, one of them broken, and the symptom is the IDE quietly disagreeing with the build about which link types and need types exist. sphinx-needs settles which way this has to be fixed: it reads exactly one TOML file's [needs] table and implements no include mechanism at all. So the one file has to be complete. spl-core's base configuration is vendored in, and `extend` is gone. Vendored while modernizing `extra_links` and `extra_options` into the `links` and `fields` tables -- both deprecated in sphinx-needs and warned about on every build, and this file already used the new form for its own `image` field, so it was mixing the two. `integrity` carries an explicit empty-string default because that is what the deprecated form implies; without it the field would have flipped to null on all 87 needs. Verified: same 87 needs, same IDs, zero differing fields, three fewer warnings. Vendoring is a copy and copies rot, so test_ubproject_config.py compares the copy against what the installed spl-core ships and fails on drift. The two deliberate deviations are asserted as such rather than merely tolerated. The larger one is `extend_exclude = ["build/**"]`. spl-core turns respect_gitignore off and then excludes nothing under the build directory, so ubCode indexed every variant and kit ever built on the machine: every need ID many times over, and no possibility of agreeing with any one build. Sphinx never saw it because conf.py narrows its source set per build shape. The generated source listings are still indexed, through `generated/**` -- the current variant's build directory -- so it is one variant's listings instead of all of them. conf.py also has to apply the per-build-shape data file after sphinx-needs reads the TOML, which happens on config-inited: the pointer in the TOML would otherwise win and hand the reports build the docs data, silently evaluating every report fence false. files.readonlyInclude makes the editor refuse edits under build/ and generated/. It is the one guard rail that applies to an assistant as well as a person. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ariant Phase 5 of doc/variant-handling-without-jinja-plan.md. The seven per-component [[source.mounts]] become [[source.variant_sources]] rules. Mounts exist for trees that are not in the project; these documents are. What changes beyond the tidying is the condition. They were gated on the KConfig feature that owns the component, except the integration suite, which was gated on `var.build_config.variant == "Disco"` because parts.cmake adds it for the test kit only and a feature could not express that. Both re-encode what parts.cmake already states. The variant-name gate was that drift made concrete: it claimed the suite for Disco's prod kit too, where parts.cmake does not add it. Every rule now asks the one question that matches the build -- is this component in the variant's component list -- and a test refuses any rule that names a variant directly. Rules are subtractive: a FALSE rule removes the files it names, a TRUE one does nothing. So two rules over the same file compose as AND, which is what gates the generated report pages on the build shape AND the component. A `docs` build with a populated build directory no longer reads report pages it will not show. doc/components/index.md becomes a static 150% toctree instead of an empty one that sphinx-mounts filled in. An entry naming an excluded document is reported as INFO by both tools by design, so the page can simply list everything. The /build/** glob toctrees are gone. They matched every variant and build type on disk and only ever resolved to one page because conf.py narrowed the source set behind the scenes -- an invisible gate holding up a visible one. Documents now name `generated/`, the configured variant's build directory, and conf.py's filtering is deleted with them. Verified: the page set is byte-identical to the mount-based build for Disco and Sleep. Against one `generated` tree holding every component's reports, the reports target builds exactly the variant's own and the docs target builds none. A CMake configure produces the pointer, the symlink, the marker and both SPL_VARIANT_DATA_FILE_* cache entries. AGENTS.md now states the rules as rules rather than describing the mechanism. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Phase 7 of doc/variant-handling-without-jinja-plan.md.
The documentation is derivable from the sources alone -- KConfig is pure Python
and the documents are text -- and only CMake's top-level project() call needs a
C toolchain. So the gate for it should not need one either, and now does not.
test_documentation.py generates the whole variant matrix and builds each
variant's documents, then asserts the property the entire design exists to
provide: a component's document is in the build exactly when that variant's
parts.cmake adds the component. Not when a feature suggests it, and never
because the variant is called something. It also asserts what must not come
back -- no Jinja construct in any document, no source-read handler in conf.py --
because with the hook gone an accidental {{ ... }} is shipped as literal text
rather than failing the build.
It is a pytest module rather than bespoke CI script so that CI stays the thin
wrapper it is: the new `documentation` job installs a Python and the locked
dependencies and runs `pytest -m "docs and <gate>"`, the same command a
developer runs. 14 tests, 14 seconds, all five variants -- where the three build
jobs only cover the variants they build, and take tens of minutes to say so.
VS Code gets tasks for the working mode this enables: select a variant for the
docs and ubCode (KConfig only, no CMake configure, no compiler), generate the
whole matrix, build one variant's documents, and run the gate.
Note on ubc: the plan also proposed `ubc check` per variant in this job. Left
out deliberately -- the CLI is not installed in this environment, so an
unverified step would be a guess about how it installs in CI. The Sphinx build
covers the same document sets meanwhile, and the conditions are asserted with
sphinx-mounts' own interpreter in test_ubproject_config.py, which is the same
grammar ubCode evaluates.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Phases 0-5 and 7 are done. Phase 6 is not, and the reason is worth writing down rather than leaving as a gap: investigating it with sphinx-test-reports 2.0.0 actually installed showed that two of its premises do not hold here. link_properties cannot replace sple_tr_link, because it reads a GoogleTest RecordProperty and this repository has none -- the traceability lives in `:tests:` RST comment blocks above each TEST(). Adopting it means adding a RecordProperty call duplicating the list three lines above it, which is the same one-fact-two-places failure this plan exists to remove, reintroduced a layer down. Keeping the comment blocks and joining by name is the alternative, and choosing between them is a decision, not a detail. Relaxing the version pin alone is not safe either: the CLI writes result = "failed" while doc/test_report_template.txt filters on "failure" in four places, so imported data would silently report zero failures. Both confirmed by running the CLI on a GoogleTest XML, not inferred. The good news is also recorded: 2.0.0 still ships the test-report directive, so the upgrade is backward compatible whenever it happens. conf.py's setup() moves to the end of the file, where a reader expects it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
I skipped the `ubc check` half of Phase 7 on the belief that the CLI was not
available. It is -- it ships inside the ubCode VS Code extension, just not on
PATH. Running it immediately found two real defects in the configuration work,
both invisible to every Sphinx build, which is precisely the failure mode this
whole plan is about: a configuration only one reader executes cannot be
validated by that reader.
1. `[source] extend_include` was inert. Configuring parsers puts ubCode in
parser mode, where the document set comes from each `[parse.parsers.*]`
include and extend_include is ignored. The generated source listings were
therefore not indexed at all, while the setting looked correct. They are
named in [parse.parsers.rst].include now. (extend_exclude IS honoured --
`ubc config` shows build/** excluded, and shows the variant_sources rules
compiled into the same list, which is how the gating is implemented.)
2. The two readers had different document sets. `[parse.parsers.md] include =
["*.md"]` matched every Markdown file in the tree, so ubCode parsed
AGENTS.md, README.md, CLAUDE.md and three dozen agent skill definitions as
project documents -- 38 files Sphinx never sees. Narrowed to match conf.py's
include_patterns; diagnostics went from 58 to 15.
It also found the last "key only one reader knows": doc/component_report.md
used the {variant} role on build_config.component_info.long_name, which spl-core
writes per BUILD for one component. The variant data cannot carry it -- the
generator does not know which component a per-component report is for -- so it
resolved inside that one build and nowhere else.
The gate is now a test rather than a thing I ran once. Per variant it asserts
that ubCode removes EXACTLY the component documents the variant's component list
omits: the design's central claim, checked against the reader that never runs
conf.py instead of argued. Verified to fail on divergence by regressing a rule
to gate on a variant name, which breaks IDEA/Sloemada.
`ubc` is on neither PyPI nor npm, so there is no install step this repository
can own; the tests find it on PATH, via $UBC, or in the extension directory, and
skip when absent rather than pretending to cover it. Not added to the CI job for
the same reason.
Two pre-existing content issues surfaced, both agreed on by both readers: an
empty toctree in components/examples/flight_controller/doc (removed), and
SWIMPL_BC-003c linking to SWDD_BC-203, a spec inside an {if} fence that does not
exist in every variant where the implementation does. The second is left alone
-- it is a traceability decision, not part of this plan.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
All four "what is still wrong" items and all seven prioritised gaps in the note are closed, each verified against the tree rather than from memory -- including planting a decoy need under build/ to confirm ubCode no longer indexes it. Three deliberate deviations from the target concept are now stated as such: the variant matrix is four-dimensional because the note's own requirements (kit and target in the data) do not fit its proposed filename; the generated reports are gated by a variant_sources rule rather than a mount, because rules compose as AND with the per-component ones and a mount cannot; and conf.py keeps its Sphinx-only plumbing, with both pointers now named explicitly rather than one of them resting on a library default. The note asks to verify rather than assume that ubCode might report the codelinks preprocessor.compile_commands key as unknown. It does not, on 0.35.0 -- `ubc config` resolves the table with defaults filled in. New finding, from testing rather than reading: `generated` is a symlink, and ubCode does not descend symlinked directories -- its own diagnostic text says so. Sphinx follows it, so the reports build is fine, but the IDE sees no generated pages at all. Verified both ways: with `generated` a real directory a planted need appears under the reports pointer and is absent under the docs pointer, so the gating itself is correct; with the symlink the same file is invisible and both pointers yield an identical needs count. This costs nothing today, because no needs live under generated/ yet. It starts costing when Phase 6 lands, since that is exactly where imported test-result needs would go. The fix is to materialise the generated .rst files into a real directory after the reports target produces them -- a post-build step, not configure time -- and it is not written here because the reports target cannot be run on this machine. Recorded at the symlink, in ubproject.toml next to the now-knowingly-inert include, and in the plan. Also drops a GENERATED marker in build/ itself, not only build/variants/, since build/ is the directory someone is most likely to open. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
CI failed all three build jobs and the documentation gate: 51 failures, two
causes.
The branch depended on an unreleased spl-core. tools/variant_data.py called
KConfig.declared_boolean_symbols(), which exists only in the fork I had
installed as an editable override here. CI installs the version pinned in
pyproject.toml, so it raised AttributeError -- and because CMakeLists.txt runs
the generator at configure time and fails hard, that took every variant build
down with it, not just the docs gate. It worked on exactly one machine: the one
with the local checkout. The accessor is now asked for and fallen back from, so
the generator works against both.
Tests read generated state they did not generate. test_ubproject_config.py
loaded build/variants/** with no fixture; build/ is gitignored, so on a fresh
checkout the files are simply absent. It passed locally only because earlier
runs had left them there. A module fixture generates the matrix first.
Chasing this found a third problem CI would NOT have caught. The
SPL_VARIANT_DATA_FILE_* cache variables exist only in the fork, so against the
pinned spl-core the reports build got no VARIANT_DATA_FILE, fell back to the
pointer holding the DOCS cell, and every report fence would have evaluated
false: a reports target with no reports in it, no error anywhere, and a green
test_reports, because that test asserts the build succeeded and junit.xml
exists rather than that any report page was produced.
So the dependency is removed instead of trusted. CMake publishes the two cells
under fixed names and conf.py selects the one matching its build shape, which
spl-core already tells it through the directory holding the per-target
config.json. VARIANT_DATA_FILE still takes precedence where something passes it,
so the fork and any later release keep working. Choosing which generated file to
read is not synthesizing data -- the file is complete and conf.py still adds
nothing to it. A test pins it, since silent regression is the failure mode.
Verified from a clean tree against spl-core 8.6.0: 84 tests pass.
One degradation remains and needs the spl-core release: the pinned version
always passes --jinja-raw-tags to clanguru, so generated source listings carry
literal {% raw %} lines now that the Jinja pass is gone. Confirmed on a built
page. Cosmetic, confined to the reports target, breaks nothing.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Second CI round: five failures, all the reports test not finding
reports/html/build/<V>/<kit>/<type>/components/<c>/reports/coverage.html.
Not a test-path quibble. spl-core writes the gcovr tree at
`reports/html/${component_reports_output_dir}/coverage/index.html` -- the
component's build-relative path -- SplBuild looks the pages up in the same
place, and the coverage.rst it generates links to `coverage/index.html`
relative to itself. Page docname and gcovr tree have to stay siblings.
Phase 5b moved the docname to generated/components/<c>/reports/coverage and
left the tree where spl-core put it, so every variant report would have shipped
a broken coverage link. The artifact assertion caught it only because it happens
to sit on the same path the link depends on.
A static toctree entry cannot name a build-directory-dependent docname, so
without a stable path it has to be a glob, narrowed by conf.py to the configured
variant's build directory. That is gap 4 of the concept note, and it is now
deferred rather than closed -- behind the same spl-core release as the
{% raw %} opt-out and Phase 6. Fixing it properly means spl-core emitting the
gcovr tree next to wherever the page lands, and
TARGET_COMPONENT_BUILD_ARTIFACTS following; both small, neither doable from here
against a pinned release.
Kept: all of Phase 5a. Membership gating, the 150% toctree, no variant name in
any condition -- the part that was about correctness rather than paths. The
variant_sources rule over generated/** stays as the right shape for the day the
pages move, inert until then.
The reports-shape test now asserts on the rendered page ("Verification" present
for reports, absent for docs) rather than on a file list, which is what the
fence actually controls and does not depend on where the pages land.
Verified from a clean tree against the pinned spl-core: 84 tests pass, ubc
reports the same 8 warnings and 5 info as before.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Linux and the devcontainer went green; Windows failed on one test with
UnicodeDecodeError: 'charmap' codec can't decode byte 0x9d
Python on Windows defaults text I/O to the ANSI code page, cp1252 on the
runner, not UTF-8. test_no_document_is_rendered_through_jinja reads every
document to check none of them contains a Jinja construct, and
doc/cross-platform-alignment-proposal.md -- which predates this branch --
carries bytes cp1252 cannot decode. The test was right; the read was
platform-dependent.
Every read_text/write_text/open in the files this branch added now names
encoding="utf-8" explicitly. That is the correct fix rather than skipping the
file: the documents are UTF-8, and a check that walks all of them must not
depend on which platform runs it.
Verified with PYTHONWARNDEFAULTENCODING=1 and -W error::EncodingWarning that
neither tools/variant_data.py nor the two unit-test modules perform any
implicitly-encoded text I/O. (Running the whole suite under that flag also trips
sphinx and spl-core internals through the subprocesses it spawns; those are not
this branch's to fix.)
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`ubc build html` reported five toctree.not_included warnings where Sphinx reports four. The extra one was doc/component_report.md, the root document of spl-core's PER-COMPONENT report -- a build shape ubCode never performs. conf.py drops it from every variant-wide build, so Sphinx never sees it as an orphan; ubCode had no such exclusion and indexed it. Four orphans in one reader and five in the other is the two of them disagreeing about the document set, which is the one thing this configuration exists to prevent. Both now report the same four, verified by diffing the two orphan lists. `[parse.parsers.*]` has no `exclude`, but `extend_exclude` IS honoured in parser mode -- unlike `extend_include`, which is not -- so that is the right knob. The existing test pinned extend_exclude to exactly ["build/**"], which is why it did not catch this: it asserted a list rather than a property. It now asserts that build output is excluded, and a new test ties the ubCode exclusion to the conf.py exclusion it mirrors, so the two cannot drift apart silently. The remaining four warnings are genuine orphans that predate this branch and that both readers agree on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review finding 2. spl-core writes unit_test_spec.rst, unit_test_results.rst and coverage.rst per component at configure time and lists them among its include patterns for BOTH build shapes. conf.py forwarded every build/ pattern unfiltered, so a docs build read all three per component -- and referenced none of them, because the fences that would have linked them are false in a docs build. Fifteen orphan warnings on Spa, for pages nobody asked to see. Measured on a real CMake docs build: 19 toc.not_included before, 4 after, the four being the genuine orphans both readers already agree on. The build shape is now derived once. It was computed for the variant data file and would have been computed again here; two derivations of one fact drift. The review notes the compiler-free gate cannot see this, because a bare build has no spl-core config.json. It can: the only thing CMake contributes is that file, and it is three lines of JSON. Two tests fabricate it -- one asserts the docs shape orphans nothing, the other that the reports shape still renders the pages, because narrowing one shape must not starve the other. The first version of that test passed with the fix reverted: it scanned stdout, and Sphinx writes warnings to stderr. Caught by reverting the fix and watching it stay green, which is the only way to find a vacuous test. It now fails with three named warnings. The same blind spot made several failure messages quote stdout alone; they include stderr now. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review finding 1, taken as the in-repo option because the other one does not
exist yet: no released spl-core lets the flag be turned off. 8.8.0 is the newest
stable and 9.0.1rc4 the newest prerelease, and both hardcode --jinja-raw-tags.
So this is not a stopgap ahead of a pin bump -- it is load-bearing for an
unknown period, and it is written as a compatibility shim with its removal
condition stated, not as a TODO.
Every generated listing under __source_docs wraps its code-block in
`{% raw %}` / `{% endraw %}`. The only thing that ever consumed those markers
was the global Jinja pass this branch deletes, so in a reports build they
reached the reader as two literal paragraphs per listing.
The handler is a line filter, not a template render: nothing is evaluated, no
brace elsewhere is touched, and hand-written documents are never seen. The
markers become EMPTY LINES rather than disappearing, so the file keeps its line
count -- broken source mapping was one of the reasons the Jinja pass had to go,
and re-creating it in the replacement would have missed the point.
Two things that forbade any source-read handler are corrected rather than worked
around, because a rule the code visibly breaks is worse than no rule. The test
now asserts the shape -- exactly one handler, named, scoped to __source_docs, no
render_string anywhere -- and AGENTS.md states the exception and its expiry.
The fixture is generated by clanguru itself, so the test tracks what spl-core
actually emits rather than what it emitted once. Verified non-vacuous by
unregistering the handler and watching it fail on the marker.
A second markup trap caught in passing: asserting "int add" against raw HTML
fails because syntax highlighting splits it across spans. The test strips tags
before asserting, the same mistake that hid this bug from me a week ago.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review findings 3 and 4. The Windows symlink fallback copied the whole CMake binary directory. Wrong three times over: it ran at CONFIGURE time, when the reports the path exists for have not been generated, so it only ever copied objects, binaries and CMakeFiles; dirs_exist_ok meant a later configure never cleared an earlier one's leftovers, so the copy went stale rather than obviously wrong; and nothing reads generated/ yet, so the cost bought nothing. It writes a marker explaining itself instead, and the fallback now has a test rather than leaving it to the one platform nobody develops on. ubc now runs in the documentation job, via useblocks/ubc-action, which puts the binary on PATH where the tests already look for it. CI_REQUIRE_UBC turns a missing binary into one clear failure instead of eleven skips: the parity tests are the only check that the two readers agree, and a check that silently does not run is worse than none, because the green tick claims it did. The other tests still skip rather than erroring on a missing binary, so the reason is stated once. Verified in all three modes -- optional and present, required and present, required and missing. Correcting the plan on one point: I recorded this as blocked on UBCODE_LICENSE_KEY / UBCODE_LICENSE_USER secrets. It is not. `ubc license show` reports no licence on this machine and there is no licence file, yet `ubc check` and `ubc build html` both work -- every local run in this branch was unlicensed. The action's licence inputs are wired anyway, so adding the secrets later changes nothing, but nothing waits on them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Review findings 5, 6, 7 and 8. Sphinx walks with followlinks=True, so `generated` -- a symlink to the configured variant's build directory -- was descended in full, under a second set of paths that the `build/modules` and `build/deps` exclusions never match. Excluding it prunes the walk: get_matching_files applies exclude_patterns to directories and writes the result back to `dirs`, so the descent stops rather than the files merely being filtered. Measured 0.09s -> 0.04s on a 301-file build directory, and it scales with the tree. That makes the forward-looking `generated/` configuration definitively inert for Sphinx as well as for ubCode, which is the right state but a fragile one: when spl-core writes the gcovr tree relative to the page and `generated/` becomes real, the `build/` forwarding has to be dropped in the SAME change or every report page is discovered under two docnames. That coupling was a comment, which is read once. It is now a test that fails when only half the change is done, verified by making the wrong half and watching it fire. The variant-selection task no longer passes a target: the pointer is always the docs cell. Choosing `reports` there opened every report fence in the IDE, each globbing into build/**, which ubCode excludes -- an unmatched glob per component. CMake writes the docs cell for the same reason. The target choice stays on the build task, where it changes what is produced rather than what is linted. AGENTS.md claimed documents name `generated/` instead of globbing `/build/**`. All seven still glob. It now describes the deferral, why it cannot be lifted from this repository, and what has to move together when it is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
avengineers/SPLed develop has two commits the useblocks fork lacks: spl-core 8.9 with a refreshed lock (f5ba89e), and the light controller state machine fix (b8beee9). Conflicts, and how they were resolved: - components/light_controller/doc/index.md: upstream nests the blink states in LIGHT_ON, inside a Jinja `{% if config.BLINKING %}` block. This branch turned that block into `{if} var.features.BLINKING` fences, so the fix goes into the BLINKING diagram, without the Jinja markers. - pypeline.yaml: upstream only quoted `python_version: "3.11"`; the fork had already replaced that key with `python_executable: python3`. Kept the fork's. - poetry.lock: upstream's refreshed lock, re-resolved against this branch's pyproject.toml. sphinx-codelinks stays at 1.4.0, the version this branch ran, with typer 0.26.7, because codelinks 1.4.0 caps typer below 0.26.8. - uv.lock, which only the fork keeps: aligned to the same versions. One conflict git could not see: the refreshed lock brings sphinx-needs 8.5.0, which loads needs_from_toml at config-inited priority 10 and resolves the variant data at 11. conf.py selected the variant data file at priority 20, too late, so every build either failed on a missing `build/autoconf.json` or silently read that pointer instead of the cell it was given. conf.py now selects it at priority 10, between the two.
The documentation changes this project needs from spl-core are not in a release yet: optional Jinja raw tags on generated listings, a variant data file per Sphinx build shape, a stable path to the build directory for the generated pages, and KConfig.declared_boolean_symbols(). They live on the feat/configurable-docs-pipeline branch of useblocks/spl-core, five commits on top of spl-core 8.9.0. Pin that commit, so CI and every machine build the same code. In both lock files only spl-core's source changes. Nothing uses the new settings yet; the following commits do.
KConfig omits a promptless boolean from its output when it evaluates to n, so the variant data defaults every declared boolean to false first. Finding them meant reading the kconfiglib instance spl-core keeps privately, behind a fallback for spl-core versions without an accessor. The pinned spl-core has KConfig.declared_boolean_symbols(), so call it and drop the fallback.
…orkarounds
The pinned spl-core can now be told what this project's documentation
needs, so each workaround in conf.py and CMakeLists.txt gives way to one
setting.
Jinja markers. SPL_SOURCE_DOCS_JINJA_RAW_TAGS OFF: clanguru no longer wraps
the generated listings in `{% raw %}`, and conf.py's source-read handler that
blanked the markers again is deleted. conf.py registers no handler at all.
Variant data per build shape. SPL_VARIANT_DATA_FILE_DOCS and _REPORTS name
the cells, and spl-core passes the right one to every Sphinx build as
`-D needs_variant_data_file=`. The fixed-name copies CMake published and
conf.py's fallback that read them are deleted. Nothing in conf.py selects a
cell any more, which is also the only way that works with sphinx-needs 8.5:
it replaces conf.py's values with ubproject.toml's and resolves the variant
data before a config-inited handler at the old priority runs, so the old
selection was silently ignored there. The tests and the VS Code task select a
cell the same way, and it is the same key `ubc check -c` overrides.
Stable report paths. SPL_SPHINX_BINARY_DIR points spl-core at `generated`,
the link tools/variant_data.py maintains, so every generated page is named
`generated/...`. The report sections name their pages directly instead of
globbing `/build/**`, spl-core writes each coverage report next to its page,
and SplBuild finds the report artifacts there. conf.py prunes `build` from
the walk, so each page has exactly one name, and forwards only spl-core's
`generated/` patterns: the `generated/**` rule in ubproject.toml keeps them
out of a docs build, declaratively, for both readers. On Windows without
Developer Mode, `generated` becomes a junction instead of a marker directory,
because the report pages are now read through it.
The tests follow: they drive the fake report tree through `generated`,
check the settings CMake hands spl-core, and assert conf.py registers no
source-read handler. The strip's line-number test is gone with the strip.
VARIANTS.md walks a newcomer through the declarative variant handling in 18 use cases: switching the variant ubCode shows, previewing any variant with Sphinx or ubc, comparing two variants, trying a change on a copy of a variant data file, and writing, extending and checking variant- dependent documentation. Every command in it was run against this branch. It sits at the root, outside both readers' document sets, so neither Sphinx nor ubCode indexes it. README.md and AGENTS.md link to it.
Nothing reads it. poetry.lock locks the project, and upstream deleted uv.lock in June 2025.
Pygments knows no jsonc lexer, so Sphinx warned twice. Its JSON lexer accepts // and /* */ comments, and ubc has no lexer check.
The three developer notes are orphan: true, which both readers honour. The requirements traceability matrix joins the start page's toctree, because it is product documentation. Four 'not in any toctree' warnings go in each reader.
tools/config_checks.py holds the checks the tests ran: the vendored needs model against spl-core's, ubCode's parser includes against conf.py's include_patterns, what ubproject.toml must leave to the generated files, and the generated rules against the grammar both engines share. conf.py runs them at config-inited and reports each finding as a warning, so a drift fails the build under -W instead of waiting for someone to run the tests.
The configuration tests read the generated rules and evaluate them, and the scope rules of a component report, with sphinx-mounts' own interpreter against the real cells; they check the one extend chain, the mount's condition and conf.py's source set by executing conf.py, and run the checks conf.py runs. The generator tests cover the component cells, the selection files with and without a build and for a component report, removing what earlier versions left, and hand-written rules. The documentation tests build through a selection file that mounts a fake build, where they used to re-point the link; ubCode reports a glob of the component toctree once, so the ubc comparison is made per glob.
ubc refuses an image in imported need content unless the import source is listed under [[needs.import_sources]] with allow_assets; since the image moved into the content, that is the CSV import. Sphinx needs no such permission and ignores the key.
sphinx-codelinks does not find the git root where .git is a file, and warns once per source (codelinks.git_ref). The gate tolerates exactly that warning, and only in a linked worktree; a clone, as on CI, has no such warning.
Two things let every component into a per-component report. A rule pattern without a slash matches at every depth, so the variant-wide rule's index.md also removed the component's own doc/index.md: the root index is no longer named, and is an orphan page, whose toctree entries a component report excludes anyway. And both readers match a rule's patterns against a mount's files relative to the mounted directory, so generated/<component>/** never narrowed the mounted build: each component's rule now names <component>/**, which covers its documentation in the tree and its pages in the build alike, and the variant's own pages are reports/**.\n\nA component report's links to the rest of the product dangle by design. Its selection file says so in ubc's terms (lint.ignore needs.dead_link), and conf.py suppresses sphinx-needs' needs.link_outgoing for that report. The rule tests now decide real paths with sphinx-mounts' own matcher, instead of comparing pattern strings, which is what let this through.
Every component report spl-core builds, in the docs shape, in both readers, strictly, with the needs compared: 24 of them next to the 10 variant cells. Each run selects its cell first, because codelinks reads the selected build's compile database.
ubmarco
changed the title
tools/try_variants.py sets up .venv with uv and the locked dependencies and walks through the cases -- one variant's documents in both readers, a CMake build with its reports, one component's report, a comparison of two variants, and the checks -- saying before each step what happens and after it what to look at. Each case configures the variants it shows, so the needs in the code follow their own #ifdef branches, and compare puts the developer's selection back. VARIANTS.md points at it from Setup.
Ruff 0.16 with the project's settings. Behaviour is unchanged: the subprocess calls that inspect their result say check=False, the logging filter catches the errors a bad record raises rather than every exception, the scripts with a shebang are executable, and noqa directives no current rule needs are gone. try_variants' docs case also says that it selects the variant it shows.
ubCode read 92 needs where Sphinx read 134 in Disco's reports: it does not mount a directory inside the project, and the selection mounted the build directory at `generated`. A mount injects sources from outside the project; these pages are inside it. The selection now names the selected build's pages by their own paths, as upstream spl-core names them, and both readers read the same 132 needs (the two untitled imports apart), field for field. - tools/variant_data.py: a reports selection declares the rst parser include of its build (one component's pages for a component report, none for the docs shape); the rules file gains one rule that shows a build's pages only for its own variant's and kit's reports, which the mount's condition used to do. build/selected_build.txt names the build for tools/compile_commands.py, since ubc refuses keys it does not know. - conf.py adds the selection's rst include to include_patterns and prunes only build/ directories that never hold a document. ubproject.toml drops its rst parser and the build/** exclude, keeping build/modules/** out: googletest's docs/index.md matched `index.md`. - Report toctrees glob /build/**/<component>/reports/...; the variant's coverage page gets a pattern per depth of the variant's name, since ** would match every component's coverage page too. - CMakeLists.txt no longer sets SPL_SPHINX_BINARY_DIR; spl-core is pinned at 5b98203, where it is reverted. - The gate also builds the reports shape of every test-kit variant and of every component report. It fails for Spa: clanguru drops the -isystem feature-header directory, so the listings show the wrong #ifdef branch (TS_BC-001 instead of TS_BC-002/003). That is fixed in clanguru. - The tests put back every file of the developer's selection.
The build jobs select the gate's tests by their quality-gate markers too, without ubc, and the gate failed whenever CI was set: 63 failures in the Linux job, 'ubc is required on CI'. It now fails only where CI_REQUIRE_UBC is set, the switch the documentation job sets for the same purpose in test_documentation.py, and skips elsewhere.
The source listings showed the #ifdef branch of "no feature defined": clanguru took only -I and -D from the compile database, and the feature header autoconf.h reaches the test sources through -isystem. Spa's report listed TS_BC-001, a manual-brightness test specification, instead of TS_BC-002/003, which Spa runs. clanguru also listed the declarations of the headers a file includes, whose tabs ubc warned about. Both are fixed in useblocks/clanguru (fix/docs-main-file-only, 07b8843), proposed to cuinixam/clanguru; this pins that commit until a release has them. The per-file ignore for the listings' tabs goes, and the gate passes in full: 63 runs, the reports of every test-kit variant included.
ubmarco
changed the title
ubmarco
changed the base branch from
feat/adopt-spl-core-docs-pipeline
to
develop
October 1, 2026 13:39
ubmarco
marked this pull request as ready for review
October 1, 2026 14:10
Rubyfi
approved these changes
Oct 1, 2026
Rubyfi
left a comment
Rubyfi
left a comment
There was a problem hiding this comment.
PR Review
Reviewed in the Linux dev container on 2026-10-01 at commit 867d16b.
Findings
1. [Medium] The spl-core pin depends on temporary refs
pyproject.toml pins spl-core to
5b982030ab8e6bf192991a6613669d95a9e37af5. The commit is currently the tip of
only these remote refs:
refs/heads/feat/docs-parity-sweeprefs/pull/5/head
Deleting or rebasing the branch and eventually expiring the pull-request ref
would make fresh dependency installation fail. A durable tag or an upstream
release should protect the pinned revision.
2. [Low-medium] The parts.cmake parser accepts unsupported syntax
tools/variant_data.py recognizes the test-kit guard with a case-folded
substring check. Direct execution confirmed that it accepts all of these as the
plain test branch:
if(NOT BUILD_KIT STREQUAL test)
if(BUILD_KIT STREQUAL test OR FOO)
if(BUILD_KIT STREQUAL testing)
if(BUILD_KIT STREQUAL Test)The first condition reverses the intended branch, while the others exceed the
documented grammar. The parser also returns "components/a" with its quotes
intact for a quoted component, so generated membership conditions do not match
the actual component. A multiline spl_add_component() call raises the bare
error ValueError: substring not found rather than a contextual grammar error.
Match the normalized condition exactly, handle quoted arguments deliberately,
and reject multiline calls with the file and line in the error message.
3. [Low] The VS Code coverage tasks open paths that are not generated
The Open variant coverage report and Open component coverage report tasks
expect standalone files such as:
build/Disco/test/Debug/reports/coverage/index.html
That path does not exist after a successful build. The report is copied into
the consolidated Sphinx output instead:
build/Disco/test/Debug/reports/html/build/Disco/test/Debug/reports/coverage/index.html
Update the task commands to use the consolidated report paths, or have the
build produce stable redirects for the task paths.
4. [Low] Tests do not protect the required CMake setting order
The spl-core documentation settings currently appear before the
parts.cmake include, which is required because spl-core computes its output
paths while adding components. Existing tests assert the setting values but not
their position. Moving them below the include could therefore break report
generation without failing the focused tests.
Add an assertion that each required setting precedes the parts.cmake include.
5. [Nit] Two comments describe obsolete configuration
.gitignoredescribesgeneratedas the current-variant pointer and mentions
a Windows copy fallback, although selection now uses TOML files.doc/cross-platform-alignment-proposal.mdsays the project pins
spl-core==8.6.0, while the installed and declared dependency is spl-core
8.9.0 from the pinned fork commit.
Validation
Environment:
- Python 3.12.3
- spl-core 8.9.0
- sphinx-needs 8.5.0
- pytest 8.4.2
Focused review suite:
.venv/bin/python -m pytest \
test/test_variant_data.py \
test/test_ubproject_config.py \
test/test_documentation.py -qResult: 118 passed, 12 skipped in 37.87s. All skips require ubc, which is not
installed in the container.
Variant builds:
./build.sh --build --variant Disco --build-kit test --build-type Debug
./build.sh --build --variant Spa --build-kit test --build-type Debug
./build.sh --build --variant Sleep --build-kit test --build-type Debug
./build.sh --build --variant Disco --build-kit test --build-type Debug| Variant | Tests | Failures | Errors | Skipped |
|---|---|---|---|---|
| Disco | 22 | 0 | 0 | 0 |
| Spa | 18 | 0 | 0 | 0 |
| Sleep | 37 | 0 | 0 | 0 |
Each build configured, compiled, ran its tests, and generated its consolidated
HTML report. Switching from Spa and Sleep back to Disco also succeeded. The
Disco Sphinx report completed schema validation with zero warnings.
Test Gaps
- Windows was not tested.
- ubc parity checks were not run because ubc is unavailable.
- Parallel variant builds were not exercised.
The test-kit guard was found by a case-folded substring, so if(NOT BUILD_KIT STREQUAL test) was read as the test branch, and a compound condition, another kit name or STREQUAL Test passed as well. A quoted component kept its quotes, so its rule never matched, and a call over two lines raised a bare 'substring not found'. Each statement is now matched whole: command names in any case, as CMake reads them, arguments exactly. Everything else raises with the file and the line. Found in the review of #4.
The coverage tasks opened build/<V>/test/Debug/reports/coverage/index.html, which no build writes: gcovr's tree is part of the variant's HTML report, at reports/html/<build-relative path>/coverage/. The same holds upstream. The component report task opened the report's index.html, which is the orphan variant start page since the component report has a root of its own, doc/component_report. Found in the review of #4.
spl-core computes what it writes for a component while parts.cmake adds it, so a setting moved below the include applies to none, and every test of its value still passes. Found in the review of #4.
.gitignore still described the generated link as the current-variant pointer, and the cross-platform proposal named spl-core 8.6.0 as the pin. Found in the review of #4.
Member
Author
|
Thanks for the thorough review, @Rubyfi. All findings confirmed and reproduced. 2. 3. VS Code report tasks — fixed in ed69f22. Both coverage tasks open the consolidated report paths (the wrong path was there upstream too). While checking, I found that Open component test report opened the component report's 4. Settings order — 0717018 adds 5. Obsolete comments — c452ecb fixes 1. Temporary pins — agreed. The clanguru pin (useblocks/clanguru 07b8843) has the same exposure. The durable fix is the upstream releases (spl-core#5, cuinixam/clanguru#9). Until then I'd protect both commits with tags in the forks; I'll do that separately. Also re-checked: Sphinx |
A bare 'ubc build html' writes _build/html under the project root, and Sphinx's convention is _build/<builder>. SPLed's own commands write under build/, but a bare run left an untracked _build/ behind.
ubc rendered `remote-url` as plain text: sphinx-codelinks registers the field and adds its link rule only while Sphinx runs, in Python, so ubCode never saw either. ubproject.toml now declares the field and a rule for the URL ubCode stores there. sphinx-needs applies only the first rule naming a field, matching or not, and the TOML rule came before codelinks' own, which left Sphinx's value unlinked; conf.py drops it for Sphinx between loading the TOML and compiling the rules. The duplicate-field filter covers remote-url too. The readers store different values in the field (the path in Sphinx, the whole URL in ubCode), so the gate lists it with that reason in docs_exceptions.toml instead of comparing it.
…branches spl-core ce62088 is the merge of useblocks/spl-core#5 into the fork's develop, which mirrors avengineers' develop; clanguru 50a25d7 the merge of useblocks/clanguru#1 into the fork's main. Both contain the commits pinned before (5b98203, 07b8843), which were reachable only from feature branches and pull-request refs. Answers finding 1 of the review of #4.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Variant-dependent documentation without Jinja, read the same way by Sphinx and by ubCode/ubc, for every variant.
This PR targets
developdirectly and carries the whole line of work: what #2 and #3 started, and this sweep. Those two PRs are not merged separately.spl-core changes: useblocks/spl-core#5, pinned as a fork commit. clanguru fixes: useblocks/clanguru#1, proposed upstream as cuinixam/clanguru#9, also pinned as a fork commit. Both pins move to PyPI releases once upstream has released them.
What changes
Selecting a variant generates everything the readers need. CMake configure, or
tools/variant_data.pyon its own, writes these files. Nothing is decided inconf.pyor on the command line, and nothing per component is maintained by hand.build/variants/<V>/<kit>/<target>.jsonvar.*: every KConfig symbol (promptless booleans included), the variant, kit, target and component list (fromparts.cmake). There are cells for per-component reports too.ubproject.variants.tomlbuild/selection.toml<build>/selection/[<component>/]<shape>.tomlubCode reaches the selection through
extend, Sphinx throughconf.py, and spl-core's runs through-D spl_selection=. Adding a component means adding it toparts.cmakeand writing its documentation.No Jinja anywhere.
{if}directive.source-readpass, the Jinja raw tags in the listings and the per-variant JSON copies are all gone.Generated pages are read where spl-core writes them, under upstream's names. That covers test specifications, test results, coverage and source listings.
/build/**/<component>/reports/..., which resolves to exactly one page because one build is read.ubc diff -c "$(cat <build>/selection/reports.toml)"compares them.Traceability in both readers:
@needcomments in the C sources, shown on each component's page. codelinks takes each variant's#ifdefbranches from the selected build's compile database.needextend. sphinx-test-reports and thesple_tr_linkfunction are gone.One configuration file,
ubproject.toml, with spl-core's needs model vendored into it.tools/config_checks.pyruns on every Sphinx read and catches drift:A documentation gate in CI (
test/test_docs_gate.py). For every variant and kit, and every per-component report, it builds withsphinx-build -Wand with ubc and compares the two needs.json files need by need. It does this in two shapes:The only allowed differences are listed in
test/docs_exceptions.toml:REQ_37andREQ_58, which are untitled in the upstream ubConnect export.Fixes found on the way:
SWDD_BC-203was implemented outside its#ifdef; upstream has the same bug.#ifdefbranch and the system headers' functions. That's fixed in clanguru: Spa's report listedTS_BC-001, a manual-brightness test specification, instead ofTS_BC-002/TS_BC-003, which Spa actually runs.For people new to it:
VARIANTS.mdis a step-by-step guide by use case.tools/try_variants.pyis a cross-OS guided tour, with setup, docs, build, component, compare and check cases. Each step says what it does and what to look at afterwards.AGENTS.mddescribes the design.Results
Backwards compatibility
ubproject.tomlno longer extends spl-core's base configuration; it is vendored, and a test fails when it drifts.build/andubproject.variants.tomlare generated and opened read-only in VS Code. Ageneratedlink from an earlier version of this work is removed on the next selection.Not in this PR
develop's nightly run fails the same way, so it doesn't come from this branch.REQ_37andREQ_58need titles in the ubConnect export.