Add AGENTS.md - #12028
Conversation
| main.nf **MUST** define one `process` with the following elements: | ||
| - Nextflow directives: | ||
| - `tag`: used for distinguishing processes in logs, usually set to "$meta.id" | ||
| - one or more `label`: used to set default resources | ||
| - `conda`: always set to "${moduleDir}/environment.yml" | ||
| - `container`: contains links to Docker and Singularity containers, gated by a fixed container engine check | ||
| - you **MUST NOT** add any other directives, they will be set by the pipeline | ||
| - Input section (`input:`): generally one tuple per logical entity (sample, reference, etc.) and `val` inputs for mandatory options; see https://nf-co.re/docs/specifications/components/modules/input-output-options for details | ||
| - Output section (`output:`): generally one output per logical file (e.g. one for sam/bam/cram and one for bai/crai); there must also be one topic output for each tool (to report the version); see https://nf-co.re/docs/specifications/components/modules/input-output-options for details | ||
| - `when:` section: you **MUST** leave the boilerplate intact | ||
| - Main script (`script:`): contains the shell script to execute, along with Nextflow code to generate the arguments; the definitions of `args` and `prefix` **MUST** be left intact | ||
| - Stub script (`stub`): contains a script to simulate the action of the module without running the tool | ||
| - All output file names **MUST** match the main script | ||
| - Compressed files **MUST** be generated by compressing an empty file | ||
| - The definitions of `args` and `prefix` **MUST** be left intact | ||
|
|
There was a problem hiding this comment.
can't we just point to the specifications and to use nf-core m lint, otherwise this will be a pain to keep in sync
| └── nextflow.config // Nextflow configuration used for testing ONLY; multiple config files may exist in some cases | ||
| ``` | ||
|
|
||
| ## Key terms |
There was a problem hiding this comment.
I don't think I know much about prompt engineering so my instincts may not be useful, but to a human reader, I think this terminology is better to have defined before they are used.
Co-authored-by: Matthias Hörtenhuber <mashehu@users.noreply.github.com>
| Required structure: | ||
|
|
||
| ``` | ||
| modules/nf-core/{module}/ |
There was a problem hiding this comment.
not sure if we should also add the gpu ones here, or give the bots the link to the documentation somewhere else: https://nf-co.re/docs/developing/components/gpu-modules
There was a problem hiding this comment.
Added in another place
| You can find the detailed documentation for each subcommand at https://nf-co.re/docs/nf-core-tools/cli/{modules|subworkflows}/{subcommand}. | ||
|
|
||
| ## nf-test and testing | ||
| - Run module tests with `nf-core modules test {name}`. |
There was a problem hiding this comment.
This will be interesting. Me and my bots prefer nf-test commands. I'd like to see how this will affect my bots judgements on what to use. Might need to remove this AGENTS.md file on my side...
There was a problem hiding this comment.
Same for me, I do prefer nf-test commands
There was a problem hiding this comment.
nf-test has more useful output (sorry infra team), but nf-core is the "canonical" approach. Not sure what to do about this.
There was a problem hiding this comment.
Okay, changed it, useful output wins
There was a problem hiding this comment.
I think for me the advantage of running the nf-core command is that the tags are built-in
| ## nf-test and testing | ||
| - Run module tests with `nf-core modules test {name}`. | ||
| - Run subworkflow tests with `nf-core subworkflows test {name}`. | ||
| - If you expect the output to change (e.g. after a tool update), you **SHOULD** update the snapshot with `nf-core modules/subworkflows test {name} --update`. You **MUST** regenerate snapshots on the same CPU architecture as CI. |
There was a problem hiding this comment.
something about picking test names?
There was a problem hiding this comment.
There is info in the main.nf.test section above
Co-authored-by: Evangelos Karatzas <32259775+vagkaratzas@users.noreply.github.com>
Co-authored-by: Maxime U Garcia <max.u.garcia@gmail.com>
| - You **MUST** use special commit titles and no commit body in the following cases: | ||
| - only module version bump: "Bump versions in {module name}" | ||
| - only nf-test snapshot update: "Update snapshots in {component name}" | ||
| - Before each commit, you **MUST** run `prek`, stage all changes it generates, resolve all errors and all solvable warnings, and repeat until none remain. |
There was a problem hiding this comment.
if pref is properly set up, there is no need to run prek before each commit
There was a problem hiding this comment.
I've had issues with it, especially when prettier checks failed (prek would block the PR instead of adding the necessary changes). Running prek manually fixes the issue. Not sure if that's my fault or a problem with the setup.
There was a problem hiding this comment.
It might have been discussed somewhere else and I missed it. But are we really encouraging commits by agents? That pushes even more of the review load onto reviewers (because it makes it too easy to push garbage, that an eager reviewer quickly jumps on instead of having a bit of a barrier what people actually commit)
There was a problem hiding this comment.
Concerning prek: it (and from the sound of it also you @itrujnara 😜) should just install the pre-commit rules via prek install -f and thats it 🙂
There was a problem hiding this comment.
I am fine with commits made by AGENTS as long as the human review them before asking an external review.
There was a problem hiding this comment.
Which is not happening. Maybe we should instruct the agents to only open draft PRs to add a distinction step
| ## Agent self-disclosure | ||
| - If you generated a majority of the code in a commit, you **MUST** add "Generated by {your name}" at the end of the commit message body. | ||
| - If you open a PR autonomously, you **MUST** add "Generated by {your name}" at the end of the PR message (above the checklist). |
There was a problem hiding this comment.
Should we say something about AGENTS replying to comments in PRs?
There was a problem hiding this comment.
I'm not sure it will fully cure the issue (due to OpenClaw and the like), but it's worth a try
There was a problem hiding this comment.
I have added a bullet about this
There was a problem hiding this comment.
We have noticed some non human like reply on both Slack and GitHub.
I am happy to help out a fellow human contributor.
I am less than happy helping what I thought was a human contributor that might not fully understand me but is actually an agent badly setup by a human.
Personal POV:
I see no issue discussing with an agent, if the replies are supervised, but otherwise I will just closed things, and I will consider removing the user from the github org.
There was a problem hiding this comment.
I think a complete AI policy is outside the scope of this PR, this only contains specific instructions for agents
Co-authored-by: Maxime U Garcia <max.u.garcia@gmail.com>
Make sure this is covered^. I dont think it is mentioned in the documentation. |
vagkaratzas
left a comment
vagkaratzas
left a comment
There was a problem hiding this comment.
Left a final comment
I believe it's sufficiently addressed in the updated push routine section |
maxulysse
left a comment
maxulysse
left a comment
There was a problem hiding this comment.
fine with me.
I would just like someone else to double check the part about prek.
Co-authored-by: Matthias Hörtenhuber <mashehu@users.noreply.github.com>
| - You **MUST** use special commit titles and no commit body in the following cases: | ||
| - only module version bump: "Bump versions in {module name}" | ||
| - only nf-test snapshot update: "Update snapshots in {component name}" |
There was a problem hiding this comment.
I don't understand the need for this?
| - If you expect the output to change (e.g. after a tool update), you **SHOULD** update the snapshot with `nf-core modules/subworkflows test {name} --update`. You **MUST** regenerate snapshots on the same CPU architecture as CI. | ||
|
|
||
| ## git and branch policy | ||
| - You **MUST NOT** commit any code to `master`. Use pull requests instead. |
There was a problem hiding this comment.
It won't let you, so does it really need stating?
| - Run tests with `nf-test test {modules|subworkflows}/{path}/tests --profile=+{docker|singularity|conda}` | ||
| - If you expect the output to change (e.g. after a tool update), you **SHOULD** update the snapshot with `nf-core modules/subworkflows test {name} --update`. You **MUST** regenerate snapshots on the same CPU architecture as CI. |
There was a problem hiding this comment.
This is saying to use two different methods?
I think nf-test is the more reliable one personally, it returns errors in a more verbose way. Can also recommand ``--stop-on-first-failure`.
| └── tests | ||
| ├── main.nf.test // nf-test unit tests for the module | ||
| ├── main.nf.test.snap // snapshots for the tests, **SHOULD ONLY** be editted through `nf-test` commands | ||
| └── nextflow.config // Nextflow configuration used for testing ONLY; multiple config files may exist in some cases |
There was a problem hiding this comment.
We don't want multiple config files, they should use module_args.
| ``` | ||
| subworkflows/nf-core/{subworkflow}/ | ||
| ├── main.nf // Nextflow code of the subworkflow | ||
| ├── meta.yml // YAML file containing information about the subworkflow |
There was a problem hiding this comment.
Subworkflows can include a nextflow.config to make them work, this gets highlighted when you run nf-core subworkflows install (gives the includeConfig line)
|
|
||
| ### tests/main.nf.test | ||
| The test file **MUST** follow the specificaton at https://nf-co.re/docs/specifications/components/modules/testing. | ||
| - The name of each test **SHOULD** contain the name of the tool, the main input format, and other inputs if relevant (example: `test("samtools - bam - index")`). Stub test names **SHOULD** end with `- stub`. |
There was a problem hiding this comment.
This is not the naming structure that we have in place. See: https://nf-co.re/docs/specifications/components/modules/testing#test-names
Co-authored-by: Simon Pearce <24893913+SPPearce@users.noreply.github.com>
…into add_agents_md
Co-authored-by: Chris Hakkaart <chris.hakkaart@seqera.io>
|
I have addressed your suggestions @SPPearce. I completely forgot about the squasher, so the commit-related rules are indeed useless. I was also mistaken about test naming; that said, I feel like we should stick to the |
Co-authored-by: Matthias Hörtenhuber <mashehu@users.noreply.github.com>
That went at least a year ago. I don't see any point in naming the species for the 98% of the modules where only one species is tested. I don't care about the names of the tests at all, provided they are vaguely meaningful (so |
8 participants
This PR is part of the agent steering project (nf-core/proposals#143). It adds the initial version of AGENTS.md to facilitate AI-assisted development in this repository and increase submission quality.
The file is intended to provide basic guidance for agents working within the repository. It is not intended as a complete replacement for the component documentation or for the module developer agent skill. That said, I appreciate feedback on any missing basic information.
PR checklist