Repository navigation
🐛 sphinx-needs: need-id order for calc_sum and copy(filter=) - #2078
Merged
Merged
Conversation
A whole-project `calc_sum` adds floats in the order the needs reached the environment, and `copy` with a `filter` copies from the first match in that order, so both answers depend on document names, on which documents the last build re-read, and on `-j`. These tests pin need-id order, comparing ids as plain strings: the total ubCode's `dynamic_functions_sum_order` fixture gives (its page verbatim, and the same needs one page each), the lowest-id copy source, string order rather than natural order, the same values after an incremental build that re-reads one document, the written link order of a `links_only` sum, and the name `calc_sum` reports when it is called outside a need. All but the `links_only` test fail against this commit's code.
A whole-project `calc_sum` now adds the needs' values in ascending need-id order, and `copy` with a `filter` copies from the match with the lowest id, both comparing ids as plain strings (code-point order, so `REQ_10` comes before `REQ_9`; not the natural order links are sorted in). Before, both read the needs in the order they reached the environment, so a total of non-integer values changed in its last digits, and the copy source moved, with the documents the last build re-read and with `-j`. The order is the byte order ubCode sorts ids by, and the addition stays sequential as ubCode's is, so the two tools give the same total. `calc_sum` with `links_only` keeps the order the links are written in, and its error for a call outside a need names `calc_sum` rather than `check_linked_values`. The docstrings of both functions state the order, and the changelog entry goes under `Unreleased`.
The `ndf` role, a need's `:style:` and a needtable's `:style_row:` call
the dynamic functions after resolution with a `NeedsView`, a different
mapping from the plain `dict` the resolution pass passes, and no test
pinned the order there. A new html-build test puts `calc_sum("hours")` and
`copy("id", filter=...)` in `ndf` roles inside a need, over summands
written in the reverse of need-id order, and asserts the rendered
`0.6000000000000001` and `SUM_A`: the unfixed code renders `0.6` and
`SUM_C`, and sorting only when the mapping is a `dict` renders `0.6`.
The changelog entry now limits the ubCode claim to a sum of literal values
over every need, and says that the error for a `calc_sum` outside a need
names `calc_sum`.
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 #2078 +/- ##
==========================================
+ Coverage 92.21% 92.26% +0.04%
==========================================
Files 136 136
Lines 19061 19062 +1
==========================================
+ Hits 17578 17587 +9
+ Misses 1483 1475 -8
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.
Summary
A whole-project
calc_sumadded its values in the order the needs reached the build environment,and
copy(filter=…)copied from the first match in that order.That order is insertion order: external needs, then documents in name order on a scratch build,
but a re-read document's needs move to the end on an incremental build, and
-jmerges workers in completion order.Float addition is not associative, so the same sources gave different values depending on the build's history.
Measured on one project (
a.rstholdsSUM_C :hours: 0.3,b.rstSUM_B 0.2,c.rstSUM_A 0.1,index.rstholdsTOTAL :total: [[calc_sum("hours")]]and:pick: [[copy("id", filter="hours is not None and hours > 0")]]):TOTAL.totalTOTAL.pick0.6SUM_Ca.rst0.6000000000000001SUM_BWith this change both builds give
0.6000000000000001andSUM_A.calc_sumwithoutlinks_onlyadds in ascending need-id order:sorted(needs)on the mapping's keys(the key is the need id, for the
dictthe resolution pass passes and for theNeedsViewthat:ndf:and:style_row:pass), thenneeds[id]. The addition stays sequential (nomath.fsum).copy(filter=…)copies from the match with the lowest id:min(result, key=itemgetter("id")), O(matches),at every seam (the resolution pass,
:ndf:,:style_row:).NEED_10 < NEED_2 < NEED_9. It is deliberately not the natural ordersort_links()uses(
REQ_9 < REQ_10), because it is the byte order ubCode sorts ids by.calc_sum(links_only=True)is unchanged: it adds in the order the links are written,which is already deterministic and is what ubCode does.
on a sum of literal values over every need (the ubCode
dynamic_functions_sum_orderfixture page is one of thenew test projects, verbatim). ubCode does not evaluate
calc_sum(filter=…)yet, and a summand that is itselfcomputed still depends on the resolution loop's order (phase 1).
calc_sumcalled outside a need now saysNo need given for calc_sum(the message namedcheck_linked_values).calc_sumcall at 10 000 needs in two measurements(0.6 s → 1.0 s and 0.66 s → 1.17 s for 100 carriers): the sort (≈ 2.5 ms at 10 000 needs) plus walking the needs
in id order. The whole 10 000-need build is unchanged in practice (15.4 s → 15.6 s).
links_onlysums andcopy(filter=…)(which only takes the minimum of its matches) are unaffected.A per-pass memo of the sorted ids is left to a follow-up (below).
copyandcalc_sumstate the order (they are autodoc'd into the docs),and the changelog gets an
Unreleased→Bug fixesentry marked (changed output):a float total can change in its last digits, and a
copy(filter=…)with several matches now copies from the lowest id.Part of #2064 (first item).
Tests
New tests in
packages/sphinx-needs/tests/test_dynamic_functions.py(inlinetest_appprojects; T1 to T5 use theneedsbuilder and readneeds.json, T6 and T7 are html builds). T1 to T6 were committed first and were red againstthe unfixed code; T7 came from the review round and is red against it too:
test_calc_sum_adds_in_need_id_order(one page per summand; the ubCode fixture page verbatim)total == 0.60000000000000010.6test_copy_filter_copies_from_the_lowest_idpick == "SUM_A"(the higher id is written first)SUM_Ctest_need_id_order_compares_ids_as_stringsNEED_10/NEED_2/NEED_9written in natural order:total == 0.6000000000000001, a copy overNEED_9/NEED_10givesNEED_100.6test_need_id_order_does_not_depend_on_the_build_historya.rstre-read (asserts0 added, 1 changed, 0 removed): identicaltotalandpick(0.6000000000000001, 'SUM_B') != (0.6, 'SUM_C')test_calc_sum_links_only_adds_in_the_written_link_order0.6and0.6000000000000001, on purposetest_need_id_order_in_the_ndf_role:ndf:calc_sum("hours")andcopy("id", filter=…)inside a need (theNeedsViewseam, also used by:style:/:style_row:): renders0.6000000000000001andSUM_A0.6andSUM_Ctest_calc_sum_outside_a_need_names_itselfcalc_sumcheck_linked_valuesMutation proofs (each applied to the fixed code, the file's tests run, then reverted):
calc_sumback toneeds.values()sorted(needs, key=_natural_sort_key)copyback toresult[0]links_onlylinks sorteddict(the pass), not for theNeedsViewNo existing assertion or snapshot changed: the full suite is 2084 passed / 13 skipped (2076 + the 8 new), 304 snapshots passed.
uv run poe lintanduv run poe typecheckpass.uv run poe docs-needswas not run locally (its intersphinxinventories need network access); the two changed docstrings and the changelog section parse without docutils warnings.
Follow-ups
calc_sumcarriers sorts once per passrather than once per call — phase 1, together with the resolution loop's own order.
links_onlykeeps the written order or moves to need-id order is a phase-1 ruling:the design's §5.3 (
derived-need-values.md, every set a derivation reads in need-id order) disagrees withregister row 280 (
links_onlyin the written order, which both tools do today).When squashing, strip the
Co-authored-bytrailer GitHub proposes.