Skip to content

Link registry modules to the guide section that documents them - #71477

Merged
Lee-W merged 19 commits into
apache:mainfrom
astronomer:registry-module-guide-links
Oct 7, 2026
Merged

Lee-W merged 19 commits into
apache:mainfrom
astronomer:registry-module-guide-links

Conversation

@Lee-W

@Lee-W Lee-W commented Aug 12, 2026 •

Copy link
Copy Markdown
Member

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.

image
Was generative AI tooling used to co-author this PR?
  • Yes (please specify the tool below)

Generated-by: [Claude] following the guidelines


  • Read the Pull Request Guidelines for more information. Note: commit author/co-author name and email in commits become permanently public when merged.
  • For fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
  • When adding dependency, check compliance with the ASF 3rd Party License Policy.
  • For significant user-facing changes create newsfragment: {pr_number}.significant.rst, in airflow-core/newsfragments. You can add this file in a follow-up commit after the PR is created so you know the PR number.

Comment thread dev/registry/registry_tools/docs_guides.py Outdated
Comment thread dev/registry/extract_parameters.py Outdated
Comment thread dev/registry/extract_versions.py
Comment thread registry/AGENTS.md
Comment thread dev/registry/extract_parameters.py
@Lee-W
Lee-W force-pushed the registry-module-guide-links branch from 78c01a0 to 72aeea4 Compare September 18, 2026 08:00
@kaxil

kaxil commented Sep 18, 2026

Copy link
Copy Markdown
Member

Round-1 items all landed, thanks. Four follow-ups and two nits, none blocking.

extract_versions.py: the is_guide_page filter runs before git_show now, but it removes very little at a release tag. At providers-amazon/9.36.0 the docs tree holds 111 .rst, of which 2 are _-prefixed and 2 are changelog.rst/commits.rst, so 107 git show spawns remain and the per-provider-version cost from round 1 is still paid. Feeding <tag>:<path> lines to a single git cat-file --batch on stdin builds a byte-identical dict in 0.019s against 1.254s for the loop, with git_ls_tree and is_guide_page unchanged. --all-versions pays the current cost once per provider-version.

extract_parameters.py:481: my round-1 off-by-one prompted the wrong correction. attach_guide_urls does if not anchor: continue, so guide_url is set only for classes a guide documents, which is what the new test_module_contract_omits_guide_url_for_undocumented_classes asserts. Entries for undocumented classes carry 12 keys, not 13. "all 12 Module fields, plus guide_url when a how-to guide documents the class" would be accurate.

test_module_contract_omits_guide_url_for_undocumented_classes cannot fail. _validate in registry_contract_models.py validates and then returns the input payload rather than model_dump(), so the assertion checks that a dict built without guide_url has no guide_url. ModuleContract.model_validate(_module_payload()).guide_url is None pins the field itself. test_module_contract_round_trips_guide_url is not vacuous, since extra="forbid" rejects an undeclared field, but its name promises a round-trip it does not perform.

AGENTS.md:468 "Nothing declares that link" is not quite right. provider.yaml declares integrations[].how-to-guide and transfers[].how-to-guide, and check_doc_files in run_provider_yaml_files_check.py set-compares that declared mapping against a glob of the operator, sensor and transfer guide paths, so it fails CI rather than rotting silently. The justification that does hold is the one worth writing down: the declared field names a page and never a section, and it does not reach the toolset, hook or decorator pages this reads. Worth correcting because AGENTS.md is a durable instruction file.

Two nits. .provider-detail-page .module-actions in main.css is a flex row with no flex-wrap, and this PR takes it from two links to three, so flex-wrap: wrap is cheap insurance at narrow viewports or a raised root font size. And git_ls_tree in extract_versions.py runs with core.quotePath at its default, which C-quotes a non-ASCII path so it fails the .endswith(".rst") test while the working-tree reader accepts it; -c core.quotePath=false closes that divergence. No such path exists under providers/ today.

@Lee-W

Lee-W commented Sep 19, 2026

Copy link
Copy Markdown
Member Author

read_guide_docs now uses git cat-file --batch: at providers-amazon/9.36.0 that is one subprocess and ~0.05s against 107 and ~1.9s, for a dict that compares equal to the loop's key for key. Parsing stays on bytes since the header counts them, and content decodes as explicit UTF-8 rather than following locale, with a decode failure left to propagate.

The discover_classes_from_provider docstring now uses "all 12 Module fields, plus guide_url when a how-to guide documents the class".

test_module_contract_omits_guide_url_for_undocumented_classes now asserts on
ModuleContract.model_validate(_module_payload()).guide_url, and the round-trip one is renamed test_module_contract_preserves_guide_url_value and reads the attribute off the model as well. _validate still returns the payload it was handed — switching it to model_dump() would inject defaults into every emitted entry — so the tests moved to the model instead.

AGENTS.md drops "Nothing declares that link" for the justification that holds:
provider.yaml's how-to-guide is CI-enforced by check_doc_files, but it names a whole page and never a section, and it covers only operators, sensors and transfers — not the toolset, hook and decorator pages this reads.


.provider-detail-page .module-actions gets flex-wrap: wrap, and git_ls_tree runs with core.quotePath=false. I kept your framing on the second one: no
non-ASCII path exists under providers/ today, so it closes the divergence between the two readers rather than fixing an observed break.

@Lee-W
Lee-W force-pushed the registry-module-guide-links branch from 72aeea4 to 850b7d2 Compare September 24, 2026 03:05
@Lee-W
Lee-W requested a review from kaxil September 24, 2026 06:43
@Lee-W
Lee-W requested a review from kaxil September 30, 2026 07:52
@Lee-W
Lee-W force-pushed the registry-module-guide-links branch from e11a7c9 to d1bb360 Compare October 1, 2026 02:40
@kaxil

kaxil commented Oct 5, 2026

Copy link
Copy Markdown
Member

Thanks, this looks good to me. A few non-blocking things, fine as a follow-up or in this PR, your call:

  • _LEADING_LITERAL_NAME_RUN has no $ anchor, so "SandboxToolset parameters" gives SandboxToolset its only Guide link, a parameter table on sandbox/configuration.html, not the sandbox/index.rst guide. Anchoring it to the end of the title changes only that link at HEAD, or sandbox/index.rst could end its title in SandboxToolset.
  • When the registry is built from a ref ahead of a provider's last release (manual dispatch, or the publish workflow's full-rebuild fallback), extract_parameters.py reads unreleased titles but links /stable. Today that's 8 of common.ai's 32 links, the four retitled pages, which still land at the top of the right page. Reading the docs at providers-{id}/{version} when the tag exists, via the read_guide_docs path extract_versions.py already has, would close it.
  • _extract_class_level_modules in test_extract_versions.py doesn't patch read_guide_docs, so 12 existing tests now shell out to a real git ls-tree and pass because the CalledProcessError is swallowed. Patching extract_versions.read_guide_docs (autospec, return_value={}) there fixes it.
  • test_skips_generated_and_release_note_pages_before_calling_git_show no longer reaches git_show; adding a .png and conf.py path to it would also pin the .rst filter, which nothing covers today.
  • Small comment fixes: drop #73523 and "reviewer-reported" from the tests and docs_guides.py, and the "Mutation canary" narration; the git_cat_file_batch docstring says git_show fails loud on CalledProcessError, but it returns None; the guide_url comment in registry_contract_models.py leaves out decorator modules.

Lee-W added 16 commits October 6, 2026 16:56
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.
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.
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.
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.
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.
`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.
"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.
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.
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.
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.
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.
@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.
@Lee-W
Lee-W force-pushed the registry-module-guide-links branch from d1bb360 to cac4877 Compare October 6, 2026 17:07
Lee-W added 3 commits October 6, 2026 18:11
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.
@Lee-W

Lee-W commented Oct 6, 2026

Copy link
Copy Markdown
Member Author
  • _LEADING_LITERAL_NAME_RUN is now anchored to the end of the title. That drops SandboxToolset's link entirely rather than moving it to sandbox/index.rst, since that page's title doesn't name the class. It's the only link that changes across the current provider docs and the release tags.
  • Guide docs are read at providers-{id}/{version} when the tag exists, via read_released_guide_docs, and fall back to the working tree otherwise.
  • _extract_class_level_modules now patches read_guide_docs, so those tests no longer shell out to a real git ls-tree.
  • The skip test now includes a .png and conf.py path, so the .rst filter is covered.
  • Dropped #73523, "reviewer-reported" and the "Mutation canary" narration, fixed the git_cat_file_batch docstring (it raises; only git_show returns None), and added decorator modules to the guide_url comment.

@Lee-W
Lee-W merged commit 07bed4f into apache:main Oct 7, 2026
308 of 309 checks passed
@Lee-W
Lee-W deleted the registry-module-guide-links branch October 7, 2026 05:28
@github-project-automation github-project-automation Bot moved this from Backlog to Done in Airflow Registry Oct 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants