From 258180fa6f819be4c97e705f6884a389a70f1396 Mon Sep 17 00:00:00 2001 From: Wei Lee Date: Wed, 12 Aug 2026 11:01:02 +0800 Subject: [PATCH 01/19] Link registry modules to the guide section that documents them A module card offers only generated API reference, which lists arguments but never says how the thing is meant to be used. The prose guides say it, and they already mark which class each section is about by titling that section with the class name -- so the pointer exists in the docs and just wasn't being read. Resolving it from the guides rather than from a declared list is deliberate: a hand-maintained class-to-guide table goes stale silently every time a guide is split or renamed, and a link that lands on the wrong section is worse than no link at all. A class documented only in prose gets no link. Both extraction paths resolve it, since a superseded version's page is rendered only from its own per-version metadata. --- dev/registry/extract_parameters.py | 15 ++ dev/registry/extract_versions.py | 38 +++++ dev/registry/registry_contract_models.py | 2 + dev/registry/registry_tools/docs_guides.py | 124 ++++++++++++++ dev/registry/tests/test_docs_guides.py | 155 ++++++++++++++++++ dev/registry/tests/test_extract_parameters.py | 30 +++- dev/registry/tests/test_extract_versions.py | 54 ++++++ .../tests/test_registry_contract_models.py | 11 ++ registry/AGENTS.md | 18 ++ registry/src/css/main.css | 9 + registry/src/provider-version.njk | 6 + 11 files changed, 459 insertions(+), 3 deletions(-) create mode 100644 dev/registry/registry_tools/docs_guides.py create mode 100644 dev/registry/tests/test_docs_guides.py diff --git a/dev/registry/extract_parameters.py b/dev/registry/extract_parameters.py index 7f776ae164599..c98c895bca28d 100644 --- a/dev/registry/extract_parameters.py +++ b/dev/registry/extract_parameters.py @@ -56,6 +56,7 @@ import yaml from extract_metadata import fetch_provider_inventory, read_inventory from registry_contract_models import validate_modules_catalog, validate_provider_parameters +from registry_tools.docs_guides import attach_guide_urls, collect_guide_anchors from registry_tools.types import ( BASE_CLASS_IMPORTS, CLASS_LEVEL_CATEGORY_OVERRIDES, @@ -98,6 +99,7 @@ class Module: provider_name: str supports_durable_execution: bool supports_deferrable: bool + guide_url: str | None = None def get_category(integration_name: str) -> str: @@ -738,6 +740,16 @@ def _resolve_decorated_operator_class(decorator_fn: object) -> type | None: return candidate if inspect.isclass(candidate) else None +def read_guide_docs(docs_dir: Path) -> dict[str, str]: + """Read a provider's authored reST docs from the working tree, keyed by path relative to ``docs_dir``.""" + if not docs_dir.is_dir(): + return {} + return { + path.relative_to(docs_dir).as_posix(): path.read_text(encoding="utf-8") + for path in sorted(docs_dir.rglob("*.rst")) + } + + def discover_classes_from_provider( provider_yaml_path: Path, base_classes: dict[str, type], @@ -985,6 +997,9 @@ def make_entry( } ) + guide_docs = read_guide_docs(provider_yaml_path.parent / "docs") + attach_guide_urls(discovered, collect_guide_anchors(guide_docs), base_docs_url) + return discovered diff --git a/dev/registry/extract_versions.py b/dev/registry/extract_versions.py index d831725f1c31e..77df6a38fa0f1 100644 --- a/dev/registry/extract_versions.py +++ b/dev/registry/extract_versions.py @@ -60,6 +60,7 @@ sys.exit(1) from extract_metadata import fetch_provider_inventory, read_connection_urls, resolve_connection_docs_url +from registry_tools.docs_guides import attach_guide_urls, collect_guide_anchors from registry_tools.types import ( CLASS_LEVEL_CATEGORY_OVERRIDES, CLASS_LEVEL_SECTIONS, @@ -130,6 +131,21 @@ def git_show(tag: str, path: str) -> str | None: return None +def git_ls_tree(tag: str, prefix: str) -> list[str]: + """List the file paths under a prefix at a specific git tag.""" + try: + result = subprocess.run( + ["git", "ls-tree", "-r", "--name-only", tag, "--", prefix], + capture_output=True, + text=True, + cwd=AIRFLOW_ROOT, + check=True, + ) + except subprocess.CalledProcessError: + return [] + return [line for line in result.stdout.splitlines() if line] + + def git_tag_exists(tag: str) -> bool: """Check if a git tag exists locally.""" result = subprocess.run( @@ -182,6 +198,26 @@ def get_source_file_path(layout: str, dir_path: str, module_path: str) -> str: return f"providers/src/{rel_file}" +def read_guide_docs(tag: str, layout: str, dir_path: str) -> dict[str, str]: + """Read a provider's authored reST docs at a tag, keyed by path relative to its docs dir. + + Only the per-provider layout keeps docs beside the provider; under the old flat + layout they lived in a top-level ``docs/`` tree, so those tags get no guide + links rather than links guessed from a path that moved. + """ + if layout != "new": + return {} + + docs_prefix = f"providers/{dir_path}/docs/" + docs: dict[str, str] = {} + for path in git_ls_tree(tag, docs_prefix): + if not path.endswith(".rst"): + continue + if content := git_show(tag, path): + docs[path[len(docs_prefix) :]] = content + return docs + + def parse_pyproject_toml_content(content: str, layout: str) -> dict[str, Any]: """Parse pyproject.toml content for dependencies, requires-python, and extras.""" result: dict[str, Any] = {"requires_python": "", "dependencies": [], "optional_extras": {}} @@ -383,6 +419,8 @@ def process_module(module_path: str, module_type: str, integration: str, categor } ) + attach_guide_urls(modules, collect_guide_anchors(read_guide_docs(tag, layout, dir_path)), base_docs_url) + return modules diff --git a/dev/registry/registry_contract_models.py b/dev/registry/registry_contract_models.py index 31f4001dc4927..d9b351b30f396 100644 --- a/dev/registry/registry_contract_models.py +++ b/dev/registry/registry_contract_models.py @@ -146,6 +146,8 @@ class ModuleContract(BaseModel): provider_name: str | None = None supports_durable_execution: bool = False supports_deferrable: bool = False + # Only set for classes a how-to guide documents in a section of their own. + guide_url: str | None = None class ModulesCatalogContract(BaseModel): diff --git a/dev/registry/registry_tools/docs_guides.py b/dev/registry/registry_tools/docs_guides.py new file mode 100644 index 0000000000000..37ea6b59c829b --- /dev/null +++ b/dev/registry/registry_tools/docs_guides.py @@ -0,0 +1,124 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +"""Map a provider's classes to the how-to guide sections that document them. + +A module's ``docs_url`` points at generated API reference, which tells a reader +what the arguments are but not how the thing is meant to be used. The prose +guides carry that, and they already mark it: a how-to guide documents one class +per section, titled with the class name (``HookToolset``, ``SQLToolset``). + +So the mapping is read back out of the guides rather than curated anywhere: a +hand-maintained class-to-guide table would rot silently every time a guide is +split, renamed, or a class is dropped, and a rotten link is worse than none. +Callers supply the reST they can see (a git tag, or the working tree) and get +back only the anchors those sources actually contain. +""" + +from __future__ import annotations + +import re +from collections.abc import Mapping +from typing import Any + +# reST underlines an (optionally overlined) section title with a run of one +# punctuation character, at least as long as the title itself. +_ADORNMENT_CHARS = "!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~" + +# Only titles opening with a class name as an inline literal are treated as +# documenting it, so prose headings ("Bounded query results") never produce a link. +_LEADING_CLASS_LITERAL = re.compile(r"^``([A-Za-z_][A-Za-z0-9_]*)``") + + +def slugify_section_anchor(title: str) -> str: + """Return the HTML id Sphinx gives a section with this title. + + Mirrors docutils' ``make_id``: lower-case, every run of non-alphanumeric + characters becomes a single hyphen, and leading/trailing hyphens are + dropped -- e.g. the section titled ``HookToolset`` is served at + ``#hooktoolset``. + """ + return re.sub(r"[^a-z0-9]+", "-", title.lower()).strip("-") + + +def _extract_class_name_from_title(title: str) -> str | None: + """Return the class a section title leads with, or None if it names prose. + + A guide marks a section as being *about a class* by opening its title with the + class as an inline literal -- ``HookToolset``, or + ``AgentOperator`` & ``@task.agent`` where one section covers the operator and + its decorator. Requiring that markup is what keeps a single-word prose heading + ("Guidelines") from claiming to document a class of the same name, and it is a + convention the guides already follow rather than one imposed on them. + """ + match = _LEADING_CLASS_LITERAL.match(title) + return match.group(1) if match else None + + +def _is_adornment(line: str) -> bool: + """Whether a line is a reST title overline/underline rather than a title.""" + return bool(line) and len(set(line)) == 1 and line[0] in _ADORNMENT_CHARS + + +def _extract_section_titles(text: str) -> list[str]: + """Return every section title in a reST document, in document order.""" + titles = [] + lines = text.splitlines() + for index, line in enumerate(lines[:-1]): + title = line.strip() + # Guides title these sections with an inline literal (``HookToolset``), + # so a title can legitimately start with an adornment character; only a + # line that is *entirely* one repeated character is an adornment. + if not title or _is_adornment(title): + continue + underline = lines[index + 1].strip() + if len(underline) >= len(title) and _is_adornment(underline): + titles.append(title) + return titles + + +def collect_guide_anchors(docs: Mapping[str, str]) -> dict[str, str]: + """Map class name -> ``.html#`` for every documented class. + + ``docs`` maps a page path relative to the provider's docs directory (e.g. + ``toolsets.rst``) to its reST source. When two pages document the same class + name, the first page in sorted order wins, so a rebuild of the same sources + always produces the same link. + """ + anchors: dict[str, str] = {} + for page in sorted(docs): + page_url = re.sub(r"\.rst$", ".html", page) + for title in _extract_section_titles(docs[page]): + class_name = _extract_class_name_from_title(title) + if not class_name or class_name in anchors: + continue + anchors[class_name] = f"{page_url}#{slugify_section_anchor(title)}" + return anchors + + +def attach_guide_urls(modules: list[dict[str, Any]], anchors: Mapping[str, str], base_docs_url: str) -> int: + """Set ``guide_url`` on every module a guide section documents. + + Mutates ``modules`` in place; returns how many got a link. + """ + attached = 0 + for module in modules: + anchor = anchors.get(module["name"]) + if not anchor: + continue + module["guide_url"] = f"{base_docs_url.rstrip('/')}/{anchor}" + attached += 1 + return attached diff --git a/dev/registry/tests/test_docs_guides.py b/dev/registry/tests/test_docs_guides.py new file mode 100644 index 0000000000000..a8f0d333e5f47 --- /dev/null +++ b/dev/registry/tests/test_docs_guides.py @@ -0,0 +1,155 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +import pytest +from registry_tools.docs_guides import ( + attach_guide_urls, + collect_guide_anchors, + slugify_section_anchor, +) + +TOOLSETS_GUIDE = """ +.. _howto/toolsets: + +Toolsets: Airflow hooks as AI agent tools +========================================== + +Intro prose. + +``HookToolset`` +--------------- + +How to use it. + +Guidelines +^^^^^^^^^^ + +More prose. + +.. _bounded-query-results: + +Bounded query results +^^^^^^^^^^^^^^^^^^^^^ + +``SQLToolset`` bounds that. + +``DataFusionToolset`` +--------------------- + +Another one. +""" + + +@pytest.mark.parametrize( + ("title", "expected"), + [ + # Verified against the published guide: the section titled ``HookToolset`` + # is served at .../toolsets.html#hooktoolset. + ("HookToolset", "hooktoolset"), + ("AgentSkillsToolset", "agentskillstoolset"), + ("Bounded query results", "bounded-query-results"), + ("Agent_Skills", "agent-skills"), + ("Direct PydanticAI MCP toolsets", "direct-pydanticai-mcp-toolsets"), + ("``AgentOperator`` & ``@task.agent``", "agentoperator-task-agent"), + ], +) +def test_slugify_section_anchor_matches_sphinx_ids(title, expected): + assert slugify_section_anchor(title) == expected + + +def test_collect_guide_anchors_finds_class_named_sections_at_any_depth(): + anchors = collect_guide_anchors({"toolsets.rst": TOOLSETS_GUIDE}) + + assert anchors == { + "HookToolset": "toolsets.html#hooktoolset", + "DataFusionToolset": "toolsets.html#datafusiontoolset", + } + + +def test_collect_guide_anchors_ignores_prose_headings(): + anchors = collect_guide_anchors({"toolsets.rst": TOOLSETS_GUIDE}) + + # "Guidelines" is shaped like a class name but isn't marked up as one. + assert "Guidelines" not in anchors + assert "Bounded query results" not in anchors + + +def test_collect_guide_anchors_ignores_classes_only_mentioned_in_prose(): + # SQLToolset appears in the guide's body but has no section of its own, so + # there is no anchor to link to. + assert "SQLToolset" not in collect_guide_anchors({"toolsets.rst": TOOLSETS_GUIDE}) + + +def test_collect_guide_anchors_keeps_nested_page_paths(): + guide = "``AgentOperator``\n-----------------\n\nProse.\n" + + assert collect_guide_anchors({"operators/agent.rst": guide}) == { + "AgentOperator": "operators/agent.html#agentoperator" + } + + +def test_collect_guide_anchors_prefers_first_page_in_sorted_order(): + guide = "``SQLToolset``\n--------------\n\nProse.\n" + + anchors = collect_guide_anchors({"toolsets.rst": guide, "operators/sql.rst": guide}) + + assert anchors["SQLToolset"] == "operators/sql.html#sqltoolset" + + +def test_collect_guide_anchors_handles_a_title_covering_more_than_the_class(): + # Verified against the published guide: this heading is served at + # .../operators/agent.html#agentoperator-task-agent, so the anchor comes from + # the whole title while the class name comes from the leading literal. + guide = "``AgentOperator`` & ``@task.agent``\n===================================\n\nProse.\n" + + assert collect_guide_anchors({"operators/agent.rst": guide}) == { + "AgentOperator": "operators/agent.html#agentoperator-task-agent" + } + + +def test_collect_guide_anchors_requires_a_long_enough_underline(): + # An underline shorter than the title isn't a section in reST, so it must not + # produce a link to an anchor Sphinx never emitted. + assert collect_guide_anchors({"toolsets.rst": "``HookToolset``\n---\n\nProse.\n"}) == {} + + +def test_attach_guide_urls_only_links_documented_classes(): + modules = [ + {"name": "HookToolset", "docs_url": "https://example.test/_api/hook/index.html"}, + {"name": "UndocumentedToolset", "docs_url": "https://example.test/_api/other/index.html"}, + ] + + attached = attach_guide_urls( + modules, + {"HookToolset": "toolsets.html#hooktoolset"}, + "https://airflow.apache.org/docs/apache-airflow-providers-common-ai/0.7.0", + ) + + assert attached == 1 + assert modules[0]["guide_url"] == ( + "https://airflow.apache.org/docs/apache-airflow-providers-common-ai/0.7.0/toolsets.html#hooktoolset" + ) + assert "guide_url" not in modules[1] + + +def test_attach_guide_urls_does_not_double_up_the_base_separator(): + modules = [{"name": "HookToolset"}] + + attach_guide_urls(modules, {"HookToolset": "toolsets.html#hooktoolset"}, "https://example.test/docs/") + + assert modules[0]["guide_url"] == "https://example.test/docs/toolsets.html#hooktoolset" diff --git a/dev/registry/tests/test_extract_parameters.py b/dev/registry/tests/test_extract_parameters.py index eac805cc49a37..3430d963a6727 100644 --- a/dev/registry/tests/test_extract_parameters.py +++ b/dev/registry/tests/test_extract_parameters.py @@ -22,7 +22,7 @@ import builtins import json import types -from dataclasses import fields +from dataclasses import MISSING, fields from unittest.mock import patch import pytest @@ -999,6 +999,26 @@ def test_discovers_operator(self, provider_yaml_path, base_classes): assert operators[0]["import_path"] == "airflow.providers.amazon.aws.operators.s3.FakeOperator" assert operators[0]["provider_id"] == "amazon" + def test_guide_section_becomes_a_guide_url(self, provider_yaml_path, base_classes): + """A class documented by a section of its own gets a link to that section; + one that is only in the API reference keeps just its ``docs_url``.""" + docs_dir = provider_yaml_path.parent / "docs" / "operators" + docs_dir.mkdir(parents=True) + (docs_dir / "s3.rst").write_text("``FakeOperator``\n----------------\n\nProse.\n") + + with ( + patch("extract_parameters.PROVIDERS_DIR", provider_yaml_path.parent.parent), + patch("extract_parameters.importlib.import_module", side_effect=self._mock_import), + ): + result = discover_classes_from_provider(provider_yaml_path, base_classes) + + by_name = {r["name"]: r for r in result} + assert by_name["FakeOperator"]["guide_url"] == ( + "https://airflow.apache.org/docs/apache-airflow-providers-amazon/stable" + "/operators/s3.html#fakeoperator" + ) + assert "guide_url" not in by_name["FakeSensor"] + def test_discovers_sensor(self, provider_yaml_path, base_classes): with ( patch("extract_parameters.PROVIDERS_DIR", provider_yaml_path.parent.parent), @@ -1093,14 +1113,18 @@ def test_extracts_short_description(self, provider_yaml_path, base_classes): assert operators[0]["short_description"] == "Copy objects in S3." def test_all_module_fields_present(self, provider_yaml_path, base_classes): - """Every discovered entry has every `Module` dataclass field (derived, not hardcoded).""" + """Every discovered entry has every required `Module` dataclass field (derived, not hardcoded). + + Fields with a default (e.g. ``guide_url``) are attached separately and only + when applicable, so they are allowed to be absent here. + """ with ( patch("extract_parameters.PROVIDERS_DIR", provider_yaml_path.parent.parent), patch("extract_parameters.importlib.import_module", side_effect=self._mock_import), ): result = discover_classes_from_provider(provider_yaml_path, base_classes) - required_fields = {f.name for f in fields(Module)} + required_fields = {f.name for f in fields(Module) if f.default is MISSING} for entry in result: missing = required_fields - entry.keys() assert not missing, f"Missing fields {missing} in {entry['name']}" diff --git a/dev/registry/tests/test_extract_versions.py b/dev/registry/tests/test_extract_versions.py index 6a3128ce01a32..1e2a3ad9d246e 100644 --- a/dev/registry/tests/test_extract_versions.py +++ b/dev/registry/tests/test_extract_versions.py @@ -266,3 +266,57 @@ def test_filesystem_schemes_read_from_release_tag( {"scheme": "tfs", "filesystem": "airflow.providers.test.fs.testfs"}, ] mock_git_show.assert_any_call("providers-test/1.0.0", fs_source_path) + + +class TestExtractModulesGuideUrls: + """A class the provider's guides document in a section of its own must get a + ``guide_url`` for every version, not just the latest. A superseded version's + page is rendered only from the per-version file this module writes, so a link + resolved on the latest path alone disappears the moment a new version lands. + """ + + PROVIDER_YAML = { + "toolsets": [ + { + "integration-name": "Test", + "python-modules": ["airflow.providers.test.toolsets.hook"], + } + ] + } + SOURCE = 'class HookToolset:\n """A toolset."""\n' + GUIDE = "``HookToolset``\n---------------\n\nProse.\n" + + def _extract(self, layout="new", docs_paths=("providers/test/docs/toolsets.rst",)): + def fake_git_show(_tag, path): + if path.endswith("toolsets.rst"): + return self.GUIDE + return self.SOURCE if path.endswith(".py") else None + + with ( + patch("extract_versions.git_ls_tree", autospec=True, return_value=list(docs_paths)), + patch("extract_versions.git_show", autospec=True, side_effect=fake_git_show), + ): + return extract_modules_from_yaml( + self.PROVIDER_YAML, "providers-test/1.0.0", layout, "test", "test", "1.0.0" + ) + + def test_documented_class_gets_a_versioned_guide_url(self): + modules = self._extract() + + assert [m["name"] for m in modules] == ["HookToolset"] + assert modules[0]["guide_url"] == ( + "https://airflow.apache.org/docs/apache-airflow-providers-test/1.0.0/toolsets.html#hooktoolset" + ) + + def test_undocumented_class_gets_no_guide_url(self): + modules = self._extract(docs_paths=()) + + assert [m["name"] for m in modules] == ["HookToolset"] + assert "guide_url" not in modules[0] + + def test_old_layout_gets_no_guide_url(self): + # Pre-per-provider tags kept docs in a top-level tree, so there is no + # provider-relative page path to build a link from. + modules = self._extract(layout="old") + + assert "guide_url" not in modules[0] diff --git a/dev/registry/tests/test_registry_contract_models.py b/dev/registry/tests/test_registry_contract_models.py index 3c2b44daf7433..5753fc6cd2740 100644 --- a/dev/registry/tests/test_registry_contract_models.py +++ b/dev/registry/tests/test_registry_contract_models.py @@ -114,6 +114,17 @@ def test_module_contract_preserves_supports_deferrable_true(): assert validated["modules"][0]["supports_deferrable"] is True +def test_module_contract_omits_guide_url_for_undocumented_classes(): + validated = validate_modules_catalog({"modules": [_module_payload()]}) + assert "guide_url" not in validated["modules"][0] + + +def test_module_contract_round_trips_guide_url(): + guide_url = "https://example.invalid/docs/toolsets.html#exampletoolset" + validated = validate_modules_catalog({"modules": [_module_payload(guide_url=guide_url)]}) + assert validated["modules"][0]["guide_url"] == guide_url + + def test_connection_type_contract_defaults_external_services_to_empty_list(): """Legacy connection-types entries (provider.yaml without `external-services`) must still validate, with the field defaulting to an empty list.""" diff --git a/registry/AGENTS.md b/registry/AGENTS.md index 99416cb89311c..3c5cfd070a5f8 100644 --- a/registry/AGENTS.md +++ b/registry/AGENTS.md @@ -462,6 +462,24 @@ They run inside Breeze where all providers are installed. `extract_metadata.py` the CI workflow can run the fast scripts (metadata, ~30s per provider) without spinning up Breeze, while parameter/connection extraction is a separate step. +### How a module gets a "Guide" link + +A module card links to the how-to guide section that documents it, alongside the +generated API reference. Nothing declares that link: `registry_tools/docs_guides.py` +reads the provider's own `docs/*.rst` and matches a class to a section when the +section's title *opens with the class name as an inline literal* — ``` ``HookToolset`` ``` +or ``` ``AgentOperator`` & ``@task.agent`` ```. The anchor is derived from the whole +title the way docutils derives its HTML id. + +That convention is what the guides already do, and it is deliberately the only +signal: a hand-maintained class-to-guide table would keep pointing at sections +that have since been renamed or split, and a link that lands on the wrong section +is worse than no link. A class documented only in prose gets no Guide link. + +Both extraction paths resolve it — `extract_parameters.py` from the working tree +for the latest release, `extract_versions.py` from the git tag for superseded ones +— because a superseded version's page is rendered only from its own metadata file. + ### Relationship to `run_provider_yaml_files_check.py` `scripts/in_container/run_provider_yaml_files_check.py` (run by the diff --git a/registry/src/css/main.css b/registry/src/css/main.css index d5077f8c8887e..a7e1820861428 100644 --- a/registry/src/css/main.css +++ b/registry/src/css/main.css @@ -3558,6 +3558,7 @@ main { } .provider-detail-page .module-actions .docs-link, +.provider-detail-page .module-actions .guide-link, .provider-detail-page .module-actions .source-link { display: inline-flex; align-items: center; @@ -3575,6 +3576,14 @@ main { color: var(--color-cyan-300); } +.provider-detail-page .module-actions .guide-link { + color: var(--accent-secondary); +} + +.provider-detail-page .module-actions .guide-link:hover { + color: var(--color-cyan-300); +} + .provider-detail-page .module-actions .source-link { color: var(--text-muted); } diff --git a/registry/src/provider-version.njk b/registry/src/provider-version.njk index e28be209f1a82..7d0c16b9c4912 100644 --- a/registry/src/provider-version.njk +++ b/registry/src/provider-version.njk @@ -475,6 +475,12 @@ eleventyComputed: {% endif %} + {% if module.guide_url %} + + Guide + + + {% endif %} {% if module.source_url %} Source From b4a11d8d13e57fa48589a61a9871aff7cf58876f Mon Sep 17 00:00:00 2001 From: Wei Lee Date: Fri, 18 Sep 2026 14:03:38 +0900 Subject: [PATCH 02/19] Give a section's decorator name a guide link too The catalog carries task-flow decorators as modules of their own, named `@task.agent` and `@task.llm_file_analysis`, and a guide documents such a decorator in the same section as the operator it wraps -- titled ``AgentOperator`` & ``@task.agent``. Only the leading literal was read, so the decorator half of those sections never got a link although the title named it. A title's leading run of inline literals is now collected, one anchor shared by every name in the run. The run stops at the first thing that is neither a name nor a "&", "," or "/" separator, so a prose title that happens to mention a literal still produces nothing. --- dev/registry/registry_tools/docs_guides.py | 51 ++++++++++++++-------- dev/registry/tests/test_docs_guides.py | 34 ++++++++++++++- 2 files changed, 64 insertions(+), 21 deletions(-) diff --git a/dev/registry/registry_tools/docs_guides.py b/dev/registry/registry_tools/docs_guides.py index 37ea6b59c829b..fe271a6ae93cf 100644 --- a/dev/registry/registry_tools/docs_guides.py +++ b/dev/registry/registry_tools/docs_guides.py @@ -19,10 +19,11 @@ A module's ``docs_url`` points at generated API reference, which tells a reader what the arguments are but not how the thing is meant to be used. The prose guides carry that, and they already mark it: a how-to guide documents one class -per section, titled with the class name (``HookToolset``, ``SQLToolset``). +(or a class and its task-flow decorator) per section, titled with the name(s) +(``HookToolset``, ``SQLToolset``, ``AgentOperator`` & ``@task.agent``). So the mapping is read back out of the guides rather than curated anywhere: a -hand-maintained class-to-guide table would rot silently every time a guide is +hand-maintained name-to-guide table would rot silently every time a guide is split, renamed, or a class is dropped, and a rotten link is worse than none. Callers supply the reST they can see (a git tag, or the working tree) and get back only the anchors those sources actually contain. @@ -38,9 +39,19 @@ # punctuation character, at least as long as the title itself. _ADORNMENT_CHARS = "!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~" -# Only titles opening with a class name as an inline literal are treated as -# documenting it, so prose headings ("Bounded query results") never produce a link. -_LEADING_CLASS_LITERAL = re.compile(r"^``([A-Za-z_][A-Za-z0-9_]*)``") +# A single inline-literal name: a class (``HookToolset``) or a task-flow +# decorator (``@task.llm_file_analysis``) -- narrow enough that it still can't +# match arbitrary prose wrapped in backticks. +_INLINE_LITERAL_NAME = r"``(@?[A-Za-z_][A-Za-z0-9_]*(?:\.[A-Za-z_][A-Za-z0-9_]*)*)``" +_INLINE_LITERAL_NAME_RE = re.compile(_INLINE_LITERAL_NAME) + +# Only titles opening with a run of inline-literal names are treated as +# documenting them, so prose headings ("Bounded query results") never produce a +# link. A run is one or more names joined by "&", "," or "/" -- how guides write +# a section that covers both an operator and its decorator +# (``AgentOperator`` & ``@task.agent``). The run stops at the first thing that +# is neither a name nor a separator, so it never reaches into prose. +_LEADING_LITERAL_NAME_RUN = re.compile(rf"^{_INLINE_LITERAL_NAME}(?:\s*[&,/]\s*{_INLINE_LITERAL_NAME})*") def slugify_section_anchor(title: str) -> str: @@ -54,18 +65,18 @@ def slugify_section_anchor(title: str) -> str: return re.sub(r"[^a-z0-9]+", "-", title.lower()).strip("-") -def _extract_class_name_from_title(title: str) -> str | None: - """Return the class a section title leads with, or None if it names prose. +def _extract_leading_names_from_title(title: str) -> list[str]: + """Return the names a section title leads with, or [] if it names prose. - A guide marks a section as being *about a class* by opening its title with the - class as an inline literal -- ``HookToolset``, or + A guide marks a section as being *about* one or more names by opening its + title with them as inline literals -- ``HookToolset``, or ``AgentOperator`` & ``@task.agent`` where one section covers the operator and its decorator. Requiring that markup is what keeps a single-word prose heading ("Guidelines") from claiming to document a class of the same name, and it is a convention the guides already follow rather than one imposed on them. """ - match = _LEADING_CLASS_LITERAL.match(title) - return match.group(1) if match else None + match = _LEADING_LITERAL_NAME_RUN.match(title) + return _INLINE_LITERAL_NAME_RE.findall(match.group(0)) if match else [] def _is_adornment(line: str) -> bool: @@ -91,21 +102,23 @@ def _extract_section_titles(text: str) -> list[str]: def collect_guide_anchors(docs: Mapping[str, str]) -> dict[str, str]: - """Map class name -> ``.html#`` for every documented class. + """Map name -> ``.html#`` for every documented class or decorator. ``docs`` maps a page path relative to the provider's docs directory (e.g. - ``toolsets.rst``) to its reST source. When two pages document the same class - name, the first page in sorted order wins, so a rebuild of the same sources - always produces the same link. + ``toolsets.rst``) to its reST source. A title can lead with more than one name + (``AgentOperator`` & ``@task.agent``), in which case every leading name gets + the same anchor. When two pages document the same name, the first page in + sorted order wins, so a rebuild of the same sources always produces the same + link. """ anchors: dict[str, str] = {} for page in sorted(docs): page_url = re.sub(r"\.rst$", ".html", page) for title in _extract_section_titles(docs[page]): - class_name = _extract_class_name_from_title(title) - if not class_name or class_name in anchors: - continue - anchors[class_name] = f"{page_url}#{slugify_section_anchor(title)}" + for name in _extract_leading_names_from_title(title): + if name in anchors: + continue + anchors[name] = f"{page_url}#{slugify_section_anchor(title)}" return anchors diff --git a/dev/registry/tests/test_docs_guides.py b/dev/registry/tests/test_docs_guides.py index a8f0d333e5f47..37e34c8e4e428 100644 --- a/dev/registry/tests/test_docs_guides.py +++ b/dev/registry/tests/test_docs_guides.py @@ -114,14 +114,44 @@ def test_collect_guide_anchors_prefers_first_page_in_sorted_order(): def test_collect_guide_anchors_handles_a_title_covering_more_than_the_class(): # Verified against the published guide: this heading is served at # .../operators/agent.html#agentoperator-task-agent, so the anchor comes from - # the whole title while the class name comes from the leading literal. + # the whole title while both the operator and its decorator get linked to it. guide = "``AgentOperator`` & ``@task.agent``\n===================================\n\nProse.\n" assert collect_guide_anchors({"operators/agent.rst": guide}) == { - "AgentOperator": "operators/agent.html#agentoperator-task-agent" + "AgentOperator": "operators/agent.html#agentoperator-task-agent", + "@task.agent": "operators/agent.html#agentoperator-task-agent", } +def test_collect_guide_anchors_links_a_decorator_name_with_underscores(): + # ``@task.llm_file_analysis`` is the real decorator name for the common.ai + # provider's LLMFileAnalysisOperator; the leading-literal charset must admit + # "@" and "." for it to ever get a link. + guide = ( + "``LLMFileAnalysisOperator`` & ``@task.llm_file_analysis``\n" + "==========================================================\n\n" + "Prose.\n" + ) + + assert collect_guide_anchors({"operators/llm_file_analysis.rst": guide}) == { + "LLMFileAnalysisOperator": ( + "operators/llm_file_analysis.html#llmfileanalysisoperator-task-llm-file-analysis" + ), + "@task.llm_file_analysis": ( + "operators/llm_file_analysis.html#llmfileanalysisoperator-task-llm-file-analysis" + ), + } + + +def test_collect_guide_anchors_ignores_a_prose_title_mentioning_a_literal(): + # The title doesn't *open* with the literal, so nothing after "Using" should + # ever be scanned for further inline literals -- a prose heading that happens + # to mention one in passing must not produce a link. + guide = "Using ``foo`` in a pipeline\n============================\n\nProse.\n" + + assert collect_guide_anchors({"toolsets.rst": guide}) == {} + + def test_collect_guide_anchors_requires_a_long_enough_underline(): # An underline shorter than the title isn't a section in reST, so it must not # produce a link to an anchor Sphinx never emitted. From 65fa06bc358d3bbdd3913d3e0e6593d0bdecf179 Mon Sep 17 00:00:00 2001 From: Wei Lee Date: Fri, 18 Sep 2026 15:19:24 +0900 Subject: [PATCH 03/19] Resolve guide links from guide pages only Both readers handed every `.rst` under a provider's docs directory to the anchor collector. `_api/` is gitignored autoapi output, so the working-tree reader saw those pages on any tree where the docs had been built while the git-tag reader never can -- the anchor set a provider ended up with depended on whether a build had run where the extractor happened to execute. `_`-prefixed paths also sort ahead of lowercase ones, so they won the first-page tie-break against the guide that actually documents the class. `is_guide_page` now gates both readers on the same rule: nothing under a `_`-prefixed path segment at any depth, and neither `changelog.rst` nor `commits.rst` -- real pages, but release notes rather than how-to guides, whose headings can be inline-literal-formatted by coincidence. Gating both readers on one predicate is what keeps the two paths from drifting apart again. The filter is not a cost fix: at `providers-amazon/9.17.0` it takes the file count from 102 to 98 and leaves the wall time where it was (~1.9s), because the cost is one `git show` subprocess per file rather than the bytes read. Batching those reads is left to a change of its own. --- dev/registry/extract_parameters.py | 13 ++-- dev/registry/extract_versions.py | 7 ++- dev/registry/registry_tools/docs_guides.py | 26 ++++++++ dev/registry/tests/test_docs_guides.py | 59 +++++++++++++++++++ dev/registry/tests/test_extract_parameters.py | 18 ++++++ dev/registry/tests/test_extract_versions.py | 27 ++++++++- 6 files changed, 142 insertions(+), 8 deletions(-) diff --git a/dev/registry/extract_parameters.py b/dev/registry/extract_parameters.py index c98c895bca28d..81a8a2c0066ab 100644 --- a/dev/registry/extract_parameters.py +++ b/dev/registry/extract_parameters.py @@ -56,7 +56,7 @@ import yaml from extract_metadata import fetch_provider_inventory, read_inventory from registry_contract_models import validate_modules_catalog, validate_provider_parameters -from registry_tools.docs_guides import attach_guide_urls, collect_guide_anchors +from registry_tools.docs_guides import attach_guide_urls, collect_guide_anchors, is_guide_page from registry_tools.types import ( BASE_CLASS_IMPORTS, CLASS_LEVEL_CATEGORY_OVERRIDES, @@ -744,10 +744,13 @@ def read_guide_docs(docs_dir: Path) -> dict[str, str]: """Read a provider's authored reST docs from the working tree, keyed by path relative to ``docs_dir``.""" if not docs_dir.is_dir(): return {} - return { - path.relative_to(docs_dir).as_posix(): path.read_text(encoding="utf-8") - for path in sorted(docs_dir.rglob("*.rst")) - } + docs = {} + for path in sorted(docs_dir.rglob("*.rst")): + relative = path.relative_to(docs_dir).as_posix() + if not is_guide_page(relative): + continue + docs[relative] = path.read_text(encoding="utf-8") + return docs def discover_classes_from_provider( diff --git a/dev/registry/extract_versions.py b/dev/registry/extract_versions.py index 77df6a38fa0f1..a6db301becd50 100644 --- a/dev/registry/extract_versions.py +++ b/dev/registry/extract_versions.py @@ -60,7 +60,7 @@ sys.exit(1) from extract_metadata import fetch_provider_inventory, read_connection_urls, resolve_connection_docs_url -from registry_tools.docs_guides import attach_guide_urls, collect_guide_anchors +from registry_tools.docs_guides import attach_guide_urls, collect_guide_anchors, is_guide_page from registry_tools.types import ( CLASS_LEVEL_CATEGORY_OVERRIDES, CLASS_LEVEL_SECTIONS, @@ -213,8 +213,11 @@ def read_guide_docs(tag: str, layout: str, dir_path: str) -> dict[str, str]: for path in git_ls_tree(tag, docs_prefix): if not path.endswith(".rst"): continue + relative = path[len(docs_prefix) :] + if not is_guide_page(relative): + continue if content := git_show(tag, path): - docs[path[len(docs_prefix) :]] = content + docs[relative] = content return docs diff --git a/dev/registry/registry_tools/docs_guides.py b/dev/registry/registry_tools/docs_guides.py index fe271a6ae93cf..5faa3174ab287 100644 --- a/dev/registry/registry_tools/docs_guides.py +++ b/dev/registry/registry_tools/docs_guides.py @@ -33,12 +33,38 @@ import re from collections.abc import Mapping +from pathlib import PurePosixPath from typing import Any # reST underlines an (optionally overlined) section title with a run of one # punctuation character, at least as long as the title itself. _ADORNMENT_CHARS = "!\"#$%&'()*+,-./:;<=>?@[\\]^_`{|}~" +_SKIPPED_PAGE_NAMES = frozenset({"changelog.rst", "commits.rst"}) + + +def is_guide_page(relative_path: str) -> bool: + """Whether a path relative to a provider's docs directory is a how-to guide page. + + Callers hand every ``.rst`` they can see to this before it ever reaches + ``collect_guide_anchors``. Two kinds of real, built pages must not go + further: + + - Anything under a ``_``-prefixed path segment, at any depth + (``_api/hook/index.rst``, ``operators/_partials/foo.rst``, top-level + ``_partials/foo.rst``): Sphinx/autoapi output and partials are directive + markup, not the hand-written, reST-underlined titles this module's + leading-inline-literal convention parses. + - ``changelog.rst`` and ``commits.rst``: real release-note pages, not + how-to guides, that can carry inline-literal-formatted headings by + coincidence. + """ + path = PurePosixPath(relative_path) + if any(part.startswith("_") for part in path.parts): + return False + return path.name not in _SKIPPED_PAGE_NAMES + + # A single inline-literal name: a class (``HookToolset``) or a task-flow # decorator (``@task.llm_file_analysis``) -- narrow enough that it still can't # match arbitrary prose wrapped in backticks. diff --git a/dev/registry/tests/test_docs_guides.py b/dev/registry/tests/test_docs_guides.py index 37e34c8e4e428..1a9a994e315d5 100644 --- a/dev/registry/tests/test_docs_guides.py +++ b/dev/registry/tests/test_docs_guides.py @@ -16,10 +16,15 @@ # under the License. from __future__ import annotations +from unittest.mock import patch + import pytest +from extract_parameters import read_guide_docs as read_guide_docs_from_worktree +from extract_versions import read_guide_docs as read_guide_docs_from_tag from registry_tools.docs_guides import ( attach_guide_urls, collect_guide_anchors, + is_guide_page, slugify_section_anchor, ) @@ -183,3 +188,57 @@ def test_attach_guide_urls_does_not_double_up_the_base_separator(): attach_guide_urls(modules, {"HookToolset": "toolsets.html#hooktoolset"}, "https://example.test/docs/") assert modules[0]["guide_url"] == "https://example.test/docs/toolsets.html#hooktoolset" + + +@pytest.mark.parametrize( + ("relative_path", "expected"), + [ + # A `_`-prefixed path segment marks autoapi/partial content, at any + # depth. Mutation canary: removing the `_` check turns these four + # from False back to True. + ("_api/index.rst", False), + ("_api/hook/index.rst", False), + ("operators/_partials/foo.rst", False), + ("_partials/foo.rst", False), + # Real, built release-note pages, not how-to guides. Mutation canary: + # removing the changelog/commits check turns these two from False + # back to True. + ("changelog.rst", False), + ("commits.rst", False), + ("toolsets.rst", True), + ("operators/agent.rst", True), + ], +) +def test_is_guide_page(relative_path, expected): + assert is_guide_page(relative_path) == expected + + +def test_readers_agree_on_which_pages_are_guides(tmp_path): + """Both `read_guide_docs` implementations delegate to `is_guide_page`, so a + working-tree read and a git-tag read of the same paths must end up with the + same set of pages -- regardless of which source produced them.""" + relative_paths = ["_api/index.rst", "changelog.rst", "commits.rst", "toolsets.rst"] + for relative in relative_paths: + target = tmp_path / relative + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text("Prose.\n") + + from_worktree = read_guide_docs_from_worktree(tmp_path) + + docs_prefix = "providers/test/docs/" + with ( + patch( + "extract_versions.git_ls_tree", + autospec=True, + return_value=[docs_prefix + relative for relative in relative_paths], + ), + patch("extract_versions.git_show", autospec=True, return_value="Prose.\n"), + ): + from_tag = read_guide_docs_from_tag("providers-test/1.0.0", "new", "test") + + # Mutation canary: reverting the `is_guide_page` call in only one of the + # two readers (e.g. extract_parameters.read_guide_docs but not + # extract_versions.read_guide_docs) fails this assertion even though each + # reader's own test (test_extract_parameters / test_extract_versions) may + # still pass on its own. + assert set(from_worktree) == set(from_tag) == {"toolsets.rst"} diff --git a/dev/registry/tests/test_extract_parameters.py b/dev/registry/tests/test_extract_parameters.py index 3430d963a6727..c67b8449e3166 100644 --- a/dev/registry/tests/test_extract_parameters.py +++ b/dev/registry/tests/test_extract_parameters.py @@ -38,6 +38,7 @@ get_category, is_durable_capable, load_resumable_job_mixin, + read_guide_docs, supports_deferrable, ) @@ -911,6 +912,23 @@ def fake_decorator_task(python_callable=None, **kwargs): } +# --------------------------------------------------------------------------- +# read_guide_docs +# --------------------------------------------------------------------------- +def test_read_guide_docs_skips_generated_and_release_note_pages(tmp_path): + (tmp_path / "_api" / "x").mkdir(parents=True) + (tmp_path / "_api" / "x" / "index.rst").write_text("Generated.\n") + (tmp_path / "changelog.rst").write_text("Release notes.\n") + (tmp_path / "toolsets.rst").write_text("``HookToolset``\n---------------\n\nProse.\n") + + result = read_guide_docs(tmp_path) + + # Mutation canary: if the `is_guide_page` filter in read_guide_docs is + # removed, this dict grows two more keys -- "_api/x/index.rst" and + # "changelog.rst" -- and this assertion goes red. + assert set(result) == {"toolsets.rst"} + + # --------------------------------------------------------------------------- # TestDiscoverClassesFromProvider # --------------------------------------------------------------------------- diff --git a/dev/registry/tests/test_extract_versions.py b/dev/registry/tests/test_extract_versions.py index 1e2a3ad9d246e..a87dc5de182cb 100644 --- a/dev/registry/tests/test_extract_versions.py +++ b/dev/registry/tests/test_extract_versions.py @@ -19,7 +19,7 @@ from __future__ import annotations import textwrap -from unittest.mock import patch +from unittest.mock import call, patch import pytest from extract_versions import ( @@ -28,6 +28,7 @@ SCRIPT_DIR, extract_modules_from_yaml, extract_version_data, + read_guide_docs, ) from registry_tools.types import CLASS_LEVEL_SECTIONS, DICT_SHAPED_CLASS_LEVEL_SECTIONS @@ -320,3 +321,27 @@ def test_old_layout_gets_no_guide_url(self): modules = self._extract(layout="old") assert "guide_url" not in modules[0] + + +class TestReadGuideDocs: + def test_skips_generated_and_release_note_pages_before_calling_git_show(self): + docs_prefix = "providers/test/docs/" + paths = [ + docs_prefix + "_api/x/index.rst", + docs_prefix + "changelog.rst", + docs_prefix + "toolsets.rst", + ] + + with ( + patch("extract_versions.git_ls_tree", autospec=True, return_value=paths), + patch("extract_versions.git_show", autospec=True, return_value="Prose.\n") as mock_git_show, + ): + result = read_guide_docs("providers-test/1.0.0", "new", "test") + + # Mutation canary: if the `is_guide_page` filter in read_guide_docs is + # removed, this dict grows two more keys and this assertion goes red. + assert set(result) == {"toolsets.rst"} + # Mutation canary: without the filter, git_show is also called for the + # two skipped paths -- this assertion goes red too, proving the + # `continue` runs before git_show, not just before the dict write. + assert mock_git_show.call_args_list == [call("providers-test/1.0.0", docs_prefix + "toolsets.rst")] From 849976a85dacabb1bebee580bb33d4e86e08b3d7 Mon Sep 17 00:00:00 2001 From: Wei Lee Date: Fri, 18 Sep 2026 15:27:35 +0900 Subject: [PATCH 04/19] Say where Guide-link coverage comes from Covering another provider's modules is a matter of that provider's section titles leading with an inline literal -- the extractor needs no change for it. Worth saying next to the convention, along with the fact that a matching title still needs a same-named module in the catalog before a link appears. --- registry/AGENTS.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/registry/AGENTS.md b/registry/AGENTS.md index 3c5cfd070a5f8..aea31f3bf276b 100644 --- a/registry/AGENTS.md +++ b/registry/AGENTS.md @@ -476,6 +476,13 @@ signal: a hand-maintained class-to-guide table would keep pointing at sections that have since been renamed or split, and a link that lands on the wrong section is worse than no link. A class documented only in prose gets no Guide link. +Growing the set of modules that get a Guide link means changing that provider's +section titles to lead with an inline literal, not touching this extractor. +`common/ai` follows the convention most thoroughly; a couple of other providers +use the same title shape for a config option name or a single decorator rather +than a class. Having the right title doesn't guarantee a link — that still +depends on a same-named module existing in the catalog. + Both extraction paths resolve it — `extract_parameters.py` from the working tree for the latest release, `extract_versions.py` from the git tag for superseded ones — because a superseded version's page is rendered only from its own metadata file. From 57823f5e8eece34a7d78c01b0200a1e0657e7ce5 Mon Sep 17 00:00:00 2001 From: Wei Lee Date: Sat, 19 Sep 2026 10:36:06 +0900 Subject: [PATCH 05/19] Say guide_url is present only when a guide documents the class The earlier field-count fix counted `guide_url` as a fixed thirteenth field. `attach_guide_urls` skips a class no guide documents, so those entries carry twelve keys, and a bare count cannot say which one a reader will get. Naming the field alongside its condition does, and matches what `make_entry` already says a few lines below. --- dev/registry/extract_parameters.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/dev/registry/extract_parameters.py b/dev/registry/extract_parameters.py index 81a8a2c0066ab..8d326f05015cc 100644 --- a/dev/registry/extract_parameters.py +++ b/dev/registry/extract_parameters.py @@ -763,7 +763,8 @@ def discover_classes_from_provider( """Discover classes from a single provider by importing its modules at runtime. Reads the provider.yaml to find which modules/classes to inspect, imports them, - and returns metadata for each discovered class with every `Module` dataclass field. + and returns metadata for each discovered class with every required `Module` + dataclass field, plus ``guide_url`` when a how-to guide documents the class. """ with open(provider_yaml_path) as f: provider_yaml = yaml.safe_load(f) From e08f244fe6b8bf1b9162aa6f034e57165c7adcad Mon Sep 17 00:00:00 2001 From: Wei Lee Date: Sat, 19 Sep 2026 10:49:50 +0900 Subject: [PATCH 06/19] Pin the guide_url contract tests to the model, not the payload `validate_modules_catalog` returns the dict it was handed rather than a `model_dump()`, so asserting on that dict said nothing about `ModuleContract`: the absent-field test could not fail at all, and the one named for a round trip performed none. Both now read the attribute off `ModuleContract.model_validate`, where the default and the parsed value actually live, and the second test is named for what it checks. --- dev/registry/tests/test_registry_contract_models.py | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/dev/registry/tests/test_registry_contract_models.py b/dev/registry/tests/test_registry_contract_models.py index 5753fc6cd2740..ea926d131881f 100644 --- a/dev/registry/tests/test_registry_contract_models.py +++ b/dev/registry/tests/test_registry_contract_models.py @@ -115,14 +115,12 @@ def test_module_contract_preserves_supports_deferrable_true(): def test_module_contract_omits_guide_url_for_undocumented_classes(): - validated = validate_modules_catalog({"modules": [_module_payload()]}) - assert "guide_url" not in validated["modules"][0] + assert ModuleContract.model_validate(_module_payload()).guide_url is None -def test_module_contract_round_trips_guide_url(): +def test_module_contract_preserves_guide_url_value(): guide_url = "https://example.invalid/docs/toolsets.html#exampletoolset" - validated = validate_modules_catalog({"modules": [_module_payload(guide_url=guide_url)]}) - assert validated["modules"][0]["guide_url"] == guide_url + assert ModuleContract.model_validate(_module_payload(guide_url=guide_url)).guide_url == guide_url def test_connection_type_contract_defaults_external_services_to_empty_list(): From d298157599753e34579c322b5081f0a4fb6a8aec Mon Sep 17 00:00:00 2001 From: Wei Lee Date: Sat, 19 Sep 2026 11:03:24 +0900 Subject: [PATCH 07/19] Say what provider.yaml's how-to-guide does not reach "Nothing declares that link" was wrong: `provider.yaml` declares `how-to-guide` for integrations and transfers, and `check_doc_files` set-compares it against the operator, sensor and transfer guide paths, so a stale entry fails CI rather than rotting. What that declaration cannot do is name a section inside a page, and it never covers the toolset, hook and decorator pages this reads -- which is the gap the title convention fills, and the reason worth recording in a file future readers take instructions from. --- registry/AGENTS.md | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/registry/AGENTS.md b/registry/AGENTS.md index aea31f3bf276b..f1a192cf27ccd 100644 --- a/registry/AGENTS.md +++ b/registry/AGENTS.md @@ -465,16 +465,20 @@ up Breeze, while parameter/connection extraction is a separate step. ### How a module gets a "Guide" link A module card links to the how-to guide section that documents it, alongside the -generated API reference. Nothing declares that link: `registry_tools/docs_guides.py` -reads the provider's own `docs/*.rst` and matches a class to a section when the -section's title *opens with the class name as an inline literal* — ``` ``HookToolset`` ``` -or ``` ``AgentOperator`` & ``@task.agent`` ```. The anchor is derived from the whole -title the way docutils derives its HTML id. +generated API reference. `provider.yaml`'s `how-to-guide` fields are CI-enforced +by `check_doc_files`, but they name a whole page, never a section, and cover only +operators, sensors and transfers — not the toolset, hook and decorator pages this +needs. So `registry_tools/docs_guides.py` reads the provider's own `docs/*.rst` +and matches a class to a section when the section's title *opens with the class +name as an inline literal* — ``` ``HookToolset`` ``` or ``` ``AgentOperator`` & +``@task.agent`` ```. The anchor is derived from the whole title the way docutils +derives its HTML id. That convention is what the guides already do, and it is deliberately the only -signal: a hand-maintained class-to-guide table would keep pointing at sections -that have since been renamed or split, and a link that lands on the wrong section -is worse than no link. A class documented only in prose gets no Guide link. +signal this resolves a section from: a hand-maintained class-to-guide table +would keep pointing at sections that have since been renamed or split, and a +link that lands on the wrong section is worse than no link. A class documented +only in prose gets no Guide link. Growing the set of modules that get a Guide link means changing that provider's section titles to lead with an inline literal, not touching this extractor. From ae9cca9b5802d04e6a79a141f15f47e3206f2f60 Mon Sep 17 00:00:00 2001 From: Wei Lee Date: Sat, 19 Sep 2026 11:08:11 +0900 Subject: [PATCH 08/19] Let a module's action links wrap The row holds three links now that a Guide link sits between Docs and Source, and it was a flex row with no wrap, so the third one is pushed out of the card at narrow viewports or a raised root font size. --- registry/src/css/main.css | 1 + 1 file changed, 1 insertion(+) diff --git a/registry/src/css/main.css b/registry/src/css/main.css index a7e1820861428..42b6d1a618e91 100644 --- a/registry/src/css/main.css +++ b/registry/src/css/main.css @@ -3552,6 +3552,7 @@ main { /* Module Actions (View Docs, Source) */ .provider-detail-page .module-actions { display: flex; + flex-wrap: wrap; align-items: center; gap: var(--space-3); margin-top: var(--space-3); From f6ecd395b18f140e7bc565e6f99db9982f251f44 Mon Sep 17 00:00:00 2001 From: Wei Lee Date: Sat, 19 Sep 2026 12:01:35 +0900 Subject: [PATCH 09/19] Read a release tag's guide pages in one git call Filtering to guide pages ahead of the read, as the previous change did, took `providers-amazon/9.36.0` from 111 files to 107 -- the cost was never the bytes read, it was one `git show` subprocess per file, and `--all-versions` pays it once per provider-version. `git_cat_file_batch` hands the survivors to a single `git cat-file --batch` on stdin: 107 subprocesses and ~1.9s become one and ~0.05s, for a dict that compares equal to the loop's, key for key and value for value. The batch protocol has to be parsed on bytes, because its header counts bytes and decoding before slicing would drift on multi-byte content. Content is then decoded as UTF-8 explicitly rather than following the process locale, and a decode failure is left to propagate: `.rst` is Sphinx-convention UTF-8, and bad data should fail loudly here the way a missing object already does. `git_ls_tree` now runs with `core.quotePath=false`. It C-quoted a non-ASCII path into a string that fails the `.endswith(".rst")` test the working-tree reader passes, so the two readers disagreed about the same file. No such path exists under `providers/` today -- this closes the divergence rather than fixing an observed break. --- dev/registry/extract_versions.py | 64 +++++++++- dev/registry/tests/test_docs_guides.py | 6 +- dev/registry/tests/test_extract_versions.py | 124 ++++++++++++++++++-- 3 files changed, 179 insertions(+), 15 deletions(-) diff --git a/dev/registry/extract_versions.py b/dev/registry/extract_versions.py index a6db301becd50..035bbd4d827b2 100644 --- a/dev/registry/extract_versions.py +++ b/dev/registry/extract_versions.py @@ -132,10 +132,17 @@ def git_show(tag: str, path: str) -> str | None: def git_ls_tree(tag: str, prefix: str) -> list[str]: - """List the file paths under a prefix at a specific git tag.""" + """List the file paths under a prefix at a specific git tag. + + Decodes via the process locale (`text=True`), unlike `git_cat_file_batch`'s + UTF-8 decode -- and `git_cat_file_batch` re-encodes these same strings + (passed straight through by `read_guide_docs`) into its stdin, so a + non-UTF-8 locale could break byte-for-byte round-trip. No such path exists + under `providers/` today. + """ try: result = subprocess.run( - ["git", "ls-tree", "-r", "--name-only", tag, "--", prefix], + ["git", "-c", "core.quotePath=false", "ls-tree", "-r", "--name-only", tag, "--", prefix], capture_output=True, text=True, cwd=AIRFLOW_ROOT, @@ -146,6 +153,48 @@ def git_ls_tree(tag: str, prefix: str) -> list[str]: return [line for line in result.stdout.splitlines() if line] +def git_cat_file_batch(tag: str, paths: list[str]) -> dict[str, str]: + """Read multiple files at a specific git tag in one `git cat-file --batch` call. + + Returns a mapping of path -> content for paths that exist at the tag; a path + git reports as missing is simply absent from the result, matching git_show's + "return None for a missing path" semantics. + + Decode failures are left unguarded on purpose: .rst files are Sphinx + convention UTF-8, an explicit "utf-8" decode is more predictable than + following the process locale, and a UnicodeDecodeError should surface loudly + rather than being swallowed -- the same posture git_show takes toward + CalledProcessError (fails loud, doesn't paper over bad data). + """ + if not paths: + return {} + + stdin = ("\n".join(f"{tag}:{p}" for p in paths) + "\n").encode("utf-8") + result = subprocess.run( + ["git", "cat-file", "--batch"], + input=stdin, + capture_output=True, + cwd=AIRFLOW_ROOT, + check=True, + ) + + output = result.stdout + pos = 0 + contents: dict[str, str] = {} + for path in paths: + newline_idx = output.index(b"\n", pos) + header = output[pos:newline_idx].decode("utf-8") + pos = newline_idx + 1 + if header.endswith(" missing"): + continue + _sha1, _obj_type, size_str = header.split(" ") + size = int(size_str) + content_bytes = output[pos : pos + size] + pos += size + 1 # skip the protocol's trailing LF, which isn't counted in size + contents[path] = content_bytes.decode("utf-8") + return contents + + def git_tag_exists(tag: str) -> bool: """Check if a git tag exists locally.""" result = subprocess.run( @@ -209,16 +258,19 @@ def read_guide_docs(tag: str, layout: str, dir_path: str) -> dict[str, str]: return {} docs_prefix = f"providers/{dir_path}/docs/" - docs: dict[str, str] = {} + survivors: list[tuple[str, str]] = [] for path in git_ls_tree(tag, docs_prefix): if not path.endswith(".rst"): continue relative = path[len(docs_prefix) :] if not is_guide_page(relative): continue - if content := git_show(tag, path): - docs[relative] = content - return docs + survivors.append((relative, path)) + + batch_result = git_cat_file_batch(tag, [full_path for _relative, full_path in survivors]) + return { + relative: batch_result[full_path] for relative, full_path in survivors if batch_result.get(full_path) + } def parse_pyproject_toml_content(content: str, layout: str) -> dict[str, Any]: diff --git a/dev/registry/tests/test_docs_guides.py b/dev/registry/tests/test_docs_guides.py index 1a9a994e315d5..975db3366a50a 100644 --- a/dev/registry/tests/test_docs_guides.py +++ b/dev/registry/tests/test_docs_guides.py @@ -232,7 +232,11 @@ def test_readers_agree_on_which_pages_are_guides(tmp_path): autospec=True, return_value=[docs_prefix + relative for relative in relative_paths], ), - patch("extract_versions.git_show", autospec=True, return_value="Prose.\n"), + patch( + "extract_versions.git_cat_file_batch", + autospec=True, + side_effect=lambda tag, paths: {p: "Prose.\n" for p in paths}, + ), ): from_tag = read_guide_docs_from_tag("providers-test/1.0.0", "new", "test") diff --git a/dev/registry/tests/test_extract_versions.py b/dev/registry/tests/test_extract_versions.py index a87dc5de182cb..bfe23f2bf9cdd 100644 --- a/dev/registry/tests/test_extract_versions.py +++ b/dev/registry/tests/test_extract_versions.py @@ -19,7 +19,7 @@ from __future__ import annotations import textwrap -from unittest.mock import call, patch +from unittest.mock import MagicMock, call, patch import pytest from extract_versions import ( @@ -28,6 +28,8 @@ SCRIPT_DIR, extract_modules_from_yaml, extract_version_data, + git_cat_file_batch, + git_ls_tree, read_guide_docs, ) from registry_tools.types import CLASS_LEVEL_SECTIONS, DICT_SHAPED_CLASS_LEVEL_SECTIONS @@ -289,13 +291,15 @@ class TestExtractModulesGuideUrls: def _extract(self, layout="new", docs_paths=("providers/test/docs/toolsets.rst",)): def fake_git_show(_tag, path): - if path.endswith("toolsets.rst"): - return self.GUIDE return self.SOURCE if path.endswith(".py") else None + def fake_git_cat_file_batch(_tag, paths): + return {p: self.GUIDE for p in paths} + with ( patch("extract_versions.git_ls_tree", autospec=True, return_value=list(docs_paths)), patch("extract_versions.git_show", autospec=True, side_effect=fake_git_show), + patch("extract_versions.git_cat_file_batch", autospec=True, side_effect=fake_git_cat_file_batch), ): return extract_modules_from_yaml( self.PROVIDER_YAML, "providers-test/1.0.0", layout, "test", "test", "1.0.0" @@ -334,14 +338,118 @@ def test_skips_generated_and_release_note_pages_before_calling_git_show(self): with ( patch("extract_versions.git_ls_tree", autospec=True, return_value=paths), - patch("extract_versions.git_show", autospec=True, return_value="Prose.\n") as mock_git_show, + patch( + "extract_versions.git_cat_file_batch", + autospec=True, + side_effect=lambda tag, paths: {p: "Prose.\n" for p in paths}, + ) as mock_git_cat_file_batch, ): result = read_guide_docs("providers-test/1.0.0", "new", "test") # Mutation canary: if the `is_guide_page` filter in read_guide_docs is # removed, this dict grows two more keys and this assertion goes red. assert set(result) == {"toolsets.rst"} - # Mutation canary: without the filter, git_show is also called for the - # two skipped paths -- this assertion goes red too, proving the - # `continue` runs before git_show, not just before the dict write. - assert mock_git_show.call_args_list == [call("providers-test/1.0.0", docs_prefix + "toolsets.rst")] + # Mutation canary: without the filter, the two skipped paths would also + # be included in the batch call's path list -- this assertion goes red + # too, proving the filtering runs before the batch call, not just + # before the dict write. + assert mock_git_cat_file_batch.call_args_list == [ + call("providers-test/1.0.0", [docs_prefix + "toolsets.rst"]) + ] + + def test_skips_a_page_whose_content_is_an_empty_string(self): + # Mutation canary: replacing `batch_result.get(full_path)` with + # `full_path in batch_result` in read_guide_docs's comprehension makes + # this page reappear as `{"empty.rst": ""}`, turning this assertion red. + docs_prefix = "providers/test/docs/" + paths = [docs_prefix + "empty.rst"] + + with ( + patch("extract_versions.git_ls_tree", autospec=True, return_value=paths), + patch( + "extract_versions.git_cat_file_batch", + autospec=True, + return_value={docs_prefix + "empty.rst": ""}, + ), + ): + result = read_guide_docs("providers-test/1.0.0", "new", "test") + + assert result == {} + + +class TestGitLsTree: + def test_passes_quote_path_false_to_git(self): + mock_result = MagicMock() + mock_result.stdout = "providers/test/docs/toolsets.rst\n" + with patch("extract_versions.subprocess.run", return_value=mock_result) as mock_run: + git_ls_tree("providers-test/1.0.0", "providers/test/docs/") + + assert mock_run.call_args.args[0] == [ + "git", + "-c", + "core.quotePath=false", + "ls-tree", + "-r", + "--name-only", + "providers-test/1.0.0", + "--", + "providers/test/docs/", + ] + + +def _batch_hit(sha1: str, obj_type: str, content: bytes) -> bytes: + return f"{sha1} {obj_type} {len(content)}\n".encode() + content + b"\n" + + +def _batch_missing(spec: str) -> bytes: + return f"{spec} missing\n".encode() + + +class TestGitCatFileBatch: + def test_empty_paths_returns_empty_dict_without_subprocess(self): + with patch("extract_versions.subprocess.run") as mock_run: + result = git_cat_file_batch("providers-test/1.0.0", []) + + assert result == {} + mock_run.assert_not_called() + + def test_two_hits_with_different_sizes_and_multibyte_content(self): + tag = "providers-test/1.0.0" + paths = ["providers/test/docs/a.rst", "providers/test/docs/b.rst"] + first_content = b"short\n" + second_content = "café prôse with more text\n".encode() + payload = _batch_hit("aaa1", "blob", first_content) + _batch_hit("bbb2", "blob", second_content) + + mock_result = MagicMock() + mock_result.stdout = payload + with patch("extract_versions.subprocess.run", return_value=mock_result): + result = git_cat_file_batch(tag, paths) + + assert result == { + paths[0]: "short\n", + paths[1]: "café prôse with more text\n", + } + + def test_hit_followed_by_missing_path(self): + tag = "providers-test/1.0.0" + paths = ["providers/test/docs/a.rst", "providers/test/docs/missing.rst"] + payload = _batch_hit("aaa1", "blob", b"content\n") + _batch_missing(f"{tag}:{paths[1]}") + + mock_result = MagicMock() + mock_result.stdout = payload + with patch("extract_versions.subprocess.run", return_value=mock_result): + result = git_cat_file_batch(tag, paths) + + assert result == {paths[0]: "content\n"} + + def test_all_missing_returns_empty_dict(self): + tag = "providers-test/1.0.0" + paths = ["providers/test/docs/a.rst", "providers/test/docs/b.rst"] + payload = _batch_missing(f"{tag}:{paths[0]}") + _batch_missing(f"{tag}:{paths[1]}") + + mock_result = MagicMock() + mock_result.stdout = payload + with patch("extract_versions.subprocess.run", return_value=mock_result): + result = git_cat_file_batch(tag, paths) + + assert result == {} From e969cbbd4cd1184543d19a9208c75cba0ce8dbba Mon Sep 17 00:00:00 2001 From: Wei Lee Date: Mon, 28 Sep 2026 12:14:51 +0900 Subject: [PATCH 10/19] Decode ls-tree output the same way as cat-file --batch Following the process locale for ls-tree while cat-file --batch decoded explicit UTF-8 meant a non-UTF-8 locale could break the round-trip of paths between the two. The subprocess mocks now carry autospec like the rest of the file. --- dev/registry/extract_versions.py | 12 ++------- dev/registry/tests/test_extract_versions.py | 29 ++++++++++++++------- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/dev/registry/extract_versions.py b/dev/registry/extract_versions.py index 035bbd4d827b2..af3f5a94e205d 100644 --- a/dev/registry/extract_versions.py +++ b/dev/registry/extract_versions.py @@ -132,25 +132,17 @@ def git_show(tag: str, path: str) -> str | None: def git_ls_tree(tag: str, prefix: str) -> list[str]: - """List the file paths under a prefix at a specific git tag. - - Decodes via the process locale (`text=True`), unlike `git_cat_file_batch`'s - UTF-8 decode -- and `git_cat_file_batch` re-encodes these same strings - (passed straight through by `read_guide_docs`) into its stdin, so a - non-UTF-8 locale could break byte-for-byte round-trip. No such path exists - under `providers/` today. - """ + """List the file paths under a prefix at a specific git tag.""" try: result = subprocess.run( ["git", "-c", "core.quotePath=false", "ls-tree", "-r", "--name-only", tag, "--", prefix], capture_output=True, - text=True, cwd=AIRFLOW_ROOT, check=True, ) except subprocess.CalledProcessError: return [] - return [line for line in result.stdout.splitlines() if line] + return [line for line in result.stdout.decode("utf-8").splitlines() if line] def git_cat_file_batch(tag: str, paths: list[str]) -> dict[str, str]: diff --git a/dev/registry/tests/test_extract_versions.py b/dev/registry/tests/test_extract_versions.py index bfe23f2bf9cdd..2ddbaa0d288fb 100644 --- a/dev/registry/tests/test_extract_versions.py +++ b/dev/registry/tests/test_extract_versions.py @@ -18,6 +18,7 @@ from __future__ import annotations +import subprocess import textwrap from unittest.mock import MagicMock, call, patch @@ -379,9 +380,9 @@ def test_skips_a_page_whose_content_is_an_empty_string(self): class TestGitLsTree: def test_passes_quote_path_false_to_git(self): - mock_result = MagicMock() - mock_result.stdout = "providers/test/docs/toolsets.rst\n" - with patch("extract_versions.subprocess.run", return_value=mock_result) as mock_run: + mock_result = MagicMock(spec=subprocess.CompletedProcess) + mock_result.stdout = b"providers/test/docs/toolsets.rst\n" + with patch("extract_versions.subprocess.run", autospec=True, return_value=mock_result) as mock_run: git_ls_tree("providers-test/1.0.0", "providers/test/docs/") assert mock_run.call_args.args[0] == [ @@ -396,6 +397,14 @@ def test_passes_quote_path_false_to_git(self): "providers/test/docs/", ] + def test_decodes_stdout_as_utf8(self): + mock_result = MagicMock(spec=subprocess.CompletedProcess) + mock_result.stdout = "docs/café.rst\ndocs/b.rst\n".encode() + with patch("extract_versions.subprocess.run", autospec=True, return_value=mock_result): + result = git_ls_tree("providers-test/1.0.0", "providers/test/docs/") + + assert result == ["docs/café.rst", "docs/b.rst"] + def _batch_hit(sha1: str, obj_type: str, content: bytes) -> bytes: return f"{sha1} {obj_type} {len(content)}\n".encode() + content + b"\n" @@ -407,7 +416,7 @@ def _batch_missing(spec: str) -> bytes: class TestGitCatFileBatch: def test_empty_paths_returns_empty_dict_without_subprocess(self): - with patch("extract_versions.subprocess.run") as mock_run: + with patch("extract_versions.subprocess.run", autospec=True) as mock_run: result = git_cat_file_batch("providers-test/1.0.0", []) assert result == {} @@ -420,9 +429,9 @@ def test_two_hits_with_different_sizes_and_multibyte_content(self): second_content = "café prôse with more text\n".encode() payload = _batch_hit("aaa1", "blob", first_content) + _batch_hit("bbb2", "blob", second_content) - mock_result = MagicMock() + mock_result = MagicMock(spec=subprocess.CompletedProcess) mock_result.stdout = payload - with patch("extract_versions.subprocess.run", return_value=mock_result): + with patch("extract_versions.subprocess.run", autospec=True, return_value=mock_result): result = git_cat_file_batch(tag, paths) assert result == { @@ -435,9 +444,9 @@ def test_hit_followed_by_missing_path(self): paths = ["providers/test/docs/a.rst", "providers/test/docs/missing.rst"] payload = _batch_hit("aaa1", "blob", b"content\n") + _batch_missing(f"{tag}:{paths[1]}") - mock_result = MagicMock() + mock_result = MagicMock(spec=subprocess.CompletedProcess) mock_result.stdout = payload - with patch("extract_versions.subprocess.run", return_value=mock_result): + with patch("extract_versions.subprocess.run", autospec=True, return_value=mock_result): result = git_cat_file_batch(tag, paths) assert result == {paths[0]: "content\n"} @@ -447,9 +456,9 @@ def test_all_missing_returns_empty_dict(self): paths = ["providers/test/docs/a.rst", "providers/test/docs/b.rst"] payload = _batch_missing(f"{tag}:{paths[0]}") + _batch_missing(f"{tag}:{paths[1]}") - mock_result = MagicMock() + mock_result = MagicMock(spec=subprocess.CompletedProcess) mock_result.stdout = payload - with patch("extract_versions.subprocess.run", return_value=mock_result): + with patch("extract_versions.subprocess.run", autospec=True, return_value=mock_result): result = git_cat_file_batch(tag, paths) assert result == {} From 0893fd8fd2ecd987a096917d0f7a839b582bc220 Mon Sep 17 00:00:00 2001 From: Wei Lee Date: Mon, 28 Sep 2026 12:57:06 +0900 Subject: [PATCH 11/19] Link guide pages titled "Prose: ``Class``" to their class again The common.ai docs reorganization retitled its dedicated pages to lead with prose and end with the class name, so most of its modules lost their Guide link, and a subsection that happened to open with a name won over the page actually dedicated to it (HookToolset landed on the security guidelines). Older release tags keep the leading-literal titles, so both shapes are read. --- dev/registry/registry_tools/docs_guides.py | 80 +++++++---- dev/registry/tests/test_docs_guides.py | 148 ++++++++++++++++++--- registry/AGENTS.md | 22 +-- 3 files changed, 197 insertions(+), 53 deletions(-) diff --git a/dev/registry/registry_tools/docs_guides.py b/dev/registry/registry_tools/docs_guides.py index 5faa3174ab287..32c64af93da46 100644 --- a/dev/registry/registry_tools/docs_guides.py +++ b/dev/registry/registry_tools/docs_guides.py @@ -20,7 +20,9 @@ what the arguments are but not how the thing is meant to be used. The prose guides carry that, and they already mark it: a how-to guide documents one class (or a class and its task-flow decorator) per section, titled with the name(s) -(``HookToolset``, ``SQLToolset``, ``AgentOperator`` & ``@task.agent``). +either at the start (``HookToolset``, ``SQLToolset``, ``AgentOperator`` & +``@task.agent``) or after a colon at the very end, as in a section titled +"Airflow hooks as tools: ``HookToolset``". So the mapping is read back out of the guides rather than curated anywhere: a hand-maintained name-to-guide table would rot silently every time a guide is @@ -54,7 +56,7 @@ def is_guide_page(relative_path: str) -> bool: (``_api/hook/index.rst``, ``operators/_partials/foo.rst``, top-level ``_partials/foo.rst``): Sphinx/autoapi output and partials are directive markup, not the hand-written, reST-underlined titles this module's - leading-inline-literal convention parses. + inline-literal title convention parses. - ``changelog.rst`` and ``commits.rst``: real release-note pages, not how-to guides, that can carry inline-literal-formatted headings by coincidence. @@ -71,13 +73,22 @@ def is_guide_page(relative_path: str) -> bool: _INLINE_LITERAL_NAME = r"``(@?[A-Za-z_][A-Za-z0-9_]*(?:\.[A-Za-z_][A-Za-z0-9_]*)*)``" _INLINE_LITERAL_NAME_RE = re.compile(_INLINE_LITERAL_NAME) -# Only titles opening with a run of inline-literal names are treated as -# documenting them, so prose headings ("Bounded query results") never produce a -# link. A run is one or more names joined by "&", "," or "/" -- how guides write -# a section that covers both an operator and its decorator -# (``AgentOperator`` & ``@task.agent``). The run stops at the first thing that -# is neither a name nor a separator, so it never reaches into prose. -_LEADING_LITERAL_NAME_RUN = re.compile(rf"^{_INLINE_LITERAL_NAME}(?:\s*[&,/]\s*{_INLINE_LITERAL_NAME})*") +# Only titles that either open with, or end a colon-led clause with, a run of +# inline-literal names are treated as documenting them, so prose headings +# ("Bounded query results") never produce a link. A run is one or more names +# joined by "&", ",", "/" or "and" -- how guides write a section that covers both +# an operator and its decorator (``AgentOperator`` & ``@task.agent``). +_NAME_SEPARATOR = r"(?:\s*[&,/]\s*|\s+and\s+)" +_NAME_RUN = rf"{_INLINE_LITERAL_NAME}(?:{_NAME_SEPARATOR}{_INLINE_LITERAL_NAME})*" + +# Shape one (predates #73523, and still what older release tags' docs use): +# the title opens with the name run. The run stops at the first thing that is +# neither a name nor a separator, so it never reaches into prose. +_LEADING_LITERAL_NAME_RUN = re.compile(rf"^{_NAME_RUN}") +# Shape two (since #73523): a prose lead-in, a colon, then the name run runs to +# the very end of the title. Anchoring to "$" is what keeps a colon earlier in +# the title, with prose after it, from being mistaken for this shape. +_TRAILING_LITERAL_NAME_RUN = re.compile(rf":\s+{_NAME_RUN}\s*$") def slugify_section_anchor(title: str) -> str: @@ -91,17 +102,21 @@ def slugify_section_anchor(title: str) -> str: return re.sub(r"[^a-z0-9]+", "-", title.lower()).strip("-") -def _extract_leading_names_from_title(title: str) -> list[str]: - """Return the names a section title leads with, or [] if it names prose. +def _extract_names_from_title(title: str) -> list[str]: + """Return the names a section title documents, or [] if it names prose. - A guide marks a section as being *about* one or more names by opening its - title with them as inline literals -- ``HookToolset``, or - ``AgentOperator`` & ``@task.agent`` where one section covers the operator and - its decorator. Requiring that markup is what keeps a single-word prose heading - ("Guidelines") from claiming to document a class of the same name, and it is a + A guide marks a section as being *about* one or more names by titling it + with them as inline literals, either at the start (``HookToolset``, or + ``AgentOperator`` & ``@task.agent`` where one section covers the operator + and its decorator) or after a colon at the very end ("Airflow hooks as + tools: ``HookToolset``"). The leading shape is tried first, since a title + that satisfies both (e.g. "``A``: ``B``") should still only name the class + it actually opens with. Requiring that markup is what keeps a single-word + prose heading ("Guidelines") -- or a literal appearing elsewhere in a prose + title -- from claiming to document a class of the same name, and it is a convention the guides already follow rather than one imposed on them. """ - match = _LEADING_LITERAL_NAME_RUN.match(title) + match = _LEADING_LITERAL_NAME_RUN.match(title) or _TRAILING_LITERAL_NAME_RUN.search(title) return _INLINE_LITERAL_NAME_RE.findall(match.group(0)) if match else [] @@ -131,21 +146,30 @@ def collect_guide_anchors(docs: Mapping[str, str]) -> dict[str, str]: """Map name -> ``.html#`` for every documented class or decorator. ``docs`` maps a page path relative to the provider's docs directory (e.g. - ``toolsets.rst``) to its reST source. A title can lead with more than one name - (``AgentOperator`` & ``@task.agent``), in which case every leading name gets - the same anchor. When two pages document the same name, the first page in - sorted order wins, so a rebuild of the same sources always produces the same - link. + ``toolsets.rst``) to its reST source. A title can name more than one name + (``AgentOperator`` & ``@task.agent``), in which case every one gets the + same anchor. When two pages document the same name: a page's own title + (its first section) beats a subsection found on any other page, since that + page is the one dedicated to the class; among two page titles -- or two + subsections neither page titles -- the first page in sorted order wins, so + a rebuild of the same sources always produces the same link. "Page title" + is simply the first title _extract_section_titles finds, not a checked + top-level adornment, so a heading-shaped block earlier on the page (say, + inside a directive) would take that role. """ - anchors: dict[str, str] = {} + found: dict[str, tuple[bool, str]] = {} # name -> (from a page title?, anchor) for page in sorted(docs): page_url = re.sub(r"\.rst$", ".html", page) - for title in _extract_section_titles(docs[page]): - for name in _extract_leading_names_from_title(title): - if name in anchors: + for index, title in enumerate(_extract_section_titles(docs[page])): + is_page_title = index == 0 + for name in _extract_names_from_title(title): + current = found.get(name) + # A page title replaces a subsection found earlier; nothing else + # replaces what was found first, so rebuilds stay deterministic. + if current is not None and (current[0] or not is_page_title): continue - anchors[name] = f"{page_url}#{slugify_section_anchor(title)}" - return anchors + found[name] = (is_page_title, f"{page_url}#{slugify_section_anchor(title)}") + return {name: anchor for name, (_, anchor) in found.items()} def attach_guide_urls(modules: list[dict[str, Any]], anchors: Mapping[str, str], base_docs_url: str) -> int: diff --git a/dev/registry/tests/test_docs_guides.py b/dev/registry/tests/test_docs_guides.py index 975db3366a50a..2fa48710af8bb 100644 --- a/dev/registry/tests/test_docs_guides.py +++ b/dev/registry/tests/test_docs_guides.py @@ -36,8 +36,8 @@ Intro prose. -``HookToolset`` ---------------- +Airflow hooks as tools: ``HookToolset`` +---------------------------------------- How to use it. @@ -53,8 +53,8 @@ ``SQLToolset`` bounds that. -``DataFusionToolset`` ---------------------- +Files with DataFusion: ``DataFusionToolset`` +----------------------------------------------- Another one. """ @@ -81,8 +81,8 @@ def test_collect_guide_anchors_finds_class_named_sections_at_any_depth(): anchors = collect_guide_anchors({"toolsets.rst": TOOLSETS_GUIDE}) assert anchors == { - "HookToolset": "toolsets.html#hooktoolset", - "DataFusionToolset": "toolsets.html#datafusiontoolset", + "HookToolset": "toolsets.html#airflow-hooks-as-tools-hooktoolset", + "DataFusionToolset": "toolsets.html#files-with-datafusion-datafusiontoolset", } @@ -101,25 +101,29 @@ def test_collect_guide_anchors_ignores_classes_only_mentioned_in_prose(): def test_collect_guide_anchors_keeps_nested_page_paths(): - guide = "``AgentOperator``\n-----------------\n\nProse.\n" + guide = "Agents with tools: ``AgentOperator``\n-------------------------------------\n\nProse.\n" assert collect_guide_anchors({"operators/agent.rst": guide}) == { - "AgentOperator": "operators/agent.html#agentoperator" + "AgentOperator": "operators/agent.html#agents-with-tools-agentoperator" } -def test_collect_guide_anchors_prefers_first_page_in_sorted_order(): - guide = "``SQLToolset``\n--------------\n\nProse.\n" +def test_collect_guide_anchors_prefers_first_sorted_page_among_page_titles(): + # Both pages title themselves after SQLToolset, so neither title beats the + # other on that basis alone; the tie is broken by sorted page order. + guide = "SQL databases: ``SQLToolset``\n------------------------------\n\nProse.\n" anchors = collect_guide_anchors({"toolsets.rst": guide, "operators/sql.rst": guide}) - assert anchors["SQLToolset"] == "operators/sql.html#sqltoolset" + assert anchors["SQLToolset"] == "operators/sql.html#sql-databases-sqltoolset" def test_collect_guide_anchors_handles_a_title_covering_more_than_the_class(): # Verified against the published guide: this heading is served at # .../operators/agent.html#agentoperator-task-agent, so the anchor comes from # the whole title while both the operator and its decorator get linked to it. + # This is the leading shape from before #73523; older release tags' docs + # (read by extract_versions.py) still use it, so it must keep working. guide = "``AgentOperator`` & ``@task.agent``\n===================================\n\nProse.\n" assert collect_guide_anchors({"operators/agent.rst": guide}) == { @@ -131,7 +135,8 @@ def test_collect_guide_anchors_handles_a_title_covering_more_than_the_class(): def test_collect_guide_anchors_links_a_decorator_name_with_underscores(): # ``@task.llm_file_analysis`` is the real decorator name for the common.ai # provider's LLMFileAnalysisOperator; the leading-literal charset must admit - # "@" and "." for it to ever get a link. + # "@" and "." for it to ever get a link. Also a leading-shape fixture kept for + # the same reason as the test above: older release tags' docs still use it. guide = ( "``LLMFileAnalysisOperator`` & ``@task.llm_file_analysis``\n" "==========================================================\n\n" @@ -149,9 +154,9 @@ def test_collect_guide_anchors_links_a_decorator_name_with_underscores(): def test_collect_guide_anchors_ignores_a_prose_title_mentioning_a_literal(): - # The title doesn't *open* with the literal, so nothing after "Using" should - # ever be scanned for further inline literals -- a prose heading that happens - # to mention one in passing must not produce a link. + # The title neither opens with the literal nor ends a colon-led clause with + # it, so a prose heading that happens to mention one in passing -- anywhere + # in the title -- must not produce a link. guide = "Using ``foo`` in a pipeline\n============================\n\nProse.\n" assert collect_guide_anchors({"toolsets.rst": guide}) == {} @@ -160,7 +165,118 @@ def test_collect_guide_anchors_ignores_a_prose_title_mentioning_a_literal(): def test_collect_guide_anchors_requires_a_long_enough_underline(): # An underline shorter than the title isn't a section in reST, so it must not # produce a link to an anchor Sphinx never emitted. - assert collect_guide_anchors({"toolsets.rst": "``HookToolset``\n---\n\nProse.\n"}) == {} + guide = "Airflow hooks as tools: ``HookToolset``\n---\n\nProse.\n" + + assert collect_guide_anchors({"toolsets.rst": guide}) == {} + + +def test_collect_guide_anchors_reads_a_name_trailing_a_colon(): + guide = "Airflow hooks as tools: ``HookToolset``\n========================================\n\nProse.\n" + + assert collect_guide_anchors({"toolsets/hook.rst": guide}) == { + "HookToolset": "toolsets/hook.html#airflow-hooks-as-tools-hooktoolset" + } + + +def test_collect_guide_anchors_reads_two_names_joined_by_and(): + guide = ( + "Agents with tools: ``AgentOperator`` and ``@task.agent``\n" + "=========================================================\n\n" + "Prose.\n" + ) + + assert collect_guide_anchors({"operators/agent.rst": guide}) == { + "AgentOperator": "operators/agent.html#agents-with-tools-agentoperator-and-task-agent", + "@task.agent": "operators/agent.html#agents-with-tools-agentoperator-and-task-agent", + } + + +@pytest.mark.parametrize( + "title", + [ + # Leading shape, one separator per case. + "``A`` & ``B``", + "``A``, ``B``", + "``A``/``B``", + "``A`` and ``B``", + # Trailing shape, one separator per case. + "Both: ``A`` & ``B``", + "Both: ``A``, ``B``", + "Both: ``A``/``B``", + "Both: ``A`` and ``B``", + ], +) +def test_collect_guide_anchors_accepts_every_separator_in_both_title_shapes(title): + underline = "=" * (len(title) + 1) + + anchors = collect_guide_anchors({"page.rst": f"{title}\n{underline}\n\nProse.\n"}) + + expected_anchor = f"page.html#{slugify_section_anchor(title)}" + assert anchors == {"A": expected_anchor, "B": expected_anchor} + + +def test_collect_guide_anchors_prefers_a_page_title_over_an_earlier_pages_subsection(): + # The reviewer-reported case: a subsection on an unrelated page happens to + # be titled after the class, but a later page is dedicated to it. + docs = { + "agent_security.rst": ( + "Securing agent tools\n" + "=====================\n\n" + "``HookToolset`` guidelines\n" + "^^^^^^^^^^^^^^^^^^^^^^^^^^^\n\n" + "Prose.\n" + ), + "toolsets/hook.rst": ( + "Airflow hooks as tools: ``HookToolset``\n========================================\n\nProse.\n" + ), + } + + assert collect_guide_anchors(docs) == { + "HookToolset": "toolsets/hook.html#airflow-hooks-as-tools-hooktoolset" + } + + +def test_collect_guide_anchors_prefers_the_page_title_over_its_own_subsection(): + guide = ( + "Batch processing: ``LLMBatchOperator``\n" + "=======================================\n\n" + "How it works.\n\n" + "``LLMBatchOperator`` or the vendor batch operators?\n" + "----------------------------------------------------\n\n" + "Prose.\n" + ) + + assert collect_guide_anchors({"operators/llm_batch.rst": guide}) == { + "LLMBatchOperator": "operators/llm_batch.html#batch-processing-llmbatchoperator" + } + + +def test_collect_guide_anchors_keeps_the_first_subsection_when_no_page_title_names_it(): + docs = { + "a.rst": "Prose page\n===========\n\n``X`` notes\n------------\n", + "b.rst": "Other page\n===========\n\n``X`` details\n--------------\n", + } + + assert collect_guide_anchors(docs) == {"X": "a.html#x-notes"} + + +@pytest.mark.parametrize( + "title", + [ + # Colon present, but prose follows it before the literal -- must not be + # scanned for a literal anywhere after the colon. + "Role in a Dag: use ``MCPToolset``, not the hook directly", + # Title ends with the literal, but there's no colon to lead it. + "Using HITL review with ``AgentOperator``", + # Colon immediately precedes the literal, but prose follows it -- the + # literal doesn't reach the end of the title. + "Guidelines: ``HookToolset`` and its allow-list", + ], +) +def test_collect_guide_anchors_ignores_a_literal_that_does_not_end_the_title(title): + underline = "=" * (len(title) + 1) + + assert collect_guide_anchors({"toolsets.rst": f"{title}\n{underline}\n\nProse.\n"}) == {} def test_attach_guide_urls_only_links_documented_classes(): diff --git a/registry/AGENTS.md b/registry/AGENTS.md index f1a192cf27ccd..b82377a2569cc 100644 --- a/registry/AGENTS.md +++ b/registry/AGENTS.md @@ -469,10 +469,14 @@ generated API reference. `provider.yaml`'s `how-to-guide` fields are CI-enforced by `check_doc_files`, but they name a whole page, never a section, and cover only operators, sensors and transfers — not the toolset, hook and decorator pages this needs. So `registry_tools/docs_guides.py` reads the provider's own `docs/*.rst` -and matches a class to a section when the section's title *opens with the class -name as an inline literal* — ``` ``HookToolset`` ``` or ``` ``AgentOperator`` & -``@task.agent`` ```. The anchor is derived from the whole title the way docutils -derives its HTML id. +and matches a class to a section when the section's title names it as an inline +literal, either at the start — ``` ``MCPHook`` ``` or after a colon at the very +end — ``` Airflow hooks as tools: ``HookToolset`` ``` or ``` Agents with tools: +``AgentOperator`` and ``@task.agent`` ```. A literal elsewhere in a prose title +does not count. The anchor is derived from the whole title the way docutils +derives its HTML id. When a name is titled in more than one place, a page's own +title wins over a subsection on any other page, so the link lands on the page +dedicated to the class rather than on a passing section about it. That convention is what the guides already do, and it is deliberately the only signal this resolves a section from: a hand-maintained class-to-guide table @@ -480,11 +484,11 @@ would keep pointing at sections that have since been renamed or split, and a link that lands on the wrong section is worse than no link. A class documented only in prose gets no Guide link. -Growing the set of modules that get a Guide link means changing that provider's -section titles to lead with an inline literal, not touching this extractor. -`common/ai` follows the convention most thoroughly; a couple of other providers -use the same title shape for a config option name or a single decorator rather -than a class. Having the right title doesn't guarantee a link — that still +Growing the set of modules that get a Guide link means titling that provider's +sections in one of those two shapes, not touching this extractor. `common/ai` +titles its dedicated operator, hook and toolset pages this way; a couple of other +providers use the same shapes for a config option name or a single decorator +rather than a class. Having the right title doesn't guarantee a link — that still depends on a same-named module existing in the catalog. Both extraction paths resolve it — `extract_parameters.py` from the working tree From e6a4182e50e2178c3a983e4b3111fdb46eac6c5d Mon Sep 17 00:00:00 2001 From: Wei Lee Date: Mon, 28 Sep 2026 23:08:36 +0900 Subject: [PATCH 12/19] Give common.ai's remaining task decorators a guide link @task.llm_file_analysis, @task.llm_sql, @task.llm_branch and @task.llm_schema_compare were documented only under an untitled "TaskFlow Decorator" subsection, so the registry found no section naming them. Each page's title now names its decorator alongside its operator, as the LLMOperator and AgentOperator pages already do. --- providers/common/ai/docs/operators/llm_branch.rst | 4 ++-- providers/common/ai/docs/operators/llm_file_analysis.rst | 4 ++-- providers/common/ai/docs/operators/llm_schema_compare.rst | 4 ++-- providers/common/ai/docs/operators/llm_sql.rst | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/providers/common/ai/docs/operators/llm_branch.rst b/providers/common/ai/docs/operators/llm_branch.rst index 35b15f0f2b970..0e113aed05028 100644 --- a/providers/common/ai/docs/operators/llm_branch.rst +++ b/providers/common/ai/docs/operators/llm_branch.rst @@ -17,8 +17,8 @@ .. _howto/operator:llm_branch: -Branch on an answer: ``LLMBranchOperator`` -========================================== +Branch on an answer: ``LLMBranchOperator`` and ``@task.llm_branch`` +=================================================================== Use :class:`~airflow.providers.common.ai.operators.llm_branch.LLMBranchOperator` for LLM-driven branching, where the LLM decides which downstream task(s) to diff --git a/providers/common/ai/docs/operators/llm_file_analysis.rst b/providers/common/ai/docs/operators/llm_file_analysis.rst index 4fb0931410d77..eed8c86628189 100644 --- a/providers/common/ai/docs/operators/llm_file_analysis.rst +++ b/providers/common/ai/docs/operators/llm_file_analysis.rst @@ -17,8 +17,8 @@ .. _howto/operator:llm_file_analysis: -Analyze files and images: ``LLMFileAnalysisOperator`` -===================================================== +Analyze files and images: ``LLMFileAnalysisOperator`` and ``@task.llm_file_analysis`` +===================================================================================== .. note:: diff --git a/providers/common/ai/docs/operators/llm_schema_compare.rst b/providers/common/ai/docs/operators/llm_schema_compare.rst index cafde3900de5e..47b06cf22255f 100644 --- a/providers/common/ai/docs/operators/llm_schema_compare.rst +++ b/providers/common/ai/docs/operators/llm_schema_compare.rst @@ -17,8 +17,8 @@ .. _howto/operator:llm_schema_compare: -Detect schema drift: ``LLMSchemaCompareOperator`` -================================================= +Detect schema drift: ``LLMSchemaCompareOperator`` and ``@task.llm_schema_compare`` +================================================================================== .. note:: diff --git a/providers/common/ai/docs/operators/llm_sql.rst b/providers/common/ai/docs/operators/llm_sql.rst index 5d06ec97e3978..42e51de795582 100644 --- a/providers/common/ai/docs/operators/llm_sql.rst +++ b/providers/common/ai/docs/operators/llm_sql.rst @@ -17,8 +17,8 @@ .. _howto/operator:llm_sql_query: -Natural language to SQL: ``LLMSQLQueryOperator`` -================================================ +Natural language to SQL: ``LLMSQLQueryOperator`` and ``@task.llm_sql`` +====================================================================== .. note:: From 763245f16a5ca3504cb87a6fc4a0f5098a51c665 Mon Sep 17 00:00:00 2001 From: Wei Lee Date: Tue, 6 Oct 2026 17:40:54 +0100 Subject: [PATCH 13/19] Stop linking parameter-table pages as a class's guide --- dev/registry/registry_tools/docs_guides.py | 13 ++++++------- dev/registry/tests/test_docs_guides.py | 12 +++++++++--- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/dev/registry/registry_tools/docs_guides.py b/dev/registry/registry_tools/docs_guides.py index 32c64af93da46..d28180222288a 100644 --- a/dev/registry/registry_tools/docs_guides.py +++ b/dev/registry/registry_tools/docs_guides.py @@ -82,9 +82,10 @@ def is_guide_page(relative_path: str) -> bool: _NAME_RUN = rf"{_INLINE_LITERAL_NAME}(?:{_NAME_SEPARATOR}{_INLINE_LITERAL_NAME})*" # Shape one (predates #73523, and still what older release tags' docs use): -# the title opens with the name run. The run stops at the first thing that is -# neither a name nor a separator, so it never reaches into prose. -_LEADING_LITERAL_NAME_RUN = re.compile(rf"^{_NAME_RUN}") +# the title is nothing but the name run. Anchoring to "$" keeps a title that +# merely opens with a literal and continues in prose ("``SandboxToolset`` +# parameters") from claiming to document that class. +_LEADING_LITERAL_NAME_RUN = re.compile(rf"^{_NAME_RUN}\s*$") # Shape two (since #73523): a prose lead-in, a colon, then the name run runs to # the very end of the title. Anchoring to "$" is what keeps a colon earlier in # the title, with prose after it, from being mistaken for this shape. @@ -106,12 +107,10 @@ def _extract_names_from_title(title: str) -> list[str]: """Return the names a section title documents, or [] if it names prose. A guide marks a section as being *about* one or more names by titling it - with them as inline literals, either at the start (``HookToolset``, or + with them as inline literals, either as the whole title (``HookToolset``, or ``AgentOperator`` & ``@task.agent`` where one section covers the operator and its decorator) or after a colon at the very end ("Airflow hooks as - tools: ``HookToolset``"). The leading shape is tried first, since a title - that satisfies both (e.g. "``A``: ``B``") should still only name the class - it actually opens with. Requiring that markup is what keeps a single-word + tools: ``HookToolset``"). Requiring that markup is what keeps a single-word prose heading ("Guidelines") -- or a literal appearing elsewhere in a prose title -- from claiming to document a class of the same name, and it is a convention the guides already follow rather than one imposed on them. diff --git a/dev/registry/tests/test_docs_guides.py b/dev/registry/tests/test_docs_guides.py index 2fa48710af8bb..081812b7b5889 100644 --- a/dev/registry/tests/test_docs_guides.py +++ b/dev/registry/tests/test_docs_guides.py @@ -108,6 +108,12 @@ def test_collect_guide_anchors_keeps_nested_page_paths(): } +def test_collect_guide_anchors_ignores_a_title_that_opens_with_a_name_then_continues_in_prose(): + guide = "``SandboxToolset`` parameters\n-----------------------------\n\nA parameter table.\n" + + assert collect_guide_anchors({"sandbox/configuration.rst": guide}) == {} + + def test_collect_guide_anchors_prefers_first_sorted_page_among_page_titles(): # Both pages title themselves after SQLToolset, so neither title beats the # other on that basis alone; the tie is broken by sorted page order. @@ -253,11 +259,11 @@ def test_collect_guide_anchors_prefers_the_page_title_over_its_own_subsection(): def test_collect_guide_anchors_keeps_the_first_subsection_when_no_page_title_names_it(): docs = { - "a.rst": "Prose page\n===========\n\n``X`` notes\n------------\n", - "b.rst": "Other page\n===========\n\n``X`` details\n--------------\n", + "a.rst": "Prose page\n===========\n\n``X``\n-----\n", + "b.rst": "Other page\n===========\n\n``X``\n-----\n", } - assert collect_guide_anchors(docs) == {"X": "a.html#x-notes"} + assert collect_guide_anchors(docs) == {"X": "a.html#x"} @pytest.mark.parametrize( From 3fa88f854457bef9b09b73aa7873aa681ba5f831 Mon Sep 17 00:00:00 2001 From: Wei Lee Date: Tue, 6 Oct 2026 17:41:42 +0100 Subject: [PATCH 14/19] Keep extract_versions module tests off real git and pin the .rst filter --- dev/registry/tests/test_extract_versions.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/dev/registry/tests/test_extract_versions.py b/dev/registry/tests/test_extract_versions.py index 2ddbaa0d288fb..56084de47d482 100644 --- a/dev/registry/tests/test_extract_versions.py +++ b/dev/registry/tests/test_extract_versions.py @@ -88,7 +88,8 @@ def test_no_other_candidates(self): } -def _extract_class_level_modules(provider_yaml: dict) -> list[dict]: +@patch("extract_versions.read_guide_docs", autospec=True, return_value={}) +def _extract_class_level_modules(provider_yaml: dict, _mock_read_guide_docs) -> list[dict]: return extract_modules_from_yaml( provider_yaml, tag="providers-test/1.0.0", @@ -334,6 +335,8 @@ def test_skips_generated_and_release_note_pages_before_calling_git_show(self): paths = [ docs_prefix + "_api/x/index.rst", docs_prefix + "changelog.rst", + docs_prefix + "diagram.png", + docs_prefix + "conf.py", docs_prefix + "toolsets.rst", ] From fe22544ec247fdf9895b2a4c7ba68eddc89ac95f Mon Sep 17 00:00:00 2001 From: Wei Lee Date: Tue, 6 Oct 2026 17:42:28 +0100 Subject: [PATCH 15/19] Tidy comments the guide-link work left behind --- dev/registry/extract_versions.py | 4 ++-- dev/registry/registry_contract_models.py | 3 ++- dev/registry/registry_tools/docs_guides.py | 4 ++-- dev/registry/tests/test_docs_guides.py | 23 +++++++------------ dev/registry/tests/test_extract_parameters.py | 3 --- dev/registry/tests/test_extract_versions.py | 10 +------- 6 files changed, 15 insertions(+), 32 deletions(-) diff --git a/dev/registry/extract_versions.py b/dev/registry/extract_versions.py index af3f5a94e205d..8c73192d8ff18 100644 --- a/dev/registry/extract_versions.py +++ b/dev/registry/extract_versions.py @@ -155,8 +155,8 @@ def git_cat_file_batch(tag: str, paths: list[str]) -> dict[str, str]: Decode failures are left unguarded on purpose: .rst files are Sphinx convention UTF-8, an explicit "utf-8" decode is more predictable than following the process locale, and a UnicodeDecodeError should surface loudly - rather than being swallowed -- the same posture git_show takes toward - CalledProcessError (fails loud, doesn't paper over bad data). + rather than being swallowed. A failing ``git cat-file`` call also raises + (``check=True``); only git_show turns CalledProcessError into ``None``. """ if not paths: return {} diff --git a/dev/registry/registry_contract_models.py b/dev/registry/registry_contract_models.py index d9b351b30f396..02eb221b1ce10 100644 --- a/dev/registry/registry_contract_models.py +++ b/dev/registry/registry_contract_models.py @@ -146,7 +146,8 @@ class ModuleContract(BaseModel): provider_name: str | None = None supports_durable_execution: bool = False supports_deferrable: bool = False - # Only set for classes a how-to guide documents in a section of their own. + # Only set for classes and task decorators (e.g. ``@task.agent``) that a how-to + # guide documents in a section of their own. guide_url: str | None = None diff --git a/dev/registry/registry_tools/docs_guides.py b/dev/registry/registry_tools/docs_guides.py index d28180222288a..27a742bf88f56 100644 --- a/dev/registry/registry_tools/docs_guides.py +++ b/dev/registry/registry_tools/docs_guides.py @@ -81,12 +81,12 @@ def is_guide_page(relative_path: str) -> bool: _NAME_SEPARATOR = r"(?:\s*[&,/]\s*|\s+and\s+)" _NAME_RUN = rf"{_INLINE_LITERAL_NAME}(?:{_NAME_SEPARATOR}{_INLINE_LITERAL_NAME})*" -# Shape one (predates #73523, and still what older release tags' docs use): +# Shape one (what older release tags' docs use): # the title is nothing but the name run. Anchoring to "$" keeps a title that # merely opens with a literal and continues in prose ("``SandboxToolset`` # parameters") from claiming to document that class. _LEADING_LITERAL_NAME_RUN = re.compile(rf"^{_NAME_RUN}\s*$") -# Shape two (since #73523): a prose lead-in, a colon, then the name run runs to +# Shape two (what current docs use): a prose lead-in, a colon, then the name run runs to # the very end of the title. Anchoring to "$" is what keeps a colon earlier in # the title, with prose after it, from being mistaken for this shape. _TRAILING_LITERAL_NAME_RUN = re.compile(rf":\s+{_NAME_RUN}\s*$") diff --git a/dev/registry/tests/test_docs_guides.py b/dev/registry/tests/test_docs_guides.py index 081812b7b5889..16044aa13c3c0 100644 --- a/dev/registry/tests/test_docs_guides.py +++ b/dev/registry/tests/test_docs_guides.py @@ -128,8 +128,8 @@ def test_collect_guide_anchors_handles_a_title_covering_more_than_the_class(): # Verified against the published guide: this heading is served at # .../operators/agent.html#agentoperator-task-agent, so the anchor comes from # the whole title while both the operator and its decorator get linked to it. - # This is the leading shape from before #73523; older release tags' docs - # (read by extract_versions.py) still use it, so it must keep working. + # This is the older leading shape; older release tags' docs (read by + # extract_versions.py) still use it, so it must keep working. guide = "``AgentOperator`` & ``@task.agent``\n===================================\n\nProse.\n" assert collect_guide_anchors({"operators/agent.rst": guide}) == { @@ -222,8 +222,8 @@ def test_collect_guide_anchors_accepts_every_separator_in_both_title_shapes(titl def test_collect_guide_anchors_prefers_a_page_title_over_an_earlier_pages_subsection(): - # The reviewer-reported case: a subsection on an unrelated page happens to - # be titled after the class, but a later page is dedicated to it. + # A subsection on an unrelated page happens to be titled after the class, + # but a later page is dedicated to it. docs = { "agent_security.rst": ( "Securing agent tools\n" @@ -315,16 +315,12 @@ def test_attach_guide_urls_does_not_double_up_the_base_separator(): @pytest.mark.parametrize( ("relative_path", "expected"), [ - # A `_`-prefixed path segment marks autoapi/partial content, at any - # depth. Mutation canary: removing the `_` check turns these four - # from False back to True. + # A `_`-prefixed path segment marks autoapi/partial content, at any depth. ("_api/index.rst", False), ("_api/hook/index.rst", False), ("operators/_partials/foo.rst", False), ("_partials/foo.rst", False), - # Real, built release-note pages, not how-to guides. Mutation canary: - # removing the changelog/commits check turns these two from False - # back to True. + # Real, built release-note pages, not how-to guides. ("changelog.rst", False), ("commits.rst", False), ("toolsets.rst", True), @@ -362,9 +358,6 @@ def test_readers_agree_on_which_pages_are_guides(tmp_path): ): from_tag = read_guide_docs_from_tag("providers-test/1.0.0", "new", "test") - # Mutation canary: reverting the `is_guide_page` call in only one of the - # two readers (e.g. extract_parameters.read_guide_docs but not - # extract_versions.read_guide_docs) fails this assertion even though each - # reader's own test (test_extract_parameters / test_extract_versions) may - # still pass on its own. + # Both readers must apply the same filter; each reader's own test can pass + # while the two drift apart. assert set(from_worktree) == set(from_tag) == {"toolsets.rst"} diff --git a/dev/registry/tests/test_extract_parameters.py b/dev/registry/tests/test_extract_parameters.py index c67b8449e3166..a6bfad26f8930 100644 --- a/dev/registry/tests/test_extract_parameters.py +++ b/dev/registry/tests/test_extract_parameters.py @@ -923,9 +923,6 @@ def test_read_guide_docs_skips_generated_and_release_note_pages(tmp_path): result = read_guide_docs(tmp_path) - # Mutation canary: if the `is_guide_page` filter in read_guide_docs is - # removed, this dict grows two more keys -- "_api/x/index.rst" and - # "changelog.rst" -- and this assertion goes red. assert set(result) == {"toolsets.rst"} diff --git a/dev/registry/tests/test_extract_versions.py b/dev/registry/tests/test_extract_versions.py index 56084de47d482..7ecc7d94dff6e 100644 --- a/dev/registry/tests/test_extract_versions.py +++ b/dev/registry/tests/test_extract_versions.py @@ -350,21 +350,13 @@ def test_skips_generated_and_release_note_pages_before_calling_git_show(self): ): result = read_guide_docs("providers-test/1.0.0", "new", "test") - # Mutation canary: if the `is_guide_page` filter in read_guide_docs is - # removed, this dict grows two more keys and this assertion goes red. assert set(result) == {"toolsets.rst"} - # Mutation canary: without the filter, the two skipped paths would also - # be included in the batch call's path list -- this assertion goes red - # too, proving the filtering runs before the batch call, not just - # before the dict write. + # Filtering must happen before the batch call, not just before the dict write. assert mock_git_cat_file_batch.call_args_list == [ call("providers-test/1.0.0", [docs_prefix + "toolsets.rst"]) ] def test_skips_a_page_whose_content_is_an_empty_string(self): - # Mutation canary: replacing `batch_result.get(full_path)` with - # `full_path in batch_result` in read_guide_docs's comprehension makes - # this page reappear as `{"empty.rst": ""}`, turning this assertion red. docs_prefix = "providers/test/docs/" paths = [docs_prefix + "empty.rst"] From cac4877719cc8a9e3cd1e7449c4c59df91b1bfd7 Mon Sep 17 00:00:00 2001 From: Wei Lee Date: Tue, 6 Oct 2026 17:53:19 +0100 Subject: [PATCH 16/19] Pin that a colon title names only its trailing literal run --- dev/registry/registry_tools/docs_guides.py | 2 +- dev/registry/tests/test_docs_guides.py | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/dev/registry/registry_tools/docs_guides.py b/dev/registry/registry_tools/docs_guides.py index 27a742bf88f56..d19f97757040f 100644 --- a/dev/registry/registry_tools/docs_guides.py +++ b/dev/registry/registry_tools/docs_guides.py @@ -73,7 +73,7 @@ def is_guide_page(relative_path: str) -> bool: _INLINE_LITERAL_NAME = r"``(@?[A-Za-z_][A-Za-z0-9_]*(?:\.[A-Za-z_][A-Za-z0-9_]*)*)``" _INLINE_LITERAL_NAME_RE = re.compile(_INLINE_LITERAL_NAME) -# Only titles that either open with, or end a colon-led clause with, a run of +# Only titles that consist solely of, or end a colon-led clause with, a run of # inline-literal names are treated as documenting them, so prose headings # ("Bounded query results") never produce a link. A run is one or more names # joined by "&", ",", "/" or "and" -- how guides write a section that covers both diff --git a/dev/registry/tests/test_docs_guides.py b/dev/registry/tests/test_docs_guides.py index 16044aa13c3c0..53fcfe4367d89 100644 --- a/dev/registry/tests/test_docs_guides.py +++ b/dev/registry/tests/test_docs_guides.py @@ -114,6 +114,12 @@ def test_collect_guide_anchors_ignores_a_title_that_opens_with_a_name_then_conti assert collect_guide_anchors({"sandbox/configuration.rst": guide}) == {} +def test_collect_guide_anchors_names_only_the_trailing_run_of_a_colon_title(): + guide = "``A``: ``B``\n-------------\n\nProse.\n" + + assert collect_guide_anchors({"page.rst": guide}) == {"B": "page.html#a-b"} + + def test_collect_guide_anchors_prefers_first_sorted_page_among_page_titles(): # Both pages title themselves after SQLToolset, so neither title beats the # other on that basis alone; the tie is broken by sorted page order. From 8a9c8e4abadb8ea1f98fbf36fc8602688867ba86 Mon Sep 17 00:00:00 2001 From: Wei Lee Date: Tue, 6 Oct 2026 17:41:38 +0100 Subject: [PATCH 17/19] Build registry guide links from the docs at the provider's release tag Guide links point at /stable, which serves released docs, so the anchors now come from the docs at the provider's release tag when it exists, instead of a working tree that may be ahead of it. --- dev/registry/extract_parameters.py | 25 ++++++++++++++- dev/registry/tests/test_extract_parameters.py | 31 +++++++++++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/dev/registry/extract_parameters.py b/dev/registry/extract_parameters.py index 8d326f05015cc..ab0f7791c870a 100644 --- a/dev/registry/extract_parameters.py +++ b/dev/registry/extract_parameters.py @@ -55,6 +55,7 @@ import yaml from extract_metadata import fetch_provider_inventory, read_inventory +from extract_versions import detect_layout, git_tag_exists, read_guide_docs as read_guide_docs_at_tag from registry_contract_models import validate_modules_catalog, validate_provider_parameters from registry_tools.docs_guides import attach_guide_urls, collect_guide_anchors, is_guide_page from registry_tools.types import ( @@ -753,6 +754,26 @@ def read_guide_docs(docs_dir: Path) -> dict[str, str]: return docs +def read_released_guide_docs( + provider_id: str, version: str, provider_rel_path: Path +) -> dict[str, str] | None: + """Read a provider's guide docs at its release tag, or None when there is no tag to read. + + The guide links point at ``/stable``, which serves the released docs, so the + anchors must come from the same content; the working tree may be ahead of it. + """ + if not version: + return None + tag = f"providers-{provider_id}/{version}" + if not git_tag_exists(tag): + return None + dir_path = provider_rel_path.as_posix() + layout = detect_layout(tag, dir_path) + if layout is None: + return None + return read_guide_docs_at_tag(tag, layout, dir_path) + + def discover_classes_from_provider( provider_yaml_path: Path, base_classes: dict[str, type], @@ -1001,7 +1022,9 @@ def make_entry( } ) - guide_docs = read_guide_docs(provider_yaml_path.parent / "docs") + guide_docs = read_released_guide_docs(provider_id, version, provider_rel_path) + if guide_docs is None: + guide_docs = read_guide_docs(provider_yaml_path.parent / "docs") attach_guide_urls(discovered, collect_guide_anchors(guide_docs), base_docs_url) return discovered diff --git a/dev/registry/tests/test_extract_parameters.py b/dev/registry/tests/test_extract_parameters.py index a6bfad26f8930..767e0d0a2ba4c 100644 --- a/dev/registry/tests/test_extract_parameters.py +++ b/dev/registry/tests/test_extract_parameters.py @@ -1034,6 +1034,37 @@ def test_guide_section_becomes_a_guide_url(self, provider_yaml_path, base_classe ) assert "guide_url" not in by_name["FakeSensor"] + @pytest.mark.parametrize( + ("tag_exists", "expected_anchor"), + [ + pytest.param(True, "fakeoperator-released-title", id="tag-exists-reads-released-docs"), + pytest.param(False, "fakeoperator", id="no-tag-reads-working-tree"), + ], + ) + def test_guide_docs_come_from_release_tag_when_it_exists( + self, provider_yaml_path, base_classes, tag_exists, expected_anchor + ): + docs_dir = provider_yaml_path.parent / "docs" / "operators" + docs_dir.mkdir(parents=True) + (docs_dir / "s3.rst").write_text("``FakeOperator``\n----------------\n\nUnreleased title.\n") + released = { + "operators/s3.rst": "``FakeOperator``: Released title\n--------------------------------\n" + } + + with ( + patch("extract_parameters.PROVIDERS_DIR", provider_yaml_path.parent.parent), + patch("extract_parameters.git_tag_exists", return_value=tag_exists) as tag_check, + patch("extract_parameters.detect_layout", return_value="new"), + patch("extract_parameters.read_guide_docs_at_tag", return_value=released) as read_at_tag, + patch("extract_parameters.importlib.import_module", side_effect=self._mock_import), + ): + result = discover_classes_from_provider(provider_yaml_path, base_classes, version="1.2.3") + + tag_check.assert_called_once_with("providers-amazon/1.2.3") + assert read_at_tag.called is tag_exists + guide_url = {r["name"]: r for r in result}["FakeOperator"]["guide_url"] + assert guide_url.endswith(f"/operators/s3.html#{expected_anchor}") + def test_discovers_sensor(self, provider_yaml_path, base_classes): with ( patch("extract_parameters.PROVIDERS_DIR", provider_yaml_path.parent.parent), From 3e2758f49d5964824f9fac5ad8c452f527de36db Mon Sep 17 00:00:00 2001 From: Wei Lee Date: Tue, 6 Oct 2026 17:44:41 +0100 Subject: [PATCH 18/19] Pin the release-tag guide docs call and cover both fallbacks to the working tree --- dev/registry/extract_parameters.py | 2 ++ dev/registry/tests/test_extract_parameters.py | 33 ++++++++++++++++++- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/dev/registry/extract_parameters.py b/dev/registry/extract_parameters.py index ab0f7791c870a..ad3813a4bd21d 100644 --- a/dev/registry/extract_parameters.py +++ b/dev/registry/extract_parameters.py @@ -761,6 +761,8 @@ def read_released_guide_docs( The guide links point at ``/stable``, which serves the released docs, so the anchors must come from the same content; the working tree may be ahead of it. + A tag from the old flat layout yields an empty dict, meaning no guide links + rather than a fallback to the working tree. """ if not version: return None diff --git a/dev/registry/tests/test_extract_parameters.py b/dev/registry/tests/test_extract_parameters.py index 767e0d0a2ba4c..13eac332c5d42 100644 --- a/dev/registry/tests/test_extract_parameters.py +++ b/dev/registry/tests/test_extract_parameters.py @@ -1061,10 +1061,41 @@ def test_guide_docs_come_from_release_tag_when_it_exists( result = discover_classes_from_provider(provider_yaml_path, base_classes, version="1.2.3") tag_check.assert_called_once_with("providers-amazon/1.2.3") - assert read_at_tag.called is tag_exists + if tag_exists: + read_at_tag.assert_called_once_with("providers-amazon/1.2.3", "new", "amazon") + else: + read_at_tag.assert_not_called() guide_url = {r["name"]: r for r in result}["FakeOperator"]["guide_url"] assert guide_url.endswith(f"/operators/s3.html#{expected_anchor}") + @pytest.mark.parametrize( + ("version", "layout", "expect_tag_lookup"), + [ + pytest.param("", "new", False, id="no-version-skips-tag-lookup"), + pytest.param("1.2.3", None, True, id="undetectable-layout-falls-back"), + ], + ) + def test_guide_docs_fall_back_to_working_tree( + self, provider_yaml_path, base_classes, version, layout, expect_tag_lookup + ): + docs_dir = provider_yaml_path.parent / "docs" / "operators" + docs_dir.mkdir(parents=True) + (docs_dir / "s3.rst").write_text("``FakeOperator``\n----------------\n\nProse.\n") + + with ( + patch("extract_parameters.PROVIDERS_DIR", provider_yaml_path.parent.parent), + patch("extract_parameters.git_tag_exists", return_value=True) as tag_check, + patch("extract_parameters.detect_layout", return_value=layout), + patch("extract_parameters.read_guide_docs_at_tag") as read_at_tag, + patch("extract_parameters.importlib.import_module", side_effect=self._mock_import), + ): + result = discover_classes_from_provider(provider_yaml_path, base_classes, version=version) + + assert tag_check.called is expect_tag_lookup + read_at_tag.assert_not_called() + guide_url = {r["name"]: r for r in result}["FakeOperator"]["guide_url"] + assert guide_url.endswith("/operators/s3.html#fakeoperator") + def test_discovers_sensor(self, provider_yaml_path, base_classes): with ( patch("extract_parameters.PROVIDERS_DIR", provider_yaml_path.parent.parent), From 72e40d0bb299e69a00ecade89b258565a6790e32 Mon Sep 17 00:00:00 2001 From: Wei Lee Date: Tue, 6 Oct 2026 18:11:57 +0100 Subject: [PATCH 19/19] Use the title shape the anchored guide matcher accepts in the release-tag test --- dev/registry/tests/test_extract_parameters.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dev/registry/tests/test_extract_parameters.py b/dev/registry/tests/test_extract_parameters.py index 13eac332c5d42..b751ae64f0893 100644 --- a/dev/registry/tests/test_extract_parameters.py +++ b/dev/registry/tests/test_extract_parameters.py @@ -1037,7 +1037,7 @@ def test_guide_section_becomes_a_guide_url(self, provider_yaml_path, base_classe @pytest.mark.parametrize( ("tag_exists", "expected_anchor"), [ - pytest.param(True, "fakeoperator-released-title", id="tag-exists-reads-released-docs"), + pytest.param(True, "released-title-fakeoperator", id="tag-exists-reads-released-docs"), pytest.param(False, "fakeoperator", id="no-tag-reads-working-tree"), ], ) @@ -1048,7 +1048,7 @@ def test_guide_docs_come_from_release_tag_when_it_exists( docs_dir.mkdir(parents=True) (docs_dir / "s3.rst").write_text("``FakeOperator``\n----------------\n\nUnreleased title.\n") released = { - "operators/s3.rst": "``FakeOperator``: Released title\n--------------------------------\n" + "operators/s3.rst": "Released title: ``FakeOperator``\n--------------------------------\n" } with (