🐛 sphinx-needs: a read through computed links is a column read

[chrisjsewell]

What

Package: packages/sphinx-needs.

This fixes check_linked_values and calc_sum(links_only=True) in a link field whose need's own links are computed in
the same step. Until the links are computed, nobody knows which needs they name, including the ones written in them.
So the call now reads its field, and the fields its filter names, as a column, on every need. That is the same
column node a whole-project calc_sum reads:

.. req:: Reads a link type through links that are computed too
   :id: LL_WLINK
   :links: T_THREE, [[copy("links", "OTHER_L")]]
   :blocks: [[check_linked_values("T_THREE", "refs", "T_ONE", one_hit=True)]]

What the call does now depends on where its field is computed:

• Computed in step 2, the link fields, by some need (refs above): the call runs after every need that computes
  it, so it reads the computed value (above, blocks is ['T_THREE'] once a linked need's computed refs names
  T_ONE). It also still waits for its own links.
• Computed after step 2, on any need, or a back link or dead-link flag: the read is out of scope. The call reports
  needs.derive_scope and is not run, and the field keeps its written links. For example: dynamic function 'check_linked_values' for option 'blocks' reads 'links_back' on every need its links name, which is final only after the link fields are computed: the call is not run and the field is left empty.
• Computed by no need: the call reads the written value.

Two more cases:

• A call in the links themselves still reads itself, which is a cycle.
• A need that reads its own field through such a column is on a cycle with it. The message then names the links:
  …, through its links, which are computed in the same step, so every need is a candidate.

Code changes:

• functions/order.py:
  • _CallReads._linked() replaces the two walks of the stored list.
  • OutOfScope.linked records a back link or flag read on every need the links name.
  • A column's Reason can now be the reader's own links node.
• functions/functions.py: the derive_scope and derive_cycle messages for these reads.
• Docs: a paragraph in dynamic_functions.rst's processing order, a clause in the list of reads that cannot be
  ordered, and a sentence on the cycle message. The changelog's Unreleased derived-values entry gets a clause and
  cites this pull request.

Why

#2064 (derived values) and #2134 (phase 1, merged) order a call after every value it reads. #2136 was a follow-up.

ubCode's review of its own phase 1 found a gap in these reads. The order walked need.get("links"), which is []
for a computed link list, so a written target got no edge. That had three effects:

• Its back links were read as the pass had reset them, so check_linked_values(…, "links_back", …) passed silently.
• A link type it computes was read before it was computed. Only the run-time check caught this, as …the order of the pass did not account for this read; please report this….
• A filter on such a field silently gave the wrong value, because the run-time check does not see a filter's reads.

A target added by the computed list was read in order or not, depending on its id. That means the value depended on the
schedule, which the order exists to rule out.

The ruling holds for both tools: such a reader reads through a column. ubCode's phase 1 takes the same rule, with the
same wording for the back-link case.

Tests

• tests/test_dynamic_functions_strata.py::test_a_read_through_computed_links_is_a_column_read uses reviewer A's
  shapes (LL_WBACK, LL_ABACK, LL_WLINK, LL_WLINK1, LL_ALINK, and Z_ALINK three links down). It adds:

  • a filter on a computed link type (LL_WFLT);
  • a field computed in step 4 on a need the links do not name (LL_LATER);
  • a field no need computes (LL_FINAL, ['T_ONE'], no finding);
  • a cycle through the column (LL_SELF).

  It runs serially and with -j 2, and asserts every value and every warning. The run-time check is silent on every
  shape. Red before the change:

  • {'LL_WBACK': ['T_ONE']} != {'LL_WBACK': []} and {'LL_ABACK': ['T_ONE']} != {'LL_ABACK': []} (the call ran on
    the reset back links);
  • {'LL_WFLT': []} != {'LL_WFLT': ['T_ONE']} (the filter read refs before it was computed, silently).

  Before the change, its build also gave the "please report this" warning for LL_WLINK, LL_WLINK1 and LL_ALINK.

• tests/test_functions_order.py:

  • The three outcomes as unit rows: a column (Column('refs'), ('RD', 'links')) for the option, a links_only sum and
    a filter; out of scope for a later field, a back link and a dead-link flag; final for a field no need computes.
  • A link field reading through itself stays a self-cycle.
  • A cycle through the column names the links.
  • Red before the change: 9 of the 12 new items (the other 3 are regression guards that already pass).

The full suite (uv run poe test-needs -n 4 tests/) passes, except the 50 graphviz renders that need dot (the same 50
fail on master on a machine without it).

Checklist

• 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 ubCode's review finding and the code. The tests were written first and shown
  red, and every line was read.
• 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).
• Documentation is updated where behaviour or options change (docs/dynamic_functions.rst).
• The package's docs/changelog.rst has the change in the existing Unreleased derived-values entry (its :pr:
  cite now names this number).
• uv run poe lint and uv run poe typecheck pass.

Refs #2064.

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.85%. Comparing base (ec0b626) to head (4476cf7).

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2143      +/-   ##
==========================================
+ Coverage   92.82%   92.85%   +0.02%     
==========================================
  Files         138      138              
  Lines       20313    20328      +15     
==========================================
+ Hits        18856    18876      +20     
+ Misses       1457     1452       -5     

Flag	Coverage Δ
ai-index	100.00% <ø> (ø)
codelinks	95.35% <ø> (ø)
mounts	94.27% <ø> (ø)
pytests	92.46% <100.00%> (+0.04%)	⬆️
reports	88.82% <ø> (ø)
ub-test-reports	90.20% <ø> (ø)

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.