🐛 Register sphinx-needs fields with a typed schema

[ubmarco]

Problem

sphinx-codelinks registers its fields (project, file, directory, and the
local/remote url fields) via add_extra_option(app, name) without a schema.
Untyped fields default to "" on every need, so when a project enables strict
sphinx-needs schema validation (unevaluatedProperties: false) those empty
fields are reported as unexpected on needs that never set them:

Need 'REQ_1' has schema violations:
  Unevaluated properties are not allowed ('directory', 'file', 'project' were unexpected)

This is the codelinks-side counterpart of the same bug found and fixed in
sphinx-test-reports.

Fix

Register each field with a typed (string) schema so an unset value defaults to
None. sphinx-needs strips None from the "reduced need" before schema
validation, so a strict unevaluatedProperties: false schema no longer flags a
field a need never set (an untyped field stays "", which is not stripped).

Registration adapts to the installed sphinx-needs by capability detection
(no packaging dependency, matching sphinx-test-reports):

• add_field(name, description, schema=...) — the modern API, used wherever it
  is importable.
• otherwise add_extra_option(app, name, schema=...) when add_extra_option
  accepts a schema keyword (sphinx-needs >= 6).
• otherwise add_extra_option(app, name) for sphinx-needs 5, which has no schema
  validation, so the empty-default bug cannot arise there anyway.

The schema capability is detected with
"schema" in signature(add_extra_option).parameters.

Test

Adds tests/doc_test/schema_strictness/ + tests/test_schema_validation.py: a
strict-schema project with a plain need, asserting no schema violation.
Confirmed red before the fix ('directory', 'file', 'project' were unexpected) and green after. The test is skipped when add_extra_option
has no schema keyword (sphinx-needs < 6).

Local checks: ruff + mypy clean; regression test green on sphinx-needs 6/7/8
and correctly skipped on 5; existing src-trace doctree snapshots unchanged.

[codecov-commenter]

Codecov Report

❌ Patch coverage is 66.66667% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.74%. Comparing base (32156a7) to head (884af80).

Files with missing lines	Patch %	Lines
tests/test_schema_validation.py	55.00%	9 Missing ⚠️
...phinx_codelinks/sphinx_extension/source_tracing.py	77.77%	2 Missing and 2 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main      #80      +/-   ##
==========================================
- Coverage   91.04%   90.74%   -0.30%     
==========================================
  Files          31       32       +1     
  Lines        2847     2875      +28     
  Branches      306      307       +1     
==========================================
+ Hits         2592     2609      +17     
- Misses        156      166      +10     
- Partials       99      100       +1     

☔ 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.

[osynge]

What's correct

• Three-tier dispatch — add_field (≥ 8) → add_extra_option(app, name, schema=schema) (6/7) → add_extra_option(app, name) (5) is exactly right. The add_field API signature (name, description, *, schema=..., no app) matches sphinx-needs
  8.1 docs.
• {"type": "string"} schema — standard JSON Schema; sphinx-needs correctly defaults typed fields to None, which gets stripped before schema validation. That's the entire mechanism.
• _USE_FIELD_SCHEMA at module load — safe; sphinx-needs version doesn't change at runtime.
• Conditional URL fields — correctly still conditional; only registered when enabled.
• skipif SN_SUPPORTS_SCHEMAS — correct skip condition; < 6 has no schema validation so the bug can't manifest there anyway.
• Test fixtures — the shutil.copytree + make_app(freshenv=True) pattern matches existing tests in the suite.

Issues

1. packaging not declared as a direct dep — moderate

  # source_tracing.py line 8
  from packaging.version import Version

packaging was not in the original imports and is not listed in pyproject.toml dependencies. It's always present transitively (sphinx requires it), but undeclared direct deps are a maintenance hazard. Add "packaging" to dependencies in
pyproject.toml, or use importlib.metadata.version("sphinx-needs") + a string comparison instead.

2. Wrong type annotation on tmpdir — minor

  def test_strict_schema_ignores_unset_codelinks_fields(
      tmpdir: Path,          # ← wrong: pytest's tmpdir is py.path.local
      make_app: Callable[..., SphinxTestApp],

tmpdir is py.path.local, not pathlib.Path. Works at runtime (py.path.local supports truediv and fspath), but mypy will flag it. The modern replacement is tmp_path: Path, which the existing tests in this repo already use. Switch
to tmp_path.

3. KeyError risk on the assertion — minor

  # line 62
  assert report["validation_warnings"] == {}, ...

If sphinx_needs ever renames or omits this key, this raises KeyError instead of a clean assertion failure. Use report.get("validation_warnings", {}) == {}.

4. Weak "validation actually ran" guard — minor

  assert report.get("validated_needs_count", 0) >= 1

The .get(..., 0) fallback means if sphinx-needs renames validated_needs_count, this assertion trivially passes with 0 >= 1 → False... wait, 0 >= 1 is False, so the test would fail but with a confusing message. Actually more concerning:
if the key simply isn't present, the .get returns 0, and 0 >= 1 is False — test fails with a non-obvious message about count rather than about the actual regression. Consider asserting the key's presence explicitly:

  assert "validated_needs_count" in report, "sphinx-needs didn't write schema_violations.json"
  assert report["validated_needs_count"] >= 1

5. PR description drift — cosmetic

The description says: "A capability check ('schema' in signature(add_extra_option)) keeps sphinx-needs 5 working", but the actual implementation uses Version(sphinx_needs.version) >= Version("6.0.0"). The version comparison is
cleaner — just update the description to match.

[osynge]

see comments

[ubmarco]

Addressed in 5d43b9e.

1 — packaging Reverted to a capability probe instead of a version compare, so packaging isn't needed at all (same approach as sphinx-test-reports):

_USE_FIELD_SCHEMA = "schema" in signature(add_extra_option).parameters

Confirmed across the tox matrix: False on needs5, True on needs6/7/8 — and add_extra_option keeps its schema param even on needs8 (where add_field is preferred), so the same probe also gates the test's skip. packaging turned out to be imported in three spots, including a pre-existing dead Version(sphinx) >= "1.6" logging guard in directives/src_trace.py (the else is unreachable under sphinx>=7.4). All removed, so no pyproject.toml dependency change is needed.

2 — tmpdir annotation Now tmp_path: Path, and dropped the then-redundant Path() wrapper.

3 & 4 — .get() on the report assertions Keeping direct subscript access (report["validation_warnings"], report["validated_needs_count"]) on purpose. schema_violations.json and these keys are precisely the sphinx-needs contract this regression test exists to pin down. If a future sphinx-needs renames or drops a key, .get(..., default) silently substitutes the default and the assertion goes green — the test passes having validated nothing, a false negative that hides the very change it should surface. Direct access raises KeyError at the exact key: the loud, actionable failure we want. Defensive .get() is the right call in production code reading optional data; in a test asserting an external contract it's the wrong default. (For #4 this also avoids the confusing 0 >= 1 message.)

5 — PR description Updated to match: capability detection + the three-tier add_field → add_extra_option(schema=…) → add_extra_option dispatch.