Repository navigation
📚 sphinx-needs: document the processing order of needs - #2081
Merged
Merged
Conversation
Dynamic functions and variants are resolved in one pass, need by need, in
the order the needs reached the environment, so a `[[…]]` or `<<…>>` that
reads a field another one computes sees the computed value or the
unresolved one depending on document names, the documents the last build
re-read, and `-j`. Nothing reports it today.
These tests pin a `needs.derive_unresolved` warning for such a read, once
per reading call, at the reading need: a `copy` of another need's field
and of the need's own, a whole-project and a `links_only` `calc_sum`
(including the need's own computed `links`), `check_linked_values` up to
its early exit, and variant conditions up to the first true expression.
A read is reported when the field carries a `[[…]]`, `<<…>>` or `<{…}>`
after `needextend`, whether or not the pass had reached it: each case is
built in both layouts, and an incremental build that flips the value read
leaves the warning unchanged. A call that fails because of what it read
gets both warnings, and each subtype is suppressed on its own. Authored
values, `links_from_content`, an `ndf` role and a condition name supplied
by `needs_filter_data` are not reported. ubCode's
`dynamic_functions_unresolved` fixture, adapted where sphinx-needs refuses
it, gives ubCode's findings.
All but the four negative tests fail against this commit's code.
A `[[…]]` or `<<…>>` that reads a field which itself carries a `[[…]]`,
`<<…>>` or `<{…}>` is now reported as `needs.derive_unresolved`, once per
reading call, at the reading need's location. The resolution pass writes
each result into its need as it goes, in the order the needs reached the
environment, so such a read sees the computed value or the unresolved one
depending on document names, on what the last build re-read, and on `-j`.
A read is reported when the field read is a key of the read need's dynamic
fields after `needextend` (`NeedItem.carries_dynamic_value`, a read-only
accessor over a set the pass never clears), not when the pass has yet to
reach that need: the second would make the warning flip with the build
history, and a `-W` build go red or green by which file was edited last.
It is also ubCode's rule, so both tools report the same reads.
`resolve_functions` opens a private record (a context variable) around
each dynamic function call and each variant, and reports it once the call
is over, whatever its outcome, so a call that fails because of what it
read also says what it read. The built-ins note their reads into it:
`copy` its source field, `calc_sum` each summand and, with `links_only`,
its own `links`, `check_linked_values` its own `links` and each target it
compares until it stops, and a variant the fields of its own need each
evaluated condition names (not a name `needs_filter_data`, `build_tags` or
`var` supplies). Outside the pass no record is open, so an `ndf` role, a
`:style_row:` and user functions record nothing. A `filter` argument's
reads are not reported.
The message keeps ubCode's words up to the reads and ends with a clause
that holds whether or not the value had been computed yet. The new
subtype is listed with the build warnings, so `suppress_warnings =
["needs.derive_unresolved"]` silences it alone. The documentation of
dynamic functions describes the warning, and the changelog entry goes
under `Unreleased`.
A `filter` is evaluated on values the pass may or may not have computed yet, so a filter that reads a computed field keeps or drops a need by the order the needs are resolved in. `calc_sum` and `check_linked_values` noted a summand or target only after their filter kept it, so a filtered sum over a computed field warned in one build and not in the next: on a two-page project, no warning on a scratch build, one after re-reading the reader's page, none again after re-reading the target's. Both now note the read before the filter: a filtered sum names every need it considers whose value is computed, kept or not, since its result depends on them through the filter anyway. A new test builds that project twice and asserts the same two warnings both times. Two read sites had no test: `check_linked_values` reading the need's own computed `links`, and `copy(filter=)` reading the lowest-id match it copies from rather than the first match in read order. Both have one now. `calc_sum` fetches the call's record once per call (`_open_reads`) rather than once per summand, and skips every note outside the pass. The documentation says what a filter on a computed field still decides (a `copy` with such a filter can be reported in one build and not in another, and the needs a message names can vary after the first computed value a call reads), and that the reads your own functions make themselves are not reported, while a built-in they call during the pass is, under your function's name.
…en builds The caveat in the dynamic-functions docs names check_linked_values beside copy: a check that stops at an authored target its filter kept, before it reaches a computed one, is reported in one build and not in another, as the review measured. The fence test's docstring is limited to its own layout.
github-actions
Bot
added
the
pkg: sphinx-needs
The only stage barrier the documentation of dynamic functions stated was
that a function cannot read back links. A new "Processing order" section
on that page replaces it with the four steps every build runs once all
documents are read: `needextend` applied in (document name, line) order,
each filter seeing the earlier extends' changes but never the result of
a `[[…]]`, `<<…>>` or `<{…}>`; those evaluated need by need and, within
a need, in a fixed field order (core fields, then `needs_fields` as
declared, then link fields, a field a `needextend` turned into a call
last) whatever the order of the options, with the need-id order of
`calc_sum` and `copy(filter=…)` and a pointer to the
`needs.derive_unresolved` subsection; back links, link conditions and
unknown links, so no call can read a back link; then constraints,
freezing, schema validation and `needs_warnings`.
The `needextend` page says that extends are applied in that order and
links to the section, and its claim that it modifies "all string-based
and list-based options" is corrected: it modifies `status`, `tags`,
`style`, `layout`, `hide` and `collapse`, and every extra field and link
option whatever its type, while `+option` takes only string and list
options. The tutorial's "Resolve" step links to the section, and the
changelog entry goes under `Unreleased`.
chrisjsewell
force-pushed
the
claude/magical-hopper-uw9j1q-docs
branch
from
October 6, 2026 16:09
3ae3e80 to
80bfaf6
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2081 +/- ##
=======================================
Coverage 92.47% 92.47%
=======================================
Files 136 136
Lines 19406 19406
=======================================
Hits 17946 17946
Misses 1460 1460
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
added a commit
that referenced
this pull request
Oct 6, 2026
…epends on earlier extends (#2083) ## Summary `needextend` directives were applied in `(docname, lineno)` order only, and each filter was evaluated against the needs as the extends applied before it had left them. So which `:status:` won a conflict depended on file names, and renaming a file could change which needs a filter reached: in a project where `b.rst` holds `needextend:: TGT_1 :status: closed` and then `needextend:: status == "closed"`, that filter tags `TGT_1`, while the identical filter in `a.rst` matches nothing. - **`:extend_priority:`**, an integer of 0 or more, default **500**; extends are applied sorted by `(extend_priority, docname, lineno)`, **lower first**. That is Sphinx's own convention for event handlers (`app.connect(event, handler, priority=500)`: lower runs first, default 500), so on a conflicting `:status:` the higher priority is applied last and wins, and `+tags` values append in ascending priority. A project that never sets the option keeps exactly today's order (pinned by a `needs.json` snapshot recorded before the change). - *Why not `priority`*: a `needextend`'s options are field names (the directive has a catch-all option spec), and `priority` is a field projects define (this package's own schema guide and changelog examples declare one), so `:priority:` on a `needextend` would be ambiguous with setting that field. The `extend_` prefix avoids the collision. (The issue proposed `extend_order`; the name follows Sphinx's `priority`.) - *Why not default 0*: with lower-first, a default of 0 leaves no room to run anything before the unmarked extends without negative numbers. 500 leaves room on both sides, and negatives are refused. - The value is parsed with docutils' `nonnegative_int`, as `max_items` is in the view directives. An invalid value (`abc`, `-1`, `1.5`, empty) is handled exactly as a bad `:strict:` is: one `needs.needextend` warning at the directive (`Invalid value for 'extend_priority' option: …`), and nothing recorded, so that extend is not applied at all (not applied with the default). - Like `:strict:`, the option is popped before the options that modify fields, so a field named `extend_priority` (or `strict`) cannot be replaced by a `needextend`; `:+extend_priority:` / `:-extend_priority:` still reach it. Documented on the page, and pinned by a test. - **Matching is unchanged in this release, with a dual evaluation.** Before the first extend is applied, the filters are evaluated against the needs as written (no copy of the needs is needed, as `id` cannot be extended; logging is suppressed for that pass so a filter's own errors are still reported once). That evaluation is done once per filter string and document (every one reads the same unmodified needs, and only `c.this_doc()` depends on the document). Suppressing the warning silences it through Sphinx's own filtering; the evaluation itself is not skipped (a skip keyed on `suppress_warnings` was tried and dropped as not worth a parameter and a hand-rolled check for a one-release notice). The apply loop then evaluates each filter against the live needs, as today, and where the two match sets differ, the extend is reported once, at its location, as the new **`needs.needextend_match_order`**: ``` b.rst:7: WARNING: the needs matched by this needextend depend on modifications applied by earlier needextend directives: it matches 1 need (TGT_1) now and 0 against the needs as written; from the next release filters are evaluated against the needs as written, before any needextend is applied [needs.needextend_match_order] ``` Both sets are named in need-id order (plain string order), the first three ids and a count of the rest. Id-targeted extends are never compared: their target is fixed. **The next release flips matching to the needs as written**, so the reported extends are exactly the ones whose reach will change. `:extend_priority:` orders the modifications; it is not a way to choose what a filter matches. In this release a filter sees the changes of the extends applied before it, so a priority can change its matches, and such an extend is reported; from the next release it never does (the issue comment's "does not affect which needs a filter matches" describes the semantics after the flip, not this release, and the docs, the changelog and the subtype's description now say which is which). - **`ENV_DATA_VERSION` 8 → 9**: `NeedsExtendType` gains `extend_priority`, which changes the pickled environment. Measured: a rebuild over a `_build` written before this change ends with exit 2 (`KeyError: 'extend_priority'`, surfaced from `build_needs_json`) at 8, and re-reads every document and exits 0 at 9. - **Cost**, measured on a generated project (10 000 needs over 20 pages, 100 filter-targeted and 100 id-targeted extends, `-b needs -E`, the subtype not suppressed, two runs each; "without" stubs the as-written evaluation out): - 100 **distinct** filter strings: `extend_needs_data` 6.6 / 6.9 s → 10.9 / 10.7 s (+3.8–4.3 s, +54–65 %; 100 as-written evaluations, 3.6 s), whole build 21.2 / 22.1 s → 25.6 / 25.1 s (+3.0–4.4 s). - 100 copies of **one** filter string: one as-written evaluation (20–22 ms); `extend_needs_data` 3.2 / 3.0 s → 2.8 / 2.7 s and whole build 17.8 / 18.0 s → 17.5 / 17.2 s, i.e. no measurable cost. So the cost is one filter evaluation per distinct filter string and document, for the one release that carries the notice. - **Docs**: the `needextend` page gets an `extend_priority` subsection (the sort key, the direction, the default, an example where a `:status:` set at 600 wins over one set at 400 whatever the file order, the shadowed field) and a "Filters and earlier needextend directives" section (the dual evaluation, the notice for the next release, `suppress_warnings = ["needs.needextend_match_order"]`). Both carry `versionadded:: 9.0.0`, as the pages already do for this release; **release writer: check the number**, and the wording "the next release" in the warning text and the docs. - **Changelog**: `Unreleased` → `Improvements`, marked **(changed output)**: a project building with `-W` that has an order-dependent filter goes red until the filter is rewritten or the subtype is suppressed, with `"needs.needextend_match_order"`: `"needs.needextend"` in `suppress_warnings` does not cover it (Sphinx matches the subtype exactly). Closes #1658; part of #2064 (fifth item). Independent of #2080 and #2081; it touches two sentences of `docs/directives/needextend.rst` and `docs/dynamic_functions.rst` that #2081 also edits ("document-name and line order" → "extend priority, then document-name and line order"), and the changelog `Unreleased` list: whichever lands second resolves those by hand. ## Tests New file `packages/sphinx-needs/tests/test_needextend_priority.py` (inline `test_app` projects, exact `build_warnings(app) == [...]`), committed first and run against the unchanged code: 13 of its 16 items failed, and the 3 guards passed. The review round added a priority-0 extend to T3 and a test for the memo (17 items). | test | what it pins | before the change | |---|---|---| | T1 `test_without_the_option_needs_json_is_unchanged` | no option anywhere: the whole `needs.json` equals the snapshot recorded before the change; no warning | green (guard) | | T2 `test_higher_priority_is_applied_last[serial]` | `a.rst` 600 `late`, `b.rst` 400 `early` → `late` | `'early' == 'late'` | | T3 `test_appends_follow_ascending_priority` | `+tags` at 600, 0, 400 and the default → `p0, p400, p500, p600` (0 runs first, it is not "unset") | `['p600', 'p400', 'p500']` (before the 0 was added) | | T4 `test_equal_priorities_keep_document_order` | an explicit 500 and no option tie, `(docname, lineno)` decides; every recorded extend carries 500 | unknown-option warning | | T5 `test_invalid_priority_is_reported_and_not_applied[abc, -1, 1.5, empty]` | the exact warning, the need untouched, nothing recorded | unknown-option warning, and applied | | T5b `test_priority_option_shadows_a_field_of_that_name` | `:extend_priority: 7` sets the priority; `:+extend_priority:` still appends to the field | `'7' == 'authored appended'` | | T6 `test_filter_depending_on_an_earlier_extend_is_reported[serial]` | the recon project as it is: exactly one warning, at `b.rst:7`; `needs.json` values as today | no warning | | T7 `test_no_warning_where_the_matches_agree` | a filter on a field no extend changes, and id-targeted extends after other extends: no warning | green (guard) | | T7 `test_match_order_warning_is_suppressed_alone[reported, suppressed]` | `suppress_warnings` silences the subtype and not the other `needextend` warning | `[reported]` red; `[suppressed]` green (guard) | | T8 `test_needs_as_written_ignore_an_earlier_priority` | a priority-100 extend in `z.rst` closes REQ_1 to REQ_5 before the two `status` filters at 500 in `a.rst` (which sorts first): both warn, `6 needs (REQ_1, REQ_2, REQ_3 and 3 more) now and 1 (REQ_6)` and `0 needs now and 5 (…)`; the needs are written in reverse id order | unknown-option warning, no match-order warning | | T9 `[j2]` variants of T2 and T6 | identical results under `-j 2` (padded to 7 documents, since Sphinx 7 reads in parallel only above 5) | red, as their serial twins | | `test_identical_filters_are_evaluated_once_per_document` | the recon project plus a second identical filter in `b.rst`: both `b.rst` filters reported at their own lines; the as-written evaluations are exactly `(filter, "a")` and `(filter, "b")` | (review round) red without the memo: three evaluations | Mutation proofs (each applied to the finished code, the new file run, the code restored): | mutation | red | |---|---| | M1 sort key without `extend_priority` | T2 serial and j2, T3, T8 | | M2 the "as written" sets evaluated inside the apply loop (the live needs) | T6 serial and j2, T7 `[reported]`, T8 | | M3 default 0 instead of 500 | T3, T4, T8 | | M4 an invalid value falls back to the default instead of skipping | T5, all four | | `:extend_priority: 0` treated as unset (`nonnegative_int(...) or DEFAULT_EXTEND_PRIORITY`) | T3 (`['p400', 'p0', 'p500', 'p600']`) | | the memo removed (one evaluation per extend) | the once-per-document test only | No existing assertion or snapshot changed: before `master` was merged in, the full suite was 2102 passed / 13 skipped (2084 + 18 new; one of those went with the dropped skip, so the file now holds 17), 305 snapshots passed (304 + the new one); after the merge the needextend test files, `uv run poe lint` and `uv run poe typecheck` pass. `uv run poe docs-needs` was not run (it creates its own environment); the whole docs tree was built with the package's development environment and stand-ins for three docs-only extensions, with no warning (nitpicky included), and the new example renders `late` with `applied_first, applied_second`. ## Follow-ups - **The flip**, in the next release: apply each filter-targeted extend to the needs it matched as written (the sets this change already computes), and retire or reword `needs.needextend_match_order`. One case is not reported now: a filter whose evaluation raises against the needs as written but not live (in practice only a non-bool result on the fast path) is not compared; after the flip it would be an `Invalid filter` warning. - **ubCode** mirrors `:extend_priority:` (same default, direction and sort key) and, if it evaluates filters against the mutated needs too, the same notice. - **`options.popitem()`** records one directive's options in reverse written order. The same key twice is refused by docutils (`duplicate option`), but different keys on the same field are applied in reverse, measured: `:-tags:` then `:+tags: new` gives `[]` (written order would give `['new']`), and `:status: replaced` then `:+status: appended` gives `replaced` (written order: `replaced appended`). It is within one directive, so the priority does not touch it; applying in written order would be a change of output of its own.
…lved branch # Conflicts: # packages/sphinx-needs/docs/changelog.rst
… the docs branch # Conflicts: # packages/sphinx-needs/docs/changelog.rst
The three source and test files that both sides changed take master's versions (the explicit-record refactor and its review fix); the one section that both sides placed, the needs.derive_unresolved subsection, is kept once, under Processing order as this branch intended, with master's final wording.
chrisjsewell
pushed a commit
that referenced
this pull request
Oct 8, 2026
…ut of scope (sphinx-needs) dynamic_functions.rst: the processing order is rewritten for the strata (needextend, link fields, back links, every other field, checks), with what each built-in reads, the order a set and a link list are read in, user functions last, and two new sections, Cycles (needs.derive_cycle) and Reads that cannot be ordered (needs.derive_scope), which replace the one on needs.derive_unresolved; predicates filters are not reported yet. The need.<field> argument form is documented with the rule that a field selecting what a call reads must be final first. needextend.rst: a filter naming a computed field is needs.derive_scope. changelog: the #2080 and #2081 entries become the 9.0.0 statement of the ordered pass, with needs.derive_cycle and needs.derive_scope, need.<field> admitted and a None result adding nothing, and a breaking-change entry says what moves for a project.
5 tasks done
chrisjsewell
added a commit
that referenced
this pull request
Oct 8, 2026
… order; cycles and out-of-scope reads warn (#2134) ## What Package: `packages/sphinx-needs`. Every `[[…]]`, `<<…>>` and `<{…}>` is now computed after every value it reads, instead of need by need in the order the needs reached the environment. Phase 1 of the derived-values contract shared with ubCode (its `planning/derived-need-values.md` §5.3): ```text needextend (as written) ─► stratum 1: link fields ─► back links ─► stratum 2: every other field ─► checks (user functions last) (need-id order) (user functions last) (link conditions, dead links, sort, constraints) ``` ```rst .. req:: A 🆔 REQ_A :status: [[copy("status", "REQ_B")]] .. req:: B 🆔 REQ_B :status: [[copy("status", "REQ_C")]] .. req:: C 🆔 REQ_C :status: done ``` On master `REQ_A` is `done` or `None` by which document Sphinx read first (and `needs.derive_unresolved` says so); now it is `done` in every build, serial or `-j`, scratch or incremental. - **`functions/order.py` (new)**: the nodes (`(need id, field)` for every field that carries a call after `needextend`), what each reads read off the call text (S/R1 edges for `copy`, one column node per field for `calc_sum` and a filter naming computed fields, the precomputed candidates of a filter on final values, the targets of the need's links in written order for `links_only` and `check_linked_values`, every condition's names for a variant), iterative Tarjan for the cycles, Kahn with a heap for the order (ties by need id, then field). Unit-tested per edge rule (a table over call texts) and on a 20 000-need chain. - **`resolve_functions`** (same signature) drives it: stratum 1, `build_backlinks`, stratum 2. The per-field body moved into `_resolve_field` unchanged but for the record's argument and the join (below). - **`resolve_links` splits**: `build_backlinks` (between the strata; each back link list in need-id order; the dead-link flags final there) and `check_links` (after stratum 2: link conditions read computed values and complete back links); `sort_links()` stays last. `resolve_links` remains, as the three in a row, for the benchmark. - **Cycles** (`needs.derive_cycle`): no member is computed; each holds its *placeholder* — a link or array field the items written in it (`LIT_1, [[copy("links")]]` keeps `LIT_1`, which keeps its back link), any other field (and a list with nothing written) its typed empty value (`None`, or `""`/`0`/`0.0`/`False`/`[]`). One warning per member field naming the members, saying which column the member itself reads the cycle through (a filter naming a computed field, a sum over every need, a filter on final values that keeps a need on the cycle) or that its variant reads the field it sets. `:hours: [[calc_sum("hours")]]` is one. - **Reads that cannot be ordered** (`needs.derive_scope`). A node one of whose calls reads what its stratum cannot wait for is a **sink**: it is not run, it reads nothing (so it is on no cycle), it holds its placeholder, and each such call is one warning. That is a link field's call or variant reading a computed field that is not a link field, a back link, or `has_dead_links`/`has_forbidden_dead_links` (set with the back links); and a `need.<field>` that selects what a call reads while it is computed in the same stratum. Also reported: a `needextend` filter naming a computed field (once per extend), and a built-in reading a field a user function computes. The last is found at run time by the read record phase 0 added, kept as the check that the static order covers every read a built-in makes: across the whole suite it fires only in the two tests built for that case, and never for a missed edge (whose message asks to be reported). A filter's reads are not seen by that check, so build tests pin each filter edge (below). - **Your own functions** run after the built-ins of their stratum, in need-id order; one in a link field (sphinx-test-reports' `tr_link`) runs at the end of stratum 1, so its links are in the back links. - **`need.<field>` arguments** are accepted in field and link values (the six parse sites), so a need using one is created instead of refused. One that selects what a built-in reads (an id, a field, a filter, `links_only`) must be final first, and fails the call (`needs.dynamic_function`) when its field is unset — `copy("title", need.src)` no longer copies the need's own title; one in a value argument (`result`, `search_value`, `upper`, …) is read like the field, after it is computed, and is `None` when unset; the `:ndf:` role in a need's content fails the same way (`??`). `copy(x, "ID", filter=…)` orders its `current_need[…]` reads on `ID`, which is the `current_need` it evaluates the filter with. - **A failed call** (it raises, or its result fails the type check) leaves its field at its placeholder, as a cycle member's: `:links: LIT_1, [[copy("hours")]]` keeps `LIT_1` (it was `[]`), with its `needs.dynamic_function` warning. - **A `None` result** adds nothing to a field: alone in a nullable string or array field it leaves it unset (it was stored as the text `"None"`, or `[None]` that schema validation refused). - A field a `needextend` sets to a call holds its placeholder until computed (the TODO in `extend_needs_data`'s REPLACE case): a replacing call the empty value, a call appended to written array items those items. - `needs.derive_unresolved` (#2080, never released) is retired; `ENV_DATA_VERSION` 9 → 10 (values change meaning: an upgraded build re-writes every page), pinned to the docstring paragraph that says so. - The back links are reset once per pass, at its start (so a function of your own in a link field reads none of an earlier in-process build), and `build_backlinks` builds into the empty lists at the barrier; the report of `needextend` filters naming a computed field walks the needs only when an extend has a filter. - The docs' own build adds `needs.derive_scope` to its `suppress_warnings`: its examples share one project, so the `needextend` examples filtering on `tags` or `status` name fields other pages' dynamic-function and variant examples compute (three reports), as the build already does for `needs.link_outgoing`. - Docs: `dynamic_functions.rst`' processing order rewritten for the strata, with **Cycles** and **Reads that cannot be ordered** replacing the `needs.derive_unresolved` section; the `need.<field>` argument form; on the needextend page, that a filter naming a computed field is `needs.derive_scope`, and that a `[[…]]` or `<<…>>` (which sees the needs after every `needextend`) is the way to react to what another `needextend` set. ## Why #2064 (the derived-values tracking issue; phase 0: #2078, #2079, #2080, #2081, #2083; the needextend flip: #2127). A computed value that read another one depended on the build history: document names, which documents an incremental build re-read, `-j`. Phase 0 named every such read; phase 1 orders them, so the value is a function of the sources, and turns what cannot be ordered into a finding instead of a silent placeholder. ## Tests The tests were committed first and shown red against the code before them (`68 failed` of the build tests, `ModuleNotFoundError` for the unit tests). After review, each behaviour change's tests were shown red against the reviewed head's sources (9 build tests), and the tests that pin behaviour already right (the filter edges the run-time check cannot see, a function named like a built-in, the `ENV_DATA_VERSION` pin, the appended-call hold, the empty list, the part's back links, the back-link reset) are each red under the deletion of what they pin. The reset is at the start of the pass: the rebuild test is red with it at the barrier only (`(['C1', 'C2'], ['C1', 'C2']) == ([], [])`), with no reset, and with a second reset at the barrier (the per-build count). | test | pins | |---|---| | `test_dynamic_functions_strata.py` (new, 46 items) | a chain in two layouts, serial and `-j 2`; a core field reading an extra one; extras in either declaration order; a sum over computed sums in two layouts; own links mixing a literal and a call (7.0); variants on a computed field, core and extra; cycles: a pair and a self-copy, a self-sum, a filter that makes every need a candidate, a self-reading variant, a call a needextend sets, a member an earlier in-process build had resolved; `copy("links_back")` in need-id order, `has_dead_links` final at the barrier; a link condition on `links_back` in both layouts; the `derive_scope` cases (link field reading a later value and a back link, a `need.<field>` selector computed in the same step, needextend filters, a built-in reading a user function's field); a `tr_link`-shaped user link function running last in stratum 1; `need.<field>` in field and link values; a `None` result. After review: a list field on a cycle keeps its written items, also after a `needextend` appended the call (`C_LINKS`, `X_ARR`, `X_ITEMS`; `X_REP`, a mixed string empty); the sinks — a link field reading another field (a literal kept, no dead link to `''`, no type error, no no-match arm), a back link, a later selector, and a blocked selector whose field another field reads (one `derive_scope`, no cycle); a link field reading `has_dead_links`; an unset `need.<field>` selector failing the call (also from a configured `default`) while an unset value is `None`; a `need.<field>` value chaining (`check_linked_values`' `search_value` and `result`, `copy`'s `upper`); each cycle member naming its own column; and, because the run-time check cannot see a filter's reads, one build per filter edge laid out so the reader sorts first: `copy`'s `current_need["k"]` (of the caller, and of the need named), `copy`'s and `check_linked_values`' filter on a computed field, `check_linked_values`' second target, `links_from_content`'s filter, and a user function registered under `copy`'s name; a field a `needextend` appended a call to, read before the call is computed (a string its empty value, not its written one; an array its written items); a call returning `[]` storing `[]`; a part's back links in need-id order at the barrier; an in-process rebuild dropping a removed back link; on both builds of an in-process rebuild, a function of your own in a link field reading no back link (its own need's or another's), with one reset per need per build; the `:ndf:` role failing on an unset `need.<field>` selector like the field does; a failed call, or a result its field cannot hold, leaving a link or array field its written items | | `test_env_data_version.py` (new, 1 item) | `ENV_DATA_VERSION` is the highest `Version N` its docstring names, so a bump without its paragraph, or a revert, is red | | `test_functions_order.py` (new, 78 items) | the edge table over call texts (a sink reads nothing: out-of-scope reads and blocked selectors carry no edge), the filter names, the typed empty values, cycles and each member's column, the step order, user functions last, the three causes of a run-time report, the 20 000-node chain | | `test_dynamic_functions_unresolved.py` (retargeted) | phase 0's 39 items on their projects with the ordered values: the message builder (now `derive_scope`'s), 17 value flips as phase-1 regressions (`test_chain_across_needs` stays THE layout test, `test_the_warning_does_not_depend_on_the_build_history` the incremental one), a failing reader of a cycle, the suppressions (and the retired type as a no-op), the subtype list; `test_ubcode_fixture` is the parity test on ubCode's fixture, `NEED_ATTR` back; `test_ubcode_own_field_copy_fixture` builds ubCode's `dynamic_functions_own_field_copy` pages verbatim (`BACKLINK`'s copy of no back link is `[]`) | | `test_dynamic_functions.py` | `TEST_7` created and resolved, `TEST_8` a `needs.dynamic_function`; the snapshot gains both | | `test_needextend_invalid_function.py` | the `need.<field>` cases now apply and resolve | | `test_variant_data_integration.py` | its six warnings in the order the values are computed (need id), not document order; its snapshot's `REQ_BADTYPE_ARRAY` keeps the written `a`, `c` beside the variant data value the array cannot hold | Mutations (each applied, the tests run, reverted): M1 stratum 2 in insertion order → `test_chain_across_needs[target-later]` red; M2 back links after stratum 2 → the `links_back` test red; M3 no precomputed candidates → `test_copy_filter_reads_the_match_it_copies_from` red (the false cycle); M4 no placeholder for cycle members → the in-process-rebuild cycle test and the written-items test red (with the needextend hold dropped too, the needextend-installed cycle test); a placeholder without the written items → the written-items and sink tests red; M5 a recursive Tarjan → the 20 000 chain red; M6 no variant-condition edges → the read record reports `derive_scope` and the variant test is red; M7 user functions in insertion order → the two sink tests red; M8 a sink still run → the `need.<field>` selector test and the sink test red (`V_LINK` `['LIT_2']`, `OTHER_RD` `['']`). Each filter edge the run-time check cannot see is red under its deletion: `copy`'s `current_need["k"]` read, `copy`'s and `check_linked_values`' filter-name reads, `check_linked_values`' targets after the first, `links_from_content`'s filter reads, and built-ins recognised by name instead of by the registered function. The full suite at the head (`uv run poe test-needs -n 4 tests/`): **50 failed, 2320 passed, 13 skipped** — the 50 are the graphviz renders (`tests/test_needflow.py`, the needflow conformance corpus, `test_doc_github_1664_legend[graphviz]`), which need `dot` and fail identically without this change on a machine that lacks it (the same 50 ids). 2320 = the base's 2191 (#2127's measured count; the base adds no sphinx-needs test to it) + 129 new items (46 + 78 + 1 in the three new files, two more in the retargeted file, two more in the needextend file). No test outside the files in the table changed value. The sibling suites that call into the resolver pass: `uv run poe test-reports` 157 passed, `uv run poe test-ub-test-reports` 356 passed (2 skipped); `uv run poe test-codelinks` 658 passed (3 skipped). `uv run poe docs-needs` builds with no warning of this change (the docs suppress the `needextend`-filter report their shared examples trigger, below); in this sandbox it still ends with 70 environmental warnings: the intersphinx hosts are refused by the proxy, and `dot` is absent. **Costs** (no sphinx-needs gate; quoted, not argued). Base `b4fe430` vs this head, `sphinx-build -b needs -E -q`, serial, every post-processing step timed (reviewer B's timer and driver), three rounds, base and head back to back per shape with the order alternating, paired per round, the 1-min load logged (1.1–5.2); synthetic projects of 10 needs per page, each `:links:` the previous need. The columns count the link-sorting loop on both sides (it moved out of `resolve_links` into `post_process_needs_data`): base = `resolve_functions` + `resolve_links`, head = `resolve_functions` (which now holds the graph and `build_backlinks`) + `check_links` + the sort loop. | shape · N | dynamic fields | base (median) | head (median) | paired delta (per round) | per dynamic field | of which the graph (`build_stratum`) | whole `post_process_needs_data`, paired | build wall base / head | |---|---|---|---|---|---|---|---|---| | copyall · 20 000 | 20 000 | 0.890 s | 1.597 s | +0.696, +1.144, +0.680 s | **+34.8 µs** (low-load round: +34.0) | 0.424 s = 21.2 µs | +1.903, +2.723, +1.851 s | 85.7 / 86.3 s | | chain10 · 20 000 | 18 000 | 2.214 s | 1.471 s | −0.946, −0.743, −0.528 s | **−41.3 µs** | 0.425 s = 23.6 µs | +0.111, +0.433, +0.870 s | 85.5 / 83.1 s | On `copyall · 20 000` (every need copies a field) phase 1 adds 0.70 s, 34.8 µs per dynamic field: 2.0× the recon's 17 µs upper estimate for the graph build and schedule. The graph itself is 21 µs (1.25×); the other ≈ 14 µs is the driver's bookkeeping per node (the `Project`, the pending set, what each node reads, a step each). Chain-heavy projects get faster (`chain10 · 20 000` −0.74 s: the phase-0 emission is gone), and `calcall · 2 000` is not slower (reviewer B's paired A/B runs: head faster in three of four pairs). The whole `post_process_needs_data` is +1.9 s on `copyall` and +0.4 s on `chain10` at 20 000 because it now carries a full garbage collection: every head run of both shapes has one gen-2 collection of 1.16–1.35 s in post-processing (inside `process_constraints`, 1.44–1.73 s against 0.25–0.31 s at base), and no base run has one there. It is not new: the build makes the same number of full collections in both versions (reviewer B measured 18, ≈ 8.6–9.0 s, over the whole build); at base the last one falls in the write phase, at head in `process_constraints`, so the build's wall time is unchanged within its ±7 s run-to-run spread. (An earlier version of this PR placed that collection inside the graph build, and an allocation fix moved it one step later; it did not remove it.) ## ubCode This is phase 1 of ubCode's `planning/derived-need-values.md` (§5.3): both tools compute derived values in the same strata and order, as one contract. Measured at this PR's head on ubCode's own phase-1 build fixtures (a frozen copy of ubCode's phase-1 change; each fixture converted to a Sphinx project reading its `ubproject.toml`, built with `-b needs`), per `(need, field)` that carries a call or a variant here: the value against the fixture's `__expected__/needs.json`, and the finding (`derive_cycle` / `derive_scope` / `dynamic_function`) against its `diagnostics.json`. ubCode's main is at `47ceb2f` (#3911, the type check); **its phase-1 pull request follows this one** (the number will be added here when it opens). **113 rows over the 14 fixtures; 110 agree** (value and finding): | fixture | rows | agree | the differing row | status | |---|---|---|---|---| | `dynamic_functions_chain` | 8 | 8 | — | | | `dynamic_functions_cycle` | 8 | 8 | — (7 `derive_cycle` in both, the same members) | | | `dynamic_functions_backlink` | 4 | 4 | — (`BAD_LINKS` `derive_scope` in both; its written link kept) | | | `dynamic_functions_removed_link` | 5 | 5 | — | | | `dynamic_functions_sum_column_resolved` | 3 | 3 | — | | | `dynamic_functions_unresolved` | 25 | 25 | — (`CYC_A`/`CYC_B` `derive_cycle`, `NEED_ATTR` `derive_scope` in both) | | | `dynamic_functions_unresolved_call_only` | 5 | 5 | — | | | `dynamic_functions_unresolved_page_added` | 1 | 1 | — | | | `dynamic_functions_unresolved_page_deleted` | 2 | 2 | — | | | `dynamic_functions_sum_column` | 10 | 10 | — | | | `dynamic_functions_cross_need` | 5 | 5 | — (`PROJECT_TOTAL` `dynamic_function` in both) | | | `dynamic_functions_own_field_copy` | 5 | 4 | `BACKLINK.incoming` (`copy('links_back')`, no need links to it): `[]` here, `null` in ubCode | ubCode moves: an empty back link list is `[]`; until ubCode's phase-1 change carries that, this row differs | | `dynamic_functions_type_check` | 21 | 20 | `F_BOOL_INT.line`: a boolean result in an integer field is stored here (Python's `bool` is an `int`; schema validation then reports it), unset + `needs.dynamic_function` in ubCode | registered (the boolean-in-integer row) | | `needs_variants` (with the fixture's `-t html -t draft`) | 11 | 10 | `REQ_006.status` (`<<[(((unclosed]: …>>`): unset + `needs.dynamic_function` here, the fallback arm + `needs.variant` in ubCode | registered (the unparsable-variant-condition row) | `cache_reproducibility` (no `__expected__`; built live in both tools, trimmed to what Sphinx builds): 20 rows, 16 agree, the 4 others the same unparsable-variant-condition row. An integer and an equal float count as the same value (the register's integer-in-number row). On review probes outside the fixtures (85 rows on my copies, 84 by reviewer A's counter; 80/79 agree) both tools now agree on: a link field reading another computed field, a back link or a computed array (not run, its written links kept), a variant in a link field reading a computed field, a blocked selector whose field another field reads (one `derive_scope`, no cycle), a link list on a cycle keeping its literal part, an array whose written items a `needextend` appended a call to (on a cycle: the written items), an unset `need.<field>` selector (a failed call), a `need.<field>` value argument (chained), the typed empty value of every type on a cycle, and a link list mixing a written link and a call that fails or returns a value the list cannot hold (`LL_NUM :links: LIT_1, [[copy("hours")]]`: `['LIT_1']` and `needs.dynamic_function` in both). The five rows that differ, by kind: | kind | rows | here | ubCode | |---|---|---|---| | registered in ubCode's divergence register | a link field's variant naming `has_dead_links` (`:links: NOPE, <<[has_dead_links]:LIT_1, LIT_2>>`) | not evaluated: `['NOPE']` + `needs.derive_scope` (the flag is final only with the back links) | the flag is not in its vocabulary: `needs.variant` "Unknown field", the no-match arm `['LIT_2', 'NOPE']` | | ubCode moves, in its own phase-1 pull request | `copy("links_back")` of an empty back link list, into an array field and into a string field (×2) | `[]`; into a string field the type-check finding | `null`, no finding | | ubCode moves, in its own phase-1 pull request | an authored array on a cycle mixing a written item and a call (`:items: a, [[copy("items", "Y")]]`); the same array whose call returns a value it cannot hold (`:items: a, [[copy("hours")]]`, measured outside the probe set) | keeps its written items, `['a']`, as a field a `needextend` appended a call to does | `null` (its authored placeholder is the typed empty value; its `needextend` placeholder already keeps the written items) | | not evaluated by ubCode | a `calc_sum` with a `filter` on a cycle (`FILT.amountn`) | `None` + `needs.derive_cycle` | `None`; ubCode does not evaluate the built-ins' `filter` arguments at all (its differences page says so), so this is no phase-1 row | The same three kinds are what the changelog names, with the fixtures' registered rows (a boolean in an integer field, an unparsable variant condition). The finding *texts* differ (each tool words its own); the members, needs and counts agree. A `None` result in a nullable field is now stored as `None`, not the text `"None"`, which closes the sphinx-needs half of the register's "None"-string row. ## Checklist - [x] I wrote this change myself and have read every line of it; it was not generated automatically from an issue. Built in an orchestrated session from #2064, ubCode's derived-values plan and the code; every test was run by hand, the tests were committed first and shown red, and every line was read. - [x] I ran the package's tests (`uv run poe test-needs`) and they pass (the graphviz renders excepted on a machine without `dot`, identically to master); `uv run poe test-reports` and `uv run poe test-ub-test-reports` pass. - [x] Documentation is updated where behaviour or options change (`docs/dynamic_functions.rst`, `docs/directives/needextend.rst`). - [x] The package's `docs/changelog.rst` has entries under *Unreleased* (#2080's and #2081's entries rewritten into the 9.0.0 statement, and a *Breaking changes* entry). - [x] `uv run poe lint` and `uv run poe typecheck` pass. The changelog: the #2080 and #2081 entries become ```rst - ✨ Dynamic functions and variants are computed in dependency order, and a value that cannot be computed is reported **(changed output)** (:issue:`2064`, :pr:`2080`, :pr:`2081`, :pr:`2134`) ``` and *Breaking changes* gains ```rst -‼️ Computed values are computed in dependency order **(changed output)** (:issue:`2064`, :pr:`2134`) ``` whose body lists what moves for a project (chained values, sums of computed summands, variants seeing computed fields, `links_back` readable, user functions after the built-ins, needs with `need.<field>` created) and that a cycle or a read that cannot be ordered is a new warning a `-W` build fails on — a cycle member, a link field whose call reads a later value and is not run, and a field whose call fails or returns a result the field cannot hold keep the items written in them if they are link or array fields and lose the computed ones, any other such field is left empty — with the `suppress_warnings` entries to keep it green meanwhile. The ✨ entry says of ubCode: ```rst `ubCode`_'s phase 1 computes the same values and reports the same findings, but for three kinds of difference. Registered in ubCode's divergence register: a boolean result in an integer field, an unparsable variant condition, and the dead-link flags ``has_dead_links`` and ``has_forbidden_dead_links``, which are not in ubCode's vocabulary (a condition naming one is reported there). Removed on ubCode's side by its own phase 1: a copy of an empty back link list (``[]`` here), and an authored array field that is not computed (on a cycle, or whose result the field cannot hold), which keeps its written items here. Not evaluated by ubCode at all: the ``filter`` arguments of the built-in functions. ``` Refs #2064.
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
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.
Stacked on #2080 (its branch is this pull request's base, so the diff is the one docs commit; GitHub retargets it to
masterwhen #2080 merges).Summary
The documentation stated one barrier between the stages that run once every document is read:
a dynamic function cannot read back links ("Restrictions / incoming_links" in
docs/dynamic_functions.rst).Everything else about the order was undocumented, and three of its effects surprise users:
needextendfilter never sees the result of a[[…]]or<<…>>:needextend:: late == "D"matches nothingalthough the need's
late: [[copy("title")]]resolves to"D";earlydeclared beforelateinneeds_fields, a need thatwrites
:late: [[copy("title")]]before:early: [[copy("late")]]still computesearlyfirst, which readslateunresolved ("None");:band: <<[early == "E"]:hit, miss>>givesmissand the identical:after:giveshit,because
bandis declared beforeearlyandafterafter it.This replaces that paragraph with a Processing order section (label
needs_processing_order) that lists thefour steps, each checked against the code and against a probe project built on this branch:
needextend, applied in (document name, line) order; each filter sees the needs as written plus the earlierextends' changes, never a
[[…]]/<<…>>/<{…}>result; an extend may set a field it can modify to a call,evaluated in step 2;
[[…]],<<…>>and<{…}>, need by need, and within a need in the fixed field order whatever the optionorder, a field a
needextendturned into a call last; a whole-projectcalc_sumandcopy(filter=…)take theneeds in need-id order; reads of a value computed in this step point to the
needs.derive_unresolvedsubsection,which becomes a child of the new section;
needs_warningsat the end of thebuild see the final values.
Also:
docs/directives/needextend.rstsays that extends are applied in document-name and line order and that a filtersees the earlier extends' changes, linking to the section; its claim that
needextend"can modify allstring-based and list-based options" is corrected: it modifies
status,tags,style,layout,hide,collapse, and every extra field and link option whatever its type (a number or boolean field included), and+optiontakes only string and list options;Unreleased→ Improvements.Part of #2064 (third item); builds on the need-id order PR and the
needs.derive_unresolvedPR, whose semantics itdocuments.
Docs only, no tests.
uv run poe lint,uv run poe typecheckand the two dynamic-function test files pass; thechanged pages parse with docutils (Sphinx-only roles and directives stubbed) without warnings or errors, and every
:ref:they use resolves to a label.uv run poe docs-needswas not run locally (its intersphinx inventories neednetwork access); the docs CI job is the authority.
Follow-up
constraintsset byneedextendor by a dynamic function is silently dropped #2075:constraintsset by aneedextendor by a[[…]]is silently dropped:NeedItem.__setitem__checks that theconstraint results are not computed yet and then never stores the value. The
needextendpage therefore does notlist
constraintsamong the options it modifies.When squashing, strip the
Co-authored-bytrailer GitHub proposes.