Add agents.md file for ai assistant coding #39
Conversation
Summary of ChangesHello @victor-villar, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces foundational documentation for the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
victor-villar
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds an agents.md file, which serves as a comprehensive guide for developers and AI assistants to understand the TwisteRL project structure, setup, and conventions. The document is well-structured and provides valuable information. I've added a few suggestions to improve clarity and consistency, such as making shell commands safer to execute and aligning the directory structure for better readability. Also, please note the typo 'assitant' in the pull request title and description; it should be 'assistant'.
| pip install -e . | ||
|
|
||
| # Rust check | ||
| cd rust && cargo check && cargo test |
There was a problem hiding this comment.
The cd rust && ... command changes the current directory. If a user is copying commands one by one, they will end up in the rust directory, which might not be the intention for subsequent commands. To make the commands safer to copy-paste, consider running the directory-specific commands in a subshell. This ensures the user's shell session remains in the project root.
| cd rust && cargo check && cargo test | |
| (cd rust && cargo check && cargo test) |
| pytest tests/ | ||
|
|
||
| # Rust unit tests | ||
| cd rust && cargo test |
There was a problem hiding this comment.
Similar to the setup command, this cd command changes the current directory. Using a subshell makes it safer for users to run these commands from the project root without unexpectedly changing their working directory.
| cd rust && cargo test | |
| (cd rust && cargo test) |
| ## Directory Structure | ||
|
|
||
| ``` | ||
| twisteRL/ |
There was a problem hiding this comment.
The project's package name is twisterl (as defined in pyproject.toml and rust/Cargo.toml). The directory structure diagram uses twisteRL/ as the root directory name, which is inconsistent. It should be twisterl/ to accurately reflect the project's naming.
| twisteRL/ | |
| twisterl/ |
| ├── rust/ # Rust crate (core RL) | ||
| │ ├── src/ # Rust source | ||
| │ └── Cargo.toml # Rust deps | ||
| ├── src/twisterl/ # Python package | ||
| ├── tests/ # Python tests | ||
| ├── examples/ # Configs and notebooks | ||
| └── pyproject.toml # Python build config |
There was a problem hiding this comment.
The comments in the directory structure diagram are not vertically aligned, which slightly impacts readability. Aligning them would make the diagram cleaner and easier to parse visually.
| ├── rust/ # Rust crate (core RL) | |
| │ ├── src/ # Rust source | |
| │ └── Cargo.toml # Rust deps | |
| ├── src/twisterl/ # Python package | |
| ├── tests/ # Python tests | |
| ├── examples/ # Configs and notebooks | |
| └── pyproject.toml # Python build config | |
| ├── rust/ # Rust crate (core RL) | |
| │ ├── src/ # Rust source | |
| │ └── Cargo.toml # Rust deps | |
| ├── src/twisterl/ # Python package | |
| ├── tests/ # Python tests | |
| ├── examples/ # Configs and notebooks | |
| └── pyproject.toml # Python build config |
cbjuan
left a comment
cbjuan
left a comment
There was a problem hiding this comment.
Review: agents.md
Thanks for adding documentation for AI coding assistants. A few issues to address before merging:
Factual Error
Line 28 references TwistableEnv, but this trait doesn't exist. The actual API is the twists() method on the Env trait (rust/src/rl/env.rs:59):
fn twists(&self) -> (Vec<Vec<usize>>, Vec<Vec<usize>>) {(vec![], vec![])}Suggested fix:
-- **Symmetry**: Implement `TwistableEnv` for environments with symmetries.
+- **Symmetry**: Override the `twists()` method on the `Env` trait for environments with symmetries. See [docs/twists.md](docs/twists.md).Shell Commands
Lines 17 and 37 use cd rust && ... which changes the working directory permanently. Use subshells instead:
-cd rust && cargo check && cargo test
+(cd rust && cargo check && cargo test)Missing Content
Consider adding:
docs/directory in the structure diagram- Reference to the
grid_worldcustom environment example - AlphaZero algorithm support (currently only PPO shown)
Minor
- Line 46:
twisteRL/→twisterl/for consistency with package name - Commit message typo: "assitatn" → "assistant"
Everything else looks good — the setup commands, trait path (twisterl::rl::env::Env), and conventions are all accurate.
2 participants
Add agents.md file for ai assistant coding