✨ Add variant role for inline variant data references

[chrisjsewell]

Summary

Adds a new variant role that resolves a reference into needs_variant_data immediately during parsing, replacing :variant:`a.b` with a plain text node holding the looked-up value.

This complements the existing <{ var.a.b }> field-value references (from #3ba0da0) by exposing the same lookup as an inline role usable anywhere in body text. Unlike the ndf/need_func roles — which leave a placeholder node in the doctree for a later post-processing pass — this role resolves in run() and returns a nodes.Text directly.

Behaviour

Given:

needs_variant_data = {"platform": "arm", "build": {"opt_level": 2}, "archs": ["arm", "x86"]}

• :variant:`platform` → arm
• :variant:`build.opt_level` → 2
• :variant:`archs` → arm, x86 (lists are comma-joined)

The role content is a dotted path rooted at var (the var root is implicit). It reuses the existing constrained lookup_variant_data() resolution — no operators, function calls, or item access are permitted. A missing config, unknown key, or a path that resolves to a mapping emits a needs.variant warning and produces empty text.

Changes

• sphinx_needs/roles/variant.py — new VariantRole(SphinxRole).
• sphinx_needs/needs.py — register app.add_role("variant", VariantRole()).
• docs/roles.rst — document the new role; changelog entry added.
• tests/test_variant_role.py + tests/doc_test/doc_variant_role/ — coverage for scalars, nested values, lists, and the warning path.

[chrisjsewell]

Review: variant role for inline variant data references

Nice, focused addition. The implementation is clean, reuses the existing constrained lookup_variant_data() resolver, matches the ndf/need_func stringification conventions (lists comma-joined, scalars via str), and the docs + changelog entry are consistent (the :ref: targets role_variant and needs_variant_data both resolve, and versionadded:: 8.2.0 lines up with the next unreleased minor). Below are a few things worth considering before merge.

1. Parse-time resolution vs. incremental-build caching (main concern)

VariantRole.run() resolves the value and bakes a nodes.Text into the doctree at read/parse time. This is the deliberate difference from ndf that the PR description calls out — but it interacts badly with Sphinx's caching:

• needs_variant_data and needs_variant_data_file are registered with rebuild="html" (sphinx_needs/config.py:740, :758), not "env".
• With rebuild="html", changing the variant data on an incremental build re-writes output but does not re-read unchanged source docs. Their pickled doctrees keep the previously-baked value, so the output goes stale until a full -E rebuild.

The field-value <{ var.a.b }> references don't have this problem because they resolve later, during dynamic-value resolution (resolve_dynamic_values → _get_variant_data, sphinx_needs/functions/functions.py:352-353), which runs on every build after reads. So the two mechanisms the PR positions as siblings actually behave differently on incremental builds.

Two options:

• Resolve in a post-processing pass like NeedFunc (leave a lightweight placeholder node, replace it during a needs-resolve callback) so re-writes always pick up current data; or
• If parse-time resolution is intended to stay, at minimum document the incremental-build caveat, and consider whether needs_variant_data should trigger an env rebuild for this role to be reliable.

(Note the file-contents case is a pre-existing gap for both mechanisms: a changed needs_variant_data_file whose path is unchanged isn't tracked as a build dependency, so neither mechanism detects it. Worth a docs note, out of scope to fix here.)

2. "not configured" warning wording (minor)

sphinx_needs/roles/variant.py — the if not variant_data: branch reports "needs_variant_data is not configured", but this also fires when the user has configured needs_variant_data/needs_variant_data_file and it merely resolves to an empty mapping. Consider softening to e.g. "no variant data is available" so the message isn't misleading in the empty-but-configured case.

3. Test coverage gaps (minor)

tests/test_variant_role.py covers scalar/nested/list and the unknown-key warning — good. Not covered:

• The variant_data_file path (relevant given #1 — a fixture using the file would exercise the resolved/merged dict the role reads back from config.variant_data).
• The "path resolves to a mapping" warning branch (:variant:build``) and the "not configured"/empty branch.

Cheap to add and they lock in the two remaining warning paths.

4. Naming (subjective, non-blocking)

variant sits next to the existing "variant options" / needs_variants support (the [value] field syntax) and the needs_variant_data filter namespace. A reader could reasonably expect a role called variant to relate to variant options rather than pure data lookup. The choice is defensible as the inline sibling of needs_variant_data; just flagging that the vocabulary is getting crowded.

Nothing here is a hard blocker for a first cut — #1 is the one I'd want a decision on, since it affects correctness on incremental builds.

---

Generated by Claude Code

[chrisjsewell]

Thanks for the thorough review! Addressed in 341467f:

#1 (incremental-build caching) — kept parse-time resolution but switched needs_variant_data and needs_variant_data_file from rebuild="html" to rebuild="env" (config.py), so changing variant data now forces a re-read and the baked role values stay current. Added a docs note covering the remaining file-contents-with-unchanged-path gap (recommend a clean -E build there), which as you noted is pre-existing for both mechanisms and out of scope to fix here.

#2 (warning wording) — softened to 'variant' role used but no variant data is available: ..., which is accurate for the configured-but-empty case.

#3 (test coverage) — added tests for the mapping-reference warning (:variant:build``), the empty/not-configured branch, and a variant_data_file fixture exercising the file + inline-override merge that the role reads back from `config.variant_data`.

#4 (naming) — left as variant for now, positioned as the inline sibling of needs_variant_data; happy to rename (e.g. variant_data / vd) if you'd prefer to disambiguate from the [value] variant-options syntax — just say the word.

---

Generated by Claude Code

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.41%. Comparing base (4e10030) to head (13b1eea).
⚠️ Report is 302 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1737      +/-   ##
==========================================
+ Coverage   86.87%   89.41%   +2.53%     
==========================================
  Files          56       73      +17     
  Lines        6532    10580    +4048     
==========================================
+ Hits         5675     9460    +3785     
- Misses        857     1120     +263     

Flag	Coverage Δ
pytests	89.41% <100.00%> (+2.53%)	⬆️

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.

[ubmarco]

I wonder how the various field types are serialised to text. I could not find anything in the docs about this, also not for the <{ wheel.speed }> syntax in fields.
That should be documented and tested. Users could pass a whole dictionary, or array, or float.

[chrisjsewell]

Good point @ubmarco — the variant role's serialisation was indeed undocumented. Fixed in 13b1eea:

Added a Type handling table to the role docs spelling out exactly how each value renders (the role always emits a single text node):

Variant data value	Rendered text
String "arm"	arm
Integer / float 2, 1.5	str() form → 2, 1.5
Boolean True	True / False
Array ["arm", "x86"]	comma-separated → arm, x86
Mapping {...}	not allowed — warning + empty text (reference a leaf instead)

Also added a float test case alongside the existing string/int/bool/array/mapping ones in tests/test_variant_role.py.

For the <{ ... }> field syntax: type behaviour there is already both documented and tested, though it's spread across two places, which is probably why it was hard to find:

• Docs — dynamic_functions.rst "Rules for variant data references": resolved values are type-validated against the field schema, non-string scalars keep their native type, and a path that doesn't resolve to a leaf (i.e. a mapping) warns.
• Tests — doc_variant_data_fields covers the positive cases (string, embedded-string, integer, array, links), and doc_variant_data_field_errors covers the mismatched-type and mapping cases.

Note the key semantic difference: the field syntax preserves native types (an int stays an int in needs.json), whereas the role always renders to inline text — hence the separate serialisation rules. Happy to consolidate/expand the field docs further if you think they're still too buried.

---

Generated by Claude Code