Repository navigation
✨ sphinx-needs: needextend :extend_priority:; warn where a filter depends on earlier extends - #2083
Merged
Merged
Conversation
`needextend` directives are to be applied in `(extend_priority, docname, lineno)` order, lower priority first, with 500 as the default, so a project that never sets the option keeps today's order. These tests pin that order for conflicting `:status:` values and for `+tags` appends, the equal-priority tie-break, the stored default, and the invalid values (`abc`, `-1`, `1.5`, empty), which are reported like a bad `:strict:` and leave the extend unapplied. They also pin that the option shadows a field named `extend_priority`, as `:strict:` does a field of its name. Which needs a filter matches stays as it is in this release, but where a filter matches different needs against the needs as written than against the needs the earlier extends left, the extend is to be reported once as `needs.needextend_match_order`. The recon project for #1658 is pinned as it is, plus a project where a priority-100 extend changes the field two later filters name, a project where the two match sets agree, and the suppression of the new subtype alone. The needs.json snapshot of a project without the option was recorded before the option exists. The priority and the warning tests fail until the option is implemented; the snapshot test and the two no-warning tests pass already, as guards.
`needextend` gains an `:extend_priority:` option, an integer of 0 or more with 500 as the default, and the extends are now applied in `(extend_priority, docname, lineno)` order: lower first, as for Sphinx's event handlers, so on a conflicting option the higher priority is applied last and wins, and `+option` values append in ascending priority. A project that never sets the option keeps its `(docname, lineno)` order. The value is parsed with docutils' `nonnegative_int`, as `max_items` is elsewhere, and an invalid one is handled exactly as a bad `:strict:` is: one `needs.needextend` warning at the directive, and nothing recorded, so that extend is not applied. Like `:strict:`, the option shadows a field of its name for the replace form; the docs say so. Which needs a filter matches does not change in this release: it is still evaluated against the needs as the earlier extends left them. Before the first extend is applied, every filter-targeted extend's filter is also evaluated once against the needs as written (no copy is needed, as `id` cannot be extended; logging is suppressed for that pass so a filter's errors are still reported once). Where the two match sets differ, the extend is reported once at its location as the new `needs.needextend_match_order`, naming both sets in need-id order, three ids and a count of the rest. The next release evaluates filters against the needs as written, so these are the extends whose reach will change. Id-targeted extends are never compared. `NeedsExtendType` gains `extend_priority`, which changes the pickled environment's shape, so `ENV_DATA_VERSION` goes to 9: measured, a rebuild over a `_build` written before this change otherwise ends with a `KeyError` on the missing key. The needextend page documents the option, the sort key, that the priority never changes what a filter matches, and the notice with its `suppress_warnings` entry; the changelog entry goes under `Unreleased`.
…n pass The docs, the changelog and the description of `needs.needextend_match_order` said that `:extend_priority:` never changes what a filter matches. That holds only once filters are evaluated against the needs as written, from the next release: in this one a filter sees the changes of the extends applied before it, and the priority decides which those are. All four now say so, and the page no longer says both that a priority can change a needextend's reach and that it cannot. The as-written evaluation is now done once per filter string and document, as every such evaluation reads the same unmodified needs and only `c.this_doc()` depends on the document, and not at all when `needs.needextend_match_order` is suppressed, as nothing else reads those sets. `extend_needs_data` takes Sphinx's `suppress_warnings` for that, keyword-only and empty by default, and checks it with Sphinx's own `is_suppressed_warning`. T3 gains a priority-0 extend, so 0 is pinned as the first priority rather than an unset one, and two tests pin the two cuts: two identical filters in one document share one evaluation and are both reported, the same string in another document is evaluated on its own, and a suppressed build evaluates nothing as written. The changelog adds that `"needs.needextend"` in `suppress_warnings` does not cover the new type.
github-actions
Bot
added
the
pkg: sphinx-needs
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2083 +/- ##
=======================================
Coverage 92.26% 92.27%
=======================================
Files 136 136
Lines 19076 19121 +45
=======================================
+ Hits 17601 17644 +43
- Misses 1475 1477 +2
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:
|
The as-written evaluation ran unless needs.needextend_match_order was suppressed, which threaded Sphinx's suppress_warnings into extend_needs_data and re-implemented the logger's filtering by hand. For a one-release notice that is not worth a parameter and a check: the sets are computed (once per filter string and document), the warning is logged, and Sphinx's own suppress_warnings filtering silences it like any other. The test for the skip goes with it.
chrisjsewell
pushed a commit
that referenced
this pull request
Oct 7, 2026
The match-order tests of #2083 become tests of what is applied: each filter matches the needs as written, whatever the extends before it changed (serial and -j 2), the priority never changes a match, two identical filters of one document share one evaluation, the retired needs.needextend_match_order in suppress_warnings is a no-op, and a filter that fails as written applies to nothing. Red at this commit: the extends still apply to the live matches.
chrisjsewell
pushed a commit
that referenced
this pull request
Oct 7, 2026
The needextend page's section on filters and earlier extends becomes "Filters see the needs as written" (versionchanged 9.0.0, with an example), its two identical paragraphs on the priority collapse into one sentence, and the processing-order step 1 says the same. The unreleased #2083 changelog entry becomes the 9.0.0 statement: the priority, and filters evaluated against the needs as written; the notice warning type it introduced is gone.
5 tasks done
chrisjsewell
added a commit
that referenced
this pull request
Oct 7, 2026
## What Package: `packages/sphinx-needs`. The flip #2083 announced. Which needs a `needextend` modifies is now decided before any `needextend` is applied: an ID names its need, and a filter is evaluated against the needs **as written**. The extends are then applied in `(extend_priority, docname, lineno)` order, as #2083 made them, to those needs. ```rst .. req:: One 🆔 REQ_1 :status: open .. needextend:: REQ_1 :status: closed .. needextend:: status == "open" :+tags: written_open ``` On master the filter is evaluated after the first `needextend` closed `REQ_1`, matches nothing, and reports `needs.needextend_match_order`; now it matches `REQ_1` as written, and `REQ_1` ends closed and tagged `written_open`, with no warning. So no `needextend` changes which needs another one's filter matches, whatever their priorities and the names of their files. A project upgrading from 8.5.0 gets no warning where a filter's matches change (the notice never shipped), so the changelog files the flip under *Breaking changes*, saying who is affected and how to keep what such a filter modified. - `extend_needs_data` (`directives/needextend.py`): one pass, in apply order, resolves every extend's targets — the ID lookup (unknown-ID warning, or `NeedsInvalidFilter` under `strict`, unchanged) and the filter, evaluated once per distinct `(filter, document)` (the memo #2083 introduced; `c.this_doc()` makes the document part of the key). That evaluation is now the only one, so it logs: a filter that cannot be evaluated at all (it does not compile) is the existing `Invalid filter '…': …` `needs.needextend` warning, once per extend that carries it, at its own line, and that extend modifies nothing. The modifications are then applied to the matched needs in need-id order (a fixed order; each need is modified on its own, so nothing depends on it). The live re-evaluation and the comparison with it are removed. - The warning type `needs.needextend_match_order`, added by #2083 and never released, is removed with its message (`logging.py`); a `suppress_warnings` entry naming it is a silent no-op (pinned by a test). - The "first three ids and a count" formatter of that message moves to `sphinx_needs.utils.counted_ids`, with unit tests of its own, for the dependency-ordered pass's cycle and scope messages to reuse. - Docs: the needextend page's section "Filters and earlier needextend directives" becomes "Filters see the needs as written" (`versionchanged:: 9.0.0`, with the example above), its two identical paragraphs on the priority become one sentence ("`:extend_priority:` orders the modifications; it never changes what a filter matches"), and step 1 of the processing order says the same. One behaviour beyond the flip itself, deliberate: a filter whose evaluation reports its own failure for some need (`needs.filter`, e.g. an unknown name) is evaluated once per document, so two identical such filters in one document now give one `needs.filter` warning, at the first of them to be applied, where master gave one per extend. The warning order is otherwise unchanged: targets are resolved in apply order, so unknown-ID and filter warnings interleave exactly as before (`test_doc_needextend_warnings` is untouched and green). ## Why #1658, and #2083's notice: a filter evaluated against the needs as the earlier extends left them made a project's result depend on the order of its extends, i.e. on file names and, since #2083, on priorities. #2083 added the priority and a notice warning naming every extend whose matches would change; 9.0.0 carries the flip itself, so the notice is retired before it is ever released. ## Tests The tests were committed before the code and were red against it (7 of 22 in `tests/test_needextend_priority.py`); the value test for the document half of the memo key was added in review: green at head, red under that key mutation. In `tests/test_needextend_priority.py`, #2083's match-order warning tests become tests of what is applied: | test | pins | |---|---| | `test_filter_matches_the_needs_as_written[serial, j2]` | after an ID extend closes `TGT_1`, a later filter on `closed` does not match it and one on `open` does; no warning; the `-j 2` cell is the determinism fence | | `test_filter_on_an_unmodified_field_matches_as_before` | a filter on a field no extend changes, and ID extends, apply exactly as before | | `test_retired_match_order_type_is_a_no_op_in_suppress_warnings[absent, listed]` | the build has only its real warning (an unknown ID), with or without the retired type in `suppress_warnings`, and the as-written values | | `test_filters_ignore_an_earlier_priority` | a priority-100 extend that closes five needs first does not change what two later `status` filters match | | `test_identical_filters_are_evaluated_once_per_document` | one evaluation per `(filter, document)` (counted), and both extends sharing it apply its result | | `test_a_filter_reading_its_document_is_shared_within_that_document_only[serial, j2]` | the same `c.this_doc() and status == "open"` in two documents: each tag lands on its own document's need only (the document half of the key, fenced by values) | | `test_filter_that_fails_as_written_applies_to_nothing[raises, reports]` | a filter that does not compile: one `Invalid filter` warning per extend, nothing applied, the extends around it applied (this covers the `except` Codecov flagged on #2083); a filter that reports `needs.filter`: one warning, at the first applied | | `test_strict_unknown_id_ends_the_build` | the ID lookup moved into the target pass; `:strict:` still raises `NeedsInvalidFilter` (green before and after: it had no test) | T1's `needs.json` snapshot (`test_without_the_option_needs_json_is_unchanged`) does not move. `tests/test_utils.py::test_counted_ids` pins the moved formatter (none, one, three, four, string order, a set). Mutation checks (each applied, the file run, reverted): | mutation | red | |---|---| | apply each filter extend to its LIVE matches again | 7 of 22, incl. `test_filter_matches_the_needs_as_written[serial, j2]` (`['saw_closed_later'] == ['open_as_written']`) | | memo per extend instead of per `(filter, document)` | `test_identical_filters_are_evaluated_once_per_document` (one call too many), `[reports]` (`2 == 1`) | | memo per filter, without the document | `test_a_filter_reading_its_document_is_shared_within_that_document_only[serial, j2]` (`['a_this', 'a_this_again', 'b_this'] == ['a_this', 'a_this_again']`), and the counted memo test (one call too few) | `uv run poe lint` and `uv run poe typecheck` pass. The full suite (`uv run poe test-needs -n 4 tests/`) gives 50 failed, 2191 passed, 13 skipped (at the head of this PR): 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; no test outside the needextend files changed. ## 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 #1658, #2083 and the code; every test was run by hand, the tests were committed first and shown red against the unflipped code, 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). - [x] Documentation is updated where behaviour or options change (`docs/directives/needextend.rst`, `docs/dynamic_functions.rst`). - [x] The package's `docs/changelog.rst` has entries under *Unreleased* (#2083's entry, trimmed to the priority, and the flip under a new *Breaking changes* heading). - [x] `uv run poe lint` and `uv run poe typecheck` pass. The changelog: #2083's entry, trimmed to the priority, stays under *Improvements*, ```rst - ✨ ``needextend`` gains ``:extend_priority:`` (default 500, lower applied first) (:issue:`1658`, :issue:`2064`, :pr:`2083`) ``` and the flip is a new *Breaking changes* entry: ```rst -‼️ ``needextend`` filters are evaluated against the needs as written **(changed output)** (:issue:`1658`, :issue:`2064`, :pr:`2083`, :pr:`2127`) ``` whose body says that no ``needextend`` changes which needs another one's filter matches, whatever their priorities and the names of their files, then **who is affected** (a filter that relied on a change made by an earlier ``needextend`` now modifies the needs written with that value, and no warning says so) and the remedy (name those needs by ID, or write the condition on the values as written), and ends: "The notice warning ``needs.needextend_match_order`` of the unreleased :pr:`2083` is gone, and a ``suppress_warnings`` entry naming it is a no-op." ## ubCode Nothing to match in ubCode beyond useblocks/ubcode#3897. ubCode already gathers every extend's matches against the pre-extend state and applies them per need in `(docname, page_order, extend_idx)` order: `update_extends` (`rust/ubc_needs/src/index.rs:3794`) collects the short-circuit matches (`:3811`) and the complex checks in one pass over the unmodified needs (`:3859-3917`), then sorts each need's extends (`:3950-3955`); its own doc comment records the filter-timing difference this PR removes (`:3776-3783`). It does not parse `:extend_priority:` yet: the option is reported and ignored, which is useblocks/ubcode#3897 (`design/divergence-register.md:237`). Once this ships in a release, the register row on filter timing (`design/divergence-register.md:236`, "🚧 D7") closes. (Measured at ubCode `main`, `a758b53`.)
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
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
needextenddirectives were applied in(docname, lineno)order only, and each filter was evaluated againstthe 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.rstholdsneedextend:: TGT_1 :status: closedand thenneedextend:: status == "closed",that filter tags
TGT_1, while the identical filter ina.rstmatches 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
+tagsvalues append in ascending priority.A project that never sets the option keeps exactly today's order (pinned by a
needs.jsonsnapshot recordedbefore the change).
priority: aneedextend's options are field names (the directive has a catch-all option spec),and
priorityis a field projects define (this package's own schema guide and changelog examples declare one),so
:priority:on aneedextendwould be ambiguous with setting that field. Theextend_prefix avoidsthe collision. (The issue proposed
extend_order; the name follows Sphinx'spriority.)without negative numbers. 500 leaves room on both sides, and negatives are refused.
nonnegative_int, asmax_itemsis in the view directives.An invalid value (
abc,-1,1.5, empty) is handled exactly as a bad:strict:is:one
needs.needextendwarning 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).
:strict:, the option is popped before the options that modify fields, so a field namedextend_priority(or
strict) cannot be replaced by aneedextend;:+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
idcannot beextended; 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 ownfiltering; the evaluation itself is not skipped (a skip keyed on
suppress_warningswas tried and dropped asnot 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: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 filtermatches. 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_VERSION8 → 9:NeedsExtendTypegainsextend_priority, which changes the pickled environment.Measured: a rebuild over a
_buildwritten 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):extend_needs_data6.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).
extend_needs_data3.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
needextendpage gets anextend_prioritysubsection (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 carryversionadded:: 9.0.0, as the pages already dofor 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-Wthat has an order-dependent filter goes red until the filter is rewritten or the subtype is suppressed,
with
"needs.needextend_match_order":"needs.needextend"insuppress_warningsdoes 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.rstanddocs/dynamic_functions.rstthat #2081 also edits ("document-name and lineorder" → "extend priority, then document-name and line order"), and the changelog
Unreleasedlist: whicheverlands second resolves those by hand.
Tests
New file
packages/sphinx-needs/tests/test_needextend_priority.py(inlinetest_appprojects, exactbuild_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_without_the_option_needs_json_is_unchangedneeds.jsonequals the snapshot recorded before the change; no warningtest_higher_priority_is_applied_last[serial]a.rst600late,b.rst400early→late'early' == 'late'test_appends_follow_ascending_priority+tagsat 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)test_equal_priorities_keep_document_order(docname, lineno)decides; every recorded extend carries 500test_invalid_priority_is_reported_and_not_applied[abc, -1, 1.5, empty]test_priority_option_shadows_a_field_of_that_name:extend_priority: 7sets the priority;:+extend_priority:still appends to the field'7' == 'authored appended'test_filter_depending_on_an_earlier_extend_is_reported[serial]b.rst:7;needs.jsonvalues as todaytest_no_warning_where_the_matches_agreetest_match_order_warning_is_suppressed_alone[reported, suppressed]suppress_warningssilences the subtype and not the otherneedextendwarning[reported]red;[suppressed]green (guard)test_needs_as_written_ignore_an_earlier_priorityz.rstcloses REQ_1 to REQ_5 before the twostatusfilters at 500 ina.rst(which sorts first): both warn,6 needs (REQ_1, REQ_2, REQ_3 and 3 more) now and 1 (REQ_6)and0 needs now and 5 (…); the needs are written in reverse id order[j2]variants of T2 and T6-j 2(padded to 7 documents, since Sphinx 7 reads in parallel only above 5)test_identical_filters_are_evaluated_once_per_documentb.rst: bothb.rstfilters reported at their own lines; the as-written evaluations are exactly(filter, "a")and(filter, "b")Mutation proofs (each applied to the finished code, the new file run, the code restored):
extend_priority[reported], T8:extend_priority: 0treated as unset (nonnegative_int(...) or DEFAULT_EXTEND_PRIORITY)['p400', 'p0', 'p500', 'p600'])No existing assertion or snapshot changed: before
masterwas merged in, the full suite was 2102 passed / 13skipped (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 lintanduv run poe typecheckpass.uv run poe docs-needswas not run (it creates its own environment); the whole docs tree was built with thepackage's development environment and stand-ins for three docs-only extensions, with no warning
(nitpicky included), and the new example renders
latewithapplied_first, applied_second.Follow-ups
(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 filterwarning.:extend_priority:(same default, direction and sort key) and, if it evaluates filtersagainst the mutated needs too, the same notice.
options.popitem()records one directive's options in reverse written order. The same key twice is refusedby docutils (
duplicate option), but different keys on the same field are applied in reverse, measured::-tags:then:+tags: newgives[](written order would give['new']), and:status: replacedthen:+status: appendedgivesreplaced(written order:replaced appended). It is within one directive, so thepriority does not touch it; applying in written order would be a change of output of its own.
When squashing, strip the
Co-authored-bytrailer GitHub proposes.