Skip to content
Merged
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
83 changes: 29 additions & 54 deletions AGENTS.md
Original file line number Diff line number Diff line change
@@ -1,16 +1,17 @@
# AGENTS.md

Guidance for AI agents working in the `substrait-java` repository. This complements
`CONTRIBUTING.md` (commit conventions, style guide) with practical, codebase-specific
knowledge.
Entry point for AI agents working in the `substrait-java` repository. Read the shared,
human-facing docs first, then keep the codebase-specific notes below in mind.

## What this project is
## Start here

`substrait-java` is the Java implementation of [Substrait](https://substrait.io/) —
a cross-language specification for relational query plans. It provides an immutable
POJO model for plans/relations/expressions/types and bidirectional conversion to and
from the Substrait protobuf wire format, plus integrations (Isthmus → Apache Calcite,
Spark).
- **[`readme.md`](readme.md)** — what the project is, the module overview, and how to build
and run it.
- **[`CONTRIBUTING.md`](CONTRIBUTING.md)** — commit conventions, the style guide, and the
build / test / format / PMD command mechanics plus the JDK 17 daemon and GraalVM
native-image setup.

For GitHub work (issues, PRs), use the `gh` CLI.

## Module layout

Expand Down Expand Up @@ -115,47 +116,11 @@ Proto conversion is split into two directions, and the class name tells you whic
converters.
- POJO types are created with `TypeCreator.REQUIRED` / `TypeCreator.NULLABLE`.

## Building, testing, formatting

- Build/run a module's tests: `./gradlew :core:test`
- Single test class: `./gradlew :core:test --tests "io.substrait.<pkg>.<Class>"`
- Format code: `./gradlew spotlessApply` (Google Java Format); scope to a module with
`./gradlew :core:spotlessApply` when iterating. `spotlessCheck` runs in CI.
- Static analysis: `substrait.java-conventions` applies **PMD** (custom ruleset at
`build-logic/src/main/resources/substrait-pmd.xml`) to the Java modules, and `check`/
`build` **fail** on violations. Easy ones to trip: missing `@Override`, unused private
fields/methods/locals, `var` (rule `UseExplicitTypes` — use explicit types), and
`public` JUnit 5 test classes/methods (they must be package-private).
- Spark subprojects are nested: compile with
`./gradlew :spark:spark-3.5_2.12:compileScala`.
- Requires running Gradle with JDK 17 and the `--add-exports` flags in
`~/.gradle/gradle.properties` (see `CONTRIBUTING.md`), or `spotlessApply` may fail.
Keep the daemon on JDK 17 *consistently*: `build-logic`'s convention plugins are
compiled to bytecode matching the daemon's JDK, so switching JDKs between builds can
leave cached plugins a later daemon can't load (`UnsupportedClassVersionError`) —
`./gradlew --stop` then rebuild clears the stale daemon/cache.
- The `isthmus-cli` **native image** is the exception to JDK 17: `nativeCompile` uses
whatever JDK runs the Gradle daemon (`graalvmNative { toolchainDetection = false }` in
`isthmus-cli/build.gradle.kts`), so it needs the daemon on a **GraalVM** JDK with
`native-image` — CI uses GraalVM **25**, while everything else declares a JDK 17
toolchain. Switching the daemon between the two is a common cause of the daemon/cache
churn noted above.
- `build-logic/` is an **included build**, not a normal subproject: it does **not**
inherit the root `gradle.properties`, so its Kotlin-compile daemon heap is set in
`build-logic/gradle.properties` (raising the root heap does nothing for it). Avoid
`--no-build-cache` casually — it forces `build-logic`'s Kotlin plugins to recompile
every run.
- CI (`.github/workflows/pr.yml`) runs the **full** `./gradlew build --rerun-tasks`,
plus `yamllint`, `editorconfig-checker`, and commitlint (all also wired as local
pre-commit hooks). Narrower local tasks pass while the PR fails — build the whole
thing before pushing.
- `javadoc` runs doclint that **fails the build**, but only via `build`/`javadocJar` —
not via `compileJava` / `test` / `spotlessCheck`. So Javadoc errors surface only on
the PR; run `./gradlew :core:javadoc` before pushing.

When you add to the public expression/visitor API, verify the dependent modules still
compile — they have their own visitor implementors:
`./gradlew :core:spotlessCheck :isthmus:compileJava :spark:spark-3.5_2.12:compileScala :examples:substrait-spark:compileJava`
## Building and testing

Run `./gradlew build` and make sure it passes before pushing — narrower tasks skip checks that
CI runs, and a `:core` change can break the visitor implementors in the other modules. See
[`CONTRIBUTING.md`](CONTRIBUTING.md#building-and-testing).

## Isthmus (Calcite conversion) notes

Expand Down Expand Up @@ -195,10 +160,20 @@ compile — they have their own visitor implementors:

## Conventions & workflow

- **Conventional commits** are required (CI lints them, and PR title + body must form a
valid commit message). Scope tags seen in history: `feat(core)`, `feat(pojo)`,
`feat(isthmus)`, `feat(extensions)`, `build(deps)`, `chore(release)`. A `!` marks a
breaking change.
- **Keep PR descriptions high-signal.** The PR title and body together become the
squash-merge commit message that `semantic-release` uses to build `CHANGELOG.md`, so they
must together form a valid conventional commit (see
[`CONTRIBUTING.md`](CONTRIBUTING.md#commit-conventions)). Beyond that, leave out the noise
agents tend to add:
- **Lists of files touched** — they're in the diff.
- **Claims that CI-verified things pass** — e.g. "tests pass", "spotless clean". If they
didn't, the checks would be red.
- **Process notes that are already implicit** — e.g. "opened as draft pending review".

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for adding this bit


Do include the rationale, and for spec-tracking changes the spec version (e.g.
`spec v0.88.0`). Keep commit bodies free of git trailers (`Signed-off-by`,
`Co-authored-by`, tool-attribution lines) — `semantic-release` builds the changelog from
the commit message and history here doesn't carry them.
- **No GitHub issue/PR references in source** (comments or Javadoc) — they belong in
commit messages and PR descriptions. `Closes #NNN` in the commit/PR body is fine;
in the code, describe behavior and spec version (e.g. `spec v0.88.0`) instead.
Expand Down
43 changes: 43 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ This page provides some orientation and recommendations on how to get the best r

1. [Commit conventions](#commit-conventions)
2. [Style Guide](#style-guide)
3. [Building and testing](#building-and-testing)

## Commit Conventions

Expand Down Expand Up @@ -45,3 +46,45 @@ regardless of which JDK launches Gradle, as long as a JDK 17 is installed and di
Spotless is the exception: the `google-java-format` version it uses only runs on JDK 17 and
fails with `NoSuchMethodError` / `NoClassDefFoundError` when the Gradle daemon runs on a newer
JDK, so `./gradlew spotlessApply` (and `spotlessCheck`) require the daemon itself to be on JDK 17.

Keep the daemon on JDK 17 **consistently**. The `build-logic` convention plugins are compiled
to bytecode matching the daemon's JDK, so switching JDKs between builds can leave cached plugins
that a later daemon cannot load (`UnsupportedClassVersionError`); run `./gradlew --stop` and
rebuild to clear the stale daemon and cache.

The one exception is the `isthmus-cli` **native image**: `nativeCompile` uses whatever JDK runs
the Gradle daemon (`graalvmNative { toolchainDetection = false }` in `isthmus-cli/build.gradle.kts`),
so it needs the daemon on a **GraalVM** JDK with `native-image` (CI uses GraalVM 25). Switching
the daemon between JDK 17 and GraalVM is the most common cause of the cache churn above.

## Building and testing

`./gradlew build` builds and tests everything; see the [readme](readme.md#building) for the
high-level build and the native-image executable. Useful narrower tasks while iterating:

* **Run a module's tests:** `./gradlew :core:test`
* **A single test class:** `./gradlew :core:test --tests "io.substrait.<pkg>.<Class>"`
* **Format:** `./gradlew spotlessApply` (Google Java Format), or scope it with
`./gradlew :core:spotlessApply`. `spotlessCheck` runs in CI and requires a JDK 17 daemon
(see [Gradle & JDK 17](#gradle--jdk-17)).
* **Spark variants:** the Scala source is shared across `spark-3.4_2.12`, `spark-3.5_2.12`, and
`spark-4.0_2.13`; `./gradlew :spark:build` builds all three, or compile one with
`./gradlew :spark:spark-3.5_2.12:compileScala`. Formatting runs from the parent `:spark`.

Some checks are **not** wired into `compileJava` / `test`, so they can pass locally yet fail
CI — build the whole thing before pushing:

* **PMD** (`substrait.java-conventions`, ruleset
`build-logic/src/main/resources/substrait-pmd.xml`) runs via `check` / `build` and fails on
violations. Common tripwires: missing `@Override`, unused private fields/methods/locals,
`var` (rule `UseExplicitTypes` — use explicit types), and `public` JUnit 5 test
classes/methods (they must be package-private).
* **javadoc** doclint fails the build, but only via `build` / `javadocJar` — run
`./gradlew :core:javadoc` before pushing.
* **CI** runs the full `./gradlew build --rerun-tasks` plus `yamllint`, `editorconfig-checker`,
and commitlint (all also wired as local pre-commit hooks).

`build-logic/` is an **included build**, not a normal subproject, so it does not inherit the
root `gradle.properties`; its Kotlin-compile daemon heap is set in `build-logic/gradle.properties`.
Avoid `--no-build-cache` casually — it forces the `build-logic` Kotlin plugins to recompile on
every run.
Loading