Agent/LLM docs - #195
Agent/LLM docs#195
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughUpdates Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @AGENTS.md:
- Line 238: Fix the typo in the sentence "Optimizer runs: 10,000 (except for
some contracts that require specific runs in order to meet bytecode limtis, see
foundry.toml)" by replacing "limtis" with "limits" so the sentence reads "...in
order to meet bytecode limits, see foundry.toml"; update the AGENTS.md text
where that exact phrase appears to correct the spelling.
- Around line 370-374: Fix the two typos in the CodeRabbit section: change
"comands" to "commands" in the sentence that references the help command and
correct "coderrabit" to "coderabbit" in the example command `coderrabit
--prompt-only -t uncommitted`; ensure the `coderabbit -h` and `--prompt-only`
flags remain unchanged elsewhere.
🧹 Nitpick comments (1)
CLAUDE.md (1)
1-2: Consider adding blank line after heading for markdown compliance.The markdown linter suggests adding a blank line after the heading for proper formatting.
📋 Proposed fix
# Read the tool-agnostic file + @AGENTS.md
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.gitignoreAGENTS.mdCLAUDE.md
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: 0xJem
Repo: OlympusDAO/olympus-v3 PR: 29
File: src/modules/TRSRY/TRSRY.v1.sol:2-2
Timestamp: 2025-07-18T00:21:49.138Z
Learning: 0xJem prefers to avoid changing historical/already-deployed contracts to minimize risk and maintain stability, even when it would improve consistency across the codebase.
Learnt from: 0xJem
Repo: OlympusDAO/olympus-v3 PR: 29
File: remappings.txt:35-40
Timestamp: 2025-07-18T00:22:32.511Z
Learning: 0xJem prefers to keep OpenZeppelin 4.8.0 as the standard/default dependency in remappings.txt for stability, with newer versions like 5.3.0 available only through explicit versioned dependency paths (e.g., openzeppelin-5.3.0/) to ensure intentional opt-in to newer versions.
🪛 LanguageTool
AGENTS.md
[style] ~96-~96: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...ediately when starting - Mark completed immediately after finishing—not in batches - One ta...
(ADVERB_REPETITION_PREMIUM)
[style] ~238-~238: Consider a more concise word here.
Context: ...me contracts that require specific runs in order to meet bytecode limtis, see foundry.toml)...
(IN_ORDER_TO_PREMIUM)
[style] ~256-~256: In American English, abbreviations like “etc.” require a period.
Context: ...Role()modifiers (oronlyAdminRole()` etc if using the PolicyAdmin mix-in) for fu...
(ETC_PERIOD)
[uncategorized] ~280-~280: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... No Floating Point: Solidity has no floating point numbers - all numbers are integers. 2....
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[grammar] ~370-~370: Ensure spelling is correct
Context: ...command: coderabbit -h for details on comands available. In general, I want you to r...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🪛 markdownlint-cli2 (0.18.1)
CLAUDE.md
1-1: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Cursor Bugbot
- GitHub Check: run-ci
- GitHub Check: run-ci
- GitHub Check: run-ci
- GitHub Check: run-ci
- GitHub Check: run-ci
- GitHub Check: run-ci
- GitHub Check: run-ci
🔇 Additional comments (2)
AGENTS.md (1)
1-408: Excellent documentation for LLM agents.This is a comprehensive and well-structured guide that provides clear guidance on project conventions, development workflows, testing practices, and architectural patterns. The level of detail, especially in the math guidelines and testing sections, will be valuable for LLM tools working with this codebase.
.gitignore (1)
17-20: LGTM!The negated ignore rules correctly ensure that AGENTS.md and CLAUDE.md are tracked despite any broader ignore patterns. The comment clearly explains the intent.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
AGENTS.md (2)
22-23: Consider clarifying the guidance aboutforge test.Line 22 states "Don't use this" but line 23 immediately demonstrates using
forge testwith flags. While the intent is clear (avoid bareforge testwithout parameters), the phrasing could be more precise.📝 Suggested improvement for clarity
-- `forge test` - Test all files. Don't use this, as it will run all tests, many of which require additional parameters to run successfully. +- `forge test` - Test all files. Avoid using this without additional flags, as it will run all tests, many of which require additional parameters to run successfully. - `forge test -vvv --match-contract ContractTest` - Run a specific test contract
96-96: Optional style refinements for consistency.The static analysis tools identified a few minor style points:
- Line 96: "immediately" appears twice in close proximity—consider removing one instance or using a synonym
- Lines 115, 267: In American English, "etc." requires a period ("etc.")
- Line 249: "in order to" could be simplified to "to" for conciseness
- Line 291: "floating point" should be hyphenated as "floating-point" when used as a compound adjective
These are stylistic nitpicks that don't affect clarity or correctness.
✨ Proposed style fixes
-- Mark completed immediately after finishing—not in batches +- Mark completed after finishing—not in batches -- style: Changes that do not affect the meaning of the code (white-space, formatting, etc). +- style: Changes that do not affect the meaning of the code (white-space, formatting, etc.). -- Optimizer runs: 10,000 (except for some contracts that require specific runs in order to meet bytecode limits, see foundry.toml) +- Optimizer runs: 10,000 (except for some contracts that require specific runs to meet bytecode limits, see foundry.toml) -- Use `onlyRole()` modifiers (or `onlyAdminRole()` etc if using the PolicyAdmin mix-in) for function access control +- Use `onlyRole()` modifiers (or `onlyAdminRole()` etc.) if using the PolicyAdmin mix-in) for function access control -1. **No Floating Point**: Solidity has no floating point numbers - all numbers are integers. +1. **No Floating Point**: Solidity has no floating-point numbers - all numbers are integers.Also applies to: 115-115, 249-249, 267-267, 291-291
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
AGENTS.mdCLAUDE.md
🚧 Files skipped from review as they are similar to previous changes (1)
- CLAUDE.md
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: 0xJem
Repo: OlympusDAO/olympus-v3 PR: 29
File: src/modules/TRSRY/TRSRY.v1.sol:2-2
Timestamp: 2025-07-18T00:21:49.138Z
Learning: 0xJem prefers to avoid changing historical/already-deployed contracts to minimize risk and maintain stability, even when it would improve consistency across the codebase.
Learnt from: 0xJem
Repo: OlympusDAO/olympus-v3 PR: 29
File: remappings.txt:35-40
Timestamp: 2025-07-18T00:22:32.511Z
Learning: 0xJem prefers to keep OpenZeppelin 4.8.0 as the standard/default dependency in remappings.txt for stability, with newer versions like 5.3.0 available only through explicit versioned dependency paths (e.g., openzeppelin-5.3.0/) to ensure intentional opt-in to newer versions.
🪛 LanguageTool
AGENTS.md
[style] ~96-~96: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...ediately when starting - Mark completed immediately after finishing—not in batches - One ta...
(ADVERB_REPETITION_PREMIUM)
[style] ~115-~115: In American English, abbreviations like “etc.” require a period.
Context: ...g of the code (white-space, formatting, etc). - refactor: A code change tha...
(ETC_PERIOD)
[style] ~249-~249: Consider a more concise word here.
Context: ...me contracts that require specific runs in order to meet bytecode limits, see foundry.toml)...
(IN_ORDER_TO_PREMIUM)
[style] ~267-~267: In American English, abbreviations like “etc.” require a period.
Context: ...Role()modifiers (oronlyAdminRole()` etc if using the PolicyAdmin mix-in) for fu...
(ETC_PERIOD)
[uncategorized] ~291-~291: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... No Floating Point: Solidity has no floating point numbers - all numbers are integers. 2....
(EN_COMPOUND_ADJECTIVE_INTERNAL)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Cursor Bugbot
- GitHub Check: run-ci
- GitHub Check: run-ci
- GitHub Check: run-ci
- GitHub Check: run-ci
- GitHub Check: run-ci
- GitHub Check: run-ci
- GitHub Check: run-ci
🔇 Additional comments (1)
AGENTS.md (1)
1-419: Excellent comprehensive documentation for LLM agents.This documentation provides thorough, well-structured guidance covering:
- Operational context: Build/test commands, safety boundaries, and permission model
- Agent behavior: Clear communication style, scope management, and tool selection preferences
- Technical depth: Default Framework architecture, testing discipline with branching tree technique, and comprehensive Solidity math guidelines
- Practical examples: Test organization patterns, decimal arithmetic walkthrough, and tooling integration
The math guidelines (lines 285-376) are particularly valuable, explicitly addressing Solidity's integer-only arithmetic, precision considerations, and rounding behavior. The testing section's branching tree technique demonstrates thoughtful test organization.
This will significantly improve LLM agent effectiveness when working with the codebase.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
AGENTS.md (1)
1-431: Excellent comprehensive documentation for LLM agents.This documentation provides thorough, well-structured guidance covering project context, development workflows, agent behavior expectations, architecture, and detailed math guidelines. The content is clear, actionable, and tailored specifically for LLM tools working with the Olympus V3 codebase.
Particular highlights:
- The agent behavior section (lines 45-130) sets clear expectations around communication style, scope management, and tool usage
- The Solidity math guidelines (lines 293-384) are exceptionally detailed and address common pitfalls with decimal precision and integer division
- The git worktree workflow guidance (lines 124-130) provides practical multi-branch workflow patterns
Optional style improvements
Static analysis flagged a few minor stylistic issues that could be polished:
- Line 96: Remove duplicate "immediately" - consider: "Mark completed immediately after finishing, not in batches"
- Lines 115, 275: Add periods after "etc." for American English style
- Line 257: Consider "to meet" instead of "in order to meet" for conciseness
- Line 299: Consider hyphenating "floating-point" when used as a compound adjective
These are purely stylistic and don't impact the documentation's effectiveness.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
AGENTS.md
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: 0xJem
Repo: OlympusDAO/olympus-v3 PR: 29
File: src/modules/TRSRY/TRSRY.v1.sol:2-2
Timestamp: 2025-07-18T00:21:49.138Z
Learning: 0xJem prefers to avoid changing historical/already-deployed contracts to minimize risk and maintain stability, even when it would improve consistency across the codebase.
Learnt from: 0xJem
Repo: OlympusDAO/olympus-v3 PR: 29
File: remappings.txt:35-40
Timestamp: 2025-07-18T00:22:32.511Z
Learning: 0xJem prefers to keep OpenZeppelin 4.8.0 as the standard/default dependency in remappings.txt for stability, with newer versions like 5.3.0 available only through explicit versioned dependency paths (e.g., openzeppelin-5.3.0/) to ensure intentional opt-in to newer versions.
🪛 LanguageTool
AGENTS.md
[style] ~96-~96: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ...ediately when starting - Mark completed immediately after finishing—not in batches - One ta...
(ADVERB_REPETITION_PREMIUM)
[style] ~115-~115: In American English, abbreviations like “etc.” require a period.
Context: ...g of the code (white-space, formatting, etc). - refactor: A code change tha...
(ETC_PERIOD)
[style] ~257-~257: Consider a more concise word here.
Context: ...me contracts that require specific runs in order to meet bytecode limits, see foundry.toml)...
(IN_ORDER_TO_PREMIUM)
[style] ~275-~275: In American English, abbreviations like “etc.” require a period.
Context: ...Role()modifiers (oronlyAdminRole()` etc if using the PolicyAdmin mix-in) for fu...
(ETC_PERIOD)
[uncategorized] ~299-~299: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... No Floating Point: Solidity has no floating point numbers - all numbers are integers. 2....
(EN_COMPOUND_ADJECTIVE_INTERNAL)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Cursor Bugbot
- GitHub Check: run-ci
- GitHub Check: run-ci
- GitHub Check: run-ci
- GitHub Check: run-ci
- GitHub Check: run-ci
- GitHub Check: run-ci
- GitHub Check: run-ci
1 participant
Adds an AGENTS.md file to document expected behaviour from LLM tools
Note
Introduces documentation for LLM tooling and ensures it is versioned.
AGENTS.mddetailing project context, build/test/lint workflows, agent behavior, architecture, development/testing standards, and tooling tipsCLAUDE.mdthat redirects readers to the tool-agnosticAGENTS.md.gitignoreto unignoreAGENTS.mdandCLAUDE.mdso they are trackedWritten by Cursor Bugbot for commit e89bc19. This will update automatically on new commits. Configure here.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.