Conversation
| - Code, usually in the form of a pull request | ||
| - RFCs or design proposals | ||
| - Issue or security vulnerability reporting | ||
| - Comments and feedback on pull requests |
There was a problem hiding this comment.
I would support prohibiting the use of AI in comments.
This comment was marked as off-topic.
This comment was marked as off-topic.
Sorry, something went wrong.
There was a problem hiding this comment.
Docstrings? Not sure how they should be viewed?
There was a problem hiding this comment.
Documentation is mentioned in the section below:
As another example, using an LLM to generate documentation, which a contributor manually reviews for correctness and relevance, edits, and then posts as a PR, is an approved use of tools under this policy.
| Contributors **must be transparent and label contributions that contain | ||
| substantial amounts of tool-generated content**, and always mention it. The pull | ||
| request and issue templates contain a checkbox for that purpose. Failure to do | ||
| so, or lies when asked by a reviewer, will be considered as a violation. Our | ||
| policy on labeling is intended to facilitate reviews, and not to track which | ||
| parts of **stac-utils** repos are generated. Contributors should note tool usage | ||
| in their pull request description, commit message, or wherever authorship is | ||
| normally indicated for the work. For instance, use a commit message trailer like | ||
| Assisted-by: \<name of code assistant\>. This transparency helps the community | ||
| develop best practices and understand the role of these new tools. |
There was a problem hiding this comment.
There could be better guidance here. We should have a specific approach to citing ai use. The advice here is a little vague:
"Contributors should note tool usage in their pull request description, commit message, or wherever authorship is normally indicated for the work. For instance, use a commit message trailer like
Assisted-by: <name of code assistant>."
Also, this mentions that, regarding ai use, "The pull request and issue templates contain a checkbox ". Will we enforce that across stac-utils? It's not a bad idea, but if we aren't doing it, we should remove the text referencing the approach.
There was a problem hiding this comment.
This feels pretty specific to me:
"Contributors should note tool usage in their pull request description, commit message, or wherever authorship is normally indicated for the work. For instance, use a commit message trailer like Assisted-by: ."
@jonhealy1 Can you provide an example of language that you feel would be less vague?
"The pull request and issue templates contain a checkbox ". Will we enforce that across stac-utils? It's not a bad idea, but if we aren't doing it, we should remove the text referencing the approach.
We can create an org-wide pull request template that will be the default for repositories that don't have one. Repositories that do have a template will need to update it to include the checkbox ... I can update the repositories I maintain if/when this PR is merged, and I can post in the CNG Slack channel and Github Discussions to notify other maintainers of the change. Thanks for calling this out, I've updated the PR description to include the additional task.
There was a problem hiding this comment.
How about something like:
Checklist
- I have tested these changes locally.
- I have included / updated tests for these changes.
- Edge Cases: I have manually verified "unhappy paths" and edge cases beyond the basic success criteria.
- I can explain the implementation logic for every line of code submitted.
AI Disclosure
- AI-Assisted: Some or all of this contribution was generated or refined using AI tools. I have reviewed, tested, and take full responsibility for the quality and correctness of the output.
There was a problem hiding this comment.
For me a lot of the danger with ai relates to testing. An ai can cleverly write a dozen tests for you and make sure that they all pass, but they may only write tests that they know will pass. Someone thinks - there's a dozen new tests here, this must cover everything! Having a human making sure that everything important - ie all of the edge cases - is being tested is key.
There was a problem hiding this comment.
The checklist looks pretty good...could you open a PR on https://github.com/stac-utils/.github to add it? The only bullet I'd add would be one about documentation. And can you use the language from the GDAL PR template for the tool usage disclosure?
## AI tool usage
- [ ] AI (Copilot or something similar) supported my development of this PR. See our [policy about AI tool use](https://stac-utils.github.io/ai-contribution-policy). Use of AI tools *must* be indicated.There was a problem hiding this comment.
I would honestly prefer something like this - do we really need to know if someone used ai or not? I think as long as they understand the "human-in-the-loop" concept and have read through our ai policy.
[ ] AI Disclosure: I have read the AI Contribution Policy and confirm a "human-in-the-loop" approach. I have reviewed all tool-generated content and take full accountability for this contribution.
There was a problem hiding this comment.
Maybe we can have 2 versions available - I would definitely want to use the second version.
There was a problem hiding this comment.
Maybe we can have 2 versions available - I would definitely want to use the second version.
I think if you're maintaining a package, it's up to you how you'd like to accept contributions. My intent behind creating this stac-utils policy was to provide a useful default, not proscribe how all stac-utils projects have to be maintained.
There was a problem hiding this comment.
Good to know - I guess I was thinking more along the lines of having 2 defaults. We definitely would like to point everyone towards at least being aware of the ai policy/ recommendations.
|
|
||
| If a maintainer judges that a contribution doesn't comply with this policy, | ||
| they should paste the following response to request changes: | ||
|
|
There was a problem hiding this comment.
Guidance maybe on how maintainers can judge whether ai-generated code has violated these policies.
There was a problem hiding this comment.
There's some in the sections above, e.g.:
it is strongly recommended that contributors write PR descriptions themselves (if needed, using tools for translation or copy-editing), in particular to avoid over-verbose descriptions that LLMs are prone to generate
And
Contributors must be transparent and label contributions that contain substantial amounts of tool-generated content, and always mention it.
In my experience, LLM-generated code tends to have patterns that can be detected by a reviewer, and per these guidelines failing to mention that substantial amounts of tool-generated content without mention that it was tool-generated would be a violation.
If the provided language is not enough, @jonhealy1 could you maybe provide examples of the language you'd like to see?
There was a problem hiding this comment.
You are a very experienced programmer - I would be interested to know more about the patterns you see, as guidance for helping improve the projects I work on.
There was a problem hiding this comment.
Some common ones that I've observed:
- Re-implementing instead of importing, aka copying code from Github rather than adding a new dependency to a project
- Little "explainer" comments inside functions
- Over-refactoring into small (1-4 line) functions that are only used once or twice
- Calling functions in a unconventional way. This happens in geospatial a lot, where there's a "common" way of doing something (e.g. using core libraries like rasterio or shapely a certain way) but the code does it some other way. This isn't always bad, but sometimes can lead to strange breakages (there's some nomenclature in geospatial software that I've found LLMs don't understand correctly, e.g. stuff like
buffer)
There was a problem hiding this comment.
To be fair, I have written code I could have imported, long before I started using ai, because I didn't understand that another library I wasn't familiar with already provided the same functionality. Explainer comments and over-refactoring are easy things to tell an ai not to do - add in a few other watch out fors, and then maybe we have no idea.
There was a problem hiding this comment.
Anyways we should publish a document of things to watch out for/ things we don't like to see.
There was a problem hiding this comment.
add in a few other watch out fors, and then maybe we have no idea.
At a certain point, we're relying on the good faith of contributors — open source is a collaborative environment where we're working together to build cool stuff. The intent of this contribution policy (to me) is to help people contribute in a positive way, not to discourage contributions.
The biggest problems I've seen with LLM-generated code w.r.t. open source libraries have been:
- Trying to do too much at once. Small, focused PRs are so much easier to review
- The contributor not understanding the code they're proposing
- Expecting a quick review — even if you generated the code quickly, it can take time to review (especially on an un-funded project)
There was a problem hiding this comment.
Small, focused PRs are also a sign of experience - I always get carried away on prs - oh I'll just fix this while I'm at it. Expecting a quick review can be similar - someone may think they've done something really valuable, but then there pr can sit there forever even. An experienced programmer can be more patient, they can fork the project if they need to or install a branch in their codebase - an inexperienced programmer may not know that there are other options.
I don't know if you use ai, but I think that no matter how good ai gets, it's only going to work as good as the developer using it.
|
How about a shorter, more concise version that's easier for people to read: stac-utils AI & LLM Contribution GuidelinesOur StanceThe stac-utils community values high-quality, maintainable code. We recognize that AI and Large Language Models (LLMs) are part of the modern developer’s toolkit. We do not ban these tools; however, we require that every contribution remains human-centric. The "Human in the Loop" principle means that you, the contributor, are the sole author. You are responsible for every line of code, every docstring, and every claim made in your Pull Request. Core Expectations1. Ownership & AccountabilityIf you use an AI tool to generate code or text:
2. Rigorous Testing & Edge CasesAI tools excel at "happy path" logic but often fail at complexity. We require a high standard for testing:
3. Uniform DisclosureTo keep our review process transparent without being intrusive, we use a standard checkbox in our Pull Request templates.
Evaluating ContributionsMaintainers evaluate work based on impact and quality. A contribution may be flagged or declined if:
The Golden Rule: A contribution should be worth more to the project than the time it takes to review it. Copyright & LicensingYou are responsible for ensuring your contribution adheres to our project's license. By submitting a PR, you affirm that you have the right to contribute this work and that it does not violate third-party copyrights. |
Personally I'd prefer to stay as aligned as possible to GDAL — they've got a lot of smart, experienced folks over there and I trust their thinking. I'd be interested in others' opinions on this one. |
I don't mean to present this as some sort of a final version. I think a simplified and more concise version could be beneficial. We could keep the spirit of what they are saying but present it in a more understandable way. |
|
It makes sense to me to keep this close to GDAL rather than going fully down the rabbit hole of considering all the possible options. If you go that route it can be really hard to get something passable merged and I do feel that there is some urgency here. I guess my preference would be to start with something close to GDAL like Pete suggested and then if we run into a specific part that is chafing we can update the policy once we have more experience of what it feels like in practice. |
|
Pete said this was open for discussion, but maybe it's not. The overarching theme of the GDAL document is to ask contributors to avoid wasting a maintainers valuable time. I'm not saying my time is so valuable that I will regret adding my thoughts here for no reason, but it is worth thinking about. |
This is a discussion. @jonhealy1 you presented alternative wording for this PR, @jsignell had a different opinion on the wording. Presenting your opinion does not mean that your suggestions will be incorporated; we will work together as a community to form a consensus. |
|
@gadomski I think you were the one who dismissed my idea towards keeping things short and simple. If you are going to say, "let's just use the GDAL document", then you are not really having much of a discussion. |
|
I would ask, how is the shortened version not aligning with GDAL and then how do we improve it so it is. |
|
My perspective is coming from watching issues like this sit open on other projects (specifically on xarray) so I am super thankful for a concrete proposal. I didn't find the document as Pete suggested hard to read or understand (and I have a ridiculously short attention span) so for me I don't really see the need for any changes other than the one I suggested. |
My personal opinion is that your proposed shortened version doesn't improve on the wording enough to be worth diverging from the GDAL upstream. I'm thinking both about readability and maintainability — as GDAL's guidance evolves, I'd like to take up any of their changes. If we diverge, it's harder to uptake those changes. |
|
An argument could be made for having 2 versions. We would like contributors to review our policies surrounding AI, but asking them to wade through the GDAL version is maybe a little unreasonable. If something is too wordy, people usually don't take the time to read it. The shorter version could reference the full version for people who want more context. Also, we shouldn't give GDAL credit for the document as it seems to have come from here: https://github.com/llvm/llvm-project/blob/main/llvm/docs/AIToolPolicy.md |
3 participants
Proposed, needs to be discussed at the next STAC PSC. Adapted directly from GDAL's: https://gdal.org/en/stable/community/ai_tool_policy.html
If/when this PR is merged, we'll need to add a pull request template with the "AI checkbox" to https://github.com/stac-utils/.github and notify maintainers that they can update their repos that have a customized template.