Repository navigation
‼️ sphinx-needs: needextend filters see the needs as written - #2127
Merged
Merged
Conversation
…phinx-needs) The "count, first three ids and how many more" shape of the needextend match-order message becomes utils.counted_ids, with unit tests of its own, so that other messages about a set of needs can share it once the match-order warning is gone. No output changes.
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.
Which needs each needextend modifies is now decided before any is applied: an id names its need, and a filter is evaluated once per (filter, document) against the needs as written, with its errors reported there (an Invalid filter warning for each extend that carries a filter that cannot be evaluated). The extends are then applied in (extend_priority, docname, lineno) order to those needs, in need-id order. The live re-evaluation and the comparison with it are gone, and with them the unreleased needs.needextend_match_order warning type and its message.
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.
…sphinx-needs) The id lookup moved into the pass that resolves every extend's targets before any is applied; the raise under :strict: had no test before the move and has one now. Green before and after the flip: the behaviour is unchanged.
…only (sphinx-needs) The as-written evaluation is shared per (filter, document); until now only a call log fenced the document half of that key. Two documents each extend their own open need with the same c.this_doc() filter, serially and under -j 2: each tag lands on its own document's need, which a key without the document would break.
Each need is modified on its own, so the order of an extend's targets is fixed for determinism only; the comment no longer claims more.
…hinx-needs) The as-written filters move to a Breaking changes section of Unreleased, as a‼️ entry saying who is affected and how to keep what such a filter modified; :extend_priority: stays the ✨ improvement, trimmed to the priority itself. Both cite #2064 again. A filter's needs.filter warning is reported at the first extend APPLIED that carries it, which the needextend page and the changelog now say.
github-actions
Bot
added
the
pkg: sphinx-needs
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2127 +/- ##
==========================================
+ Coverage 92.57% 92.60% +0.02%
==========================================
Files 136 136
Lines 19459 19447 -12
==========================================
- Hits 18014 18008 -6
+ Misses 1445 1439 -6
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:
|
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.
What
Package:
packages/sphinx-needs.The flip #2083 announced. Which needs a
needextendmodifies is now decided before anyneedextendis 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.On master the filter is evaluated after the first
needextendclosedREQ_1, matches nothing, and reportsneeds.needextend_match_order; now it matchesREQ_1as written, andREQ_1ends closed and taggedwritten_open,with no warning. So no
needextendchanges which needs another one's filter matches, whatever their priorities andthe 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
NeedsInvalidFilterunderstrict, unchanged) and the filter, evaluatedonce per distinct
(filter, document)(the memo ✨ sphinx-needs: needextend:extend_priority:; warn where a filter depends on earlier extends #2083 introduced;c.this_doc()makes the document part of thekey). 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.needextendwarning, once per extend that carries it, at itsown 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.
needs.needextend_match_order, added by ✨ sphinx-needs: needextend:extend_priority:; warn where a filter depends on earlier extends #2083 and never released, is removed with its message(
logging.py); asuppress_warningsentry naming it is a silent no-op (pinned by a test).sphinx_needs.utils.counted_ids, with unittests of its own, for the dependency-ordered pass's cycle and scope messages to reuse.
"Filters see the needs as written" (
versionchanged:: 9.0.0, with the example above), its two identical paragraphson the priority become one sentence ("
:extend_priority:orders the modifications; it never changes what a filtermatches"), 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 documentnow give one
needs.filterwarning, at the first of them to be applied, where master gave one per extend. The warning order is otherwiseunchanged: targets are resolved in apply order, so unknown-ID and filter warnings interleave exactly as before
(
test_doc_needextend_warningsis 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_filter_matches_the_needs_as_written[serial, j2]TGT_1, a later filter oncloseddoes not match it and one onopendoes; no warning; the-j 2cell is the determinism fencetest_filter_on_an_unmodified_field_matches_as_beforetest_retired_match_order_type_is_a_no_op_in_suppress_warnings[absent, listed]suppress_warnings, and the as-written valuestest_filters_ignore_an_earlier_prioritystatusfilters matchtest_identical_filters_are_evaluated_once_per_document(filter, document)(counted), and both extends sharing it apply its resulttest_a_filter_reading_its_document_is_shared_within_that_document_only[serial, j2]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]Invalid filterwarning per extend, nothing applied, the extends around it applied (this covers theexceptCodecov flagged on #2083); a filter that reportsneeds.filter: one warning, at the first appliedtest_strict_unknown_id_ends_the_build:strict:still raisesNeedsInvalidFilter(green before and after: it had no test)T1's
needs.jsonsnapshot (test_without_the_option_needs_json_is_unchanged) does not move.tests/test_utils.py::test_counted_idspins the moved formatter (none, one, three, four, string order, a set).Mutation checks (each applied, the file run, reverted):
test_filter_matches_the_needs_as_written[serial, j2](['saw_closed_later'] == ['open_as_written'])(filter, document)test_identical_filters_are_evaluated_once_per_document(one call too many),[reports](2 == 1)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 lintanduv run poe typecheckpass. The full suite (uv run poe test-needs -n 4 tests/) gives50 failed, 2191 passed, 13 skipped (at the head of this PR): the 50 are the graphviz renders (
tests/test_needflow.py, the needflow conformancecorpus,
test_doc_github_1664_legend[graphviz]), which needdotand fail identically without this change on a machinethat lacks it; no test outside the needextend files changed.
Checklist
Built in an orchestrated session from needextend: User-controlled extension ordering via an
extend_orderoption #1658, ✨ sphinx-needs: needextend:extend_priority:; warn where a filter depends on earlier extends #2083 and the code; every test was run by hand, the tests werecommitted first and shown red against the unflipped code, and every line was read.
uv run poe test-needs) and they pass (the graphviz renders excepted on a machinewithout
dot, identically to master).docs/directives/needextend.rst,docs/dynamic_functions.rst).docs/changelog.rsthas entries under Unreleased (✨ sphinx-needs: needextend:extend_priority:; warn where a filter depends on earlier extends #2083's entry, trimmed to the priority, andthe flip under a new Breaking changes heading).
uv run poe lintanduv run poe typecheckpass.The changelog: #2083's entry, trimmed to the priority, stays under Improvements,
and the flip is a new Breaking changes entry:
whose body says that no
needextendchanges which needs another one's filter matches, whatever their prioritiesand the names of their files, then who is affected (a filter that relied on a change made by an earlier
needextendnow modifies the needs written with that value, and no warning says so) and the remedy (name thoseneeds by ID, or write the condition on the values as written), and ends: "The notice warning
needs.needextend_match_orderof the unreleased :pr:2083is gone, and asuppress_warningsentry naming itis 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 passover the unmodified needs (
:3859-3917), then sorts each need's extends (:3950-3955); its own doc comment recordsthe filter-timing difference this PR removes (
:3776-3783). It does not parse:extend_priority:yet: the option isreported and ignored, which is useblocks/ubcode#3897 (
design/divergence-register.md:237). Once this ships in arelease, the register row on filter timing (
design/divergence-register.md:236, "🚧 D7") closes.(Measured at ubCode
main,a758b53.)When squashing, strip the
Co-authored-bytrailer GitHub proposes.