-
Notifications
You must be signed in to change notification settings - Fork 17
Add AGENTS.md for AI agent guidance #455
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add AGENTS.md for AI agent guidance #455
Conversation
Provide persistent instructions for AI coding agents (Cursor, Copilot, Codex, etc.) covering git commit conventions, Python and C++ code style, license headers, whitespace rules, testing, and PR/issue workflows. Fixes openvdb#453 Signed-off-by: Mark Harris <[email protected]> Co-authored-by: Cursor <[email protected]>
| - Format Python code with **black** using the exact flags from CI: | ||
|
|
||
| ``` | ||
| black --target-version=py311 --line-length=120 --extend-exclude='wip/' . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have an open PR (#449 which adds black and clang format to build.sh). We might want to dry this up so build.sh is the source of truth for commands like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was just literally in the process of writing our slack channel to ask if we can add this to a "format" script that subsumes src/scripts/run_clang_format.sh.
Please let's just have one and only one way this gets done, across all our file types, that's used in the same way by both CI and locally.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think pre-commit would be the best way to consolidate all this. But it's devops work, we don't get paid for that. :)
An easier fix, at least for black, is to put the black command line arguments into pyproject.toml as mentioned in this file. I was planning to do that in a follow up.
blackencino
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any validations/fixers that are objective and fully-specified should be run through a script, like a better version of src/scripts/run_clang_format.sh. This should apply whatever we apply to any filetype we check in CI. CI should not be able to reference scripts or validators from outside this workspace.
| - Format Python code with **black** using the exact flags from CI: | ||
|
|
||
| ``` | ||
| black --target-version=py311 --line-length=120 --extend-exclude='wip/' . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was just literally in the process of writing our slack channel to ask if we can add this to a "format" script that subsumes src/scripts/run_clang_format.sh.
Please let's just have one and only one way this gets done, across all our file types, that's used in the same way by both CI and locally.
| - Reference any related issues or PRs. | ||
| - For bugs: provide clear reproduction steps, expected vs actual behavior, and | ||
| environment details. | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anywhere in this file, we should request that the agents restrict generated code and documentation to ascii characters only - no emojis.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
|
I realize that my comments are less about this being a good AGENTS.md starting point (which it clearly is) and more frustration about the misalignment of our CI validation with the tools built into our environment. If we want to separate those concerns, then I think the only actual request I have here is for the "ascii chars only" restriction. |
Since "ascii chars only" is additive (and opinionated), I don't think this is a great reason to block merging this PR as a starting point. I keep having to remind LLMs to sign-off commits and to use the correct arguments to black, and I would like ASAP for that to no longer be something I have to remind them of. I actually find some non-ascii characters useful in code. For example in program output like logs I find ✅ and ❌ and a few other non-ascii characters (e.g. unicode arrows) quite useful. |
fwilliams
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Talked with Mark and agreed to approve this as a starting point and add automation in a follow up
Signed-off-by: Mark Harris <[email protected]>
|
I added a note about avoiding excessive non-ascii/emoji and also some files to only commit if directed (e.g. until we have project specifics, .cursor/ and settings.json since I still have some local changes I don't want to commit and can't .gitignore). |
blackencino
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
|
This is markdown only and our tests don't run on markdown changes so I am admin merging this. |
Summary
AGENTS.mdfile to the repository root providing persistent instructions for AI coding agents (Cursor, Copilot, Codex, etc.).github/workflows/codestyle.yml) and project conventions (CONTRIBUTING.md,pyproject.toml)Fixes #453
Test plan
AGENTS.mdrenders correctly on GitHub.github/workflows/codestyle.yml(--target-version=py311 --line-length=120 --extend-exclude='wip/')h,cpp,cc,cu,cuh)pyproject.toml[tool.pytest.ini_options]Made with Cursor