Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
424 changes: 424 additions & 0 deletions .ai/skills/review-comet-docs/SKILL.md

Large diffs are not rendered by default.

7 changes: 4 additions & 3 deletions .ai/skills/review-comet-expression-pr/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -225,9 +225,10 @@ and the requirement to record a performance audit. Hold the PR to both.

### Generated, review the source instead

The compatibility pages under `docs/source/user-guide/latest/compatibility/expressions/` and
`docs/source/user-guide/latest/configs.md` are produced by `GenerateDocs` and regenerated by CI on
merge. Never ask the contributor to hand-edit them. Check what feeds them:
`GenerateDocs` fills in the expression compatibility tables and the config tables when the docs are
built, and CI rebuilds and publishes the docs when a merge changes their inputs. The **Generated Content** section of
`docs/source/contributor-guide/style_guide.md` says exactly which text is generated. Never ask the
contributor to hand-edit that text. Check what feeds it:

- Expression marked `Incompatible(Some("..."))` but `getIncompatibleReasons()` is empty, so the
compat page shows it as supported with no caveats.
Expand Down
23 changes: 14 additions & 9 deletions .ai/skills/review-comet-pr/SKILL.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
---
name: review-comet-pr
description: Use when reviewing a DataFusion Comet pull request. Covers the workflow that applies to every PR and routes to the area-specific review skills for expressions, FFI, memory management, shuffle, and Iceberg writes. Provides guidance to a human reviewer rather than posting comments.
description: Use when reviewing a DataFusion Comet pull request. Covers the workflow that applies to every PR and routes to the area-specific review skills for expressions, FFI, memory management, shuffle, Iceberg writes, and documentation. Provides guidance to a human reviewer rather than posting comments.
argument-hint: <pr-number>
---

Expand Down Expand Up @@ -43,6 +43,7 @@ change.
| `native/core/src/execution/memory_pools/`, `CometTaskMemoryManager`, `CometShuffleMemoryAllocator`, `MemoryConsumer` / `try_grow` / `reserve` call sites, pool configs | `review-comet-memory-pr` |
| `native/shuffle/`, `spark/src/main/scala/org/apache/spark/sql/comet/execution/shuffle/`, `spark/src/main/java/org/apache/spark/shuffle/`, `CometShuffleExchangeExec` | `review-comet-shuffle-pr` |
| `IcebergWriteStrategy`, `IcebergWriteExec`, `IcebergCommitExec`, `CometIcebergWriteExec`, `CometIcebergNativeWrite`, `iceberg_write.rs`, `iceberg_partition_path.rs`, the iceberg-rust pin | `review-comet-iceberg-write-pr` |
| `docs/source/`, `README.md`, `AGENTS.md`, `.ai/skills/**/SKILL.md`, `.doc(...)` strings in config classes, doc comments | `review-comet-docs` |

For a new operator, read `docs/source/contributor-guide/adding_a_new_operator.md` alongside this
skill. There is no dedicated operator review skill yet.
Expand All @@ -52,8 +53,8 @@ For a PR that deals with timestamps or the session timezone, read
cuts across the serde, the native kernels, the scans and the JVM/native boundary, so no area skill
owns it. "Timestamps and timezones" in step 5 says what to check.

If the PR falls outside all of these, for example build, CI, docs only, or release tooling, this
skill on its own is the review.
If the PR falls outside all of these, for example build, CI, or release tooling, this skill on its
own is the review.

## 2. Gather PR Metadata

Expand Down Expand Up @@ -95,7 +96,7 @@ Anything that reports `getSupportLevel()` must report it honestly:

- `Compatible()`, matches Spark exactly
- `Incompatible(Some("reason"))`, differs in documented ways and requires
`spark.comet.expr.allowIncompatible=true`
`spark.comet.expression.<Name>.allowIncompatible=true`
- `Unsupported(Some("reason"))`, cannot be implemented

A PR that quietly marks something `Compatible` while the diff shows a known divergence is the single
Expand Down Expand Up @@ -264,7 +265,8 @@ For a change on a default path, look for tests with the inputs that broke earlie

### CI

`gh pr checks <pr> --repo apache/datafusion-comet`, and again with `--failed` for detail.
`gh pr checks <pr> --repo apache/datafusion-comet`. For only the failed checks, add
`--json name,state,bucket,link --jq '.[] | select(.bucket == "fail")'`.

Summarize failures in the review. Do not compare against failures on `main`.

Expand Down Expand Up @@ -299,10 +301,13 @@ same PR, because a follow-up that is not filed as an issue does not happen.
Each area skill names the doc for its subsystem and the specific claims in it that go stale. When you
load an area skill, read its doc and hold the diff against it.

**Do not ask contributors to hand-edit generated docs.** The compatibility guide pages under
`docs/source/user-guide/latest/compatibility/expressions/` and
`docs/source/user-guide/latest/configs.md` are produced by `GenerateDocs` and regenerated by CI on
every merge to `main`. Review the source that feeds them instead.
**Do not ask contributors to hand-edit generated docs.** `GenerateDocs` fills in the config tables
and the expression compatibility tables when the docs are built, and CI rebuilds and publishes the
docs when a merge to `main` changes their inputs. The
**Generated Content** section of `docs/source/contributor-guide/style_guide.md` says exactly which
text is generated. Review the source that feeds it instead. The prose around the generated tables,
the `_category_template/` files, and the `index.md` pages are written by hand and are reviewed
normally.

## Review Bar

Expand Down
3 changes: 3 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ Relevant entry points:
- [Adding a New Expression](docs/source/contributor-guide/adding_a_new_expression.md) /
[Adding a New Operator](docs/source/contributor-guide/adding_a_new_operator.md).
- [Debugging Guide](docs/source/contributor-guide/debugging.md).
- [Documentation Style Guide](docs/source/contributor-guide/style_guide.md): the terms to use for
how Comet runs a query, and the conventions for docs pages. Read it before editing `docs/source`,
`README.md`, or the `.doc(...)` strings in config classes.
- [Backporting to Release Branches](docs/source/contributor-guide/backporting.md): a fix that
goes to one release branch also goes to every newer one. Cherry-pick with `-x`, and never add
an unrelated change to a backport pull request.
Expand Down
1 change: 1 addition & 0 deletions docs/source/contributor-guide/index.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ Expression Audits <expression-audits/index>
Data Source Audits <data-source-audits/index>
Supported Spark Configurations <spark_configs_support>
Configuration Conventions <config_conventions>
Documentation Style Guide <style_guide>
```

```{toctree}
Expand Down
Loading