docs: add a documentation style guide and apply it to the remaining pages by Abhishek-Mohan · Pull Request #7151 · apache/datafusion-comet

Abhishek-Mohan · 2026-10-22T00:14:25Z

Which issue does this PR close?

Part of #4419. Operator renames are left for a separate issue, because they change plan output and test files (see my comment on #4419).

Rationale for this change

#4419 proposes a shared vocabulary for Comet's docs, mainly replacing vague uses of "native" with specific terms. The README and several user guides already use it (#4428, #4650, #4651, #4652), but the rules exist only in the issue, and several pages still use the old wording.

This PR writes the rules down as a style guide and applies them to the remaining pages that #4419 lists. Each page is its own commit, so you can review one commit at a time, or I can split the PR if you'd prefer.

How this PR covers #4419

#4419 asks for	Where
Three-axis vocabulary (pipeline, language, data format)	Style guide, T1-T10
Rule for "native", with the allowed compounds	Style guide, N1-N6
"Vectorized" not used as a synonym for columnar	T10
Value prop on `README.md`	Already done in #4428
Value prop on `docs/source/index.md`	The landing page's opening line
`scala_java_udfs.md`	Already done on `main`
`understanding-comet-plans.md`	Leftover wording. The three node kinds stay (see below)
`plugin_overview.md`: "Rust-implemented operators", and how JVM Arrow expressions fit into the pipeline	Wording, plus the new codegen dispatch paragraph
`native_shuffle.md` and `jvm_shuffle.md`	Wording. Their titles stay, as #4419 suggests
`gluten_comparison.md`: the JVM-on-Arrow path as a differentiator	Its existing Codegen Dispatch section already covers this. Wording updated
Migration plan, step 1: the vocabulary first, with no code or plan changes	This PR

Deliberate departures, explained in my comment on #4419:

Hybrid keeps its shipped meaning.
Expressions are not plan nodes, so there's no fourth "Arrow-native JVM expression" node kind. Codegen dispatch is described as an expression kind instead.
Operator renames, and the open questions about them, are left for a separate issue.

What changes are included in this PR?

1. Style guide (docs/source/contributor-guide/style_guide.md), linked from the Reference section of the contributor guide and from AGENTS.md:

Vocabulary (T1-T10). The three questions from Establish nomenclature style guide and audit operator names for clarity #4419: does Comet handle the work, which language runs it, and what format is the data in. Also covers codegen dispatch, the expression kinds, Arrow IPC, Spark rows, and "vectorized".
The rule for "native" (N1-N6). Which uses are vague, which are allowed ("native" followed by a component, such as native shuffle or native Parquet reader), and the contributor-guide forms such as "native code".
Operators, plan names, and expressions (O1-O4). Use the names plan output prints, link to the existing operator reference, and don't describe renames that haven't happened.
Page conventions (P1-P18). The structure, accuracy, and hygiene checks from Add a docs-review Claude skill to enforce style guide and documentation quality #4420, plus which content is generated, a redirect rule for moved pages, and attribution of third-party content.

Every rule has a stable ID so reviews and the #4420 skill (#7153) can cite it.

Where #4419 and the current code differ, the guide follows the code:

Hybrid keeps its meaning from expressions.md: an expression with both a Rust path and a codegen-dispatch path.
Expressions are kept separate from plan nodes.
"Vectorized" is used for proper names and batch kernels, not as a synonym for columnar.
"Arrow-native" describes data format. It doesn't claim that no per-row work happens.

2. Page cleanups, one commit per page:

Page	Changes
`contributor-guide/plugin_overview.md`	Wording, plus factual corrections, listed below
`contributor-guide/native_shuffle.md`	"Comet native operators" → "Comet operators", because native shuffle accepts any `CometPlan` child. "native" used as a noun → the component it means. Also: the revert exception for converted shuffle input, and what happens when direct read can't serve a stage
`contributor-guide/jvm_shuffle.md`	Says which parts run on the JVM and which in Rust. Corrects the `spark.comet.shuffle.directRead.enabled` description
`about/gluten_comparison.md`	"native" → "Comet pipeline", "Rust", or "Velox". Corrects the claim that Comet has no Delta Lake support: there is an experimental, opt-in contrib scan (#5365). Points the two Gluten limitations links, which return 404, at Gluten's v1.7.0 sources
`user-guide/latest/understanding-comet-plans.md`	Leftover uses after #4650. Adds `CometTakeOrderedAndProject` as another exception to single-block fusion. Also: the explain section says seven configs and adds the missing `spark.comet.explain.planOnly.enabled` and `spark.comet.explain.rules` rows, and the `CometCollectLimit` row says it runs Rust limit plans unless Spark collects it at the plan root
`index.md` (landing page)	"the Apache DataFusion native engine" ×4. The opening line now uses the README's Arrow framing. Removes "powerful", and makes the benchmarking link relative

Corrections in plugin_overview.md, which need the closest review:

Rule registration. CometSparkSessionExtensions also registers post-transition rules, AQE stage optimizer rules on Spark 3.5 and later, and IcebergWriteStrategy.
Partial stage conversion. An unsupported operator stays in Spark, not the whole plan. After Spark inserts transitions, RevertNativeForTransitionHeavyStages runs first. It can revert a transition-heavy stage when spark.comet.exec.transitionRevert.enabled is set, unless reverting it is unsafe. Then EliminateRedundantTransitions turns the remaining transitions into CometColumnarToRowExec, or into the Rust CometNativeColumnarToRowExec when that opt-in config is set.
Plan leaves.
- ScanExec takes Arrow batches from the JVM through the C Stream.
- A direct shuffle read is a ShuffleScanExec.
- The Parquet, Iceberg, and CSV scans read their files in Rust.
Codegen dispatch (new paragraph). Establish nomenclature style guide and audit operator names for clarity #4419 asks this page to explain how JVM code that works on Arrow batches fits into the pipeline. The new paragraph covers expressions routed through the JVM codegen dispatcher: JvmScalarUdfExpr → CometUdfBridge → CometScalaUDFCodegen, over Arrow through JNI. Scala and Java UDFs take the same path, and the operator stays Rust-implemented.
Block formation. Blocks are formed from consecutive CometNativeExec operators, except that the write operators start a new block for their child. The native shuffle writer takes in a CometNativeExec child.

Config keys, class names, and quoted explain output are unchanged on every page.

Overlap with open PRs:

docs: add a scan contributor guide and a scan PR review skill #6052 (scan guide) rewrites the Phase 1 scan section of plugin_overview.md, so this PR leaves that section as it is.
docs: publish the newest release at user-guide/latest and main at user-guide/dev #7112 renames user-guide/latest to user-guide/dev. The guide's links use latest today, and I'll update them if docs: publish the newest release at user-guide/latest and main at user-guide/dev #7112 lands first.
feat: txt file support #6784 adds a row to the Scans table in understanding-comet-plans.md, which this PR realigns, so the two conflict.
fix: stop gating columnar shuffle on native-serde checks it never uses #6110, perf: read shuffle blocks directly in native Iceberg writes #6926 and docs: add AI-assisted contributions page to the contributor guide #6734 touch nearby lines in the shuffle guides, the plans page, AGENTS.md, and the contributor index.

I'll rebase whichever lands first.

Not included:

Operator renames.
Problems the review turned up that need new content rather than rewording:
- the out-of-date from_unixtime example;
- operators missing from the operator reference;
- the CometBatchScan row;
- two H1s in plugin_overview.md.
A link from contributing.md, which I'll add after docs: add AI-assisted contributions page to the contributor guide #6734 merges.

How are these changes tested?

Docs only.

prettier --check passes on every changed file.
I built the docs locally with Sphinx. Every page renders, the new page appears under Reference, and every changed link and anchor resolves. The build gave no new warnings.
I ran the Add a docs-review Claude skill to enforce style guide and documentation quality #4420 skill (docs: add a docs-review skill #7153) on this PR with five models. It found real errors, such as a seventh explain config, conditions missing from the native columnar-to-row and direct-read sentences, and the wrong sources in the Generated Content table. Those are fixed here.
I checked each changed claim against main. Five independent AI reviews (GPT-6 Astra, Grok 4.7, Gemini 4 Argon, Claude Opus 5.5, GPT-6.1 Sol) checked them against the code. Over two rounds, each finding was either fixed or listed under Not included.

Council review

Each round, seven models ran dry runs: they followed the style guide literally as an agent would, rewrote pages with it, and ran the review skill on this branch. Findings were checked against the code before fixing. Models already at 10/10 on both PRs were not rerun in round 4.

Rounds: 4

Delegate	Round 1	Round 2	Round 3	Round 4
Claude Opus 5.5	8/10	8/10	9/10	10/10
Gemini 4 Argon	9/10	10/10	10/10	not run
Grok 4.7	7/10	8/10	9/10	10/10
GPT-6.1 Sol	9/10	9/10	10/10	not run
GPT-6 Astra	9/10	9/10	10/10	10/10
Claude Sonnet 5.5	8/10	8/10	8/10	9/10
GPT-6 Luna	8/10	8/10	9/10	10/10

AI assistance: OpenCode helped draft the guide and the edits and checked them against the code. The reviews above were run with OpenCode subagents.

Add docs/source/contributor-guide/style_guide.md, which writes down the vocabulary from apache#4419 (pipeline membership, implementation language and data format), the rule for "native", how to name operators in prose, and the page conventions that the docs-review skill in apache#4420 cites. Each rule has a stable ID so that reviews can cite it. The guide follows the code where apache#4419 and the code differ: "Hybrid" keeps the meaning it already has in expressions.md, expressions are described separately from plan nodes, and "Arrow-native" is a claim about data format only. The Generated Content section names the generated text exactly, by the markers GenerateDocs fills in, including the config tables built from QueryPlanSerde and contrib config classes, and the cast tables built from CometCast. P18 covers attribution of third-party content, which apache#4420 lists among its checks. The guide's scope covers .doc strings in contrib config classes, and its vocabulary and accuracy rules also apply to doc comments, AGENTS.md and agent skills. Link the page from the contributor guide toctree and from AGENTS.md. Part of apache#4419. Generated-by: OpenCode

Correct statements in plugin_overview.md that no longer match the code: - CometSparkSessionExtensions registers more than CometRule: it also registers rules that run after columnar transitions are inserted, AQE query stage optimizer rules on Spark 3.5 and later, and IcebergWriteStrategy. - Comet does replace part of a query stage, and an unsupported operator stays in Spark, not the whole plan. Describe the post-transition rules in the order CometRule runs them: RevertNativeForTransitionHeavyStages first, which can decline an unsafe revert, then EliminateRedundantTransitions, which uses CometColumnarToRowExec, or CometNativeColumnarToRowExec when that opt-in config is set and the converter supports the schema. - Not every leaf of the Rust plan is a ScanExec. ScanExec takes Arrow batches from the JVM; a direct shuffle read of a Comet shuffle with supported types is a ShuffleScanExec; the Parquet, Iceberg and CSV scans read their files in Rust. - Blocks are formed from consecutive CometNativeExec operators, except that the write operators start a new block for their child. The native shuffle writer absorbs a CometNativeExec child. Add a paragraph on the JVM codegen dispatcher, which apache#4419 asks this page to cover: some expressions in a Rust plan run Spark's generated JVM code over Arrow batches through JNI, and the operator around them stays in the Comet pipeline. Scala and Java UDFs take the same path. Also replace the vague uses of "native" with the apache#4419 terms. The scan section (Phase 1) is left alone, because apache#6052 rewrites it. Part of apache#4419. Generated-by: OpenCode

Replace vague uses of "native" in native_shuffle.md. Native shuffle accepts any CometPlan child, so call the child a Comet operator rather than a native one. Where "native" was used as a noun, name the Rust component instead. Also correct statements this touched: - When RevertNativeForTransitionHeavyStages restores a stage, a native shuffle whose input came from spark.comet.convert.shuffleInput.enabled switches back to JVM columnar shuffle. - When direct read cannot serve a shuffle stage, the stage stays as it is and the operators that read it cannot join a Rust plan. The check is QueryPlanSerde.supportedDataType; supportedSinkDataType does not exist. - Drop the unverified reason given for rejecting nested range keys. - Describe spark.comet.shuffle.directRead.enabled the same way as the JVM shuffle guide. Contributor-guide forms such as "native code" and "native plan", the "native shuffle" name, and config keys are unchanged. Part of apache#4419. Generated-by: OpenCode

Replace vague uses of "native" in jvm_shuffle.md. Say which parts of JVM shuffle run on the JVM (partitioning) and which run in Rust (Arrow IPC encoding), and call a shuffle's child a Comet operator, since native shuffle accepts any CometPlan child, including JVM-side ones. Say that direct read applies when a Comet block reads the shuffle as a ShuffleScan input. Also describe spark.comet.shuffle.directRead.enabled accurately: both read paths decode in Rust, and direct read decodes inside the Rust plan, bypassing Arrow FFI, as the config's own doc string says. Part of apache#4419. Generated-by: OpenCode

Replace vague uses of "native" with the term each sentence means: the Comet pipeline, Rust, or Velox on the Gluten side. Describe Velox and DataFusion as columnar query engines rather than vectorized ones. Two factual fixes came up while rewording: - The table formats section said Comet has no Delta Lake integration. main now has an experimental, opt-in Delta Lake scan (apache#5365), so say that and link the Delta Lake guide. - Link the compatibility guide by its path in the docs rather than by its published URL, and point the two Gluten limitations links, which return 404, at the page in Gluten's v1.7.0 sources. Part of apache#4419. Generated-by: OpenCode

apache#4650 rewrote this page. Fix the uses of "native" that remain: say "Rust" where the text means the Rust implementation or the Rust plan, "in the Comet pipeline" where it means Comet as a whole, and "Spark's built-in shuffle" instead of "Spark's native shuffle". Config keys, API names, and quoted explain output are unchanged. Also say that consecutive Rust-implemented operators are usually, not always, combined into one block, and add CometTakeOrderedAndProject to the exceptions: it runs its own final Rust plan, after a single-partition shuffle when its input has several partitions. Two small fixes on the same page: the explain section documents seven configs, not five, so add the missing spark.comet.explain.planOnly.enabled and spark.comet.explain.rules rows to its table; and CometCollectLimit is not purely JVM-side, since it runs Rust limit plans unless Spark collects it at the plan root, so say so and adjust the JVM-side section's intro. prettier realigns the Scans table because two of its rows changed. Part of apache#4419. Generated-by: OpenCode

Use the apache#4419 terminology on docs/source/index.md, which the README rewrite in apache#4428 did not cover. The page now says what Comet does with DataFusion rather than calling it a native engine, and the opening line carries the README's Arrow framing, as apache#4419 asks. Say that Comet hands DataFusion the operators it runs in Rust, not all supported operators, since some run on the JVM. Also drop "powerful" from the edited paragraph, and link the benchmarking guide by its relative path rather than its published URL. Part of apache#4419. Generated-by: OpenCode

sunchao

Reviewed all seven commits and nine changed files at 56396e38e2dd066e01256201b6b3dc4772dec970 against base 2b1c6a3938cbe1c4b2d0aa6383e64360569f877a. The full PR diff contains documentation changes only. I verified four new P2 findings. The PR has no existing reviews, conversation comments, or review threads.

Summary

Prior state and problem
The guides used “native” for both work handled by Comet and work executed in Rust. The plugin overview also said a query stage could convert only as a whole.
Design approach
The style guide distinguishes which engine handles an operation, which language executes it, and which format carries its data. The revised pages apply those distinctions to plans, shuffles, and expression dispatch.
Correctness
A non-adaptive final aggregate can read a Comet shuffle through ScanExec with direct reads enabled. Separately, a stage can revert to Spark without appearing in the spark.comet.explain.rules log. Both cases contradict new documentation.
Compatibility analysis
Spark 3.4.3, 3.5.9, 4.0.4, 4.1.3, and 4.2.0 sources confirm the documented transition ordering and collect-limit behavior. The AQE stage optimizer hook is present from Spark 3.5. Celeborn adds a child-class restriction missing from the revised shuffle eligibility rule.
Key design decisions
The guide preserves printed operator names and the shipped Hybrid expression classification. A JVM-dispatched expression can remain inside a Rust-implemented operator.
Implementation sketch
AGENTS.md and the contributor index link the new guide. The plugin overview, both shuffle guides, plan reference, Gluten comparison, and landing page adopt its terminology.
Performance
The patch adds no query execution overhead. Its JVM shuffle diagram does overstate the cost for a row-forwarding bridge. The probe consumed two rows with zero Arrow batches produced by the bridge.
Design
Linking to the existing operator reference and identifying the inputs to GenerateDocs gives contributors one place to verify plan names and generated content.
Abstraction & complexity
CometPlan identifies membership in the Comet pipeline. Predicting shuffle behavior also requires the manager’s selection rule and the child’s doExecute() implementation.
Behavioral changes worth calling out
Compared with branch-1.2, the PR changes contributor and user guidance. It documents partial-stage conversion, the separate final Rust plan for multi-partition top-K, and the experimental opt-in Delta scan.
Suggested improvements
Qualify direct reads by the serialized ShuffleScan leaf, distinguish Celeborn’s child requirement, show the JVM bridge’s row path, and narrow the rule-logging guarantee.

Routed skills. review-comet-pr, review-comet-shuffle-pr, review-comet-ffi-pr, review-comet-expression-pr, review-comet-memory-pr, and review-comet-iceberg-write-pr.

Exact-head CI. Label pull requests passed. Comet CI, CodeQL, and Check PR Title each reported action_required with zero jobs executed. Comet CI run

Validation. Focused Spark 4.1.3 probes exercised shuffle serialization, manager selection, JVM row consumption, and rule logging. Sphinx 7.4.7 built the head and merge base with the same 80 warnings. All eight changed documentation pages rendered, and all 52 local article links checked resolved. Validation excluded full Spark/native suites, live Celeborn I/O, release-document assembly, and Mermaid rendering.

- plugin_overview.md: a shuffle input is a ShuffleScanExec only when it is serialized as a ShuffleScan leaf, which needs direct read, an AQE query stage, and supported output types. Without AQE, the shuffle is read through ScanExec. - understanding-comet-plans.md: spark.comet.explain.rules logs the plan before and after CometScanRule, CometExecRule and EliminateRedundantTransitions, not stage reverts. Point to spark.comet.explain.fallback.log.enabled for those. - jvm_shuffle.md: scope the columnar-to-row diagram to children that produce Arrow batches. A CometSparkToColumnarExec child returns its input rows directly. - native_shuffle.md: with the Celeborn shuffle manager, native shuffle needs a CometNativeExec child, not any CometPlan. Part of apache#4419. Generated-by: OpenCode

Abhishek-Mohan · 2026-10-28T17:34:02Z

Note

🤖 Claude Opus 5.5 responding on behalf of @Abhishek-Mohan

@sunchao all four findings are fixed in 3c982c8, and main is merged in (459c6fe). Ready for another look.

andygrove · 2026-10-28T23:41:30Z

 Spark are disabled by default, but users can opt in. See the [Comet Compatibility Guide] for more information.

-[Comet Compatibility Guide]: https://datafusion.apache.org/comet/user-guide/latest/compatibility/index.html
+[Comet Compatibility Guide]: /user-guide/latest/compatibility/index.md


Changing this link format may affect how the site gets published?

Note
🤖 Claude Opus 5.5 responding on behalf of @Abhishek-Mohan

No effect on publishing. Sphinx resolves /user-guide/latest/compatibility/index.md to the relative link ../user-guide/latest/compatibility/index.html in the built page, the same form main already uses for four other links on this page. A base-vs-head docs build shows no new warnings, and the build now warns if the target page moves.

andygrove · 2026-10-28T23:44:07Z

This looks great @Abhishek-Mohan. I've been reviewing the changes manually. Will finish reviewing tomorrow!

comphead

I checked the rewritten paragraphs against the code at this head, and the transition rules, the write-block exception, the explain configs and the renamed operators all match. Three leftovers outside the diff, so no inline comments. In plugin_overview.md the Shuffle Reads paragraph (lines 202 to 204) still says the compressed blocks go to the plan whenever a native plan consumes the shuffle and spark.comet.shuffle.directRead.enabled is on, while the rewritten paragraph at lines 156 to 163 now requires a ShuffleScan leaf, which needs an AQE shuffle query stage. Could it use the same condition, as the jvm_shuffle.md sentence already does? The overview figure comet-overview.svg still says "Apache DataFusion native engine" and "Native plan", which the new alt text and the guide's own "Before" example move away from. Line 81 of index.md still says "seamlessly", which P10 lists as a marketing word.

… the home page The Shuffle Reads paragraph in plugin_overview.md now gives the same ShuffleScan condition as the paragraph above it and jvm_shuffle.md. The overview figure and the index.md compatibility blurb write Rust instead of native (N3) and drop seamlessly (P10). Generated-by: OpenCode

…igure labels CometCollectLimit takes rows on the JVM for collect, show and take, through executeCollect, and runs Rust limit plans otherwise; where it sits in the plan does not decide this. The overview figure no longer says every supported operator runs in Rust (CometUnion does not), and drops vectorized as a synonym for columnar (T10). Generated-by: OpenCode

… the Gluten page With the Celeborn shuffle manager, native shuffle also needs spark.comet.shuffle.mode=native and the manager's own support checks (cometCelebornShuffleFallbackReason), not only a CometNativeExec child. Two changed lines on the Gluten page now address the reader (P9). Generated-by: OpenCode

N1 now covers every form of the banned phrases, such as ran natively and Comet-native. N2 allows the native shuffle path without pairing it with JVM shuffle. N3 replaces and similar with a test, names user-guide/, and says pages outside the contributor guide allow only the N2 compounds. Agents running the guide could not apply the old wording consistently. Generated-by: OpenCode

…or a fused block Under Adaptive Query Execution, CometRule also runs as the columnar rule on each new query stage, not only as a stage preparation rule. The Celeborn condition names nativeShuffleFallbackReason, which holds the manager's checks, instead of a user-guide section that only covers released clients. The pages already say Rust plan for a group of operators that run together in Rust, so the three Comet block mentions use it too. Generated-by: OpenCode

T7 lists Arrow-native, which N3's limit to N2 compounds outside the contributor guide would otherwise forbid. N3 also says any other use of native is a finding, so a phrase outside the lists has an outcome. Generated-by: OpenCode

A dry run read native cache scan as allowed by N2, but the in-memory cache scan runs on the JVM. N2 now says the scan and writer compounds apply only to Rust-implemented ones. Generated-by: OpenCode

A dry run rewrote "falls back to its native execution" and "executes natively (not falling back to Spark)" wrongly, because N1 offered only T1 or T3. N1 now says not falling back is T1, since codegen dispatch is not a fallback, and that Spark's own behavior is N5. Generated-by: OpenCode

…ame, keep the N5 test case stable The merge-base step now tries again after deepening and stops with an error if there is still no base. Branch names with a slash no longer break the state file path. The N5 case moves to the scratch page, so it holds before and after apache#7151 rewrites understanding-comet-plans.md. Generated-by: OpenCode

Abhishek-Mohan · 2026-10-29T07:15:21Z

Note

🤖 Claude Opus 5.5 responding on behalf of @Abhishek-Mohan

@comphead all three fixed in 60af6cb: the Shuffle Reads paragraph uses the ShuffleScan condition, comet-overview.svg and index.md no longer say native or seamlessly. bc3ffc2 corrects two of the figure labels.

andygrove

Thanks for writing the vocabulary down and for applying it page by page. I checked the rewritten paragraphs against the code, including the transition rules, the Celeborn condition, the supportedDataType rename, the explain configs, and the CometSparkToColumnarExec and CometExec doExecute() behavior. They all match. I also confirmed the Delta guide and the Gluten v1.7.0 limitations links resolve. I have two things I would like addressed.

The first is the scope of the style guide. The opening paragraph says the guide covers everything under docs/source, README.md and the .doc(...) strings, and N1 says it applies in any tense or form. Several pages outside this PR still use phrases N1 rules out. For example, timezones.md has "runs natively only in UTC sessions", sql-file-tests.md has "runs natively" and "the native path", memory_management.md has "native operators", and ffi.md has "a native operator consumes Comet shuffle output". As written, a reviewer can cite N1 against any of those pages, and an author who changes one line may be asked to rewrite the whole page. Could the scope paragraph say that the rules apply to new and changed text, and that existing pages move over as they are edited? If you plan a sweep of the remaining pages, could you file a tracking issue and link it from #4419 so we can see where it stands?

The second is in gluten_comparison.md. The old text called Velox and DataFusion vectorized query engines, and the new text calls them columnar query engines. That sentence is about batch-at-a-time execution, which T10 allows "vectorized" for. "Columnar" is a claim about data layout instead, and both projects describe themselves as vectorized. Would "vectorized" be more accurate here?

CI note: at the current head only the label job has run. The rest of Comet CI, including the prettier check on the markdown, still needs a workflow approval, so I could not see a result for it.

Existing pages move over as they are edited, so a review does not ask a change to restyle text it leaves alone, unless the change makes that text wrong (P16). Requested in review. Generated-by: OpenCode

The sentence is about batch-at-a-time execution, which T10 allows vectorized for, and both projects describe themselves that way. Columnar is a claim about data layout. Requested in review. Generated-by: OpenCode

Abhishek-Mohan · 2026-10-30T23:45:42Z

Note

🤖 Claude Opus 5.5 responding on behalf of @Abhishek-Mohan

Both done. The scope paragraph now says the rules apply to new and changed text, and that existing pages move over as they are edited. The same change is on #7153. gluten_comparison.md says "vectorized query engine" again. #7358 tracks the remaining pages and is linked from #4419.

On CI: since #7311 the pull request checks run on my fork, and they passed on the latest push, including prettier in Preflight (run).

sunchao

I rechecked the four earlier findings against the shuffle and AQE rules. The revised descriptions now distinguish when the transition can happen and what the driver actually logs. I also rendered the base and head with Sphinx. The head introduced no additional warnings in that comparison, and the overview page resolves to the SVG in the built site. Looks good to me.

* docs: add a documentation style guide Add docs/source/contributor-guide/style_guide.md, which writes down the vocabulary from apache#4419 (pipeline membership, implementation language and data format), the rule for "native", how to name operators in prose, and the page conventions that the docs-review skill in apache#4420 cites. Each rule has a stable ID so that reviews can cite it. The guide follows the code where apache#4419 and the code differ: "Hybrid" keeps the meaning it already has in expressions.md, expressions are described separately from plan nodes, and "Arrow-native" is a claim about data format only. The Generated Content section names the generated text exactly, by the markers GenerateDocs fills in, including the config tables built from QueryPlanSerde and contrib config classes, and the cast tables built from CometCast. P18 covers attribution of third-party content, which apache#4420 lists among its checks. The guide's scope covers .doc strings in contrib config classes, and its vocabulary and accuracy rules also apply to doc comments, AGENTS.md and agent skills. Link the page from the contributor guide toctree and from AGENTS.md. Part of apache#4419. Generated-by: OpenCode * docs: add a docs-review skill Add .ai/skills/review-comet-docs/SKILL.md, a sibling of review-comet-pr for documentation. It has three modes: PR mode reviews the docs a pull request changes, branch mode reviews a branch before it becomes a PR, and audit mode reviews an existing file or directory, including files not yet committed. The skill reads the documentation style guide on every run and cites its rule IDs rather than restating the rules. It finds the Apache remote and the PR's real base, reads everything from the reviewed revision, checks whether main has moved since, and checks only the added lines in PR and branch mode. Mechanical checks come first: vague "native" (including in Markdown italics), config keys (including keys built from a name at run time), operator names, links, anchors, images, external links, structure, redirects for moved pages, and version claims. Then it runs judgment checks, the most important being whether each claim, and each condition in it, still matches the code. Besides docs pages, it checks changed .doc strings, doc comments, AGENTS.md and agent skills. Findings are must-fix, should-fix, or nit, with nits dropped when the skill runs as part of a review-comet-pr review. It never edits files or posts comments. A short table of fixed cases says how to check the skill after changing it. In review-comet-pr, route docs changes to the new skill, mention documentation in its description, and remove "docs only" from the list of PRs it covers on its own. Fix two things in it that no longer match: `gh pr checks --failed` no longer exists, and the opt-in key is spark.comet.expression.<Name>.allowIncompatible. In review-comet-pr and review-comet-expression-pr, point at the style guide for exactly which docs text is generated. Closes apache#4420. Generated-by: OpenCode * docs: make review-comet-docs runs reproducible and check links with Sphinx - Pin the reviewed commit (HEAD_SHA) after checking it against headRefOid, and read everything at it, including the style guide. - List changed files with git diff instead of gh pr view --json files, which stops at 100 files. - Fail on a missing merge base (shallow clones) instead of diffing against an empty revision. - Check drift against the PR's own target branch, not always main. - Name the state file per run and quote its values, so concurrent reviews and paths with spaces work. - Use git grep -E with perl instead of git grep -P, so Git builds without PCRE work, and set core.quotePath=false for non-ASCII names. - Replace the hand-written link, anchor, toctree, and heading checks with a Sphinx build of the base and the head, compared by warning. It resolves the generated per-version pages the way docs/build.sh does, catches links that a renamed heading breaks on unchanged pages, handles reStructuredText, and resolves toctree entries properly. H1s are counted with markdown-it, which handles nested code fences. - Route AGENTS.md and agent skills to the skill from review-comet-pr. Part of apache#4420. Generated-by: OpenCode * docs: retry the merge base after deepening, sanitize the state file name, keep the N5 test case stable The merge-base step now tries again after deepening and stops with an error if there is still no base. Branch names with a slash no longer break the state file path. The N5 case moves to the scratch page, so it holds before and after apache#7151 rewrites understanding-comet-plans.md. Generated-by: OpenCode * docs: make the docs build and heading check fail loudly and run in audit mode build() now reports a failed Sphinx run instead of comparing empty warning lists, builds the working tree in audit mode, and uses a named virtual environment, as does the H1 check. Audit mode gets a config-key command, the stop rule no longer treats an empty search as an error, the generated-content check says what to compare, and the apache#6786 case reads the guide from main. Generated-by: OpenCode * docs: fix what seven agents hit when running the docs review skill Dry runs on Claude, GPT, Gemini and Grok models in zsh on macOS found these gaps. The venv now picks Python 3.10 or later and cleans up on failure, since the macOS system Python cannot install the docs requirements. Each call reloads the state with a given line, and a failed docs build no longer reads as a reason to stop. The guide falls back to the target when the head predates it. Audit mode reads the working tree for the H1 check and code reads, and skips the rename check. Code files are filtered to doc comments, SVG text is reviewed, protobuf message names are not operators, and the build output no longer carries the macOS /private prefix. Generated-by: OpenCode * docs: make the native rules testable N1 now covers every form of the banned phrases, such as ran natively and Comet-native. N2 allows the native shuffle path without pairing it with JVM shuffle. N3 replaces and similar with a test, names user-guide/, and says pages outside the contributor guide allow only the N2 compounds. Agents running the guide could not apply the old wording consistently. Generated-by: OpenCode (cherry picked from commit 80d4af0) * docs: allow Arrow-native in N2 and close the native rules T7 lists Arrow-native, which N3's limit to N2 compounds outside the contributor guide would otherwise forbid. N3 also says any other use of native is a finding, so a phrase outside the lists has an outcome. Generated-by: OpenCode (cherry picked from commit ad78e9b) * docs: save the file list, read the guide from the working tree as a last resort The changed files that step 3 selects are now written to a file saved in the state, so later calls pass them to Git instead of rebuilding the list by hand. When neither the head nor the target has the guide, as for a PR made before it existed, the skill reads the working tree's copy and says so, instead of stopping. Audit mode leaves the revision out of the code searches, because an empty quoted revision makes Git fail. Generated-by: OpenCode * docs: count audit files for the cap, skip test sources, name the guide fallback Audit mode has no ADDED list, so the 10-file cap now counts the files step 1 lists. Test sources no longer enter the file list, since their string literals passed the doc-comment filter. The stop rule names the step 2 guide lookup as a fallback, and the key notes cover the EXPRNAME placeholder pages use. Generated-by: OpenCode * docs: limit the N2 scan compounds to Rust-implemented scans A dry run read native cache scan as allowed by N2, but the in-memory cache scan runs on the JVM. N2 now says the scan and writer compounds apply only to Rust-implemented ones. Generated-by: OpenCode (cherry picked from commit e8b8000) * docs: point N1 at T1 for not-a-fallback and at N5 for Spark A dry run rewrote "falls back to its native execution" and "executes natively (not falling back to Spark)" wrongly, because N1 offered only T1 or T3. N1 now says not falling back is T1, since codegen dispatch is not a fallback, and that Spark's own behavior is N5. Generated-by: OpenCode (cherry picked from commit 147f3f4) * docs: save the audit file list as PATHS Audit mode used an undefined <files> placeholder. It now saves the step 1 list as PATHS, as the other modes do, and says the mechanical checks cover every file while only the judgment checks are capped. Generated-by: OpenCode * docs: apply the style guide to new and changed text Existing pages move over as they are edited, so a review does not ask a change to restyle text it leaves alone, unless the change makes that text wrong (P16). Requested in review. Generated-by: OpenCode (cherry picked from commit fdff0b1) * docs: keep paths with spaces whole, build each revision with its own requirements The file list was passed as $(cat "$PATHS"), which splits a name such as "My Page.md" into two pathspecs, so the page was silently skipped. Commands now pass it through tr and xargs -0 -r. The docs build reused one environment installed from the checkout's requirements; a new venv function installs each revision's own docs/requirements.txt into an environment named after its hash, so a change that updates a Sphinx extension is built with it. Requested in review. Generated-by: OpenCode * docs: list audit files unquoted, note generated expression anchors Audit mode listed files with Git's default path quoting, so a page such as résumé.md became an escaped string and was skipped. The docs build copies only the expression templates, so a link to a heading that GenerateDocs adds to a per-version page warns although the published page has it; the skill now says to check the serde's notes instead. Requested in review. Generated-by: OpenCode * docs: print audit search results with unquoted file names The audit searches now pass core.quotePath=false too, so a finding on a page such as résumé.md names the file as written. Generated-by: OpenCode * docs: match the category page and Spark version for generated anchors A heading GenerateDocs adds appears only on the category page whose serde map holds the expression, at the Spark versions that register it. The manual check now confirms both, so a link to the same heading on another category page is reported as broken. Requested in review. Generated-by: OpenCode

github-actions Bot added the documentation Improvements or additions to documentation label Oct 22, 2026

Abhishek-Mohan mentioned this pull request Oct 23, 2026

Establish nomenclature style guide and audit operator names for clarity #4419

Open

Abhishek-Mohan force-pushed the docs/4419-style-guide branch from a088151 to 03723eb Compare October 23, 2026 07:30

Abhishek-Mohan changed the title ~~docs: add a terminology style guide for contributor docs~~ docs: add a documentation style guide Oct 23, 2026

Abhishek-Mohan force-pushed the docs/4419-style-guide branch from 03723eb to 7cce988 Compare October 23, 2026 07:33

Abhishek-Mohan force-pushed the docs/4419-style-guide branch 2 times, most recently from 9d5fadf to 0151778 Compare October 23, 2026 07:39

Abhishek-Mohan mentioned this pull request Oct 23, 2026

docs: add a docs-review skill #7153

Merged

Abhishek-Mohan changed the title ~~docs: add a documentation style guide~~ docs: add a documentation style guide and apply it to the remaining pages Oct 23, 2026

Abhishek-Mohan force-pushed the docs/4419-style-guide branch 4 times, most recently from fd74821 to 0cc4995 Compare October 24, 2026 00:35

Abhishek-Mohan added 7 commits October 23, 2026 18:00

Abhishek-Mohan force-pushed the docs/4419-style-guide branch from 0cc4995 to 56396e3 Compare October 24, 2026 01:03

Abhishek-Mohan marked this pull request as ready for review October 24, 2026 01:46

sunchao reviewed Oct 24, 2026

View reviewed changes

Comment thread docs/source/contributor-guide/plugin_overview.md Resolved

Comment thread docs/source/user-guide/latest/understanding-comet-plans.md Resolved

Comment thread docs/source/contributor-guide/jvm_shuffle.md Resolved

Comment thread docs/source/contributor-guide/native_shuffle.md Resolved

Merge branch 'main' into docs/4419-style-guide

491f561

Abhishek-Mohan requested a review from sunchao October 28, 2026 22:43

andygrove reviewed Oct 28, 2026

View reviewed changes

comphead reviewed Oct 28, 2026

View reviewed changes

Abhishek-Mohan added 8 commits October 28, 2026 17:55

docs: limit the N2 scan compounds to Rust-implemented scans

e8b8000

A dry run read native cache scan as allowed by N2, but the in-memory cache scan runs on the JVM. N2 now says the scan and writer compounds apply only to Rust-implemented ones. Generated-by: OpenCode

Abhishek-Mohan requested review from andygrove and comphead October 29, 2026 16:48

andygrove reviewed Oct 30, 2026

View reviewed changes

Abhishek-Mohan added 3 commits October 30, 2026 16:19

Merge branch 'main' into docs/4419-style-guide

ff16d83

docs: apply the style guide to new and changed text

fdff0b1

Existing pages move over as they are edited, so a review does not ask a change to restyle text it leaves alone, unless the change makes that text wrong (P16). Requested in review. Generated-by: OpenCode

docs: call Velox and DataFusion vectorized query engines again

3981303

The sentence is about batch-at-a-time execution, which T10 allows vectorized for, and both projects describe themselves that way. Columnar is a claim about data layout. Requested in review. Generated-by: OpenCode

Abhishek-Mohan mentioned this pull request Oct 30, 2026

docs: move existing pages to the style guide's "native" rules #7358

Open

48 tasks

sunchao approved these changes Oct 31, 2026

View reviewed changes

andygrove added this pull request to the merge queue Nov 3, 2026

Merged via the queue into apache:main with commit f78c0b1 Nov 3, 2026
27 checks passed

Abhishek-Mohan mentioned this pull request Nov 4, 2026

docs: apply the style guide's "native" rules to timezones.md #7477

Open

Navigation Menu

Repository navigation

docs: add a documentation style guide and apply it to the remaining pages - #7151

docs: add a documentation style guide and apply it to the remaining pages#7151
andygrove merged 21 commits into
apache:mainfrom
Abhishek-Mohan:docs/4419-style-guide

Abhishek-Mohan commented Oct 22, 2026 •

edited

Loading

Uh oh!

sunchao left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Abhishek-Mohan commented Oct 28, 2026

Uh oh!

andygrove Oct 28, 2026

Uh oh!

Abhishek-Mohan Oct 29, 2026

Uh oh!

andygrove commented Oct 28, 2026

Uh oh!

comphead left a comment

Uh oh!

Abhishek-Mohan commented Oct 29, 2026

Uh oh!

andygrove left a comment

Uh oh!

Abhishek-Mohan commented Oct 30, 2026

Uh oh!

sunchao left a comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

Abhishek-Mohan commented Oct 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

How this PR covers #4419

What changes are included in this PR?

How are these changes tested?

Council review

Uh oh!

sunchao left a comment

Choose a reason for hiding this comment

Summary

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Abhishek-Mohan commented Oct 28, 2026

Uh oh!

andygrove Oct 28, 2026

Choose a reason for hiding this comment

Uh oh!

Abhishek-Mohan Oct 29, 2026

Choose a reason for hiding this comment

Uh oh!

andygrove commented Oct 28, 2026

Uh oh!

comphead left a comment

Choose a reason for hiding this comment

Uh oh!

Abhishek-Mohan commented Oct 29, 2026

Uh oh!

andygrove left a comment

Choose a reason for hiding this comment

Uh oh!

Abhishek-Mohan commented Oct 30, 2026

Uh oh!

sunchao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Abhishek-Mohan commented Oct 22, 2026 •

edited

Loading