Skip to content

👌 Add needs_json_include_link_conditions config option#1681

Merged
chrisjsewell merged 6 commits into
masterfrom
copilot/allow-conditional-output-in-needs-json
Mar 17, 2026
Merged

👌 Add needs_json_include_link_conditions config option#1681
chrisjsewell merged 6 commits into
masterfrom
copilot/allow-conditional-output-in-needs-json

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 17, 2026

Following e22e31d (parse link conditions from imported/external needs), the needs.json output now includes link conditions by default (e.g. NEED-1[status=="open"]). A new config option controls this behavior.

Changes

  • config.py: New json_include_link_conditions: bool = True field on NeedsSphinxConfig
  • needsfile.py: NeedsList.add_need() uses to_link_string() (with conditions) instead of to_filter_string() (ID only) when enabled, for outgoing link fields only (backlinks never carry conditions)
  • docs/configuration.rst: Documents the new option, noting it only affects outgoing link fields
  • tests/test_link_conditions.py: Tests for both True (default) and False behavior using syrupy snapshot testing with the html builder

Usage

# conf.py — default: conditions included in needs.json
needs_json_include_link_conditions = True   # "NEED-1[status==\"open\"]"
needs_json_include_link_conditions = False  # "NEED-1"

This option only affects outgoing link fields. Backlink fields (e.g. links_back) never carry conditions, so they are unaffected by this setting.

Original prompt

Following on from commit e22e31d, allow for the output needs.json to be written with/without conditions on the links. Have this controllable by a new configuration, and have it set the "with" by default


📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.

Copilot AI and others added 2 commits March 17, 2026 16:45
Add a new configuration option `needs_json_include_link_conditions` (default True)
that controls whether link conditions are included in the needs.json output.
When True, links include conditions (e.g. 'NEED-1[status=="open"]').
When False, only the target ID is written (e.g. 'NEED-1').

Co-authored-by: chrisjsewell <2997570+chrisjsewell@users.noreply.github.com>
Co-authored-by: chrisjsewell <2997570+chrisjsewell@users.noreply.github.com>
Copilot AI changed the title [WIP] Add configuration for conditional output in needs.json Add needs_json_include_link_conditions config option Mar 17, 2026
Copilot AI requested a review from chrisjsewell March 17, 2026 16:48
Comment thread docs/configuration.rst Outdated
Comment thread docs/configuration.rst Outdated
Comment thread tests/test_link_conditions.py Outdated
Comment thread tests/test_link_conditions.py Outdated
Comment thread tests/test_link_conditions.py Outdated
…rge tests

- Change versionadded from 5.2.0 to 8.0.0
- Remove unnecessary example code block from docs
- Merge HTML warning test and JSON test into single test using needs builder
- Use syrupy snapshot testing for JSON output instead of individual asserts

Co-authored-by: chrisjsewell <2997570+chrisjsewell@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 17, 2026

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • releases.astral.sh
    • Triggering command: /opt/hostedtoolcache/Python/3.12.13/x64/bin/uv /opt/hostedtoolcache/Python/3.12.13/x64/bin/uv venv -p cpython3.10 --allow-existing --prompt=sphinx-needs[py310] --python-preference system /home/REDACTED/work/sphinx-needs/sphinx-needs/.tox/py310 (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

Comment thread tests/test_link_conditions.py Outdated
[
{
"buildername": "html",
"buildername": "needs",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move back to html builder, but also add to the conf.py:

needs_build_json = True
needs_json_remove_defaults = True

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in 9e2ec20 — switched both tests to html builder and added needs_build_json = True + needs_json_remove_defaults = True to the test conf.py. Snapshots are now much more concise.

…ove_defaults

Use html builder instead of needs builder, with needs_build_json = True and
needs_json_remove_defaults = True in conf.py, producing more concise snapshots.

Co-authored-by: chrisjsewell <2997570+chrisjsewell@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 17, 2026

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • releases.astral.sh
    • Triggering command: /opt/hostedtoolcache/Python/3.12.13/x64/bin/uv /opt/hostedtoolcache/Python/3.12.13/x64/bin/uv venv -p cpython3.10 --allow-existing --prompt=sphinx-needs[py310] --python-preference system /home/REDACTED/work/sphinx-needs/sphinx-needs/.tox/py310 (dns block)

If you need me to access, download, or install something from one of these locations, you can either:

@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 17, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.03%. Comparing base (4e10030) to head (91cfc81).
⚠️ Report is 269 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1681      +/-   ##
==========================================
+ Coverage   86.87%   89.03%   +2.15%     
==========================================
  Files          56       71      +15     
  Lines        6532    10163    +3631     
==========================================
+ Hits         5675     9049    +3374     
- Misses        857     1114     +257     
Flag Coverage Δ
pytests 89.03% <100.00%> (+2.15%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 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.

Copy link
Copy Markdown
Member

@chrisjsewell chrisjsewell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remaining changes requested

1. sphinx_needs/needsfile.py — Remove redundant backlinks override (lines 169–173)

The backlinks block is unnecessary. Backlinks are always created as NeedLink(id=..., condition=None) during resolve_links(), so to_link_string() on a backlink with no condition produces the same output as the default to_filter_string() (i.e. just the bare ID). This means lines 169–173 are dead code that adds overhead without changing the output.

Remove these lines:

            # Overwrite backlink fields similarly
            for key, blinks in need_info.iter_backlinks_items(as_str=False):
                bkey = f"{key}_back"
                if bkey not in self._exclude_need_keys:
                    writable_needs[bkey] = [li.to_link_string() for li in blinks]

So the if self.needs_config.json_include_link_conditions: block becomes just:

        if self.needs_config.json_include_link_conditions:
            # Overwrite link fields with serializations that include conditions
            for key, links in need_info.iter_links_items(as_str=False):
                if key not in self._exclude_need_keys:
                    writable_needs[key] = [li.to_link_string() for li in links]

2. docs/configuration.rst — Clarify scope of the option (around line 1939)

Update the description to note this only affects forward/outgoing link fields, since backlinks never carry conditions:

needs_json_include_link_conditions
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

.. versionadded:: 8.0.0

When ``True`` (the default), outgoing link fields in the ``needs.json`` output include
any conditions attached to the link, for example ``NEED-1[status=="open"]``.

When ``False``, only the target need ID is written, for example ``NEED-1``.

This option only affects outgoing link fields. Backlink fields (e.g. ``links_back``)
never carry conditions, so they are unaffected by this setting.

Default: :need_config_default:`json_include_link_conditions`

3. sphinx_needs/config.py — Update the docstring similarly (lines 859–861)

    """If True, include link conditions in outgoing link fields of the JSON needs file output (e.g. 'NEED-1[cond]').
    If False, link conditions are stripped and only the target ID is written (e.g. 'NEED-1').
    Backlink fields are unaffected as they never carry conditions.
    """

Co-authored-by: chrisjsewell <2997570+chrisjsewell@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 17, 2026

Remaining changes requested

...

All three changes applied in 91cfc81:

  1. Removed the redundant backlinks override block in needsfile.py
  2. Updated configuration.rst to clarify the option only affects outgoing link fields
  3. Updated the docstring in config.py similarly

@chrisjsewell chrisjsewell changed the title Add needs_json_include_link_conditions config option 👌 Add needs_json_include_link_conditions config option Mar 17, 2026
@chrisjsewell chrisjsewell marked this pull request as ready for review March 17, 2026 18:09
@chrisjsewell chrisjsewell merged commit 4473aef into master Mar 17, 2026
23 checks passed
@chrisjsewell chrisjsewell deleted the copilot/allow-conditional-output-in-needs-json branch March 17, 2026 18:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants