diff --git a/CHANGES b/CHANGES index bf755da6..2f377e9c 100644 --- a/CHANGES +++ b/CHANGES @@ -18,6 +18,28 @@ $ uv add gp-sphinx --prerelease allow +### Fixes + +#### `sphinx-autodoc-fastmcp`: No duplicate IDs when a page heading matches a tool slug + +A heading whose slug matched a tool's bare cross-reference alias +produced docutils duplicate-ID warnings and an ambiguous HTML anchor. +Tool cards now expose a single canonical `fastmcp-tool-` anchor; +bare `{ref}` and tool-role links keep resolving and target the +canonical anchor — scrolling the reader to the tool card instead of +the page heading. (#49) + +```text ++------------------------------------------+ +| # Delete buffer (H1) | <- #delete-buffer +| | heading links & old URLs land here +| +------------------------------------+ | +| | delete_buffer [mutating] [tool] | | <- #fastmcp-tool-delete-buffer +| | Delete an MCP-owned buffer. | | tool links land here +| +------------------------------------+ | ++------------------------------------------+ +``` + ## gp-sphinx 0.0.1a26 (2026-05-25) ### Fixes diff --git a/packages/sphinx-autodoc-fastmcp/src/sphinx_autodoc_fastmcp/_directives.py b/packages/sphinx-autodoc-fastmcp/src/sphinx_autodoc_fastmcp/_directives.py index 326a4423..f046104d 100644 --- a/packages/sphinx-autodoc-fastmcp/src/sphinx_autodoc_fastmcp/_directives.py +++ b/packages/sphinx-autodoc-fastmcp/src/sphinx_autodoc_fastmcp/_directives.py @@ -81,14 +81,16 @@ def _register_section_label( def _component_ids(kind: str, name: str) -> tuple[str, list[str]]: - """Derive canonical section id + back-compat aliases for a component. + """Derive canonical section id + back-compat label aliases for a component. Canonical IDs always namespace by kind so a tool ``status`` and a prompt ``status`` cannot collide in ``std.labels``. Tools additionally - keep the bare slug as an alias because their unprefixed IDs were the - public ``{ref}`` shape on ``main`` and live in downstream user docs; - prompts/resources/templates are new in this branch and have no such - history, so they get the canonical ID only. + keep the bare slug as a label alias because their unprefixed names were + the public ``{ref}`` shape on ``main`` and live in downstream user docs; + the alias resolves to the canonical ID and never becomes a physical + HTML id, so it cannot collide with a same-slug page heading. + Prompts/resources/templates have no such history, so they get the + canonical ID only. Examples -------- @@ -109,10 +111,11 @@ def _register_alias_if_free( env: BuildEnvironment, *, alias: str, + target_id: str, display_name: str, kind: str, ) -> bool: - """Register a bare-slug alias in std.labels iff currently unclaimed. + """Register a bare-slug label alias for *target_id* iff unclaimed. Aliases are tool-only by policy (back-compat with v1 ``{ref}`` URLs). Calling this for any other kind is a programming error — raises @@ -120,12 +123,31 @@ def _register_alias_if_free( target, log WARNING and return ``False`` (canonical-only, no silent overwrite). - The alias maps to itself (``std.labels[alias] = (docname, alias, ...)``); - the bare slug is also pushed onto ``section["ids"]`` so the alias - resolves to a real HTML anchor without forcing existing ``:ref:`` - consumers to update href fragments. + The alias is a pure label: it points at the canonical section ID + (``std.labels[alias] = (docname, target_id, ...)``) and never becomes + a physical HTML id of its own, so a same-slug page heading keeps sole + ownership of the bare anchor (#48). Returns True if the alias was registered, False if skipped. + + Examples + -------- + >>> import types + >>> std = types.SimpleNamespace(labels={}, anonlabels={}) + >>> domains = types.SimpleNamespace(standard_domain=std) + >>> env = types.SimpleNamespace(docname="api", domains=domains) + >>> _register_alias_if_free( + ... env, + ... alias="delete-buffer", + ... target_id="fastmcp-tool-delete-buffer", + ... display_name="delete_buffer", + ... kind="tool", + ... ) + True + >>> std.labels["delete-buffer"] + ('api', 'fastmcp-tool-delete-buffer', 'delete_buffer') + >>> std.anonlabels["delete-buffer"] + ('api', 'fastmcp-tool-delete-buffer') """ if kind != "tool": msg = f"alias registration not permitted for kind={kind!r}" @@ -136,7 +158,7 @@ def _register_alias_if_free( if existing is not None: existing_doc = existing[0] existing_id = existing[1] - if (existing_doc, existing_id) != (env.docname, alias): + if (existing_doc, existing_id) != (env.docname, target_id): logger.warning( "sphinx_autodoc_fastmcp: bare alias %r for %s already claimed " "by %s#%s; using canonical id only", @@ -147,8 +169,8 @@ def _register_alias_if_free( ) return False - std.anonlabels[alias] = (env.docname, alias) - std.labels[alias] = (env.docname, alias, display_name) + std.anonlabels[alias] = (env.docname, target_id) + std.labels[alias] = (env.docname, target_id, display_name) return True @@ -157,8 +179,8 @@ class FastMCPToolDirective(SphinxDirective): Supports the standard Sphinx ``:no-index:`` flag (mirrors :func:`autofunction`/:func:`autoclass` semantics): when set, the card - still renders in full but its canonical section ID and bare-slug alias - are not registered in :class:`StandardDomain` ``labels`` / + still renders in full but its canonical section ID and bare-slug label + alias are not registered in :class:`StandardDomain` ``labels`` / ``anonlabels``. Use it when a tool needs to appear visually on more than one page (e.g. a gallery demo + a reference page) — exactly one invocation per tool should omit ``:no-index:`` so cross-references @@ -200,7 +222,6 @@ def _build_tool_section(self, tool: ToolInfo) -> list[nodes.Node]: section = nodes.section() section["ids"].append(section_id) - section["ids"].extend(aliases) section["classes"].extend((_CSS.TOOL_SECTION, API.CARD_SHELL)) if no_index: # Marker consumed by ``register_tool_labels`` in _transforms.py @@ -208,13 +229,19 @@ def _build_tool_section(self, tool: ToolInfo) -> list[nodes.Node]: section["fastmcp_no_index"] = True else: _register_section_label(self.env, section_id, tool.name) - for alias in aliases: - _register_alias_if_free( + # Marker consumed by ``register_tool_labels`` so incremental + # rebuilds restore the same alias labels from the doctree cache. + section["fastmcp_alias_labels"] = [ + alias + for alias in aliases + if _register_alias_if_free( self.env, alias=alias, + target_id=section_id, display_name=tool.name, kind="tool", ) + ] document.note_explicit_target(section) title_node = nodes.title("", "") @@ -401,7 +428,7 @@ def run(self) -> list[nodes.Node]: for tool in sorted(tier_tools, key=lambda x: x.name): first_line = first_paragraph(tool.docstring) ref = nodes.reference("", "", internal=True) - ref["refuri"] = f"{tool.area}/#{tool.name.replace('_', '-')}" + ref["refuri"] = f"{tool.area}/#{_component_ids('tool', tool.name)[0]}" ref += nodes.literal("", tool.name) rows.append( [ diff --git a/packages/sphinx-autodoc-fastmcp/src/sphinx_autodoc_fastmcp/_transforms.py b/packages/sphinx-autodoc-fastmcp/src/sphinx_autodoc_fastmcp/_transforms.py index b484146e..f7e984af 100644 --- a/packages/sphinx-autodoc-fastmcp/src/sphinx_autodoc_fastmcp/_transforms.py +++ b/packages/sphinx-autodoc-fastmcp/src/sphinx_autodoc_fastmcp/_transforms.py @@ -68,11 +68,12 @@ def collect_tool_section_content(app: Sphinx, doctree: nodes.document) -> None: def register_tool_labels(app: Sphinx, doctree: nodes.document) -> None: """Mirror autosectionlabel for fastmcp card sections (``{ref}````). - Re-registers labels for every id on each card section (canonical AND - any back-compat aliases), so incremental Sphinx rebuilds — which - purge labels when a doc changes — restore both shapes from the - doctree cache without re-running the directive's parse-time - registration. + Re-registers labels for every id on each card section, plus the + back-compat bare-slug aliases stored in the section's + ``fastmcp_alias_labels`` attribute (pointing at the canonical id), + so incremental Sphinx rebuilds — which purge labels when a doc + changes — restore both shapes from the doctree cache without + re-running the directive's parse-time registration. """ domain = app.env.domains.standard_domain docname = app.env.docname @@ -97,6 +98,11 @@ def register_tool_labels(app: Sphinx, doctree: nodes.document) -> None: for section_id in section["ids"]: domain.anonlabels[section_id] = (docname, section_id) domain.labels[section_id] = (docname, section_id, tool_name) + canonical_id = section["ids"][0] + aliases: list[str] = section.get("fastmcp_alias_labels", []) + for alias in aliases: + domain.anonlabels[alias] = (docname, canonical_id) + domain.labels[alias] = (docname, canonical_id, tool_name) def add_section_badges( diff --git a/tests/ext/fastmcp/test_fastmcp_integration.py b/tests/ext/fastmcp/test_fastmcp_integration.py index be45f2ee..0c52b396 100644 --- a/tests/ext/fastmcp/test_fastmcp_integration.py +++ b/tests/ext/fastmcp/test_fastmcp_integration.py @@ -3,6 +3,7 @@ from __future__ import annotations import textwrap +import typing as t import pytest @@ -119,7 +120,195 @@ def test_fastmcp_tool_cards_use_shared_layout( in html ) assert 'class="headerlink gp-sphinx-api-link"' in html - assert 'class="reference internal" href="#list-sessions"' in html + assert 'class="reference internal" href="#fastmcp-tool-list-sessions"' in html assert "Parameters" in html assert "readonly" in html assert "tool" in html + + +_COLLISION_MODULE_SOURCE = textwrap.dedent( + """\ + from __future__ import annotations + + import types + + + def delete_buffer(name: str) -> str: + \"\"\"Delete one buffer. + + Parameters + ---------- + name : str + Buffer name. + \"\"\" + + return "" + + + delete_buffer.__fastmcp__ = types.SimpleNamespace( + name="delete_buffer", + title="Delete buffer", + tags={"destructive"}, + annotations=None, + ) + """ +) + +_COLLISION_CONF_PY = textwrap.dedent( + """\ + from __future__ import annotations + + import sys + + sys.path.insert(0, r"__SCENARIO_SRCDIR__") + + extensions = [ + "sphinx_autodoc_fastmcp", + ] + + fastmcp_tool_modules = ["buffer_tools"] + fastmcp_area_map = {"buffer_tools": "api"} + fastmcp_collector_mode = "introspect" + """ +) + +# The page heading "Delete buffer" slugs to ``delete-buffer`` — the same +# bare alias the tool card claims for the ``delete_buffer`` tool. +_COLLISION_INDEX_RST = textwrap.dedent( + """\ + Delete buffer + ============= + + Use :toolref:`delete_buffer` for an inline link, or + :ref:`delete-buffer` for a bare label reference. + + .. fastmcp-tool:: buffer_tools.delete_buffer + + .. fastmcp-tool-summary:: + """ +) + + +@pytest.fixture(scope="module") +def fastmcp_heading_collision_result( + tmp_path_factory: pytest.TempPathFactory, +) -> SharedSphinxResult: + """Build a page whose heading slug matches a tool's bare alias.""" + cache_root = tmp_path_factory.mktemp("fastmcp-heading-collision") + scenario = SphinxScenario( + files=( + ScenarioFile("buffer_tools.py", _COLLISION_MODULE_SOURCE), + ScenarioFile( + "conf.py", + _COLLISION_CONF_PY.replace( + "__SCENARIO_SRCDIR__", SCENARIO_SRCDIR_TOKEN + ), + substitute_srcdir=True, + ), + ScenarioFile("index.rst", _COLLISION_INDEX_RST), + ), + ) + return build_shared_sphinx_result( + cache_root, + scenario, + purge_modules=("buffer_tools",), + ) + + +class CollisionRenderFixture(t.NamedTuple): + """HTML fragment that must render when a heading shares the tool's slug.""" + + test_id: str + needle: str + + +_COLLISION_RENDER_FIXTURES: list[CollisionRenderFixture] = [ + CollisionRenderFixture( + test_id="canonical-card-id", + needle='id="fastmcp-tool-delete-buffer"', + ), + CollisionRenderFixture( + test_id="tool-section-classes", + needle='class="gp-sphinx-fastmcp__tool-section gp-sphinx-api-card-shell"', + ), + CollisionRenderFixture( + test_id="badge-container", + needle='class="gp-sphinx-api-badge-container"', + ), +] + + +@pytest.mark.integration +@pytest.mark.parametrize( + list(CollisionRenderFixture._fields), + _COLLISION_RENDER_FIXTURES, + ids=[f.test_id for f in _COLLISION_RENDER_FIXTURES], +) +def test_heading_collision_card_renders( + fastmcp_heading_collision_result: SharedSphinxResult, + test_id: str, + needle: str, +) -> None: + """Card markup survives a heading/alias slug collision.""" + html = read_output(fastmcp_heading_collision_result, "index.html") + assert needle in html + + +@pytest.mark.integration +def test_heading_collision_emits_no_duplicate_id_diagnostic( + fastmcp_heading_collision_result: SharedSphinxResult, +) -> None: + """A same-slug heading + tool card produces no duplicate IDs (#48).""" + assert "Duplicate ID" not in fastmcp_heading_collision_result.warnings + + +class CollisionAnchorFixture(t.NamedTuple): + """Expected occurrence count for an anchor fragment under a slug collision.""" + + test_id: str + needle: str + expected_count: int + + +_COLLISION_ANCHOR_FIXTURES: list[CollisionAnchorFixture] = [ + CollisionAnchorFixture( + test_id="bare-id-owned-by-heading-only", + needle='id="delete-buffer"', + expected_count=1, + ), + # The toolref link wraps the tool name in ; the bare {ref} + # link wraps the label title in — the + # trailing tag disambiguates the two resolution paths. + CollisionAnchorFixture( + test_id="toolref-targets-canonical-anchor", + needle='class="reference internal" href="#fastmcp-tool-delete-buffer"> None: + """The heading owns the bare anchor; tool links target the canonical id (#48).""" + html = read_output(fastmcp_heading_collision_result, "index.html") + assert html.count(needle) == expected_count