Skip to content

Improve documentation site layout - #1578

Merged
timsaucer merged 11 commits into
apache:mainfrom
timsaucer:doc/phase1-theme-refresh
Jun 7, 2026
Merged

timsaucer merged 11 commits into
apache:mainfrom
timsaucer:doc/phase1-theme-refresh

Conversation

@timsaucer

@timsaucer timsaucer commented Jun 5, 2026

Copy link
Copy Markdown
Member

Which issue does this PR close?

There is no open issue, but based on the recent work our sibling project Comet did to improve their site.

Rationale for this change

This is phase 1 of a 4 phase project to improve documentation. Here we are updating the site dependencies and the overall layout. This is the biggest visual win of the phases. The follow on work includes:

  • Switching from rst files to markdown. This is primarily because agents will be reading these files and agents have been highly improved to focus on markdown over other formats.
  • Setting up for multi-version builds. As the feature set has been growing from version to version, it will be useful for end users to select their version to find appropriate functions.
  • Updating the CI system to push to the asf-site branch like Comet does.

What changes are included in this PR?

  • Update dependencies for site documentation
  • Configure the modern theme in docs/source/conf.py
  • Add dark-mode variants for the inline-code accent color and the table-striped row shading so both render with adequate contrast.
  • Add site icon

Are there any user-facing changes?

Yes — purely visual to the published documentation site. No API changes, no behavioral changes to the datafusion package itself. Changes a reader will notice:

  • A top navigation bar with the project logo, the user-guide / contributor-guide links, a GitHub icon, and a light/dark theme switcher.
  • Dark mode now actually renders correctly (the old overrides assumed a single light theme).
  • The site is built using a newer Sphinx theme; published HTML will look meaningfully different.
Screenshot 2026-06-05 at 9 33 55 AM

Bump pydata-sphinx-theme 0.8.0 -> 0.16 to enable the modern navbar slot
API and dark/light theme switcher. Configure top navbar with logo,
nav links, GitHub icon, and theme switcher in conf.py. Drop the custom
docs-sidebar.html override and the layout.html block that silenced the
navbar — both predate the slot API and conflict with the new theme.
Strip CSS overrides that fought the old theme (--pst-header-height: 0,
navbar-brand sizing) and add a dark-mode variant for the inline code
color and table-stripe shading. Fix the stale github_repo
("arrow-datafusion-python" -> "datafusion-python") so future Edit-on-
GitHub links resolve. Bump copyright year and project name.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@timsaucer
timsaucer marked this pull request as draft June 5, 2026 12:51
@timsaucer timsaucer changed the title docs: refresh theme — pydata-sphinx-theme 0.16, top navbar, dark mode Improve documentation site layout based on Comet improvements Jun 5, 2026
timsaucer and others added 9 commits June 5, 2026 08:53
Previous structure dumped every top-level toctree entry from index.rst
into the navbar, producing eight items including external URLs ("Github
and Issue Tracker", "Rust's API Docs", ...) that wrapped to two lines
each. Introduce user-guide/index.rst and contributor-guide/index.rst as
section landing pages with nested toctrees, then point index.rst at just
those two plus autoapi/index. The navbar now reads "User Guide",
"Contributor Guide", "API Reference" — three single-line entries. Move
the external links into the index.rst body where they're discoverable
without crowding navigation.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Add Examples and Rust API as text links in the top navbar via the
pydata-sphinx-theme external_links option. Nest the code-of-conduct
link inside the Contributor Guide toctree so it appears alongside the
other contributor pages. Drop the duplicate "Further reading" bullet
list from the landing page now that every link has a permanent home.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Move the Rust API docs entry from external_links to icon_links and use
the fa-brands fa-rust gear mark. Now sits next to the GitHub icon in
navbar_end with matching visual weight instead of a wider text link.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
The default pydata-sphinx-theme sidebar-nav-bs starts at the current
top-level section, so the root index — which has no parent section —
ends up with an empty sidebar. The theme's layout also explicitly
filters sidebar-nav-bs out of the sidebar list when suppress_sidebar_
toctree() returns true (which it does for root pages), so simply
overriding sidebar-nav-bs.html in templates doesn't help.

Add a sidebar-globaltoc.html template that calls Sphinx's toctree()
global directly to render the full document tree, and wire it through
html_sidebars under a name the theme's suppress filter doesn't strip.
Landing page now shows User Guide / Contributor Guide / API Reference
in the sidebar with the current section expanded on inner pages.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Switch the sidebar toctree call from toctree() to generate_toctree_html
with collapse=False, so nested <ul>s render into the DOM for every
branch. The pydata-sphinx-theme JS then wraps them in <details> with
fa-chevron-down toggles, matching the datafusion-comet sidebar where
each section with children can be expanded inline. show_nav_level=1
keeps deeper levels collapsed on first load.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Bump show_nav_level 1 -> 2 so the landing-page sidebar opens with
User Guide / Contributor Guide / API Reference already expanded to
their immediate children. Deeper levels remain collapsed behind
chevrons so the sidebar stays scannable.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Restore the "Links" sidebar heading that the previous site had —
GitHub and Issue Tracker, Rust API Docs, Code of Conduct, Examples.
Implemented as a second hidden toctree with :caption: Links so the
pydata-sphinx-theme sidebar renders the heading above the four
external URLs. Drop Code of Conduct from the Contributor Guide
toctree since it now lives under Links instead.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Replace the second hidden toctree (which expanded each external URL
into its own navbar entry) with a dedicated links.rst landing page,
and add a single "links" entry to the main toctree. Top navbar now
shows User Guide / Contributor Guide / API Reference / Links — four
items, no wrapping. Clicking Links opens the page that lists GitHub,
Rust API Docs, Code of Conduct, and Examples.

Drop the external_links Examples entry from conf.py since the same
URL now lives on the Links page.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Drop in the same favicon.svg the main datafusion.apache.org site
uses (just the Apache DataFusion mark, no wordmark) and wire it
through html_favicon. Browsers and bookmarks now show the project
icon instead of the generic Sphinx page glyph.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
@timsaucer
timsaucer marked this pull request as ready for review June 5, 2026 13:36
@timsaucer
timsaucer requested review from andygrove and Copilot June 5, 2026 13:36
@timsaucer timsaucer changed the title Improve documentation site layout based on Comet improvements Improve documentation site layout Jun 5, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Phase 1 of a documentation-site refresh, updating Sphinx theme/dependencies and restructuring the documentation TOC/layout to match newer styling patterns (including dark mode support and a site icon).

Changes:

  • Updated docs build dependencies (notably pydata-sphinx-theme to 0.16.x) and locked them in uv.lock.
  • Reworked the docs IA/TOC by introducing section index pages (User Guide / Contributor Guide) and simplifying the root index.rst tree.
  • Added theme/layout customizations: navbar configuration, global sidebar template, dark-mode CSS tweaks, and an SVG favicon.

Reviewed changes

Copilot reviewed 10 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
uv.lock Locks updated docs dependencies (pydata theme upgrade + new transitive deps).
pyproject.toml Updates docs extras to use newer pydata-sphinx-theme and adds sphinx-reredirects.
docs/source/index.rst Simplifies root toctree to point at section index pages + links.
docs/source/user-guide/index.rst New section landing page to collect user-guide toctree items.
docs/source/contributor-guide/index.rst New section landing page to collect contributor-guide toctree items.
docs/source/links.rst New page collecting external links under a single toctree entry.
docs/source/conf.py Switches to modern pydata theme config (navbar/logo/icons/sidebar config) + favicon.
docs/source/_templates/sidebar-globaltoc.html New sidebar template rendering the full global toctree with collapsible behavior.
docs/source/_templates/layout.html Stops silencing the navbar (inherits theme navbar again) while keeping custom footer.
docs/source/_templates/docs-sidebar.html Removes the old custom sidebar template implementation.
docs/source/_static/theme_overrides.css Updates theme overrides for dark-mode code/table stripe contrast and removes obsolete sidebar sizing rules.
docs/source/_static/favicon.svg Adds site favicon asset.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread docs/source/conf.py
Comment thread docs/source/_templates/sidebar-globaltoc.html Outdated
Two small follow-ups from the Copilot reviewer on apache#1578:

- Append .html to the html_sidebars entry. Sphinx's Jinja loader
  resolves both "sidebar-globaltoc" and "sidebar-globaltoc.html" to
  the same template, but the explicit form is closer to the spelling
  in the Sphinx docs and is harder to misread.
- Update the inline comment in sidebar-globaltoc.html that still
  claimed show_nav_level=1 after we bumped it to 2 in conf.py. Now
  describes the variable wiring instead of hard-coding a number that
  has to be kept in sync with conf.py.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>

@ntjohnson1 ntjohnson1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Built the docs and scrolled through and looked at the currently posted website.
Things look nicer with this layout and the two added pages seems reasonable. No fine grained thoughts on the details, since I know you grabbed this from Comet.

Instructions to build docs locally (from the docs readme but to potentially save looking them up)

uv sync --dev --group docs --no-install-package datafusion
uv run --no-project maturin develop --uv
uv run --no-project docs/build.sh

@timsaucer
timsaucer merged commit 407298f into apache:main Jun 7, 2026
31 checks passed
@timsaucer

Copy link
Copy Markdown
Member Author

Thanks @ntjohnson1 !

@timsaucer
timsaucer deleted the doc/phase1-theme-refresh branch June 7, 2026 13:19
timsaucer added a commit that referenced this pull request Sep 9, 2026
The user guide had no page for someone who installs an extension library
and wants to run queries with it. `with_extensions` was described only in
a 123-line docstring and in the contributor-guide FFI page, which opens by
explaining that Rust has no stable ABI — the wrong altitude for a reader
who just wants their queries to run somewhere else.

Adds `user-guide/extensions.md`: what an extension library is, which kinds
register directly versus needing `with_extensions`, the two failure modes
that actually bite (a collected context, a version mismatch), and how to
check what a session was taught. It names no capsule, no ABI, and no codec
id; the only occurrences of `FFI` and `TaskContextProvider` are inside the
error string a reader would be searching for.

Splits `distributing-work.md` into a directory. The page was entirely about
pickling expressions to worker pools and treated query-level distribution
as two stubs at the end, so nothing on the site connected `with_extensions`
to distribution at all — which is the road a data scientist is actually
looking for. The index now asks who owns the partitioning decision and
routes accordingly; `query-engines.md` carries the missing bridge and
absorbs the two upstream work-in-progress sections.

Wires `sphinx-reredirects`, which has been a declared dependency since
#1578 without ever being enabled, so the old `distributing-work.html` URL
keeps working. Also fixes three pointers that went stale in the MyST
migration and still named `.rst` files, a malformed `ref:` role in
udf-and-udfa.md that rendered as literal text, and a doubled "the" in
data-sources.md.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
timsaucer added a commit that referenced this pull request Sep 10, 2026
…1679)

* Add atomic SessionContext.with_extensions API

Installing FFI extension codecs and query planners by chaining the
existing with_* methods can bind task-context providers to intermediate
contexts that are later collected, breaking the weak provider reference
over the FFI boundary. with_extensions creates one destination context,
passes it to each extension factory so components bind to that exact
context, and installs everything in a single state write.

Co-Authored-By: Claude Fable 5 <[email protected]>

* Add extension-bundle example and with_extensions FFI tests

MyPlannerExtension in the query-planner example crate implements the
__datafusion_session_extension__ protocol from Rust: it extracts the
destination context's task-context provider, binds fresh observing
codecs and a planner to it, and returns SessionExtensionComponents. Its
codecs record the max_rows config value resolved through the weak
provider, letting tests prove the provider targets the returned context
rather than the source. Documents with_extensions as the preferred API
in the FFI guide.

Co-Authored-By: Claude Fable 5 <[email protected]>

* Document and test the context-outlives-DataFrame contract

A DataFrame does not keep its SessionContext alive. FFI components hold
a weak task-context provider, so operations that reach an FFI codec
after the context is collected fail with a clean out-of-scope error
rather than crashing. Lock that behavior in with a test and document
the ownership contract in the FFI guide and with_extensions docstring.

Co-Authored-By: Claude Fable 5 <[email protected]>

* Skip private internal methods in wrapper coverage test

Single-underscore methods on internal pyo3 classes (such as
SessionContext._install_extensions) are private support methods for the
Python wrappers and do not require a public wrapper.

Co-Authored-By: Claude Fable 5 <[email protected]>

* Test planner rebinding and codec ids in with_extensions

A codec-only bundle installed on a context that already holds an FFI
planner must rebind that planner to the new chains, so the planner
decodes through the bundle's codecs.

Codec ids are derived from the exporting class, so two bundles shipping
the same codec class collide and the install is refused. Declaring
__datafusion_codec_id__ on the object a bundle hands over resolves it,
and both chains then install.

Co-Authored-By: Claude Fable 5 <[email protected]>
Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* Fix duplicate attribute docs in SessionExtensionComponents

The docs build runs Sphinx with --fail-on-warning. SessionExtensionComponents
documented its fields in both a napoleon `Attributes:` section and the dataclass
class-body annotations, so autoapi emitted each field twice and the build failed
with six "duplicate object description" warnings.

Move each field's description to a per-field docstring under its annotation so
autoapi renders exactly one entry per field.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* Move session extension types into datafusion.extensions

QueryPlannerExportable, SessionExtensionComponents, and
SessionExtensionExportable describe how an extension library plugs into a
session, not how a SessionContext behaves. Give them their own module so
context.py does not keep absorbing the extension surface as it grows.

extensions.py imports SessionContext, the codec protocols, and CapsuleType
under TYPE_CHECKING only, so context.py can import from it at runtime
without a cycle. All three names remain importable from datafusion and
datafusion.context; QueryPlannerExportable stays out of the top-level
__all__ as before.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* Run the with_extensions docstring example in CI

The example was marked `+SKIP` because the main suite has no built FFI
extension to import, which is exactly how such an example rots. Parse the
statements out of the live docstring in the query-planner example suite,
drop the skip, and execute each one against a real extension bundle. Only
names are redirected: `my_extension` resolves to a stand-in combining this
repository's provider codecs and planner, and `SessionContext` supplies the
config that planner reads. A renamed method, a changed signature, or a wrong
expected output now fails CI, which already runs this suite.

Also drop the `extensions` Args entry's restatement of the type hint and
say instead what the hint does not: install order is chain order.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* Share the session in with_extensions instead of forking it

`_derive_for_extensions` minted a new `Arc<SessionContext>` via
`new_with_state(self.ctx.state())`. Every other `with_*` method shares
`Arc::clone(&self.ctx)`, and `new_with_state` carries the session id over, so
`with_extensions` returned a second live session claiming the same
`session_id()` as the source while holding independent `SessionState`.
Configuration and the function registry diverged, catalogs stayed shared, and
both handles reported the same `__datafusion_codec_id__` — which is
`session:<session_id>` and exists precisely to distinguish codec chains, so
installing both on a third session was refused as a duplicate id.

The fork also bought nothing. It was introduced to keep components from binding
to an intermediate context that could be collected, but there is one
`Arc<SessionContext>` per session, so no such intermediate exists; deriving one
is what creates the hazard. Rule 6 of the ffi-capsule-protocol skill already
said to mutate `SessionState` in place rather than derive a replacement.

Delete `_derive_for_extensions` and hand the receiver to the extension
factories. `_install_extensions` already returned a handle sharing
`Arc::clone(&slf.borrow().ctx)`, so removing the fork upstream of it is the
whole change. Atomicity is unaffected: both codec chains are built as locals and
state is written exactly once, at the end, in `set_session_query_planner`.

Replace `test_with_extensions_provider_targets_returned_context`, which is
vacuous once the session is shared, with
`test_with_extensions_shares_the_session_with_the_source`. It asserts matching
session ids and that a `SET` issued through the source after installation is
visible to the provider the bundle bound. Reintroducing the fork fails it.

Update the prose that described the fork-era design: the `with_extensions`
docstring and `SessionExtensionComponents` / `SessionExtensionExportable` in
`datafusion.extensions`, the `with_extensions` and "What a derived context
shares" sections of the FFI guide, the query planner example's README and
`extension.rs` comments, and two test docstrings. Note the shared-session
mechanism in Rule 6 of the skill, since `with_extensions` is where it is
easiest to get wrong.

`enable_url_table` is once again the only method that mints a second
`Arc<SessionContext>` for a session; its comment, the FFI guide, and the skill
now also record that it forks state while keeping the session id, tracked as a
bug in #1708.

Also add the missing doctest to `SessionExtensionComponents` and a pointer to
`with_extensions` from the upgrade guide, which described only the low-level
install path.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* Name a bundle's bare capsules after the bundle

A codec handed to `with_extensions` as a bare `PyCapsule` fell through to
`anon:<uuid4>`, an id private to the session that installed it. Plans written
through it are undecodable anywhere else, and `with_extensions` accepts no
`codec_id=` to override that — so the workaround was to wrap the capsule in an
object declaring `__datafusion_codec_id__`, which nothing documented. A
distributed engine has to decode its plans in another process, so the shape it
would naturally ship — a Rust bundle handing over capsules, as
`MyPlannerExtension` does — was the one shape that could not work.

The bundle is the stable name that was missing. It is a plain Python object, so
its `module.QualName` is library-owned and exactly as stable across processes as
an exporting codec class's, which arm 3 of `derive_codec_id` already trusts. The
capsule was unnameable only because a capsule carries no type of its own, not
because nothing stable was in reach.

Resolve a capsule's id through the contributing bundle, using `derive_codec_id`
itself so the bundle inherits the same `__datafusion_codec_id__` escape hatch
against a class rename. The fallback applies only where randomness would have:
an id declared on the handed-over object, or that object's own class, still
wins, so an extension can name a codec directly.

Two bare capsules of one kind from one bundle collide and are refused. Numbering
them by position would be exactly the id `codec.rs` rejects for `anon:` — one
another library can mint the same value from — and would break stored plans the
first time the bundle reordered what it returns.

`resolve_codec_id` gains the bundle argument, `_install_extensions` takes
(codec, bundle) pairs, and the collision message now names both routes to a
distinct identity; it previously offered only `codec_id=`, which is unreachable
from `with_extensions`.

Covered in `python/tests/test_context.py`, which reaches every arm without a
built extension library: the bundle-derived name, an extension pinning its own
id, an id on the handed-over object winning, an exporting object keeping its
own, and the two-capsule collision. The cross-FFI case is pinned in the query
planner example, where a Rust bundle's capsules must report
`datafusion_ffi_query_planner_example.MyPlannerExtension` and no id may be
`anon:`.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* Stop claiming _install_extensions always writes state

The doc comment said "the final state is written through this context's own
`state_ref()`", which overstates it. `set_session_query_planner` returns early
when there is no planner to bind, and the codec chains live on the returned
`PySessionContext` fields rather than in `SessionState` — so a codec-only
install onto a session with no FFI planner writes nothing.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* Tidy the loose ends from review of with_extensions

Mark `SessionExtensionExportable` `@runtime_checkable` and have
`with_extensions` check it with `isinstance` rather than `hasattr`, so the
annotation and the runtime check are the same statement, and callers can ask
the question too. Covered by a doctest on the protocol.

Replace the leading-underscore skip in `test_wrapper_coverage` with a named
allowlist. The pattern also excused `DataFrame._repr_html_`, which a wrapper
does have to provide, so a two-method need was weakening coverage for every
private name. Removing `_install_extensions` from the allowlist fails the test,
so the entry is load-bearing rather than decorative.

Say in `_CodecOnlyExtension` that retaining the context is what the protocol
tells real extensions not to do, and that it is kept only so a test can assert
which context the factory was handed.

Let the docstring-example shim in the query planner example accept a config
positionally, the way the real constructor does. Editing the docstring to
`SessionContext(config)` now fails as a doctest diff rather than as a
`TypeError` inside the harness.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* Require with_extensions codecs to be objects, not capsules

Reverses "Name a bundle's bare capsules after the bundle". Deriving a
capsule's id from the bundle that contributed it reads the identity off the
wrong object: the bundle is whatever the caller passed to with_extensions, so
an application that packages several libraries as one bundle of its own
stamps its identity onto the inner libraries' codecs. Their pinned
__datafusion_codec_id__ is discarded and there is nothing the inner library
can do about it, since its object never reaches _install_extensions. Nothing
fails at install time; the mismatch surfaces as an undecodable plan in the
process that reads it, naming an id nobody wrote in source.

So with_extensions now refuses a bare capsule and names the getter to
implement. An id read off the handed-over object is composition-stable by
construction, which the new tests pin at both layers. This also decouples a
codec's wire identity from the bundle's Python class name, which is what
__datafusion_codec_id__ exists for, and closes the case where a bundle built
by a factory function contributed a wire id containing "<locals>".

The low-level methods keep accepting capsules: they take codec_id=, so the
random anon: arm still has an escape hatch. Query planners are unaffected,
carrying no wire id.

MyPlannerExtension gains BundledLogicalCodec and BundledPhysicalCodec, small
pyclasses holding the bound FFI codec and declaring pinned ids, as the
reference shape for a library whose plans leave the process.

Also, unrelated to the above but adjacent in the docs: with_extensions never
said that a bundle-supplied planner replaces an installed one rather than
layering, and the SessionExtensionComponents example that showed a codec was
fully skipped with undefined names. Both fixed.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* Install extension codecs and planners in two phases

A session chains many codecs and dispatches between them by id, so codecs
accumulate and their order does not affect decoding. A session holds exactly
one query planner, so planners cannot accumulate — they compose by nesting,
each wrapping the one before it. Collecting both from a single hook forced
with_extensions to refuse more than one planner per call, because every factory
ran before anything was installed and so no bundle could see another bundle's
planner to wrap it. Two libraries that each ship a planner could not be
installed together at all, and splitting them across two calls silently
discarded the first.

Codecs now come from __datafusion_session_extension__ and planners from a new
__datafusion_session_planner__(ctx, fallback), which runs once per bundle in
argument order after every codec is installed. Each receives the planner built
so far; wrapping it nests this bundle outside the previous one, so the last
bundle listed ends up outermost. A bundle implements either hook or both, which
also lets a library that ships only an optimizing planner stop returning empty
components. SessionExtensionComponents loses its query_planner field.

Running the planner hooks after every codec is installed is what makes a nested
planner safe. The rebuild that follows a later codec install reaches only the
outermost layer, so a fallback captured against a partial chain would stay
stale; there is now no "afterwards" within a call.

Atomicity is unchanged. _install_extension_codecs writes nothing — the chains
belong to the returned handle — so phase one is transactional for free, and the
nest is built in memory with _install_extension_planner performing the single
session write after the last hook returns. A hook that raises in either phase
leaves the caller's context as it was.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* Give the planner example a node only its own codec can carry

The bundle shipped a planner and codecs, but the halves never met: the planner
emitted a stock GlobalLimitExec and the codecs delegated everything to the
default codec. So the example asserted by structure that a planner and its
codecs belong together without demonstrating why, and no test would have caught
a bundle whose planner emits a node its own codec cannot encode.

DistributedQueryPlanner now wraps its result in a DistributedExec, a type
private to this library, and ObservingPhysicalExtensionCodec claims it by
downcast and rebuilds it from its inputs. Nothing else in the session knows the
type, which is the reason the two ship as one bundle. The observing codecs stop
being dead weight in the process — they were previously never consulted, and
decode_max_rows_seen had no caller.

Also documents what codec order does and does not control, which building this
surfaced. Decoding routes by id and is never order-dependent. Encoding stops at
the first codec that claims the node, so a codec claiming a broad category —
MyPhysicalExtensionCodec claims any ForeignExecutionPlan — takes nodes from any
library installed after it. The query still succeeds; only the library that
wrote the bytes changes, which breaks a plan that has to decode elsewhere.

That gives a bundle two reasons to want different positions for its two halves.
The guide now says to contribute each half at its own position with a small
adapter rather than reordering, since the hooks are independent, and treats the
low-level sequence as the last resort it is: it works, but it hands back
responsibility for codec-before-planner ordering and leaves a hand-layered
fallback holding the codecs it captured. No attempt is made to express every
permutation from one call.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* Drop the planner example's logical codec observer

The physical observer earns its place now that it claims the bundle's own
DistributedExec, but the logical one never did and cannot: it declines all four
methods to the default codec, and this library defines no logical extension
node for it to claim. The FFI logical codec does not carry arbitrary
LogicalPlan::Extension nodes anyway, so there is no logical analogue to give
it. Measuring a query confirms it: every record_task_ctx firing comes from the
physical decode path and none from the logical one.

BundledLogicalCodec now wraps DefaultLogicalExtensionCodec directly, which
keeps what the logical half actually demonstrated -- a bundle contributing both
codec kinds under ids it declares -- and drops 54 lines of trait impl and
hand-written Debug that fed an accessor nothing could observe.

Also narrows PlannerObservations::used_fallback back to private. It is read
only through MyQueryPlanner::used_fallback in the same module; its neighbours
need pub(crate) because extension.rs reads them, and it does not.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* Export QueryPlannerExportable and drop the empty-extensions guard

Two loose ends from review.

QueryPlannerExportable was the only member of the extension protocol family
left in the submodule while SessionExtensionComponents,
SessionExtensionExportable and SessionPlannerExportable were exported from the
package root. It types the planner a __datafusion_session_planner__ hook
returns, so a bundle author needs it just as much, and one family member
importing differently from the rest is a papercut with no upside. Also fixes
two doc references that stopped resolving when these classes moved out of
context.py: a bare :class:`QueryPlannerExportable` and a bare
:py:class:`SessionExtensionComponents`, both now spelled with their module the
way the neighbouring datafusion.user_defined references are.

with_extensions() with no arguments raised instead of installing nothing. That
put it out of family: every sibling varargs method -- DataFrame.select, filter,
sort, drop, window -- accepts zero arguments and returns a no-op result, and
the two existing "at least one" guards in the codebase both cover cases with no
meaningful identity element, which this is not. Installing no extensions has an
obvious answer, and a caller assembling the list from a plugin registry should
not have to special-case it being empty. Phase two still runs, so the empty
case rebinds an existing FFI planner to unchanged chains; a test pins that the
planner survives it.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* Describe both extension hooks in the upgrade guide

The planner-install section still said a bundle exposes
__datafusion_session_extension__, which stopped being the whole protocol when
planners moved to a hook of their own.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* Generate heading anchors for h4 so the FFI guide's links resolve

`myst_heading_anchors` was 3, but the extension-bundles section added in this
branch cross-references its own `####` subsections. Sphinx warns
`'myst' cross-reference target not found: 'when-codec-order-does-matter'` and
renders that link as plain text; the docs build does not pass `-W`, so it went
unnoticed.

Bumping to 4 rather than promoting the heading keeps the four subsections
nested under `### Extension bundles: with_extensions`, where they belong.
Only one other `####` heading exists under `docs/source/`.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* Skip the planner commit when with_extensions installs nothing

`with_extensions` ended every call with `_install_extension_planner`, which
rebuilds `SessionState` to rebind an existing FFI planner to this handle's
codec chains. When the call installed no codec there is nothing to rebind
against, so the rebuild is at best churn — and at worst it drags a planner
that is sitting on another handle's codecs onto this one's, silently undoing
that install. `with_python_udf_inlining` already guards its no-op toggle for
exactly this reason; `with_extensions` now guards the same way.

`test_with_extensions_installing_nothing_leaves_the_planner_alone` covers both
shapes of "installed nothing": no arguments at all, and a bundle whose hooks
answer empty. Both fail without the guard, with the planner left on an empty
logical chain and the query erroring out.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* Reject a lone codec where SessionExtensionComponents wants an iterable

`logical_extension_codecs=codec` instead of `(codec,)` is the easy mistake to
make, and it surfaced as `'MyCodec' object is not iterable` raised by an
`extend` call inside `with_extensions` — naming neither the field nor the hook
that built the value. `__post_init__` now checks it, so the error lands in the
extension library's own frame and says which field is wrong and how to spell
one codec.

It also normalizes each field to a tuple. The declared type is a tuple and the
class is frozen, so a list left in place would be a mutable member of an
immutable value, and a generator would be exhausted by the first read. A str
is refused rather than normalized: it is iterable, so it would otherwise
become a tuple of characters and fail much later as that many bogus codecs.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* Widen the with_extensions annotation to both hooks

`with_extensions` accepts a bundle implementing either hook — the runtime check
tests against both protocols, `test_with_extensions_accepts_a_planner_only_extension`
pins it, and the FFI guide's `PlannerOf` adapter recommends contributing only the
planner half. The annotation named `SessionExtensionExportable` alone, so a type
checker rejected the very shape the guide tells authors to write.

Two doc comments also went stale. `test_with_extensions_no_extensions_keeps_an_installed_planner`
still described phase two running and rebinding an existing planner, which the
no-op guard now skips outright, and `__datafusion_codec_id__` listed a
`_install_extensions` method that never existed under that name.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* Correct three claims in the extension bundle docs

The `SessionExtensionComponents` doctest took its codec capsule off a
`SessionContext()` that was dropped on the same line. An FFI codec holds its
task-context provider weakly, so that capsule names a session that is already
gone — the doctest only reads an id back so it passes, but it is the exact
shape the FFI guide warns against. It now keeps the context in a name.

`SessionPlannerExportable` called returning `fallback` a wrap that "contributes
nothing". It is not: the capsule the first bundle receives wraps the session's
planner for export, so handing it back installs it as a foreign planner and
every later plan crosses an FFI boundary that was not there before. `None` is
the no-op. Corrected in the protocol docstring, the FFI guide's canonical
section, and the `_PlannerExtension` test helper that repeated the claim.

`__post_init__` walked a written-out list of field names. It now walks
`dataclasses.fields`, filtered on the `_codecs` suffix so a codec field added
later is normalized without anyone remembering to name it, and a future field
that is not a codec collection is left alone.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* Rename _rebind_query_planner to _export_query_planner

The method rebinds nothing. It imports whatever a `__datafusion_session_planner__`
hook returned — an object exposing the getter or a raw capsule — and hands back a
capsule, so the next hook receives one either way; its own doc comment already
said "re-export". Meanwhile "rebind" means something specific and different in
this file: rebuilding an installed planner against a handle's codec chains, which
is what `set_session_query_planner` does. Freeing the word keeps the two apart.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* Say which codec chains each extension hook's context carries

`__datafusion_session_planner__` was documented as receiving a context that
carries the final codec chains, and `MyPlannerExtension` relies on exactly that
when it takes the host's codecs off `ctx` instead of minting its own. The other
side was never stated: `__datafusion_session_extension__` runs before anything
is installed, so its `ctx` is the same session with the chains the receiver
already had — missing this call's codecs, including the bundle's own.

Both hooks hand back a valid task-context provider, which is what components
actually need, so the difference only bites an author who reads codec chains off
the context. Recorded on `SessionExtensionExportable`, in the FFI guide's
two-phase section, and in Rule 2 of the capsule-protocol skill.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* Make the planner-hook tests assert what their names claim

`test_with_extensions_threads_the_planner_through_in_order` never checked
an order: it asserted each hook recorded one fallback and that the second
was not None, both of which hold for a host that ran the hooks backwards.
`test_with_extensions_skips_a_planner_hook_returning_none` asserted only
that downstream ran, while its comment claimed the skipped hook had not
become downstream's fallback.

`_PlannerExtension` now takes an optional shared list the hooks append
themselves to, so order is observable. The threading test asserts that
list, plus that the second hook's fallback is not the object the first was
handed -- the host re-exports every return value before passing it on. A
capsule is opaque from Python, so that cannot separate a re-export of the
first planner from a fresh read of the session's; the comment says so and
points at the FFI suite's `test_with_extensions_nests_planners_in_argument_order`,
which pins the nesting by asserting the outer planner delegated.

The skip test records the skipped hook's fallback too, and asserts both
hooks ran, that downstream was handed a different capsule, and that the
resulting context still queries.

Each new assertion was mutation-tested: reversing the planner loop, dropping
the `if supplied is None: continue`, and replacing the re-export with a
straight pass-through each fail exactly one of these tests.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* Correct three more claims in the extension bundle docs

`QueryPlannerExportable` said `session` is the
`datafusion.context.SessionContext` the planner is being installed on. It
is not. The capsule getters are called from Rust and receive the PyO3
context, so `isinstance(session, SessionContext)` is False -- while its
repr reads `datafusion.SessionContext`, because the pyclass declares
`module = "datafusion"`. It carries every capsule getter and
`__datafusion_codec_id__`, which is all the protocol needs, so the fix is
to say duck-type it rather than to change what is passed. The two bundle
hooks are the exception and do receive the wrapper, since `with_extensions`
dispatches them from Python; the ffi.md section on capsule getters now
draws the same distinction.

The `with_extensions` `Raises:` section listed ValueError for colliding
codec ids only. A getter returning a capsule of the wrong kind also raises
it -- `Expected name 'datafusion_query_planner' in PyCapsule, instead got
'datafusion_logical_extension_codec'` -- which
`test_with_extensions_rejects_bad_codec_capsule` already pins.

The `datafusion.extensions` module docstring said phase two runs the
planner hook "once per bundle". Once per bundle that implements it; a
bundle implements either hook or both.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* docs: add a user-facing extensions page and split distributing-work

The user guide had no page for someone who installs an extension library
and wants to run queries with it. `with_extensions` was described only in
a 123-line docstring and in the contributor-guide FFI page, which opens by
explaining that Rust has no stable ABI — the wrong altitude for a reader
who just wants their queries to run somewhere else.

Adds `user-guide/extensions.md`: what an extension library is, which kinds
register directly versus needing `with_extensions`, the two failure modes
that actually bite (a collected context, a version mismatch), and how to
check what a session was taught. It names no capsule, no ABI, and no codec
id; the only occurrences of `FFI` and `TaskContextProvider` are inside the
error string a reader would be searching for.

Splits `distributing-work.md` into a directory. The page was entirely about
pickling expressions to worker pools and treated query-level distribution
as two stubs at the end, so nothing on the site connected `with_extensions`
to distribution at all — which is the road a data scientist is actually
looking for. The index now asks who owns the partitioning decision and
routes accordingly; `query-engines.md` carries the missing bridge and
absorbs the two upstream work-in-progress sections.

Wires `sphinx-reredirects`, which has been a declared dependency since
#1578 without ever being enabled, so the old `distributing-work.html` URL
keeps working. Also fixes three pointers that went stale in the MyST
migration and still named `.rst` files, a malformed `ref:` role in
udf-and-udfa.md that rendered as literal text, and a doubled "the" in
data-sources.md.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* docs: split the FFI guide into one section per audience

`contributor-guide/ffi.md` had grown to 795 lines serving three different
readers at once, filed under a section whose index says it is for people
contributing to this repository. An engineer at delta-rs or a distributed-engine
vendor is neither a contributor nor an end user; they consume a published,
versioned protocol, and the only description of it lived behind a heading
telling them the page was not for them.

Adds a third top-level section, `extension-guide/`, so the sidebar reads User
Guide / Extension Guide / Contributor Guide — one per audience. It carries the
`(ffi)=` label, so every existing reference keeps resolving, including the
`:ref:`ffi`` inside `context.py`'s docstring that ships in the wheel.

The maintainer-facing rationale moves to `contributor-guide/ffi-internals.md`:
the weak-`Arc` scheme, why repairing an orphaned provider cannot work, why
planner codec rebinding is one level deep, and the two upstream issues. An
extension vendor should not be reading "that is a bug rather than a design, do
not copy the pattern" halfway down their integration guide.

The PyO3 `frozen` policy moves to `contributor-guide/pyo3-guidelines.md`. It is
project review policy with nothing to do with FFI, and extracting it repairs a
prose bug: it had been spliced into the middle of "Implementation Details", so
the sentence "If you were interfacing with a library that provided the above
`FFI_TableProvider`" resumed 46 lines after the snippet it referred to. Those
two halves are rejoined on `capsule-protocol.md`.

Fills the coverage gap the split exposed. The codebase exports 18 capsule
getters and the old page documented 7; the remaining 11 appeared on no page
that even listed them. The section index now carries a table of all 18, and
`table-providers.md` and `functions.md` document the catalog family, table
functions, physical optimizer rules, and extension options for the first time.

Corrects the argument rule while moving it. The old section asserted that
capsule getters "receive the SessionContext they are being installed on", which
is true for the codec, planner, and table-function hooks but not for the
catalog family: `CapsuleGetterArg::LogicalCodec` passes the host's logical
codec as a bare capsule, and `__datafusion_table_provider__` gets a session
from `SessionContext.register_table` but a codec capsule from
`Schema.register_table`. Nothing breaks, because every implementation passes
the argument to `ffi_logical_codec_from_pycapsule`, which handles both — so the
rule is now stated by capability rather than by type, and the upgrade guide
says which hooks changed.

Also converts the guide's five in-page heading links to labelled `{ref}`
targets, since heading anchors rot silently on rewording, and redirects the old
`contributor-guide/ffi.html` URL.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* docs: give each docstring claim one canonical home

The extension docstrings and the FFI guide had grown the same six claims 3-5
times each, in wording that had already started to drift. `with_extensions` was
123 lines — the longest docstring in the package by 30 — and most of it argued
a design rather than stating a contract.

Routes each recurring claim to one home and leaves a one-line pointer
elsewhere. Session sharing and context lifetime move onto the `SessionContext`
class docstring, since they are properties of the type and that is exactly why
five methods had each reworded them. The `None`-vs-`fallback` contract moves
onto `SessionPlannerExportable`, since it is a return-value contract of one
method. The two-phase rationale, the objects-not-capsules argument, and the
codec-order argument stay in the guide, which owns the "why".

The duck-type-the-session rule moves from `QueryPlannerExportable` to
`LogicalExtensionCodecExportable`, which is already the designated `session`
reference for that family and where the codec protocols were pointing for it
anyway. It also gains the fact the old text was missing: across the protocol
this argument is not always a session, so duck-typing it is not a style
preference.

Not every docstring shrank. `__datafusion_query_planner__` was 4 lines with no
example while `set_query_planner` told callers to capture the fallback through
it, so it grew to 25. Several others grew by gaining the `Args`/`Returns`/
`Raises` sections they were missing, and by trading `+SKIP` examples for
runnable ones — a `SessionContext` satisfies the capsule-getter protocols, so
its own exported capsule stands in for a library's without a build step. The
total across these sixteen docstrings is roughly flat, at 577 lines before and
629 after; what changed is that the rationale left and the contract arrived.

The 21 lines of `dataclasses.fields` rationale on
`SessionExtensionComponents.__post_init__` become comments in the method body.
That is the fix rather than touching `autoapi_options`: `__post_init__` is a
*special* member, so dropping `private-members` would not hide it, and dropping
`special-members` would delete the entire `__datafusion_*__` reference surface.
`conf.py` now records why that setting is deliberately left alone, and
de-duplicates the four re-exported `extensions` classes the way it already did
for `DataFrame` and `SessionContext`.

Adds `python/tests/test_docstrings.py`, which is what would have caught the
123-liner: a 95-line ceiling with no waiver list, a doctest-presence check over
the extension protocol, and a check that a docstring naming the guide actually
links it. The last two each found a real defect on first run.

Also fixes what this branch made newly load-bearing in the serialization
surface: `plan.py`'s single-backtick RST rendered `LogicalExtensionCodec` and
friends as italics rather than links into the new API, and the `global_ctx()`
fallback in `Expr.from_bytes` now silently yields a context with no extension
codecs, which before this branch lost only registrations.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* docs: point the non-Sphinx consumers at the new pages

Four links in the two example READMEs pointed into `docs/source/.../ffi.md`
with heading anchors. Sphinx does not link-check those, so deleting the page
would have left them silently 404ing. They now use published URLs, which also
fixes a second problem: a relative path into `docs/source` only renders on
github.com and is broken for anyone reading the README from crates.io, an
sdist, or a vendored copy. `grep -rn "docs/source" examples/` is now empty.

`.ai/skills/ffi-capsule-protocol/SKILL.md` named the deleted page as "where the
truth is", and AGENTS.md sends agents to that skill before they touch a capsule
getter. It now points at the specific extension-guide pages and separately at
`ffi-internals.md`.

`llms.txt` filed the whole subject under "Optional" as "extending the Python
bindings" — the wrong shelf for the headline feature of a major release, since
that section means "skippable on a tight context budget". It gains an
Extensions and distribution section, `datafusion.extensions` and
`datafusion.ipc` in the API list, both FFI example crates, and the corrected
`distributing-work` URL.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* docs: fix extension-guide review findings, rename phase-one bundle hook

Six findings from a read-through of the new extension guide, plus the API
rename one of them turned into.

The "process local tokens" note sat in the guide index with nothing around it to
explain what a token was or why the reader should care. It moves to a new
`extension_codec_durable_metadata` section in `codecs.md`, where the reader is
already thinking about what goes in a payload: what to encode, then what the
examples do instead, and the three consequences that follow from parking live
objects in a process-local map — no double decode, no fan-out, and a leak for
any plan that never reaches a decoder. The checklist item now points there
instead of at the guide index.

The hook reference loses its `Capsule name` column, which restated
`datafusion_<thing>` for every row when the naming rule already derives it, and
gains a `Contributes` column instead. The argument column stays: those four
values are protocol, not a signature, and only 4 of the 18 hooks have a Python
definition to link at all — the rest are host-side imports, so links into Rust
source would rot faster than the table. Staleness is handled by
`test_hook_reference_table_lists_every_hook` instead, which greps `crates/` and
`python/datafusion/` for `__datafusion_*__` and diffs the set against the table
rows. Verified it fails when a row is dropped.

`capsule-protocol.md` described `abi_stable`, which datafusion-ffi no longer
uses. It now describes `stabby` and the part that is not stabby: `FFI_Option`
and `FFI_Result` are datafusion-ffi's own, because stabby's require
`T: IStable` and the `FFI_*` structs hold self-referential function pointers.
The conversion example converts to `Arc<dyn TableProvider>` rather than naming
`ForeignTableProvider`, since the `From` impl compares library markers and
returns the original `Arc` when both sides are the same library. Three other
snippets on that page had gone stale with it: `FFI_TableProvider::new` with
three arguments, `PyCapsule::new_bound`, and a receiving snippet whose variable
was named `codec`.

In `table-providers.md` all five `Registered with` cells now render
`Receiver.method`, so the schema row reads `Catalog.register_schema` rather
than a bare dotted path, with one sentence on reaching a `Catalog` first.
`Other session components` was in `functions.md`, where an optimizer rule and a
config struct are neither functions nor tables; it becomes its own page,
`other-components.md`, carrying the `extension_other_hooks` label so the index
rows still resolve.

Three guide pages named individual tests, which invites exactly the divergence
the reference is supposed to prevent. They name the suite now.

Finally the phase-one bundle hook. `__datafusion_session_extension__` reused
the name of the whole thing it is a hook on — `with_extensions` takes
extensions and `SessionExtensionExportable` is the bundle protocol — while its
sibling `__datafusion_session_planner__` is named for its content. It is now
`__datafusion_session_components__`, matching both its sibling and the
`SessionExtensionComponents` it returns, which leaves room for the UDF and
provider fields that will join the codec fields later. The protocol class
follows it to `SessionComponentsExportable`, since that file's convention is
one class per hook name. Neither name has shipped, so the upgrade guide needs
no before-and-after; it introduces both hooks as new in this release and names
the new one.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

* test: match the hook table against dispatch sites, not raw text

`test_hook_reference_table_lists_every_hook` scanned every byte of
`crates/**.rs` and `python/datafusion/**.py` for `__datafusion_*__`, so
comments and docstrings counted as evidence a hook exists. A doc-comment
contrasting a hook with one that was removed, or naming a hypothetical,
would have had to be deleted or added to the guide's table, and neither
is right.

Count sites instead. On the Rust side a site is a string literal holding
nothing but the hook name -- what `hasattr`, `getattr`, and
`call_capsule_getter` are handed -- or a `fn` of that name, which is a
hook the host implements itself. Error strings that merely embed a name
no longer count; each already sits beside a real lookup.

On the Python side, read the syntax tree rather than the text: a method
being defined, an attribute being accessed, or a string standing alone.
A docstring is one string node holding the whole docstring, so prose
drops out without a rule of its own.

The dispatched set is unchanged at 18, still matching the table exactly.
Verified in both directions: a comment naming a removed hook now passes,
while adding a real lookup for an undocumented name still fails.

Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>

---------

Co-authored-by: Claude Fable 5 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants