✨ sphinx-needs: needs.derive_unresolved for reads of same-pass values

[chrisjsewell]

Summary

All [[…]], <<…>> and <{…}> are resolved in one pass, need by need, in the order the needs reached the build
environment, and each result is written into its need as soon as it is computed. A dynamic function or variant
condition that reads a field which itself carries a [[…]], <<…>> or <{…}> (on another need, or on its own need)
therefore reads the computed value or the unresolved one depending on that order: document names, the documents the
last build re-read, -j. Nothing reported it. It is now a warning, needs.derive_unresolved, once per reading
call, located at the reading need (None for an external need, as for every pass warning).

The rule. A read of name on need T is reported iff name is a key of T's dynamic fields after needextend
(new read-only accessor NeedItem.carries_dynamic_value(name), over a set the pass never clears), whether or not the
pass has already computed it. Reporting only "not yet computed" reads would make the warning itself depend on build
history: on one project (CHAIN_X on a.rst copies CHAIN_Y.summary on b.rst, CHAIN_Y = [[copy("title")]]) a
scratch build reads 'None', an incremental build re-reading a.rst reads 'done', one re-reading b.rst reads
'None' again, so a "not yet visited" warning would turn a -W build red, green, red by which file was edited last.
So the warning also fires where today's value happens to be the computed one (CHAIN_B, CYC_B, VAR_COND in the
corpus below): that value is an accident of insertion order and flips on an incremental build (pinned by a test).
It is ubCode's rule (reads_unresolved), so both tools report the same reads.

What is read (only the built-ins and variant conditions, phase 0):

• copy: the source field: own need, need_id, or the filter's chosen match (the lowest id);
• calc_sum: the value of every need it considers (need-id order for a whole-project sum), noted BEFORE its
  filter runs, so a need the filter drops is named too; with links_only, also the need's own links;
• check_linked_values: the need's own links, then each target's search_option until it stops, again noted
  before its filter;
• a variant condition: the names of each condition evaluated (after the needs_variants lookup, up to the first that
  holds), on the need's own fields, excluding names needs_filter_data, build_tags or var supply (they win over a
  field of the same name in the evaluation context).

Mechanism. resolve_functions opens one UnresolvedReads record for each dynamic function call and each
<<…>> variant it evaluates (the conditions of one variant share it), and passes it down explicitly; no module-level
or context state is involved. _execute_dynamic_func hands the record, as the keyword-only argument reads, only to
a function carrying the records_reads mark (the built-ins copy, calc_sum and check_linked_values). The mark
names the function it was set on and is checked against the function as registered, so a wrapper that copies a
built-in's attributes (functools.wraps) is not marked. Each built-in notes, through UnresolvedReads.note_read, a
field or link it reads itself that carries a [[…]], <<…>> or <{…}> of its own once every needextend is
applied: copy the field of the need it copies from (with a filter, the match the filter chose); calc_sum the field of each candidate need, and
check_linked_values the field of each target it reaches before it stops, each before their filter decides on it, and their own need's
links where they read them. The fields a filter reads are never noted. _get_variant receives the same record
and notes the need's fields that each evaluated condition names. Every other function, every user-registered one
included, is called exactly as before and never receives reads. A built-in called after the pass (an ndf role, a
needtable :style_row:, a direct execute_func call), or by a user's own function during it, gets reads=None and
notes nothing: the user's function has no record to pass on. When the call or variant ends, whatever its outcome,
the record is reported in a finally block as one needs.derive_unresolved warning located at the reading need.

Message: ubCode's words up to the reads, then a clause that is true whether or not the value had been computed:

index.rst:60: WARNING: dynamic function 'copy' for option 'summary' read 'summary' on need 'CHAIN_B', which carries a dynamic function or variant computed in the same pass: the value read depends on the order the needs are resolved in [needs.derive_unresolved]
index.rst:69: WARNING: dynamic function 'calc_sum' for option 'total' read 'hours' on 3 needs (HRS_1, HRS_2, LIT_3), which carry a dynamic function or variant computed in the same pass: the value read depends on the order the needs are resolved in [needs.derive_unresolved]
index.rst:95: WARNING: variant condition for option 'band' read 'f1' on need 'VAR_COND', which carries a dynamic function or variant computed in the same pass: the value read depends on the order the needs are resolved in [needs.derive_unresolved]

A name read on four or more needs names the first three (… and K more); several names are joined as ubCode joins
them (a, b and c); carries is singular only for one name read on one need.

Not reported (documented): the fields a filter argument reads (current_need included; ubCode evaluates none of
these), the reads your own functions make, through a built-in they call or otherwise (no record reaches a user
function), <{…}> (variant data, not needs), links_from_content (reads the document). A need.<attr> argument
cannot occur in a field value in sphinx-needs.

Caveats (documented): a filter that reads a computed field still decides, by the order, which needs it keeps.
Noting before the filter keeps calc_sum stable, but a copy with such a filter, and a check_linked_values whose
check stops at a target the filter kept before it reaches a computed one, can be reported in one build and not in another. After the first computed value a call reads, what it reads next (an early
exit, the first true condition) can depend on that value, so the needs a message names can vary between builds while
the warning itself does not.

Opting out: suppress_warnings = ["needs.derive_unresolved"] silences exactly this subtype; it is its own subtype
(listed in the documentation's build-warnings list), so needs.dynamic_function failures stay visible.

Part of #2064 (fourth item); the notice period of #2030 step 0/E. ubCode reports the same reads as the Info-graded
needs.derive_unresolved (useblocks/ubcode#3855): the same finding set on the shared corpus, a lower severity by design.
This is changed output: a -W build with such a read fails until the read is changed or the subtype suppressed.
Docs: a "Reads of a value computed in the same pass" section in docs/dynamic_functions.rst, and a note on the
reserved reads keyword beside the built-ins; changelog entry under Unreleased → Improvements.
For the release writer: the section carries .. versionadded:: 9.0.0, after the page's existing 9.0.0 note;
renumber it if the release is not 9.0.0.

Tests

packages/sphinx-needs/tests/test_dynamic_functions_unresolved.py (inline test_app projects plus one doc_test
project; exact build_warnings(app) == [...]), committed first: 26 red, 4 negatives green, against the unchanged code;
30/30 green after. The review round added three (33/33 green): the filtered-sum fence, red against the previous code,
and two read-site tests. The explicit-record round added six more (39/39 green), under T12 below; none of the 33 changed: four with the parameter, two in its review round (a functools.wraps wrapper of a built-in, and the parameter's keyword-only None default).

test	pins
test_message_shape (8) / test_message_needs_a_read	the pure formatter: 1, 2, 3, 4, 10 ids, two names, three names, variant vs function; empty input refused
T1 test_chain_across_needs	CHAIN_A copies CHAIN_B.summary: one warning at CHAIN_A, none at CHAIN_B; target on a page read after ("") and before ("Middle") give the identical list; also under -j 2
T2 test_chain_inside_one_need	status = [[copy("comment")]], comment = [[copy("title")]]: 'comment' on need 'SAME_NEED'
T3 test_calc_sum_over_every_need	summands written in reverse id order: 'hours' on 2 needs (HRS_1, HRS_2); five: (HRS_1, HRS_2, HRS_3 and 2 more)
T4 …links_only_reads_its_own_links / …names_its_computed_targets	own links = [[copy(...)]]: 'links' on need 'OWN_LINKS'; links HRS_2, HRS_3, HRS_1: (HRS_2, HRS_1) in link order
T2b test_copy_filter_reads_the_match_it_copies_from	copy(filter=) reports the lowest-id match it copies: RD_ONE (computed match read first, authored lowest id) silent; RD_TWO (authored match first, computed lowest id SRC_A2) warns
T5 test_check_linked_values	reads WORK_1.status (already computed: reported anyway); the one_hit gate that stops at an authored target reports nothing
T5b test_check_linked_values_reads_its_own_links	own links = [[copy("links", "OTHER_9")]]: 'links' on need 'GATE_OWN'
T6 test_variant_condition	f1 declared before band (matched) and after (unmatched): identical warning; f1 named twice in the variant, named once
T6b …reads_only_the_expressions_it_evaluates	a condition after the first true one is not read; a needs_variants name reads its expression's names
T7 test_a_call_set_by_needextend_is_a_computed_value	LIT_3 :hours: 1 + needextend … [[copy("h0")]]: LIT_3 is named (total 3.0 reads the pre-extend value)
T8 test_a_failing_call_still_says_what_it_read	both warnings, derive_unresolved first; with the subtype suppressed only needs.dynamic_function remains
T9 (4)	authored reads, links_from_content and an :ndf: of a computed field report nothing; a condition name needs_filter_data supplies is not a field read; the suppressed T1 project is warning-free; the subtype is listed with its description
T10 test_the_warning_does_not_depend_on_the_build_history	T1 built twice on one app, index.rst re-read: value flips "" → "Middle", warning list identical
T10b test_a_filter_on_a_computed_field_does_not_decide_the_warning	calc_sum("hours", "summary == 'done'") and a filtered check_linked_values over TGT_F (computed summary and hours), built twice: the filter keeps TGT_F in one build only (total 0.0 → 3.0), and the two warnings are identical in both. Against the previous code, CLI builds gave 0, 2, 0 warnings for scratch, re-read reader page, re-read target page
T11 test_ubcode_fixture	ubCode's dynamic_functions_unresolved fixture as tests/doc_test/doc_df_unresolved/ (hours' default moved out of its schema; NEED_ATTR dropped): (reader, option, name, needs) set equals ubCode's 12 findings; all 12 message heads are byte-identical to ubCode's
T12 (6) …user_function_is_called_without_the_record / …builtin_a_user_function_calls_is_not_reported / …user_wrapper_of_a_builtin_is_not_handed_the_record / …builtin_called_after_the_pass_notes_nothing / test_reads_written_in_a_call_is_an_error / …take_the_record_only_as_a_keyword_defaulting_to_none	a needs_functions entry with the plain signature (no **kwargs) resolves in the pass with no warning; a copy a user function calls reads CHAIN_B's unresolved summary and is not reported; :ndf: roles, a :style_row: and a direct execute_func call on computed values render the final values and report nothing; a functools.wraps(copy) wrapper with a narrower signature resolves ('NARROW') and a forwarding one is not reported (the mark names its own function, so a copied __dict__ does not carry it); copy("title", reads=1) in a field and in a role each fail that call with needs.dynamic_function, the build completing; reads is keyword-only with a None default on all three built-ins

Mutation proofs (each applied to the committed code, the new file run, reverted):

mutation	red
M1 "not yet visited": skip the note when the pass already wrote that field	T1 target-earlier, T1 -j, T5, T6 f1-first, T6b, T10, T11
M2 report only when the call succeeded	T8, T11
M3 do not note links_only's own links	T4 own-links, T11
M4 drop the subtype from WarningSubTypes only	the listing test; poe typecheck (2 diagnostics); the suppress tests stay green (Sphinx filters on the string)
o8 drop check_linked_values' own-links note	T5b
c4c copy(filter=) notes the first match (result[0]) instead of the one it copies	T2b
calc_sum notes the summand after its filter again	T10b
check_linked_values notes the target after its filter again	T10b
r1 drop @records_reads from calc_sum	T3 (both)
r2 drop @records_reads from copy	T1 (all three)
r3 hand the variant path reads=None	T6 (both)
r4 hand reads= to every function, marked or not	T12 user-function (shout() got an unexpected keyword argument 'reads'), and the links_from_content negative of T9
r5 read the mark off the timing wrapper instead of the registered function	every reporting test (17): functools.wraps copies the mark's name, not its identity
r6 a shared module-level record as the built-ins' default	the keyword-only-None signature test

No existing assertion or snapshot changed. The full suite was last run whole on master 4e3db3b plus the first commits
(2127 passed / 13 skipped, 304 snapshots: 2094 before + the 33 new); after merging master 714123a and the
explicit-record round, the machine at hand had no graphviz dot, so its 50 render tests failed identically at the base
and at the head while everything else passed with the six new tests added — CI is the authority for the whole-suite count.

Cost, on a 10 000-need project with 100 whole-project calc_sum carriers, built with -E and timing the real
resolve_functions. Eight builds each, alternating, on a noisy machine (the base alone ranged 1305–1858 ms):
the median is 1836 ms with this change, against 1693 ms before the warning existed (+143 ms, +8.5 %);
the reviewer's session measured +17.6 % on the same point, so the cost is in the +9 % to +18 % range.
The first version of this pull request measured 2028 ms (+19.8 %) in the same session, before calc_sum
fetched the record once per call. The whole-build wall time moves by between ±0.5 % and +2.4 %. The explicit
parameter removes the per-call context-variable set/reset and the per-read lookup, so it is not slower than that.

uv run poe lint and uv run poe typecheck pass. uv run poe docs-needs was not run (its
intersphinx inventories need network access); the new docs section, the reserved-keyword note and the changelog entry
parse without docutils messages, and docs/conf.py ignores the nitpick on the record's class in the built-ins'
autodoc signatures, as it does for NeedsSphinxConfig.

Follow-ups

• Reads made by a filter argument (copy, calc_sum, check_linked_values, links_from_content) are not reported
  (see Caveats): reporting them would make a copy with a filter on a computed field stable too.
• A corpus both tools accept, to replace the adapted copy: the divergences the run surfaced are schema.default
  (accepted by ubCode, refused here), need.<attr> in a field value (accepted by ubCode, need refused here), a link
  list mixing a literal and a call (ubCode keeps the literal before resolution, sphinx-needs holds []), a
  needextend-set call's pre-resolution value (old value here, empty in ubCode), and the default needs_id_regex
  refusing GATE.

When squashing, strip the Co-authored-by trailer GitHub proposes.

[codecov]

Codecov Report

❌ Patch coverage is 95.65217% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 92.47%. Comparing base (714123a) to head (46cfd37).

Files with missing lines	Patch %	Lines
...hinx-needs/src/sphinx_needs/functions/functions.py	94.73%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2080      +/-   ##
==========================================
+ Coverage   92.39%   92.47%   +0.08%     
==========================================
  Files         136      136              
  Lines       19320    19394      +74     
==========================================
+ Hits        17850    17934      +84     
+ Misses       1470     1460      -10     

Flag	Coverage Δ
ai-index	100.00% <ø> (ø)
codelinks	95.35% <ø> (ø)
mounts	94.26% <ø> (ø)
pytests	91.88% <95.65%> (+0.13%)	⬆️
reports	88.01% <ø> (ø)
ub-test-reports	90.16% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.