[agents] Update AGENTS.MD with review-derived guidance#2106
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
|
mdboom
left a comment
There was a problem hiding this comment.
The linting guidance is wrong and should be fixed. The rest I feel less strongly about.
| - **Lint at the source**: if formatting or lint fixes affect generated files, | ||
| make the fix in the generation source (`cython-gen`, `cybind`, templates, or | ||
| sync source) or exclude generated outputs from the check. Otherwise the next | ||
| regeneration will reintroduce the same issue. |
There was a problem hiding this comment.
I would be stricter about this, since it isn't always easy to generate code that would pass all lints.
| - **Lint at the source**: if formatting or lint fixes affect generated files, | |
| make the fix in the generation source (`cython-gen`, `cybind`, templates, or | |
| sync source) or exclude generated outputs from the check. Otherwise the next | |
| regeneration will reintroduce the same issue. | |
| - **Don't lint generated files:** If a file has the comment "This code was automatically generated...", do not perform any formatting or lint fixes. |
| - Keep CUDA 12.x and 13.x build compatibility in mind. Do not directly cimport | ||
| newly generated binding symbols unless older supported CUDA-major builds are | ||
| gated or have a wrapper/fallback path. |
There was a problem hiding this comment.
Do we want to say something like "2 most recent versions of CUDA" to avoid hard-coding 12 and 13 here?
There was a problem hiding this comment.
+1. I like the wording in the cuda_bindings file above of "Preserve compatibility with the supported CUDA major-version matrix."
| - Keep public APIs minimal and intentional. Avoid exposing private helpers just | ||
| to make tests or examples easier; prefer improving the public path or keeping | ||
| helpers private. |
There was a problem hiding this comment.
"Avoid" feels too strong, unless we plan to move many more tests to Cython. Exposing private members is sort of the only feasible way to write tests in Python when implementing in Cython.
| - Do not weaken tests just to pass a platform or CI configuration. Avoid broad | ||
| platform skips such as "skip all WSL" or "skip all Windows"; query CUDA | ||
| driver/device capability or the specific missing library/feature instead. |
There was a problem hiding this comment.
I think the key point is that it's fine to "skip all Windows" if the upstream library documentation says something like "not supported on Windows". Otherwise we should be trying to use the tightest criteria possible.
| - Lint or formatting changes that touch generated files should either be made | ||
| in the generator (`cython-gen`, `cybind`, templates, or sync source) or should | ||
| exclude generated outputs from the check. |
There was a problem hiding this comment.
Ditto to above. We have historically just skipped linting and formatting on these files altogether.
Maybe saying something like "use .pre-commit-config.yaml for linting and formatting" would be better so we can just deterministically encode the rules there.
| - **Platform split files**: keep `_linux.pyx` and `_windows.pyx` variants | ||
| aligned when behavior should be equivalent. | ||
| - **Lint at the source**: if formatting or lint fixes affect generated files, | ||
| make the fix in the generation source (`cython-gen`, `cybind`, templates, or |
There was a problem hiding this comment.
cython-gen and cybind aren't public projects so a non-NVIDIA contributor will not have access to contribute fixes
| - Keep CUDA 12.x and 13.x build compatibility in mind. Do not directly cimport | ||
| newly generated binding symbols unless older supported CUDA-major builds are | ||
| gated or have a wrapper/fallback path. |
There was a problem hiding this comment.
+1. I like the wording in the cuda_bindings file above of "Preserve compatibility with the supported CUDA major-version matrix."
There was a problem hiding this comment.
Now that we're v1.0+ it would be nice to have something in here to be mindful of API breakage
Summary
Adds review-derived guidance to the root and package-specific
AGENTS.mdfiles so future agent-generated changes better reflect recurring cuda-python code review feedback.The change updates:
AGENTS.mdcuda_bindings/AGENTS.mdcuda_core/AGENTS.mdcuda_pathfinder/AGENTS.mdcuda_python/AGENTS.mdHow this guidance was generated
This PR turns recurring review feedback from cuda-python maintainers and contributors into durable agent instructions.
I analyzed inline PR review comments created from August 5, 2024 at 13:50:18 UTC through May 16, 2026 at 07:25:14 UTC. The dataset covers PR #84 through PR #2087 and includes 6,687 inline review comments across 586 unique PRs.
Data source:
MEMBERcomments: 5,471 comments from 37 commentersThe content added to the
AGENTS.mdfiles was driven by themes that appeared repeatedly in those review comments. I used keyword-based matching over review comment bodies to quantify recurrence, then converted the highest-value themes into agent-facing guidance.It is a directional analysis to identify feedback patterns that are common enough to document up front.
Research findings that informed the updates
Across the full inline review comment corpus, 2,514 of 6,687 comments, or 37.59%, matched at least one theme reflected in this PR.
Looking only at comments from GitHub
MEMBERauthors, 1,990 of 5,471 comments, or 36.37%, matched at least one of these themes. That indicates the guidance is grounded in maintainer/team review feedback, not only in occasional external contributor discussion.cuda_pathfinderand repo-wide errors to avoid silent fallback paths, validateCUDA_HOME/CUDA_PATH, and surface user-actionable diagnostics.Mapping from research to file changes
The root
AGENTS.mdcaptures review themes that apply across the monorepo:The package-specific
AGENTS.mdfiles add narrower guidance where the review feedback was package-specific:cuda_bindings/AGENTS.mdadds generated-source, Cython-copy, CUDA-version compatibility, and external-contribution guidance because review comments repeatedly flagged one-off generated edits and compatibility risks in binding code.cuda_core/AGENTS.mdadds public API design, capability-query, CUDA compatibility, resource cleanup, and testing guidance because review feedback often focused on high-level API stability and user-observable CUDA behavior.cuda_pathfinder/AGENTS.mdadds discovery/fallback/environment-root guidance because review feedback around path discovery emphasized explicit diagnostics over silent success-shaped fallbacks.cuda_python/AGENTS.mdadds packaging and docs aggregation guidance because metapackage changes need to stay aligned with component versions, constraints, release tags, and generated docs behavior.