Skip to content

🐛 Accurate directive line mapping and source-attributed nested renders#1175

Open
chrisjsewell wants to merge 7 commits into
masterfrom
claude/opus-agents-review-workflow-ww8sig
Open

🐛 Accurate directive line mapping and source-attributed nested renders#1175
chrisjsewell wants to merge 7 commits into
masterfrom
claude/opus-agents-review-workflow-ww8sig

Conversation

@chrisjsewell

@chrisjsewell chrisjsewell commented Jul 20, 2026

Copy link
Copy Markdown
Member

Closes three directive-API fidelity gaps characterised while making sphinx-jinja2's warnings line-accurate (myst-parser 4.0.1/5.1.0, re-verified against master post-#1164).

content_offset is now document-relative (rST convention)

Directives run under MyST received content_offset = body_offset (directive-relative), so the conventional content_offset + N idiom reported lines near the top of the file wherever the fence sat. Directives now receive the 0-based document line of their first content line, and self.content.items carry per-line absolute lines — both exactly what docutils' rST parser provides. Coupled with this, MockState.nested_parse treats input_offset as an absolute document offset. For the standard nested_parse(self.content, self.content_offset, node) idiom the rendered position is unchanged (the two old offsets summed to the same number), so no rendering fixture changes; the external contract is what's fixed.

Free corrections that fell out: {epigraph} attribution .line (was one too small), {parsed-literal} .line (was hardcoded 1), and myst's own _docs.py directives.

Source-attributed nested renders (supported API)

DocutilsRenderer.nested_render_text(text, lineno, source=...) attributes rendered nodes and warnings (sphinx logger + docutils reporter, symmetrically) to a caller-specified path, exception-safe and re-entrant. Since #1114 switched warning locations to env.docname, the document/reporter override dance was inert under sphinx — {include} itself was affected: warnings inside included markdown were logged against the including document, one line off. Includes now cite the included file at the true line (the #1164 "tracked separately" off-by-one), and :start-after: no longer derives line numbers from a character index (previously past-EOF lines like inc.md:19 in a 5-line file).

Supported insertion API

  • MockStateMachine.insert_input(input_lines, source=None) — docutils-compatible signature; parsed as MyST (documented caveat), rendered at the directive's position, attributed to source:1..n.
  • MockState.renderer / MockStateMachine.renderer properties bless the access path.
  • New docs/develop/extending.md documents the API with an explicit stability promise, lineno line-space semantics, eval-rst semantics (already document-absolute; now pinned by test), and limitations (deferred document-level warnings; inline tokens).

Behaviour notes for extension authors (also in changelog)

  • nested_parse with literal relative offsets (e.g. autodoc/autosummary's offset=0) now places content at the document top, as under rST.
  • self.lineno + self.content_offset arithmetic now double-counts — as it always did under rST (sphinx-design's badges/buttons hit this latent pattern).
  • Duplicate-reference-definition warnings still attribute to the containing document (deferred post-parse; documented limitation).

Verification

  • 1245 → 1285 passed, 11 skipped; ruff/format/mypy clean; docs build clean.
  • The originating scenario end-to-end: a fence at line 9 mapping an error on content line 2 via content_offset + 2 now warns at index.md:11 (was index.md:3).
  • Master-vs-branch doctrees byte-identical on real docs; only warning-location outputs changed, each hand-verified against true source lines; parse-time delta within noise.
  • Adversarially reviewed in two rounds (edge-case probes incl. nested/list/blockquote/eval-rst fences, include chains and slicing options, mutation testing of the new tests, downstream sweep: myst-nb unaffected, sphinx-design standard idioms invariant).

Downstream (sphinx-jinja2): the isinstance(state_machine, StateMachine) gate can become a version check, the override dance collapses onto nested_render_text(..., source=template_path), and its test_myst_render_error_location can assert line-accurate expectations.

claude added 7 commits July 19, 2026 23:06
Directives rendered by MyST now receive a document-relative `content_offset`
(the 0-based absolute line of the first content line, == 1-based first
content line minus 1), and `content.items` carrying each line's 0-based
absolute document line -- exactly the convention docutils' own rST parser
provides. Previously `content_offset` was the body-relative `body_offset`
(0, 1, 2... from the fence) and items were relative 0, 1, 2..., so
extensions computing `content_offset + N` (e.g. sphinx-jinja2)
mis-attributed reporter warnings.

This is coupled with `MockState.nested_parse`, whose `input_offset` is now
an absolute document line (dropping the `self._lineno +` shift). For the
standard `nested_parse(self.content, self.content_offset, node)` idiom the
rendered line is `position + body_offset` either way, so every nested node
line is byte-identical -- no rendering fixtures change.

Free line-attribution fixes made correct while here:
- `{epigraph}` attribution `.line` was one too small (latent off-by-one);
  it now equals the attribution's true document line, matching docutils'
  `parse_attribution` (`lineno = 1 + line_offset`).
- `{parsed-literal}` `.line` (docutils sets `content_offset + 1`) is now the
  true absolute content line instead of a hardcoded 1.

Internal docs directives (`_docs.py`) that passed a literal `0` to
`nested_parse` now pass `self.content_offset`, preserving their
render-at-the-directive placement under the new absolute semantics.

Behaviour note for extension authors: `MockState.nested_parse` now
interprets `input_offset` as an absolute document line
(docutils-consistent); callers that passed a literal directive-relative
offset should pass an absolute one (the common `self.content_offset` idiom
is unaffected). `DirectiveParsingResult.body_offset` docstring clarified
(relative to the line after the opening line; `0` = next line, `-1` =
merged); its semantics are unchanged.

Tests: new `tests/test_directive_line_mapping.py` pins the contract across
fence shapes (bare, merged, options, YAML, nested, list-embedded, colon,
argument) at multiple document positions, against docutils rST ground
truth, plus the eval-rst pin, the epigraph/parsed-literal fixes,
nested-line invariance, the `reporter.warning(line=content_offset + N)`
idiom, and a `{role}` smoke test. The existing
`test_directive_block_text_rst_parity` asserted the old body-relative
`content_offset` (3); updated to the document-relative value (4) --
`body line` is on document line 5.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CBANSW6TYQxwWRqGhxhdHw
Add a supported mechanism for extensions to render generated content (an
included file, a template's output, ...) at the current document position,
attributing both the rendered nodes and any warnings to a caller-specified
source path.

`DocutilsRenderer.nested_render_text` gains a `source` keyword.  While it is
set, the document/reporter are pointed at that path (so node `source` stamping
and docutils reporter warnings attribute there) and the path is pushed onto a
new renderer attribution stack consulted by `create_warning` (so the sphinx
logger attributes there too).  The override is restored on exit -- including
after an exception -- and nests re-entrantly, so a source render may itself
contain further source renders (e.g. nested includes).  The method is now
documented as supported API, with its `lineno` line-space semantics and the
MyST-not-rST parsing caveat spelled out.

`warnings_.create_warning` gains a matching `source` keyword:

- sphinx branch: an explicit `node` still wins; otherwise `source` produces a
  location *string* `"source:line"` (or `"source:"` when there is no line).
  The colon is required -- sphinx passes a colon-containing string through
  verbatim, whereas a colon-less string (or a `(source, line)` tuple) is run
  through `env.doc2path`, which would mangle an explicit source path.  The
  default `(env.docname, line)` path is byte-identical to before.
- docutils branch: `source` (with `line`) is passed straight to
  `reporter.warning`, which docutils honours as an explicit override.

The two modes are therefore symmetric: a warning inside generated content is
reported against the generating source in both.

Consumers wired in-repo:

- `MockIncludeDirective.run` now delegates the document/reporter dance to
  `nested_render_text(..., source=str(path))`, and renders at `startline`
  rather than `startline + 1`.  This fixes the include line off-by-one: a
  warning on line 3 of an included file was reported at line 4.  In sphinx mode
  it was additionally attributed to the *parent* document (a side effect of the
  logger switching to `env.docname`); it now cites the included file.  The
  relative-images/relative-docs md_env handling stays in the directive, as it
  resolves against the containing document's directory.
- `MockStateMachine.insert_input(input_lines, source=None)` renders generated
  MyST at the directive's position with a docutils-compatible signature; with
  `source` it attributes to `source:1..n`, otherwise it renders in the
  document's line-space.  Content is parsed as MyST (not rST) and rendered
  immediately into the current node (before any nodes the directive returns --
  identical to docutils for the common `return []` pattern).
- `MockState.renderer` / `MockStateMachine.renderer` read-only properties bless
  the existing `_renderer` as the supported way for extensions to reach these
  APIs from a directive.

Known limitation (documented): deferred document-level warnings (e.g. duplicate
reference definitions, emitted in `_render_finalise`) escape a render-scoped
override and attribute to the containing document; inline tokens carry no
per-line maps.

Also fixes a stale docs URL in `MockState.__getattr__` (kept with the code).

Tests: new `tests/test_renderers/test_nested_render.py` pins include
attribution (sphinx via `sphinx_doctree`, docutils via `publish_doctree` +
`tmp_path`), the source override in both modes (warnings + node `.source`
stamping), `insert_input` (ordering, source attribution, document-space
fallback, and the `renderer` properties), and restoration (nested
override-in-override attributing innermost then restoring outward, restoration
after a directive raises, and restoration when the render itself raises
mid-way).  It lives under `tests/test_renderers/` so its sphinx builds run
after the docutils role fixtures (the same ordering the existing
`test_fixtures_docutils`/`test_fixtures_sphinx` split relies on).

Fixture regenerated for the off-by-one fix (hand-verified against the source):
- `tests/test_renderers/fixtures/mock_include_errors.md`: the unknown-role
  warning for the single-line `bad.md` moves from `bad.md:2` to `bad.md:1`
  (its true line); the `bad.md` attribution itself is unchanged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CBANSW6TYQxwWRqGhxhdHw
Add `docs/develop/extending.md` (in the developer toctree after
`architecture.md`), documenting the supported API for rendering generated
content from a directive:

- reaching the renderer via `self.state.renderer` /
  `self.state_machine.renderer`;
- `nested_render_text` (its `lineno` line-space semantics, with a worked
  example, and `source=` attribution);
- `insert_input` (the docutils-compatible signature, the MyST-not-rST caveat,
  and the immediate-render ordering note);
- what `content_offset` now means (document-relative; the `content_offset + N`
  idiom reports true document lines);
- documented limitations (deferred document-level warnings attribute to the
  containing document; inline tokens carry no per-line maps).

The page is narrative and cross-references the API objects with `{py:meth}`/
`{py:obj}`/`{py:func}` roles; autodoc2 renders the docstrings, which are the
source of truth.

Add an `## Unreleased` CHANGELOG section (✨ New Features / 🐛 Bug Fixes /
📚 Documentation) covering the document-relative `content_offset` fix (with the
epigraph/parsed-literal corrections and the absolute `nested_parse` offset
note), the include attribution + off-by-one fix, the `source=` nested-render
API, `insert_input`, the `renderer` properties, and this page.

The stale `MockState` docs URL fix ships with the code in the preceding commit.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CBANSW6TYQxwWRqGhxhdHw
In `MockIncludeDirective.run`, the `:start-after:` handling advanced the
included file's `startline` by `split_index + len(split_on)` -- a *character*
index into the file content -- rather than a line count.  Because `startline`
is then used as the 0-based line shift for `nested_render_text`, a warning
inside the retained segment was reported far past the end of the file (e.g. a
directive truly on line 4 of a 5-line included file warned at `inc.md:19`).

Advance by the number of newlines in the consumed prefix instead:

    startline += file_content.count("\n", 0, split_index + len(split_on))

The remaining text's first (possibly partial) line still lies ON the marker's
line, so counting the consumed newlines makes `nested_render_text` report the
TRUE file lines -- consistent with the `:start-line:` path.  This deliberately
differs from docutils, whose `:start-after:` reports lines relative to the
retained segment; MyST reports true file lines.  `:end-before:` only truncates
`file_content` and does not touch `startline`, so it is unaffected.

Tests (`tests/test_renderers/test_nested_render.py`): a `:start-after:` include
whose warning-triggering line sits at a known true file line asserts the true
line is reported and the old character-index-derived line is absent, plus a
combined `:start-after:` + `:end-before:` case.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CBANSW6TYQxwWRqGhxhdHw
Follow-ups from the review of the nested-render/attribution work.  No runtime
behaviour changes -- tests, docs, docstrings and a changelog note only.

- Tests: make `create_warning`'s `source` kwarg standalone-load-bearing.  The
  existing extension-style tests always run with the renderer's ambient reporter
  override active, which masks the docutils-branch `source` kwarg (dropping it
  survives the suite).  Add `test_create_warning_source_docutils_arm` (calls
  `create_warning` directly on an override-free `new_document`, asserting the
  `source`/`source:line` attribution and that the document's own source does not
  leak), and `test_create_warning_source_sphinx_arm_no_line` (sphinx build,
  `line=None` -> logged `source:` trailing-colon form, not doc2path-mangled).
- Tests: parametrize `test_insert_input_renders_at_position` over `list[str]`
  and a docutils `StringList` input, pinning that `insert_input` accepts both.
- Tests: `test_downstream_reporter_warning_line` now also asserts the old,
  body-relative `<string>:{body_offset + 1}:` location is absent (guarded to
  skip if it would coincide with the true line -- it does not for these cases).
- Changelog: add a migration note for extension authors under the
  `content_offset` bullet -- `MockState.nested_parse`'s `input_offset` is now an
  absolute document offset; the standard idiom is unchanged, but literal
  relative offsets (e.g. `0`) now place docutils-identically (autodoc/autosummary
  `offset=0` renders at the document top), and `self.lineno + self.content_offset`
  now double-counts (as it always did under reStructuredText).
- Docs/docstring: `insert_input` -- state plainly that docutils requires `source`
  positionally while the mock makes it optional; that inserted content lands
  before the directive's returned nodes (docutils splices after; identical only
  for `return []`); and that a `StringList`'s per-line `(source, offset)` items
  are not preserved (joined and attributed uniformly to `source`).
- Docs: fix the `{eval-rst}` cross-reference to the `#`-prefixed label form used
  by sibling docs, and note in the limitations that `document["source"]`/the
  reporter are repointed at the override during a source-attributed render
  (extensions resolving paths against `document["source"]` mid-render see it --
  the same behaviour `{include}` has always had).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CBANSW6TYQxwWRqGhxhdHw
- docs/develop/extending.md: add a "Stability" subsection promising the page's
  APIs (`nested_render_text`/`source`, `insert_input`, the `renderer` properties,
  the document-relative `content_offset` contract) are supported public API.
- docs/develop/extending.md: document `{eval-rst}` semantics (real docutils state
  machine; document-absolute `lineno`/`content_offset`; `content_offset + N`
  works identically), pinned by `test_eval_rst_content_offset_pinned`.
- CHANGELOG.md (Unreleased): tighten to house-bullet style keeping every fact,
  and correct the autodoc/autosummary offset note (rST-consistent offset value,
  not byte-identical `StringList` attribution).
- Code polish: `_attribute_to_source` guards empty-string source (`if not
  source`); `create_warning` docstring now documents all parameters.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CBANSW6TYQxwWRqGhxhdHw
The assertion in test_include_attribution_sphinx compared against a
forward-slash <src>/inc.md path, but on Windows the include path is
joined with a backslash (<src>\inc.md), failing both windows-latest CI
jobs. The CI logs show the feature itself behaves correctly on Windows
(true file, true line); only the assertion was separator-naive.
Normalise backslashes in the captured warnings before asserting.

The other sphinx-mode tests assert verbatim caller-supplied paths and
the docutils-mode tests use prefix-independent substring matches, so
this was the only separator-sensitive assertion (all other tests passed
on windows-latest).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CBANSW6TYQxwWRqGhxhdHw
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.

2 participants