From 2a98a3f22cd3bb39f00d9a626795ab32de89e4ec Mon Sep 17 00:00:00 2001 From: Kevin Liu Date: Sun, 4 Oct 2026 08:12:48 -0700 Subject: [PATCH 01/10] docs: add agent guidance on code comments and API surface Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/copilot-instructions.md | 10 +++++++++- AGENTS.md | 19 +++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 3744d6f435..91aa06f843 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -12,7 +12,15 @@ When reviewing pull requests, follow these rules to avoid noise and redundancy: - Skip the Obvious: Do not describe what the code is doing. Assume the reader understands the code. - Ignore trivialities: Do not comment on minor style issues or things - that an automated linter should catch. + that an automated linter should catch. Comment churn, unrelated edits, + and API growth are not trivialities. +- Comment churn: Flag code comments or docs that restate the code, + narrate history, or justify review decisions. Raise once per PR, + listing the locations. +- Unrelated edits: Flag renames, reformatting, or reordering of code the + change does not otherwise need. +- API growth: Flag new `pub` items with no use outside the crate + (see `public-api.txt`) and suggest `pub(crate)`. - Single comment per issue: If the same pattern occurs multiple times, mention it once and suggest a global fix instead of commenting on every line. diff --git a/AGENTS.md b/AGENTS.md index 22ddb2bbd6..cbfcc6fab8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -22,6 +22,25 @@ This file provides repository-specific guidance for automated agents working in this repository. +## Comments, docs, and diff scope + +Default to no comment. Leave existing comments as they are unless you change +the code they describe or they are wrong. + +- Comment only what the code cannot say: invariants, non-obvious + constraints, links to external behavior. Do not restate the code. +- If a comment would need editing after a behavior-preserving refactor, it + describes internals; leave it out. +- Do not describe how the code changed or explain review history + ("previously", "instead of", "unlike X"). Put that in the commit message + or PR description. +- Public items get a one-line doc summary of what they do for the caller. + Expand only for contract the caller must know. +- Test names say what they check. Do not narrate asserts. +- Do not rename, reformat, or reorder code unrelated to the change. +- Prefer `pub(crate)`. Make items `pub` only when something outside the + crate needs them; `public-api.txt` changes show API growth. + ## Security Model When assessing potential vulnerabilities or calibrating automated security From 9a1b1244b866a4cd30ffc80375473ca4cb8d931b Mon Sep 17 00:00:00 2001 From: Kevin Liu Date: Sun, 4 Oct 2026 08:41:02 -0700 Subject: [PATCH 02/10] docs: add agent guidance on PR descriptions Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/copilot-instructions.md | 6 ++++-- AGENTS.md | 7 +++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 91aa06f843..7dca240967 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -12,8 +12,8 @@ When reviewing pull requests, follow these rules to avoid noise and redundancy: - Skip the Obvious: Do not describe what the code is doing. Assume the reader understands the code. - Ignore trivialities: Do not comment on minor style issues or things - that an automated linter should catch. Comment churn, unrelated edits, - and API growth are not trivialities. + that an automated linter should catch. The rules below are not + trivialities. - Comment churn: Flag code comments or docs that restate the code, narrate history, or justify review decisions. Raise once per PR, listing the locations. @@ -21,6 +21,8 @@ When reviewing pull requests, follow these rules to avoid noise and redundancy: change does not otherwise need. - API growth: Flag new `pub` items with no use outside the crate (see `public-api.txt`) and suggest `pub(crate)`. +- PR description: Flag descriptions that restate the diff or pad the + template. Mention it in the review summary. - Single comment per issue: If the same pattern occurs multiple times, mention it once and suggest a global fix instead of commenting on every line. diff --git a/AGENTS.md b/AGENTS.md index cbfcc6fab8..6aa089c89f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -41,6 +41,13 @@ the code they describe or they are wrong. - Prefer `pub(crate)`. Make items `pub` only when something outside the crate needs them; `public-api.txt` changes show API growth. +## Pull requests + +- Follow the PR template. Keep each section short: the problem, the + approach, and anything a reviewer would not guess from the diff. +- Do not restate the diff: no file-by-file walkthroughs, per-function + summaries, or "changes made" checklists. + ## Security Model When assessing potential vulnerabilities or calibrating automated security From 25befdd00d7f8660918f5fd2a42a7f3fc8f902f7 Mon Sep 17 00:00:00 2001 From: Kevin Liu Date: Sun, 4 Oct 2026 08:45:11 -0700 Subject: [PATCH 03/10] docs: separate always-flag review rules Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/copilot-instructions.md | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 7dca240967..bd1cbbcc68 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -12,20 +12,21 @@ When reviewing pull requests, follow these rules to avoid noise and redundancy: - Skip the Obvious: Do not describe what the code is doing. Assume the reader understands the code. - Ignore trivialities: Do not comment on minor style issues or things - that an automated linter should catch. The rules below are not - trivialities. -- Comment churn: Flag code comments or docs that restate the code, - narrate history, or justify review decisions. Raise once per PR, - listing the locations. -- Unrelated edits: Flag renames, reformatting, or reordering of code the - change does not otherwise need. -- API growth: Flag new `pub` items with no use outside the crate - (see `public-api.txt`) and suggest `pub(crate)`. -- PR description: Flag descriptions that restate the diff or pad the - template. Mention it in the review summary. + that an automated linter should catch. - Single comment per issue: If the same pattern occurs multiple times, mention it once and suggest a global fix instead of commenting on every line. - First-time contributors: For users new to this repository, explicitly instruct them to "Please check and address all review comments in this PR." + +Always flag these, once per PR with the locations: + +- Comment churn: Code comments or docs that restate the code, narrate + history, or justify review decisions. +- Unrelated edits: Renames, reformatting, or reordering of code the + change does not otherwise need. +- API growth: New `pub` items with no use outside the crate (see + `public-api.txt`). Suggest `pub(crate)`. +- PR description: Descriptions that restate the diff or pad the + template. Mention it in the review summary. From 5f0d0613a6470d62277f89321ab0f314c70f393e Mon Sep 17 00:00:00 2001 From: Kevin Liu Date: Sun, 4 Oct 2026 08:45:40 -0700 Subject: [PATCH 04/10] docs: soften always-flag wording Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/copilot-instructions.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index bd1cbbcc68..77b253e5d7 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -20,7 +20,7 @@ When reviewing pull requests, follow these rules to avoid noise and redundancy: explicitly instruct them to "Please check and address all review comments in this PR." -Always flag these, once per PR with the locations: +Also flag these, once per PR with the locations: - Comment churn: Code comments or docs that restate the code, narrate history, or justify review decisions. From c3c5d2a14026449f5163352ddefcc0c98821868d Mon Sep 17 00:00:00 2001 From: Kevin Liu Date: Sun, 4 Oct 2026 08:47:15 -0700 Subject: [PATCH 05/10] docs: drop refactor test from comment guidance Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- AGENTS.md | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 6aa089c89f..5182b792e8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -29,13 +29,11 @@ the code they describe or they are wrong. - Comment only what the code cannot say: invariants, non-obvious constraints, links to external behavior. Do not restate the code. -- If a comment would need editing after a behavior-preserving refactor, it - describes internals; leave it out. - Do not describe how the code changed or explain review history ("previously", "instead of", "unlike X"). Put that in the commit message or PR description. -- Public items get a one-line doc summary of what they do for the caller. - Expand only for contract the caller must know. +- Public items get a one-line doc summary of what they do for the caller, + not how. Expand only for contract the caller must know. - Test names say what they check. Do not narrate asserts. - Do not rename, reformat, or reorder code unrelated to the change. - Prefer `pub(crate)`. Make items `pub` only when something outside the From 8199953545a6576f3a47129481294aed456afaee Mon Sep 17 00:00:00 2001 From: Kevin Liu Date: Sun, 4 Oct 2026 08:53:21 -0700 Subject: [PATCH 06/10] docs: base pub guidance on intended API Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/copilot-instructions.md | 2 +- AGENTS.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 77b253e5d7..bbbc4a53a4 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -26,7 +26,7 @@ Also flag these, once per PR with the locations: history, or justify review decisions. - Unrelated edits: Renames, reformatting, or reordering of code the change does not otherwise need. -- API growth: New `pub` items with no use outside the crate (see +- API growth: New `pub` items the change does not need to expose (see `public-api.txt`). Suggest `pub(crate)`. - PR description: Descriptions that restate the diff or pad the template. Mention it in the review summary. diff --git a/AGENTS.md b/AGENTS.md index 5182b792e8..b985c59b9f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -36,8 +36,8 @@ the code they describe or they are wrong. not how. Expand only for contract the caller must know. - Test names say what they check. Do not narrate asserts. - Do not rename, reformat, or reorder code unrelated to the change. -- Prefer `pub(crate)`. Make items `pub` only when something outside the - crate needs them; `public-api.txt` changes show API growth. +- Prefer `pub(crate)`. Make items `pub` only when they are part of the API + the change intends to expose; `public-api.txt` changes show API growth. ## Pull requests From abe4bfc0fda9414b4c2d4e01e5979c5be9902fbb Mon Sep 17 00:00:00 2001 From: Kevin Liu Date: Sun, 4 Oct 2026 08:53:59 -0700 Subject: [PATCH 07/10] docs: clarify visibility guidance Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/copilot-instructions.md | 4 ++-- AGENTS.md | 5 +++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index bbbc4a53a4..fd3a3b89e9 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -26,7 +26,7 @@ Also flag these, once per PR with the locations: history, or justify review decisions. - Unrelated edits: Renames, reformatting, or reordering of code the change does not otherwise need. -- API growth: New `pub` items the change does not need to expose (see - `public-api.txt`). Suggest `pub(crate)`. +- API growth: New public API the change does not need. Check new entries + in `public-api.txt`. - PR description: Descriptions that restate the diff or pad the template. Mention it in the review summary. diff --git a/AGENTS.md b/AGENTS.md index b985c59b9f..395cd2e888 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -36,8 +36,9 @@ the code they describe or they are wrong. not how. Expand only for contract the caller must know. - Test names say what they check. Do not narrate asserts. - Do not rename, reformat, or reorder code unrelated to the change. -- Prefer `pub(crate)`. Make items `pub` only when they are part of the API - the change intends to expose; `public-api.txt` changes show API growth. +- Use the narrowest visibility that works. Make items `pub` only when they + are part of the API the change intends to expose; `public-api.txt` + changes show API growth. ## Pull requests From b9ca6c05f7d70d9fdb7c50cfcd6eebff6df06376 Mon Sep 17 00:00:00 2001 From: Kevin Liu Date: Sun, 4 Oct 2026 09:42:08 -0700 Subject: [PATCH 08/10] docs: add reuse and test duplication guidance Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/copilot-instructions.md | 1 + AGENTS.md | 3 +++ 2 files changed, 4 insertions(+) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index fd3a3b89e9..c00ef406b2 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -26,6 +26,7 @@ Also flag these, once per PR with the locations: history, or justify review decisions. - Unrelated edits: Renames, reformatting, or reordering of code the change does not otherwise need. +- Duplication: New helpers, types, or tests that duplicate existing ones. - API growth: New public API the change does not need. Check new entries in `public-api.txt`. - PR description: Descriptions that restate the diff or pad the diff --git a/AGENTS.md b/AGENTS.md index 395cd2e888..a8b85614c6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -35,7 +35,10 @@ the code they describe or they are wrong. - Public items get a one-line doc summary of what they do for the caller, not how. Expand only for contract the caller must know. - Test names say what they check. Do not narrate asserts. +- Do not add tests that duplicate existing coverage. - Do not rename, reformat, or reorder code unrelated to the change. +- Reuse or extend existing helpers and types before adding new ones. Do + not add an abstraction just to save a few lines. - Use the narrowest visibility that works. Make items `pub` only when they are part of the API the change intends to expose; `public-api.txt` changes show API growth. From df4975b930533eb0c4a2535e16e9e206e64835f1 Mon Sep 17 00:00:00 2001 From: Kevin Liu Date: Sun, 4 Oct 2026 09:44:59 -0700 Subject: [PATCH 09/10] docs: trim reuse guidance Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- AGENTS.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index a8b85614c6..4c98260aa5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -37,8 +37,7 @@ the code they describe or they are wrong. - Test names say what they check. Do not narrate asserts. - Do not add tests that duplicate existing coverage. - Do not rename, reformat, or reorder code unrelated to the change. -- Reuse or extend existing helpers and types before adding new ones. Do - not add an abstraction just to save a few lines. +- Reuse existing helpers and types before adding new ones. - Use the narrowest visibility that works. Make items `pub` only when they are part of the API the change intends to expose; `public-api.txt` changes show API growth. From f874bd8ba1d2e19820be43b0be231e2ec1aa7263 Mon Sep 17 00:00:00 2001 From: blackmwk <257669749+blackmwk@users.noreply.github.com> Date: Thu, 8 Oct 2026 17:07:06 +0800 Subject: [PATCH 10/10] Apply batched suggestions from code review Co-authored-by: Matt Butrovich --- AGENTS.md | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 4c98260aa5..5a1106e0b0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -33,19 +33,30 @@ the code they describe or they are wrong. ("previously", "instead of", "unlike X"). Put that in the commit message or PR description. - Public items get a one-line doc summary of what they do for the caller, - not how. Expand only for contract the caller must know. + not how. Expand only for contract the caller must know, such as the + errors it returns (`# Errors`) or when it panics (`# Panics`). - Test names say what they check. Do not narrate asserts. +- Cover the edge cases and error paths the change adds, not only the + happy path. +- Build the expected value and assert equality with it, rather than + checking fields one at a time. - Do not add tests that duplicate existing coverage. -- Do not rename, reformat, or reorder code unrelated to the change. -- Reuse existing helpers and types before adding new ones. +- Do not rename, reformat, refactor, or reorder code unrelated to the + change. A bug fix does not carry an API redesign. +- Reuse existing helpers, types, and test fixtures (such as + `TableTestFixture`) before adding new ones. Use arrow-rs kernels + instead of reimplementing them. - Use the narrowest visibility that works. Make items `pub` only when they - are part of the API the change intends to expose; `public-api.txt` - changes show API growth. + are part of the API the change intends to expose. Run + `make generate-public-api` to update each crate's `public-api.txt`, and + check its diff for API growth. ## Pull requests - Follow the PR template. Keep each section short: the problem, the - approach, and anything a reviewer would not guess from the diff. + approach, and anything a reviewer would not guess from the diff. In + AI Disclosure, name the areas of uncertainty or assumptions a reviewer + should focus on. - Do not restate the diff: no file-by-file walkthroughs, per-function summaries, or "changes made" checklists.