diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 00000000000..bbfeddb250b --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,219 @@ + + +# AGENTS.md + +Guidance for AI coding agents working on Apache Arrow. It covers the +conventions shared across the repository; component directories may +carry their own `AGENTS.md` with build and test commands, and the +closest file to the code you are editing takes precedence. + +## Repository Gotchas + +- This is a monorepo. The C++ library (`cpp/`) is the shared core; + PyArrow (`python/`), R (`r/`), and MATLAB (`matlab/`) bind to it + directly, C/GLib (`c_glib/`) wraps it, and Ruby (`ruby/`) binds to + C/GLib. A change in `cpp/` can break any of these, so check the + dependent bindings when you touch C++. +- Java, Go, Rust, C#, JavaScript, Swift, and Julia are **not** in this + repository. They live in `apache/arrow-java`, `apache/arrow-go`, + `apache/arrow-rs`, `apache/arrow-dotnet`, `apache/arrow-js`, + `apache/arrow-swift`, and `apache/arrow-julia`. Do not look for or + recreate them here. +- Generated files are committed and must not be hand-edited: + `r/R/arrowExports.R` and `r/src/arrowExports.cpp` (regenerated by + `r/data-raw/codegen.R`), and everything under `cpp/src/generated/`. +- C++ has two build systems. A file added to or removed from a + `CMakeLists.txt` usually needs the same change in the sibling + `meson.build`. +- Every new source file needs the Apache License 2.0 header. Copy it + from an existing file in the same directory. CI's license check + (`rat`) rejects files without one. + +## Building and Testing + +Each component has its own toolchain. Read the developer docs for the +component before building, rather than guessing: + +- C++: `docs/source/developers/cpp/building.rst` +- Python: `docs/source/developers/python/building.rst` +- R: `r/vignettes/developers/setup.Rmd` +- Everything else: `docs/source/developers/` +- Running tests, per component: + `docs/source/developers/guide/step_by_step/testing.rst` + +Build and run the affected tests locally before pushing. CI is not a +substitute for a component's unit tests: it is slow and shared, and a +test that was never compiled is the first thing a reviewer sees. If +you cannot build the component, enable GitHub Actions on your fork +and don't open the PR until the jobs your change touches pass there. + +Run pre-commit on the files you changed: + +```bash +pip install pre-commit +pre-commit install +pre-commit run --files +pre-commit run --all-files r # one component; aliases are listed in .pre-commit-config.yaml +pre-commit run -a # every hook +``` + +### Writing Tests + +- One test per behaviour. Fold near-identical cases into one test + with a loop or `parametrize` rather than copying the function. +- A regression test must fail without the fix. Check this before + pushing. +- Don't duplicate C++ test coverage in Python or R; binding tests + only need to show the plumbing works. +- Don't test compute kernels on invalid arrays (they are only + specified for valid input), and don't test type variants that share + a code path. Errors the code raises on purpose still need a test. +- Call `ValidateFull()` on arrays a C++ test builds. + +## Writing Code + +- Fix what the issue describes, nothing more. Don't generalise, rename, + or reformat nearby code, and keep unrelated files out of the diff. +- Internal preconditions are documented (or `DCHECK`ed), not validated + with error paths. Reviewers reject input validation nobody asked for. +- Before writing a bitmap loop, a visitor, an index normaliser, or a + test helper, look for an existing one in `cpp/src/arrow/util/` and + the nearest `testing/` or `test_util` file. There usually is one. +- Comments describe the code for a future reader, never the PR, the + bug, or what changed. If a comment says "this change", "now", + "still", or "previously", delete it or move it to the PR description. + A bare `// GH-12345` above a regression test is fine. +- Performance claims need numbers. For C++, run + `archery benchmark diff --benchmark-filter= HEAD upstream/main` + (see `docs/source/developers/benchmarks.rst`) and paste the table + into the PR description. Pass the baseline explicitly: the default + is `origin/HEAD`, which is the fork, not `apache/arrow`. A + maintainer can also trigger the CI benchmark run; note in the + description if that would help. + +## Git + +- `upstream` is `apache/arrow`; `origin` is the contributor's fork. + Fetch from `upstream`, push branches to `origin`, and check + `git remote -v` before pushing. Never push to `upstream`. +- Never open a PR from your fork's `main` branch. Fork-sync bots + overwrite it, which empties the PR. +- PRs are squash-merged, so the PR title and description become the + commit message. Spend effort there rather than on individual commits. +- Rebase on `upstream/main` when the branch is stale, and force-push + your own PR branch; reviewers expect that. An outdated branch makes + CI run every job and often fails for unrelated reasons. + +## Issues + +- Prefix the title with the component(s) in brackets: + `[C++][Python] Short description of the problem`. +- Do NOT include a `GH-` prefix in issue titles. That is for PR titles + only, where it refers to the issue being fixed. +- Search existing issues before creating one, and use the templates in + `.github/ISSUE_TEMPLATE/`. +- Before starting on an issue, check that nobody is assigned and no PR + already exists for it. +- One issue per PR, but one issue for all instances of the same + pattern. Fix them together rather than opening a PR per file. +- If working on the fix changes your understanding of the bug, update + the issue description too. + +## Pull Requests + +Title format, checked by CI: + +``` +GH-: [Component] Short description +``` + +Example: `GH-14736: [C++][Python] Ensure no validity bitmap in UnionArray::SetData` + +Use the issue number, not the PR number, and match the component +casing of existing PRs (`[Docs]`, `[Format]`, `[C++]`). + +A PR may use `MINOR: [Component] Short description` instead, with no +issue, only if it is a grammar, usage, or spelling fix touching at +most 2 files, or a documentation update touching at most 2 files and +under 500 words. Note the space after `MINOR:`. Never combine `MINOR: ` +with a `GH-` number. + +Fill in every section of `.github/pull_request_template.md`: + +- Keep each section to a sentence or two. Do not restate what the diff + already shows. +- Do not draft the PR description. The contributor writes it, in + their own words, and updates it when the code changes so it still + matches what is in the PR. +- Delete the "breaking changes" and "Critical Fix" lines unless they + genuinely apply. "Critical" is defined in the template (security, + incorrect data, or a crash), not any bug fix. +- Complete the "Was AI used for this PR?" checkboxes truthfully, + including who reviewed the change before submission. +- Don't `@mention` people in the description. It becomes the commit + message, so mentions follow it into git history and every + cherry-pick. + +Prefer one open PR at a time until your first is merged. Address +review feedback on an open PR before opening another; maintainers +close new PRs from authors who are ignoring feedback on old ones. + +After opening: fix CI failures your change caused. For failures that +are already happening on `main`, say so in a comment and leave them. + +## Working with AI Tools + +Arrow's policy on AI-generated contributions is in +`docs/source/developers/overview.rst` under "AI-generated code". +In practice: + +- The contributor reviews every change locally, understands it, and + is accountable for the PR. Do not open a PR or post anything to + GitHub on their behalf without that review. Where the contributor + does not fully understand a part of the change, the policy asks + them to say so in a comment on the PR and explain how they verified + it; prompt them to do that rather than papering over it. +- Do not reply to review comments on the contributor's behalf, and do + not draft replies for them to paste. The contributor reads each + comment and answers it in their own words. Reviewers recognise + agent-written replies and treat them as a sign nobody is reading. + If the contributor has you draft any other GitHub post (an issue, + say), it must end with a short attribution line saying so. +- Do not report a review comment as addressed until the change is + pushed and visible in the diff. A claimed-but-missing change ends + the conversation. +- Never tag or ping maintainers, and never "bump" a PR. +- Keep PRs small and focused. Avoid verbose comments, unnecessary test + cases, and changes outside the scope of the issue. + +## Boundaries + +- **Ask first:** adding a dependency, changing public APIs, changes + that span more than one component, destructive git operations + (force-pushing the contributor's own PR branch after a rebase is + the one routine exception). +- **Never:** push to `upstream`, hand-edit generated files, commit + secrets or credentials, ping maintainers. + +## Further Resources + +- [CONTRIBUTING.md](CONTRIBUTING.md) +- [New Contributor's Guide](https://arrow.apache.org/docs/developers/guide/index.html) +- [Contributing Overview](https://arrow.apache.org/docs/developers/overview.html)