✨ needflow: a portable option vocabulary, with deprecations - #1770
Draft
chrisjsewell wants to merge 39 commits into
Draft
✨ needflow: a portable option vocabulary, with deprecations#1770chrisjsewell wants to merge 39 commits into
chrisjsewell wants to merge 39 commits into
Conversation
added 24 commits
August 15, 2026 15:03
Add a `:direction:` option (down/up/right/left, plus the TB/TD/BT/LR/RL aliases) and a `needs_flow_direction` project default, resolved with the `max_items` rule: only an unset option consults the configuration. Each engine spells the intent itself. Graphviz has all four as `rankdir`. PlantUML has only `top to bottom direction` and `left to right direction` -- `bottom to top direction` and `right to left direction` are syntax errors (verified against PlantUML 1.2020.02) -- so a reversed direction degrades to its axis mate and warns once for the project. An explicit option beats a direction carried by an engine config blob: the blob is a preamble of defaults and the option a per-element value, so the statement is emitted after the blob and wins. A disagreement between the two is reported, since the author has plainly said two different things. Nothing is emitted for a diagram already drawn the way it asks to be, so a diagram that does not use the option keeps the source it had before. Also adds `match_variants_all`, the cascade counterpart of `match_variants`, and bumps ENV_DATA_VERSION to 7 for the new `NeedsFlowType` keys.
Add a `:link_labels:` option (none/outgoing/incoming/type) and a `needs_flow_link_labels` project default, and route both engines through one `GraphEdge.label()`. `:show_link_names:` and `needs_flow_show_links` become deprecated aliases for `outgoing`, honoured as before and warned about only when used. The tri-state also fixes their interaction: the two used to be OR-ed, so a project that turned labels on left no way of turning them off again for a single diagram. `:link_labels: none` is that way out. `incoming` and `type` are new: the first labels an edge with the reverse title, the second with the bare link field name for a diagram that wants the data model rather than prose. The needflow fixtures move to the new spelling, which keeps the two `warnings == ""` builds silent.
Add a `:legend:` option (types, links, or both) and a `needs_flow_legend` project default, rendered as docutils tables beside the diagram. The legend is a document, not a picture, so there is one implementation for every engine instead of one per engine: the two in-diagram legends had drifted into different scope rules -- plantuml listed every configured need type, graphviz only the drawn ones -- and neither could describe link types at all, which was a standing TODO in the graphviz engine. The new legend lists only what the diagram actually drew, need types and link types alike, and is selectable text rather than pixels. A section with nothing to describe is omitted rather than drawn as empty headings. `:show_legend:` keeps its per-engine in-diagram rendering exactly as it was, so no existing diagram changes, and is deprecated. An explicitly empty `:legend:` means "no legend", which is how a diagram opts out of a project default.
Add a `:styles:` option and a `needs_flow_styles` configuration of named style classes, so that "these needs look like that" is expressible portably. The predicate half is the variant syntax the project already uses, so no third mini language appears; the value half is a class name rather than inline properties, so a rule means the same thing on every engine. Declarations cascade like CSS: every matching rule contributes, later wins per property. The property set is closed -- fill, border, border_width, border_style, text_color, shape -- because an engine specific escape is the job of the engine config, not of a class that has to travel between engines. What an engine cannot draw degrades quietly (a wide border becomes a bold plantuml line) or with one warning (a shape plantuml has no form for); a class that is not configured is an authoring mistake and is reported per directive, with the diagram still drawn. `:highlight:` and `:border_color:` become deprecated sugar. `highlight` is a built-in class rendered in each engine's legacy red-outline form rather than through the property machinery, so moving from the option to the class leaves the diagram byte-identical. Both graphviz node paths -- plain node and subgraph -- now share one presentation renderer, so they cannot drift apart again as they had before.
Add `:engine_config:` and `needs_flow_engine_config`, the one discouraged way through to engine specific syntax: the blob lives in the configuration under the engine it belongs to, and the document only names it. Documents therefore stay portable even when a project chooses not to be. This is a rename, not a redesign. `needs_flow_configs` and `needs_graphviz_styles` are still read, under the same names and with the same values, so no project has to move anything to upgrade; `:config:` becomes the deprecated spelling of the same selector. The unknown-name warning now points at both places a name could have come from. Two failure modes stop being crashes: - A graphviz style entry that is not a mapping used to travel unchecked into the emitter and abort the build with `'str' object has no attribute 'items'`. It is now rejected where it is read, and the diagram is drawn without it. - An unusable `needs_flow_engine` tripped a bare `assert`, ending the build with a traceback rather than a message. It now warns once, names the allowed values, and falls back to the default engine.
Three ways in which the same option meant different things depending on which engine happened to draw the diagram: - `:class:` was collected by the directive and then dropped by the plantuml engine, so it styled a graphviz diagram and did nothing to a plantuml one. It now reaches the plantuml figure too. - `:debug:` emitted raw HTML on plantuml and a literal block on graphviz. Both now emit a literal block, so the source gets line numbers and the theme's code styling either way. Pygments has no PlantUML lexer, so that source is shown unhighlighted rather than wrongly highlighted. - `:scale:` sizes a raster image and has always been silently ignored by graphviz. It is deprecated -- honesty rather than loss -- pointing at `:width:` / `:height:`. `needs_flow_link_types` is deprecated as dead: the directive always defaults its `:link_types:` option to every link field, so the configuration has never been reachable, and making it work now would silently narrow every existing diagram.
`needs_links` gains `line`, `part_line` and `arrow`, and its `color` is finally honoured; `needs_types` gains `shape`. All of them say what is meant rather than how one engine spells it. The old keys held PlantUML tokens that graphviz had to translate through a lookup table with a documented "cheat", and `color` was carried by the configuration but drawn by neither engine (an identical TODO sat in both emitters). The neutral values are enumerations every engine can draw: `line` is solid/dashed/dotted/thick/invisible, `arrow` is normal/none/open/circle/cross/both, and `shape` is a ten member vocabulary whose members all exist -- exactly or as a near form -- on both engines. The legacy keys keep working, and keep drawing exactly what they drew, so a project can migrate one link type at a time; using them now says so once. `needs_types[].style` is untouched, and only a type that opts in to `shape` is drawn from the neutral vocabulary, so no existing diagram moves. The configured default `color` of `#000000` is the engines' own edge color, so it is left unsaid rather than restated on every edge of every diagram. Test fixtures whose values migrate losslessly move to the new keys; the `filter_doc` fixture deliberately keeps the old ones, since it gives a link type different colors for parts and non-parts, which one `color` cannot say.
Add a per-option reference for `:direction:`, `:legend:`, `:link_labels:`, `:styles:` and `:engine_config:`, and for the five new configuration values behind them, each with what it means, what it degrades to where an engine cannot draw it, and how it interacts with the project default. The examples throughout the documentation move to the new spellings, including the tutorial, whose per-status border colours become a small set of named style classes in `ubproject.toml` -- which is what the classes are for. The legacy spellings keep their own section, with their anchors intact, so every existing inbound link still resolves. Every superseded option and configuration value gains a `.. deprecated::` marker naming its replacement, and `needs_flow_link_types` gains the warning its documentation should always have carried: it does nothing, and has never done anything. The documentation deliberately demonstrates the deprecated spellings in the sections that document them, so `needs.deprecated` joins the suppressed warnings in `docs/conf.py`.
Formatting and typing fixes from `pre-commit run --all-files`: the new `needs_links` display keys are declared on `NeedLinksConfig`, the style default uses a `default_factory`, and two lint nits are folded away.
The legacy `style` value was passed in but never consulted: the decision is made on the neutral value alone, and an unset one simply hands the emission back to the legacy path.
…th engines `rankdir` is a graph attribute, and both the shipped `lefttoright`/`toptobottom` configs and the shape the documentation teaches key it inside a `graph [...]` block. Detection looked only at the top level, so on graphviz the blob's direction went unnoticed: the disagreement warning never fired, and with `:direction: down` nothing was emitted at all, leaving the blob to win -- the exact opposite of the published collision rule. Both halves are fixed. Detection now reads `root` then `graph`, in the emitter's own order of increasing authority, and the resolved `rankdir` is written after the whole preamble, since a later graph attribute statement overrides an earlier one (verified with dot(1)). A direction that merely restates what the diagram already draws is now dropped on both engines, rather than only when it happened to be `down`. A legacy `:config: lefttoright` diagram was gaining a second `left to right direction` statement, which moved the bytes of input that uses no new option at all. The direction-versus-config test gains its graphviz leg -- its absence is why this got through -- and asserts statement counts rather than mere presence.
`needs_flow_direction = "sideways"` reached a lookup table unchecked and ended the build with a traceback -- the exact failure mode this slice had just removed for `needs_flow_engine`, reintroduced by its own flagship option. `needs_flow_link_labels` had the opposite fault: an unusable value was silently inert. Every enumerated configuration value now goes through one validator that warns once under `needs.config`, names the allowed values, and falls back to the documented default. A mistake in one line of `conf.py` is not a reason to end a build. `mermaid` becomes a reserved engine name rather than a rejected one, for both the `:engine:` option and the configuration. ubCode draws needflows with it, and a document naming it was losing its diagram entirely to a hard docutils error -- which is precisely the unportability the reservation exists to prevent. It is accepted and degrades to a drawable engine with one warning. The unusable-engine check moves to configuration load time, so a project that misconfigures it and happens to have no needflow anywhere is still told.
Unifying the two graphviz node paths onto one presentation renderer quietly gave the subgraph path the plain node path's quoting, turning `style=filled` into `style="filled"` for every clustered need. Graphviz treats the two alike, but the generated image file is named after a hash of this source, so every needflow image of every project that nests needs was renamed on its first rebuild. Byte-preservation wins: the subgraph path writes a bare value again, and quotes only a multi-style value, which cannot be written bare. Pinned by a test, since nothing pinned it before.
…ation Three related defects in how a link type's colour is resolved. A link type migrated one key at a time lost its colour. The deprecated `style` is a compound of a colour token and line keywords; setting the neutral `line` supersedes that string, and it was being dropped wholesale, colour and all, with nothing said. The colour is now rescued when the neutral one is unset, which is what makes the incremental migration the changelog invites actually safe. `part_color` joins `part_line`, falling back to `color` exactly as `part_line` falls back to `line`. Without it the deprecated `style`/`style_part` pair could express a distinction the neutral vocabulary could not -- a different colour for part links -- which left four link types in this project's own documentation configuration, and one test fixture, permanently on the deprecated spelling. Those five now migrate. This is an additive amendment to the planned vocabulary and is recorded as one. Black becomes expressible. Treating `#000000` as "the same as unset" made the one colour a user could not ask for the one the engines happen to default to, which stops being harmless as soon as an engine config sets an edge colour of its own. Unset is the sentinel instead, and the built-in `links` type no longer ships a black that was only ever standing in for "nothing". `part_line` also gains its first diagram-source assertion, on both engines.
Three warnings were quieter or vaguer than the degradation policy asks. An unknown `:styles:` class is an authoring mistake, which the policy puts in the tier that warns per directive -- but the check ran per need, so it carried a project-wide once-filter and the same typo in twenty documents produced one warning pointing at one of them. Which classes a rule names does not depend on the need it is tried against, so the check now runs once per directive, next to the class compilation, and the per-need path stays silent. The plantuml shape and arrow degradations passed no location, so a project was told once, without being told where. Both now name the needflow they came from. `compile_style_classes` and `resolve_legend` accepted a location, documented it, and discarded it, while two comments claimed the opposite; the parameter is now threaded through to every warning it was meant to reach, including the shape and property coercions. The dead `allow_empty` parameter of `parse_legend`, whose branch nothing could reach, is gone.
…th engines The three legacy tests were single-engine, yet the deprecated `style`/ `style_part`/`style_start`/`style_end` keys reach graphviz through a whole translation layer of their own -- the layer the entire deprecation story rests on -- and labels are emitted differently per engine. All three gain their graphviz leg. `:border_color:`'s deprecation notice was only filtered out of an assertion, never asserted; eight of the nine deprecations were positively pinned and this was the ninth. Every way of misdescribing a style class is now exercised: the container, each class, the property name, and each property's own value grammar. The values are written into `conf.py`, since Sphinx refuses to override a dictionary setting with another type -- and a hand-written `conf.py` is where these mistakes are made. An out-of-enum *option* value is pinned as the one place erroring is right, against a configuration value, which degrades. Also corrects the `flow_show_links` deprecation marker, which named a version that is already released.
… suppression Four of the config rows this slice shipped had no user documentation at all, while the code deprecated the spellings the documentation still taught -- so a reader following the docs was warned off them with nowhere to go. `needs_links[].line/part_line/color/part_color/arrow` and `needs_types[].shape` are now documented beside the keys they replace, which carry `.. deprecated::` markers, and the examples teach the neutral spellings. `arrow` gets an explicit migration table, because the mapping has a trap worth naming: the member that reproduces what a link type draws today is `open`, not `normal` -- the default `->` has always rendered as an open head on graphviz, and `normal` is the solid one. The enum is not bent to hide this; it is documented. The project-wide `needs.deprecated` suppression is gone. It was blunt: it switched the notice off for the whole documentation build, for every deprecation in Sphinx-Needs, and would have hidden any accidental future use in the examples readers copy from. The sections that demonstrate a deprecated spelling now show it as a non-executed sample with a pointer to the rendered result of its replacement, and the build is warning-free without the blanket. Changelog gains the four user-visible fixes and the one output change that had none, including that a multi-name engine config no longer leaks one diagram's styles into the next.
Formatting from `pre-commit run --all-files`, and one suppression that the narrowed section lookup no longer needs.
A need title is escaped for the graphviz HTML-like label and then wrapped, so the wrapper counted the characters of an entity and could break inside one: a title holding a quote wrapped to `&quo<br/>t;`, which is invalid markup and a visibly broken label. The wrap width was also counting escaped characters rather than what the reader sees. Wrapping now happens first and each piece is escaped after, which fixes both. Found by the conformance corpus: the `percent-neutralisation` case exists to check that text significant to a diagram language stays literal, and the first generated expectation for it contained the broken entity. Also records the PlantUML version caveat for the `hexagon` shape, which older releases have no element for.
The portable vocabulary is implemented twice -- here and in ubCode -- and nothing else would make the two drift visibly. The corpus is 24 language neutral cases, each stating some needs, some portable configuration and some directive options, plus the diagram source every engine is expected to emit and the degradations it is expected to report. ubCode reads the same files. `README.md` is the shared format spec, verbatim, so it travels with the corpus; `manifest.json` checksums every case and the spec itself, which is the only thing that notices an edit that never reached the manifest. The 12 required families are covered: the no-warning fence, node-id injectivity, empty edge labels, text significant to a diagram language, colour normalisation, all four directions (including the two plantuml degrades), the three legend scopes, all four link-label kinds, the style cascade and the built-in highlight's byte parity, line/arrow/colour together, part styling, three shapes including one plantuml cannot draw, and one case per remaining degradation-registry id. Expected sources are generated from the code and then read: every one was checked against its case's stated purpose rather than accepted as a snapshot, which is how the broken-entity bug in the graphviz label was found. Mermaid expectations are deliberately absent. They belong to the ubCode slice, which owns the repo of record and will re-sync both copies before either pull request opens. The harness's own refusals are pinned too -- unknown keys, a tier-1 entry, an unknown degradation id, an unstamped checksum -- because a runner that quietly accepts a malformed case gives a green count for something that asserts nothing.
The corpus is shared byte-for-byte with ubCode and checksummed in its own manifest, so a formatter local to this repository must not rewrite it -- doing so would dirty every case the moment either copy is re-synced, and break the checksums that make a drifted copy visible. Plus formatting fallout in the harness itself, which the formatter does own.
The portable legend is one engine-independent out-of-diagram implementation (ruling D3), so it never appears in any `expect.<engine>.source` and the three legend cases had identical sources -- which left the drawn-only scope rule, the whole point of family 7, unasserted. The amended spec gives it a top-level `expect.legend` key, a sibling of the engine keys rather than a member of one, listing exactly the rows each section must show, in order. Absence is meaningful: it asserts that no legend renders at all, which every legendless case now gets for free. An empty list, an empty key, or an unknown section is refused outright, since a case claiming a legend while asserting nothing about it would pass silently. That replaces the derived check the harness carried while the format question was open. Verified to bite in all three directions: an undrawn type added to an expectation fails, a legend expected where none renders fails, and a legend rendered where the key is absent fails. The legend tables gain per-section classes so a reader's stylesheet -- and this harness -- can address one section instead of counting tables. README re-copied verbatim from the amended spec; `corpus_version` bumped to 2 and every checksum recomputed, per the rule that any change bumps it. Also adds the changelog bullet for the graphviz entity/wrap fix, which landed with its documentation but without an entry.
`needs_flow_engine` was already checked where the configuration is read, while `needs_flow_direction`, `needs_flow_link_labels` and `needs_flow_legend` were checked only as a diagram was drawn -- so a project that misconfigured one of them and happened to have no needflow anywhere heard nothing at all. All four now go through one `validate_flow_config` at load time, using the same functions the resolution uses. Those warn once per message, so a project that does draw diagrams still hears about a bad value exactly once rather than twice, and the in-place fallback during resolution is unchanged.
…st of the suite The conformance harness synthesized its projects without setting `plantuml`, so they fell back to sphinxcontrib-plantuml's default command. That renders only on a machine that happens to have a `plantuml` on PATH; CI runners have java and the vendored jar and nothing else, so every plantuml case failed its image render, and the harness -- correctly -- refused the resulting warning as outside the degradation registry. All 24 plantuml legs were red on CI while every graphviz leg passed. The projects now point at the same jar every other test uses, taken from a new `plantuml_command` fixture so there is one definition of it rather than two; `test_app` uses it as well, which is where the string previously lived. Three needflow tests that call `make_app` themselves went the same way round the fixture. They were not failing -- their assertions only look for a substring -- but they were emitting a spurious render warning on any machine without plantuml, one assertion-tightening away from the same breakage. Reproduced first by masking the container's plantuml, which gave exactly the CI failure (24 failed, 24 passed, same message); the whole suite now passes both with that mask and without it.
Member
Author
Open design questions before this mergesThree things worth settling while both this PR and the paired ubCode one are still open, since 1. Fold the new options back into the existing names, rather than adding new onesThe proposal: instead of Recommendation: do it, for all three. It removes three of the nine deprecations, and — more
For the link labels this is a clean flag→enum widening: there is no semantic fork, the bare form The one wrinkle, on That is defensible, and I'd resolve it as: bare Knock-on if adopted: 2.
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #1770 +/- ##
==========================================
+ Coverage 86.87% 90.58% +3.70%
==========================================
Files 56 76 +20
Lines 6532 11737 +5205
==========================================
+ Hits 5675 10632 +4957
- Misses 857 1105 +248
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
chrisjsewell
marked this pull request as draft
August 15, 2026 21:19
added 3 commits
August 15, 2026 21:39
Git rewrites LF to CRLF on checkout under `text=auto`, so a Windows checkout of the corpus holds different bytes for identical content -- and the manifest hashed raw bytes. Every case checksum, the README's, and the mismatch test failed on the Windows CI leg while every other platform passed. The manifest stamps content, so the hash now normalises CRLF and lone CR to LF first. `.gitattributes` additionally pins the corpus to LF, which keeps the working-tree bytes canonical and stops a re-sync between the two repositories carrying a platform's line endings across; the normalisation is what makes the contract hold when that pin is missing or a file arrives some other way. The pin is a later, more specific pattern than `* text=auto`, so it wins without fighting it. Both halves of the contract are pinned together, because normalisation must not buy portability by giving up the tamper detection it exists to serve: a line-ending rewrite must not move a checksum, and a real edit still must. Manifest regeneration also gets a committed home. It was being done by hand, with its own copy of the hashing, which is exactly how the two could have disagreed; verification and regeneration now share one function, reached with `UBC_UPDATE_CORPUS=1`. README re-copied verbatim from the amended spec, so `corpus_version` goes to 3. Every case checksum is unchanged -- they were already LF -- which is the expected shape of this diff.
The new regression test read the corpus file and asserted it held LF, which is a claim about the working tree rather than about the hashing -- and it is false in exactly the situation the normalisation exists to survive, so the corpus suite failed when run against a CRLF checkout. It now derives its reference content by normalising whatever is on disk, and compares the LF, CRLF and lone-CR encodings of that against each other and against the corpus file. The suite passes identically under both encodings, and still fails if the normalisation is removed.
The bare `:show_link_names:` flag already meant precisely one of the four values the new option offered, so this is a widening rather than a rename: the option now takes an optional value -- `none`, `outgoing`, `incoming`, `type` -- and written bare it still means `outgoing`. `needs_flow_show_links` likewise accepts those values beside the booleans it always took, where `True` is `outgoing` and `False` is `none`. `:link_labels:` and `needs_flow_link_labels` are deleted outright. Both were new in this unreleased branch, so there is nothing to deprecate and no alias worth keeping. That removes two deprecations from the two spellings most likely to be in existing documents: neither the option nor the configuration is superseded any more, so their warnings, their `.. deprecated::` markers and their changelog entries go, replaced by `.. versionchanged::`. Seven deprecations remain, and the changelog list now matches the code. The values still fix what the flag could not express: the option and the configuration used to be OR-ed together, so a project that turned labels on left no way of turning them off again for one diagram. `show_link_names` stays a bool in the shared `NeedsFilteredDiagramBaseType`, which needgantt and needsequence also use as a flag, so needflow's widened value lands beside it rather than changing its type. The fixtures that had moved to `:link_labels: outgoing` purely to silence the deprecation move back, restoring the incidental end-to-end coverage of the bare form. The conformance corpus is untouched: it carries the portable vocabulary, deliberately decoupled from what either repository calls a thing, so the cases keep saying `link_labels` and only the harness mapping changes -- to `needs_flow_show_links` in the synthesized configuration and `:show_link_names:` in the synthesized document. All 24 case checksums stay byte-identical and `corpus_version` stays at 3.
added 12 commits
August 15, 2026 23:26
Rebuilding an existing `_build` across this branch's own commits crashed with `KeyError: 'show_link_names_value'` (and symmetrically `'link_labels'` the other way round). Parent and tip both stamped 7, so Sphinx kept the pickled doctrees and the new emitter read the old shape -- exactly the situation this constant exists to prevent. Reproduced at exit 2, and resolved by the bump, which makes Sphinx report `build environment version not current` and re-read. 7 was an intermediate shape that existed only within this unreleased branch, so the released step is 6 to 8. The earlier reasoning that no further bump was needed held for the *user* upgrade path -- 6 to 7 does force a full re-read -- but not for anyone bisecting or switching branches.
`needs_flow_show_links` was declared a boolean for years, so any truthy value drew labels and drew them silently. Holding every non-boolean to the new enumeration turned `1` from labels-on into labels-off with a warning -- a real regression for a value master accepted without comment. Only a *string* is now held to the enumeration, since a string is someone naming a value rather than leaning on truthiness. Everything else is read for its truth, as before. The prose in both places claimed more than that fix delivers, so it is narrowed to what actually holds: booleans and other non-strings keep their meaning, and an unrecognised string -- which used to be truthy and draw labels -- now warns and falls back to `none`. That is the one input whose behaviour changes. The `type` row of the config test asserted only that `links` appeared, and `links` is a substring of both `links outgoing` and `links incoming`, so a mis-mapped `"type"` shipped green; proved by injecting exactly that mis-mapping. It now excludes the two values it is a substring of, as the option test already did. Also documents on the shared `show_link_names` key that for needflow it records only that the option was given, and points at `show_link_names_value` -- its meaning changed for anyone reading needflow nodes out of tree. Plus two stale parameterisation ids and a trailing blank line left by a deleted section.
Two things the widening left behind. The ubCode copy does not exist yet, so a version bump costs nothing to reconcile -- and shipping a corpus whose stated rationale is false costs more. `link-labels-outgoing` still called `:show_link_names:` "the deprecated" option and "an alias". Both became false when the option was widened rather than replaced, and it was the last place in the tree describing either kept name as deprecated. The rationale is rewritten around what is true: every implementation has a spelling that already meant this, which is why the sphinx-needs one was widened, and this case is the byte-parity target each checks its own spelling against. The `CONFIG_KEYS` row for `link_labels` was dead: all five cases using it drove it through `options`, never through `config`. Two cases now drive the configuration half -- one for a project default reaching a diagram that says nothing, which also pins `type` as the bare field name where a containment check cannot (`links` being a substring of both titles), and one for a diagram overriding a default that turns labels on, which is the case that used to be inexpressible. `corpus_version` 3 to 4, manifest regenerated through `_build_manifest()`. The three legend cases are reformatted from inline to block sequences by that regeneration, with no change of content.
Found by the ubCode review as a cross-tool divergence: `needs_flow_show_links = "Outgoing"` warned and silently fell back to `none` here, while ubCode -- which normalises both halves -- drew labelled edges for the same configuration. Two tools rendering the same project differently, with nothing to suggest that capitalisation was the cause. The asymmetry was internal to Sphinx-Needs as well. The directive options go through docutils' `choice`, which lowercases and strips before matching, so `:show_link_names: Outgoing` and `:engine: PlantUML` have always been accepted; only the configuration side matched exactly. Normalising in the shared validator fixes both problems at once, in one place. `needs_flow_engine` is included deliberately, and it is the least arguable of the group: its own option already tolerates case through `choice`, so leaving the configuration exact-match would have preserved an asymmetry rather than avoided widening one. `needs_flow_legend` already normalised through `parse_legend`, and its test passes unchanged. The non-string truthiness path is untouched: it returns before any string handling, so `1` and `True` never reach the normaliser. Normalising accepts what the author plainly meant, not anything -- a value still wrong after normalisation is still reported, and the message quotes what was actually written so it can be found in `conf.py`.
`:legend:` and `needs_flow_legend` are removed before they were ever released, and the existing `:show_legend:` flag is widened in their place. The option now takes the *key* of a legend configuration, and never the sections inline: a project names its legends whatever it likes, so an inline vocabulary would mean a legend called `types` collided with the section called `types` and needed a precedence rule nobody should have to learn. One namespace, no reserved words. `needs_flow_legends` holds the named configurations -- `parts` (types, links or both, in order) and `placement` (internal or external). `needs_flow_show_legend` names the project default. It selects *which* legend, never *whether*: presence stays per directive, so a project default cannot give a legend to a diagram that never asked -- which is also why the key namespace needs no "off" value a legend could collide with. Written bare, `:show_legend:` resolves to the engine's own default, which both engines here draw inside the diagram exactly as they always have, so the emitted source is byte-identical to before and no fixture changes. `placement` is a preference rather than a demand. An engine that cannot draw the legend inside the picture renders the table beside it instead, *silently*: the two carry identical information and differ only in where they sit, so the substitution is a decorative nearest form, and a warning would be unactionable on a configuration shared with a tool that can never satisfy the preference. Naming a legend that is not configured is a per-directive warning listing the keys that are available, and the diagram falls back to the engine default rather than losing its legend. `:show_legend:` is therefore no longer deprecated, taking the deprecation count from seven to six. ENV_DATA_VERSION is stepped to 9: `NeedsFlowType` loses `legend` and gains `show_legend_key`, so a doctree pickled by the previous shape fails with `KeyError: 'show_legend_key'` on rebuild.
The legend a diagram gets when it names none is engine specific by design, so a single shared `expect.legend` could not describe it: an engine that draws a good legend inside the diagram has it in `source` already, and one that cannot renders the external table instead. The key moves to `expect.<engine>.legend`, asserting only the out-of-diagram legend; an internal one needs no key of its own, since `source` already pins it byte-exactly. The case schema gains a `legends:` block and `show_legend` under both `config:` and `options:`. The three legend cases now pin an explicit key, so every engine agrees by construction, and `legend-engine-default` is added to make the default a contract rather than an accident. Two harness bugs surfaced while regenerating: - `_assert_legend` ran before the `UBC_UPDATE_CORPUS` rewrite, so a case whose legend changed could never be regenerated -- the harness checked the fresh rendering against the stale expectation it was about to replace. It now runs after the rewrite branch. - The two checksum tests compared case files against a manifest the same run was rewriting, so under regeneration they passed or failed on test ordering. They now skip while regenerating; the stamp is checked on the ordinary run that follows. corpus_version 5, 27 cases, README re-copied verbatim.
`:show_legend:` is documented in the portable vocabulary rather than under "Legacy options", which held nothing else and so is removed; the anchor it took with it now points at the widened option instead of a deprecation notice. The engine-default rule is stated plainly -- written bare, the option draws what it always drew -- alongside why the option takes a name and never the sections inline. `needs_flow_legends` and `needs_flow_show_legend` are documented, including that placement is a preference an engine may substitute silently, and that the project default selects which legend rather than whether there is one. The changelog drops the `:show_legend:` deprecation, leaving six. `docs/ubproject.toml` gains a `beside` legend, because the new example names it and an unknown key is a warning -- so the documentation build is itself a test of that path.
`resolve_legend` read `option_key or project_key`, so a directive key that named nothing REPLACED the project key: the unknown-key path warned and went straight to the engine default, never consulting `needs_flow_show_legend`. A typo in one diagram silently cost the project the legend it had configured. An unusable value is treated as *unset* everywhere else in this vocabulary -- an unusable `needs_flow_show_links` string warns and then behaves as though it had not been written -- and unset means the next step applies. So the resolution is a chain: the option, then the configuration, then the engine's own legend, with an undefined name warning and handing on rather than short-circuiting. The two steps warn differently, because they are different mistakes. An option key is the directive's own text, so it is reported there, every time. A project key is one `conf.py` line, so it is reported once for the build and names the configuration key at fault; repeating it at every needflow would bury the directive-level warnings an author can act on. The test that pins this uses a project key that is deliberately NOT the engine default (`beside` is external where the default is internal), so the two paths are distinguishable in the rendering. A project default that happened to coincide with the engine default could not fail either way.
State the order explicitly -- option, then `needs_flow_show_legend`, then the engine's own legend -- and say what an undefined name does at each step: it warns, and is then treated as unset so the next step applies.
Two defects in the `needs_flow_legends` value grammar, which no brief
specified and which nothing had compared against ubCode.
`parts` was never checked before being iterated, and both shapes the
mistake actually takes went badly wrong:
parts: 5 ExtensionError: 'int' object is not iterable (exit 2)
parts: "links" five warnings naming single letters, then the TYPES
legend drawn in place of the links legend asked for
Both break this slice's rule that a presentation default degrades and
never fails the build, and the second is worse than the crash: it draws a
legend that is quietly not the configured one. Only a LIST is accepted
now; a bare string is reported with the spelling that would have worked,
rather than becoming a second valid form -- `parts` is ordered, one name
cannot express an order, and an alternative spelling would have to keep
meaning the same thing in every tool that reads this configuration.
Second, neither legend key was checked at config-load time, so a project
that misconfigured one and happened to have no needflow was never told --
a regression against the release whose `needs_flow_legend` was a plain
string that `validate_flow_config` did check. Both keys are restored to
that call. Because it runs with no directive location and the per-diagram
call now uses the same text, Sphinx's `once` filter makes the read-time
warning win, which also stops a `conf.py` mistake being reported against
an `index.rst` line.
Also: `placement` becomes `Literal[...] | None`, where None is the
engine's own default rather than a fixed value -- both engines here draw
an in-image legend of need types, while an engine with no legend construct
has only the external table. Naming the engine's default is what lets the
two tools state one contract instead of each documenting its own answer.
And a legend name that is empty, blank, or carries outside whitespace is
reported and dropped: selectors are stripped before lookup and an empty
one means "no name given", so such a name can never be matched however it
is written.
`parts` is an ordered list, but nothing proved it: every existing case wrote the sections in canonical order, so a tool that treated `parts` as a set -- or as a string enum with a fixed section order -- would have passed the whole corpus. `legend-order` asks for `[links, types]` and is the only case that can catch that. The harness could not have seen it either. `_legend_rows` collected the sections in a FIXED order rather than document order, and `_assert_legend` compared them as a set, so both orders produced the same reading. Rows are now read in document order and the sections compared as a sequence; mutating the new case's expectation to the other order fails, which is what makes the case load-bearing rather than decorative. `test_manifest_has_no_orphans` joins the two checksum tests in skipping under UBC_UPDATE_CORPUS -- a fourth instance of the same race, and the one that only appears when a case is ADDED: the manifest and the case directory disagree until the rewrite lands, so whether it ran before or after decided the result. README re-copied verbatim from the amended spec, which now states that `parts` is a list in render order and that unset `placement` takes the engine's default. corpus_version 6, 28 cases.
`parts` is documented as a list whose order is contract, with the reason a bare string is refused rather than accepted as a second spelling. `placement` unset is documented as taking *the engine's own default* rather than as a fixed value, and what that is for the engines here -- which is the framing both tools need, since an engine with no legend construct at all defaults the other way.
Member
Author
Splitting this into reviewable slicesMarked as draft: at 8,695 insertions across 57 files this is too much to review as one The plan is five PRs. The first three should be near-mechanical to review; every
Ordering note: the entity-wrapping fix has to come first, because later slices record One benefit of splitting: Nothing here is discarded. Each slice is carved by subtraction from the reviewed state on Generated by Claude Code |
chrisjsewell
added a commit
that referenced
this pull request
Aug 20, 2026
Third slice of the #1770 split (after #1780 and #1781): the presentation options. It gives `needflow` an engine-neutral way to say which way the graph flows, what the edges are labelled with, and what the legend contains — additive only, **zero deprecations**: two existing options are widened rather than replaced, so the spellings in existing documents keep working and keep producing the same bytes. ## Options - **`:direction:`** (new) — `down` (default) / `up` / `right` / `left`, plus the `TB`/`TD`/`BT`/`LR`/`RL` two-letter forms Graphviz and Mermaid users already know, with a `needs_flow_direction` project default. An explicit option beats an engine-config(`:config:`)-derived direction on both engines, and disagreement warns. PlantUML has no bottom-to-top or right-to-left primitive (probed: both are syntax errors), so those degrade to their axis mate with a single warning per project. - **`:show_link_names:` widened** — the flag now takes an optional value `none` / `outgoing` / `incoming` / `type`, and written bare still means exactly what it always meant (`outgoing`), byte-for-byte. `needs_flow_show_links` likewise accepts a string as well as a boolean (`True` ≡ `outgoing`, `False` ≡ `none`). The old OR of flag and config becomes a precedence — only an unset option consults the config — which fixes the interaction that made "project default on, this one diagram off" impossible to express. - **`:show_legend:` widened** — it now takes the *name* of a legend defined in `needs_flow_legends`, or nothing. Deliberately key-only: an inline value set would collide with user-chosen names and need a precedence rule. Written bare it renders today's in-diagram legend, byte-for-byte. ## Config `needs_flow_legends = {name: {...}}` defines legends — `parts`, an **ordered list** of the sections to draw (`types`, `links`; a list only, and order is rendered as listed), and `placement` (`internal` / `external`), a preference: unset takes the engine's own default placement, which is internal for both engines here. A legend that includes `links` renders as a table beside the diagram, since neither engine can draw link rows inside it. `needs_flow_show_legend` names the legend a diagram gets when it asks for one without naming its own; it selects *which*, never *whether* — presence stays per-directive, so there is no off-switch value for a legend name to collide with. An undefined key resolves as a **chain** — the directive's key, else the project's, else the engine default — warning at each undefined step and handing on, because an unusable value is treated as unset everywhere in this vocabulary. The two steps warn at different tiers: a bad directive key per directive (`needs.needflow`), a bad `needs_flow_show_legend` once per project (`needs.config`, no directive location — it is a `conf.py` mistake). All four new configs are validated as they are read, so a project that misconfigures one and happens to have no needflow is still told; the checks warn and fall back, never crash (including non-string values, `parts: 5`, and `parts: "links"`). Enumerated config values are matched the way the option parsers already match theirs — case-insensitively, ignoring surrounding whitespace — and `needs_flow_engine`'s membership check from #1780 gets the same treatment. ## Nothing moves for existing projects — with one named edge Verified by cross-commit diff of the generated diagram source on both engines (a five-needflow probe using only pre-existing spellings, and a ten-shape matrix over every reachable `show_link_names` shape), and independently reproduced in review: projects using only existing spellings produce byte-identical output. No test fixture changed: `git diff master -- tests/doc_test/` is empty, `.rst` and `conf.py` alike. The five conformance cases merged in #1781 — including the bare-`:show_legend:` and bare-`:show_link_names:` parity targets — pass **unregenerated**, with unchanged checksums. The one edge, and why the changelog has a Breaking section: `needs_flow_show_links` set to a **string** was never a supported spelling (the config was declared `bool`, and Sphinx already warned about the type) but drew labels via truthiness. It is now read as a value: an unrecognised string warns and draws no labels, and `'none'` — the sharpest case — now silently means what its author meant instead of the opposite. Non-string truthiness (`1`, `0`) is deliberately preserved. ## Environment version `ENV_DATA_VERSION` goes 6 → 7: the directive persists the resolved options in doctrees, and an older reader over a newer doctree fails on the missing keys (reproduced by rebuilding across the change and watching it raise, then fixed by the bump). No other open PR claims 7. ## Conformance corpus `corpus_version` 1 → 2, 5 → 23 cases: direction (one per value, including the two PlantUML degradations and the option-vs-engine-config conflict), the four link-label values plus config-driven and option-beats-config cases, and six legend cases (explicit keys, order pinning, the chain). Fifteen case files are byte-identical to the reviewed umbrella branch and pass here unregenerated — a cross-check of this carve rather than a snapshot of it. The degradation mapping table gains its first three live rows (tier + subtype + pattern), regeneration now records degradation entries, and the per-engine `expect.<engine>.legend` key is exercised for the first time by the external-placement cases. Regeneration is idempotent (verified over two full runs). ## Tests and review Every behaviour was recorded failing before its implementation landed, and seven targeted mutations each turn a test red — one of them (an option-beating-`:config:` assertion satisfiable by a `rankdir` emitted in the wrong place) was caught by the byte-exact corpus case and now has a dedicated ordering fence. The adversarial review reproduced the byte-preservation, the matrix, the tier split, and the ENV crash independently, and found one real defect — a non-string `needs_flow_show_legend` crashed the build — which is fixed at both affected sites with red-first tests for both paths. Full suite: 1637 passed; the 47 remaining failures/errors are pre-existing environmental ones, byte-identical to master's set. Docs build warning-count and warning-set identical to master. ## Follow-ups (later slices of the #1770 split) - Slice 4: link and type styling (`needs_links[].line/part_line/color/part_color/arrow`, `needs_types[].shape`) with the first deprecations. - Slice 5: `:styles:` + `needs_flow_styles`, `:engine_config:`, the remaining deprecations, and moving the engine membership check into `validate_flow_config` (review measured today's #1780 behaviour reporting a `conf.py` engine mistake against a directive location, and staying silent when no needflow exists — inherited there deliberately).
This was referenced Aug 21, 2026
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
1 participant
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.
Third slice of the needflow portable-options work, riding #1768 (bug fixes) and #1769 (one
graph model, two thin emitters). It gives
needflowan engine-neutral option and configvocabulary — each option expresses intent that plantuml, graphviz, and ubCode's mermaid
renderer can honour or gracefully degrade — plus a discouraged per-engine escape hatch.
Two existing options were widened rather than replaced, so the spellings most likely to be
in existing documents keep working and gain no deprecation.
Options
:direction:(new) —down(default) /up/right/left, withTB/TD/BT/LR/RLaliases and a
needs_flow_directiondefault. An explicit option beats an engine-config-deriveddirection on both engines, and disagreement warns. PlantUML has no bottom-to-top or
right-to-left primitive (probed: both are syntax errors), so those degrade to their axis mate
with a single warning per project.
:show_link_names:widened — the flag now takes an optional valuenone/outgoing/incoming/type, and written bare still means exactly what it always meant (outgoing),byte-for-byte.
needs_flow_show_linkslikewise accepts a string as well as a boolean(
True≡outgoing,False≡none). That fixes the OR-interaction which made "projectdefault on, this one diagram off" impossible to express.
:show_legend:widened — it now takes the name of a legend defined inneeds_flow_legends, or nothing. Deliberately key-only: an inline value set would collide withuser-chosen names and need a precedence rule. Written bare it renders the legend this engine
draws best, which upstream is today's in-diagram legend, byte-for-byte.
:styles:(new) — a comma list of[<filter>]:<class>rules (the existing variant syntax)over named classes from
needs_flow_styles, with CSS-cascade semantics (declaration order,last match wins per property) and a closed property set:
fill,border,border_width,border_style,text_color,shape. The built-inhighlightclass byte-preserves today's:highlight:output.:engine_config:(new) — the escape hatch: named per-engine blobs fromneeds_flow_engine_config, deliberately config-level so documents stay portable.:config:becomes a deprecated alias.Config
needs_flow_legends = {name: {...}}defines legends —parts, an ordered list of thesections to draw (
types,links), andplacement(internal/external). Unsetplacementtakes the engine's own default placement, which is internal here and external in a renderer with
no legend construct.
needs_flow_show_legendnames the legend a diagram gets when it asks for onewithout naming its own; it selects which, never whether, so presence stays per-directive and
the key namespace needs no value meaning "off".
An undefined key resolves as a chain — the directive's key, else the project's, else the
engine default — warning at each undefined step and handing on, because an unusable value is
treated as unset everywhere in this vocabulary. The two steps warn at different tiers: a bad
directive key once per directive, a bad
needs_flow_show_legendonce per project.needs_links[].line/part_line/color/part_color/arrow(neutral enums;coloris finallyhonoured — an identical TODO sat in both emitters — and is unset-by-default, so a link type can
now ask for black) and
needs_types[].shape(a ten-member neutral enum; the legacy plantumlstylekeywords are accepted as aliases). Options consult their config default only when unset(the
max_itemsrule from #1761), and every enumerated config value is validated at load timewith a warning and a fallback rather than a crash. Those values are matched the way the directive
options already match theirs — case-insensitively, ignoring surrounding whitespace — so
needs_flow_direction = "Down"no longer warns and falls back where:direction: Downisaccepted. (docutils' own
choicenormalises, so:engine: PlantUMLhas always been accepted;the config half was the odd one out.)
part_coloris an additive amendment to the planned vocabulary: without it, the deprecatedstyle/style_partpair could express something the neutral keys could not — a distinct colourfor part links — which stranded four link types in this project's own docs configuration on the
deprecated spelling.
Six deprecations, honoured indefinitely
:config:,:scale:(deprecated without replacement — it was already a silent no-op ongraphviz),
:highlight:(kept as sugar for[<filter>]:highlight),:border_color:,needs_links[].style/style_part/style_start/style_end, andneeds_flow_link_types— the lastdeprecated as dead: it has never had any effect, and the docs that taught otherwise are
corrected. Every deprecation warns only when used; option-free builds stay warning-free. The
suite's two whole-project empty-warnings fences are intact and unmodified; three narrower fence
assertions gained a single-substring allow-list because their fixture is the deprecated feature
under test — each still asserts everything else is empty.
Nothing moves for existing projects
Review-verified by cross-commit diff of the generated diagram source on both engines: projects
using only existing spellings — including bare
:show_link_names:and bare:show_legend:—produce byte-identical output, built HTML, and image filenames. No
.rsttest fixture in the reponeeded changing:
git diff origin/master -- 'tests/doc_test/**/*.rst'is empty. The fourconf.pydiffs come from theneeds_linksneutral-key migration and, indoc_links/conf.pyonly, dropping a
needs_flow_link_typesthat the dead-config deprecation made pointless.mermaidis additionally accepted as a reserved engine name (drawing with the default engine,with a warning) so a ubCode-authored document does not lose its diagram when rendered here.
Bugs fixed on the way
markup), because escaping ran before wrapping
:config:merges mutated a shared dict, leaking one diagram's graphviz styles intothe next
:class:was silently dropped by the plantuml emitterneeds_graphviz_stylesentry crashed the buildneeds_flow_enginefailed via a bareassert(stripped underpython -O):debug:on plantuml now renders a line-numbered literal block, matching graphvizConformance corpus
tests/conformance/needflow/holds 28 language-neutral cases —(needs data, options/config) → expected emitted source per engine + expected degradations—checksummed by a manifest and duplicated into ubCode, which owns the copy of record and adds the
mermaid expectations. Because the corpus names the portable vocabulary rather than either tool's
spelling, widening
:show_link_names:changed only the mapping table. Legend expectations areper-engine (
expect.<engine>.legend), since an internal legend is already asserted byte-exactlyby that engine's
sourceand only an external one is separate document content. Expected sourceswere each reviewed against the case's stated purpose rather than snapshotted blind — that review
is what surfaced the entity-wrapping bug above. The harness refuses malformed cases and checksum
mismatches, and the refusals are themselves tested; checksums are taken over line-ending-normalised
bytes so a Windows checkout and a Linux one agree.
Environment version
ENV_DATA_VERSIONgoes 6 → 9: the directive persists the resolved options in doctrees, and anolder reader over a newer doctree fails on the missing keys (reproduced by rebuilding across the
change and watching it raise, then fixed by the bump). 7 and 8 were intermediate shapes that
existed only within this branch, so the released step is 6 → 9. A sweep of open PRs on the literal
constant name found no competing claim.
Tests and docs
Every behaviour change and every deprecation warning was recorded failing before its fix or
feature landed. The full suite passes (1534 passed, 13 skipped) and the docs build is
warning-free with the blanket
needs.deprecatedsuppression removed — the deprecated demos arenow non-executed samples pointing at their replacements' rendered examples. The docs gain a
portable-options section, per-option
.. versionadded::/.. deprecated::markers, and anarrow-migration table (today's default graphviz arrowhead is
open, notnormal).