Update contributor guidelines regarding AI spam, reviews, code

[Jefffrey]

From the discussion here:

• Policy on uptick of LLM PRs from new contributors #24413

Updating our contributing guidelines, specifically to target cases where we see spam PRs from AI origins especially from new contributors. Aiming to set some rules/guidelines around this, and also hopefully make any agents involved at least reconsider before spamming

• closes Policy on uptick of LLM PRs from new contributors #24413
• related to Add AI tooling disclosure text to contributor guide and fields to PR templates #18095
• closes Discussion: guidelines for LLM-generated PR reviews #25317

[Jefffrey]

Just an initial draft, can probably tighten up and or expand on some of the wording

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.68%. Comparing base (bd86190) to head (fe8aea6).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #24510      +/-   ##
==========================================
- Coverage   82.69%   82.68%   -0.01%     
==========================================
  Files        1147     1147              
  Lines      447204   447204              
  Branches   447204   447204              
==========================================
- Hits       369793   369779      -14     
- Misses      55002    55011       +9     
- Partials    22409    22414       +5     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Rich-T-kid]

This all make sense to me. I agree with @saadtajwar point about making a distinction between AI involvement and blind AI slop

[alamb]

maybe we can also suggest they help with the existing PR (e.g. help review it, work with the other contributor to get the PR to follow the guidelines / go through the review process)

[Jefffrey]

(i aim to revisit this soon and update with suggestions 👍)

[alamb]

I am going to try and revive / update this, given it came up with a conversation with @neilconway today and apache/arrow-rs#11209 (comment) with @Jefffrey

[alamb]

I took the liberty of pushing a bunch of commits to this branch to refine the contributor guide for AI contributions and spam

[alamb]

I am obviously being biased here but I think these look good now. We should wait for others in the community to weigh in

[2010YOUY01]

Took a read, and I think it's a great improvement.

Maybe we can make ai contribution section a top-level doc page, like open a new section next to Introduction:

[2010YOUY01]

I feel “understand the core idea” is a bit vague now, and we could make the expectation more concrete. I also think setting a higher bar for PRs makes it easier to make progress during review.

Perhaps

“Understand the PR” means more than being able to follow the diff. It means:

- Could reproduce the implementation without relying on AI.
- Understand how the change fits into the surrounding architecture.
- Can judge whether the design adds only necessary complexity and is maintainable long term.

[Jefffrey]

Could reproduce the implementation without relying on AI

I might push back on this point as its a bit vague to enforce; does it mean you can write the PR after you got knowledge from working with the LLM, for example? Or just you mainly used LLM as a shortcut for the ideas you had in your brain

I know there are cases where LLMs can help iterate on an idea and identify edge cases, etc. so it can be confusing if this "disqualifies" the PR so to speak

[2010YOUY01]

I might push back on this point as its a bit vague to enforce;

I agree this might not convey the idea clearly. I think we can agree that a PR should be opened with sufficient understanding, but “understanding” itself is still a bit vague.

In practice, I see quite a few PRs where the contributor's understanding isn't deep enough when the PR is opened. That makes review much harder, and sometimes the review still effectively turns into the reviewer driving the AI through the contributor.

I'm not sure what the best way is to define the bar for “enough understanding/confidence to open a PR.” “Being able to reimplement it manually” seems like one concrete test for that bar, rather than the principle itself. And I agree edge cases finding should be excluded.

[Jefffrey]

i do feel the "Authors should be ready to justify and help reviewers understand the design and code during review" should hopefully cover this; and if it does turn into reviewer driving the LLM then it'll fall into the part where we dont want them to just paste LLM output

[2010YOUY01]

i do feel the "Authors should be ready to justify and help reviewers understand the design and code during review" should hopefully cover this;

100%, this is a good practical test for 'enough undersantinding for the PR'

I think it also worth a separate documentation section for 'how to write good PR/Issue description', I'll give it a try later.

[alamb]

I think this is excellent discussion
I tried to incorporate this feedback in this commit: 4405473

[alamb]

Here is the updated version

AI Policy

DataFusion has the following policy for AI-assisted PRs:

• We welcome AI-assisted PRs from anyone. We do not welcome unreviewed "AI
  dumps" (defined below).
• The PR author should have personally read the entire PR they submit and
  understand the core ideas end-to-end. Authors should be ready to justify
  and help reviewers understand the design and code during review.
• Call out unknowns and assumptions. It's okay to not fully understand
  some bits of AI-generated code. Please point these cases out so we can work
  together to clear up any concerns.

While "understand the core ideas" is partly subjective, it means more than being
able to follow the diff textually. We expect the PR author to take an active
role in responding to feedback and crafting the PR to make sure it fits well
into the project as a whole. The worst situation is one where the reviewer ends
up simply driving the contributor's LLM, for the reasons described in the next
section.

[kumarUjjawal]

Maybe we can make ai contribution section a top-level doc page, like open a new section next to Introduction:

Agree!

[Jefffrey]

thanks for picking this up again 🙇

it looks pretty good 🙏

[Jefffrey]

perhaps a side discussion, but something to note is even arrow (main repo) has turned off their take action:

• GH-51232: [CI] Remove the "take" comment bot for self-assigning issues arrow#51233

it could be worth exploring as a case could be made it sometimes can stifle discussion if its too easy for someone to just come in an 'take' an issue (though i havent been keeping an eye on datafusion recently so im not sure if this is a concern)

[alamb]

I filed a separate ticket to discuss

• Consider removing the take action / open ticket assignment #26063

[Jefffrey]

Could reproduce the implementation without relying on AI

I might push back on this point as its a bit vague to enforce; does it mean you can write the PR after you got knowledge from working with the LLM, for example? Or just you mainly used LLM as a shortcut for the ideas you had in your brain

I know there are cases where LLMs can help iterate on an idea and identify edge cases, etc. so it can be confusing if this "disqualifies" the PR so to speak

[Jefffrey]

im not sure how often we see it here, but sometimes people might use it for translation. we could put a point where we allow it but we expect it to just translate. we could technically ask them to deepL it, but LLMs can be better at localizing and making it read a bit easier. the main point however is it should still represent the original message, and not fluff it up

[alamb]

This is a great point -- I tried to clarify in 57fde97 to emphasize that the point is to avoid unreviewed AI dumps. Here is the updated text:

Responding to review comments

The same policy applies to review discussion as to the code itself: reviewers
want to talk to you, not to your AI tool. Please do not respond to reviewer
comments with unreviewed, fully AI-generated responses as again this puts all the
burden on the reviewer and you learn nothing. We expect your responses to be
in your own words, though it is fine to use AI to help prepare your response
(for example to understand the comment, or help translate your response to
English). Examples of unhelpful unreviewed AI comment dumps:

• Summarizes the diff rather than answering the question that was asked.
• Lists the commands run locally (e.g. cargo fmt, cargo test)
  and whether they passed. This is not useful to reviewers because CI already runs
  these checks.
• Contains statements that don't make sense in context, such as claiming
  tests could not be run because cargo is not installed.

For example, see [this review thread in arrow-rs][arrow-rs-review-example] where
the reviewer asked a design question, and received several replies that
described what had changed and which commands had been run, rather than an
answer to the question.

Again, the point of code review is to help the project and to help you grow
as an engineer, so please read each comment, make sure you understand it, and
reply in your own words.

[alamb]

I also dropped a note to the datafusion dev list: https://lists.apache.org/thread.html/z4pfjzynsk0sx1dzgj5m9lj56swfhbn5

[mbutrovich]

I also dropped a note to the datafusion dev list: https://lists.apache.org/thread.html/z4pfjzynsk0sx1dzgj5m9lj56swfhbn5

Thanks @alamb. The email made sure I took a look.

Maybe I missed it, but I think we should mention ASF's policy like Iceberg does.

Respect ASF policy. Ensure generated content does not introduce incompatible licenses or undisclosed third-party code; review the ASF Generative Tooling Guidance and licensing rules when in doubt.

[nuno-faria]

Something I've seen lately in some repos is PRs being automatically closed when there is suspicion of being part of a PR dump (e.g., https://github.com/vitejs/vite/pulls?q=is%3Apr+state%3Aclosed+label%3A%22bot%3A+likely%22). Don't know how accurate it is.

[2010YOUY01]

Something I've seen lately in some repos is PRs being automatically closed when there is suspicion of being part of a PR dump (e.g., https://github.com/vitejs/vite/pulls?q=is%3Apr+state%3Aclosed+label%3A%22bot%3A+likely%22). Don't know how accurate it is.

The metrics they're using seem quite reasonable — you can check them by clicking through the link in the GitHub bot reply:

https://agentscan.tools/user/mikamikasuki

[Omega359]

I've read the AI policy and I think it's well written and I agree with it.

[crepererum]

thanks for writing up this very reasonable and pragmatic (IMHO) approach

[mbutrovich]

Thanks @Jefffrey and @alamb for working on this. I'm marking this as request changes so the comment below gets resolved before merge, since the PR already has several approvals.

[mbutrovich]

Should this list point contributors to the ASF Generative Tooling Guidance? I raised this in an earlier comment, and the page at fe8aea6 doesn't mention it yet.

The guidance puts conditions on the contributor. The tool's terms can't restrict use of the output in a way that conflicts with the Open Source Definition. The output also has to meet one of three conditions: it isn't copyrightable, it contains no third-party material, or any third-party material in it is used with permission. As I read the rest of this page, it covers review burden and author understanding, so a contributor who follows it wouldn't learn about the licensing side. Iceberg's contributor guide has a bullet for this that we could adapt. What do you think about something like this?

Suggested change

	- **Call out unknowns and assumptions**. It's okay to not fully understand
	some bits of AI-generated code. Please point these cases out so we can work
	together to clear up any concerns.
	- **Call out unknowns and assumptions**. It's okay to not fully understand
	some bits of AI-generated code. Please point these cases out so we can work
	together to clear up any concerns.
	- **Respect ASF policy**. Make sure generated content does not introduce
	incompatible licenses or undisclosed third-party code. See the
	[ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html)
	for the conditions contributors must meet.

[alamb]

I will add this after I get out of meetings