Conversation
WalkthroughAdds a new AGENTS.md guide, updates README, adjusts Solana test-data start/end block override handling, expands discovery catalog (clients.json and large methods.json updates to add/augment reth and many RPC methods), and bumps project version in pyproject.toml. Changes
Sequence Diagram(s)sequenceDiagram
participant CLI as CLI / ParsedOptions
participant SolanaTD as SolanaTestData
participant Blocks as BlockSource
CLI->>SolanaTD: instantiate(with parsed_options)
SolanaTD->>SolanaTD: check parsed_options.start_block?
alt start_block provided
SolanaTD->>SolanaTD: set self.start_block_number = parsed_options.start_block
else
SolanaTD->>Blocks: earliest_available_block_number
Blocks-->>SolanaTD: earliest
SolanaTD->>SolanaTD: set start to earliest
end
SolanaTD->>SolanaTD: check parsed_options.end_block?
alt end_block provided
SolanaTD->>SolanaTD: set self.end_block_number = parsed_options.end_block
else
SolanaTD->>Blocks: latest_block_number
Blocks-->>SolanaTD: latest
SolanaTD->>SolanaTD: set end to latest
end
SolanaTD->>SolanaTD: apply use_latest_blocks logic & bounds checks/warnings
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (6)
✅ Files skipped from review due to trivial changes (1)
🧰 Additional context used🧬 Code graph analysis (1)chainbench/test_data/solana.py (1)
🔇 Additional comments (10)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (10)
AGENTS.md (10)
3-3: Neutralize vendor-specific reference and fix punctuation.Avoid potentially outdated brand names and add the missing comma after “e.g.”.
-This repository contains Chainbench, a blockchain infrastructure benchmarking tool built on Locust. When working on the project interactively with an agent (e.g. the Codex CLI) please follow the guidelines below for efficient development and testing. +This repository contains Chainbench, a blockchain infrastructure benchmarking tool built on Locust. When working on the project interactively with an agent (e.g., a CLI assistant), please follow the guidelines below for efficient development and testing.
21-25: Tighten Poetry workflow; avoid redundant lock step afterpoetry add.
poetry addupdates the lock automatically. Reservepoetry lockfor when you edit pyproject.toml manually or need to refresh the lock without changing constraints.-1. Use Poetry to manage dependencies: `poetry add <package>` or `poetry add --group dev <package>`. -2. Run `poetry lock --no-update` to update the lock file. -3. Install updated dependencies with `poetry install`. -4. Verify compatibility with Python 3.10+ as specified in the project. +1. Use Poetry to manage dependencies: `poetry add <package>` or `poetry add --group dev <package>`. +2. Install updated dependencies with `poetry install`. +3. If you edited pyproject.toml manually, run `poetry lock` to regenerate the lock file. +4. Verify compatibility with Python 3.10+ as specified in the project.
28-33: Minor style and linter alignment tweaks.Add the hyphen in “120‑character” and note W503 alongside E203 for Black compatibility (common pairing).
-* Follow Black formatting (120 character line length). +* Follow Black formatting (120-character line length). * Use isort for import sorting (Black-compatible profile). -* Follow Flake8 linting rules (E203 ignored for Black compatibility). +* Follow Flake8 linting rules (ignore E203 and W503 for Black compatibility). * Use type hints where appropriate. * Keep MyPy checks passing.
34-44: Keep wording consistent in the table.Very minor: “Run type checks” reads more idiomatically.
-| `poetry run mypy .` | Run type checking | +| `poetry run mypy .` | Run type checks |
46-48: Addpre-commit installto make hooks effective locally.Without installation, hooks won’t run on commit.
Or use pre-commit hooks: ```bash +poetry run pre-commit install poetry run pre-commit run --all-files--- `77-81`: **Add a quick note on monitoring method.** Consider mentioning how to watch memory/CPU (e.g., `htop`, Docker stats, or built-in metrics) to make the advice immediately actionable. ```diff -* Monitor memory usage with larger data sizes. +* Monitor memory/CPU of both Chainbench workers and the target node (e.g., via `htop`, `docker stats`, or your cloud metrics) with larger data sizes.
91-101: Addstart --helpand--versionto the recap for quick discovery.| `poetry install` | Install all dependencies | | `poetry run chainbench --help` | Show all available commands | +| `poetry run chainbench start --help` | Show options for running a benchmark | +| `poetry run chainbench --version` | Show CLI version | | `poetry run chainbench list methods` | List supported RPC methods | | `poetry run chainbench list profiles` | List available profiles | | `poetry run chainbench list shapes` | List load pattern shapes | | `poetry run chainbench discover <url>` | Discover available methods |
102-109: Add a short privacy/ToS reminder.Helps prevent accidental leakage and policy violations during testing/reporting.
* **Use appropriate rate limits** to avoid overwhelming nodes. * **Start with light profiles** before heavy ones. * **Keep test durations short** during development. +* **Do not include private endpoints or credentials** in commands, logs, or bug reports. +* **Respect your provider’s Terms of Service** and published rate limits.
69-74: Specify the canonical profiles directoryPlease update AGENTS.md to point readers at the exact location where profiles live:
• File: AGENTS.md
Lines: 69–74Short explanation: instead of “Place custom profiles in appropriate subdirectories,” name the root path explicitly so users don’t have to guess.
Suggested diff:
-* Place custom profiles in appropriate subdirectories. +* Place custom profiles in the canonical directory: `chainbench/profile/<network>/…`Optionally, you can list the top-level folders under
chainbench/profile/(e.g.evm,solana,starknet, etc.) to make it even clearer.
1-112: Add a link to AGENTS.md in README and consider moving it under docs/I verified that:
- README.md exists but does not reference AGENTS.md.
- There is no CONTRIBUTING.md in the repo.
- A docs/ directory is already present.
Recommendations:
- In README.md, add a “Development” or “Contributing” section with a link to AGENTS.md.
- Optionally, relocate AGENTS.md into docs/ for better organization and update the README link to point to docs/AGENTS.md.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
AGENTS.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
AGENTS.md
[grammar] ~38-~38: There might be a mistake here.
Context: ...ose | | ------------------------ | -----------...
(QB_NEW_EN)
[grammar] ~39-~39: There might be a mistake here.
Context: ...-------------------------------------- | | poetry run black . | Format code...
(QB_NEW_EN)
[grammar] ~40-~40: There might be a mistake here.
Context: ...mat code to project standards | | poetry run isort . | Sort import...
(QB_NEW_EN)
[grammar] ~41-~41: There might be a mistake here.
Context: ... imports | | poetry run flake8 | Run linting...
(QB_NEW_EN)
[grammar] ~42-~42: There might be a mistake here.
Context: ... linting checks | | poetry run mypy . | Run type ch...
(QB_NEW_EN)
[grammar] ~77-~77: There might be a mistake here.
Context: ... ## 7. Working with Test Data * Start with smallest data size (--size XS) for de...
(QB_NEW_EN)
[grammar] ~77-~77: There might be a mistake here.
Context: ...data size (--size XS) for development. * Use --use-latest-blocks for nodes with...
(QB_NEW_EN)
🔇 Additional comments (3)
AGENTS.md (3)
1-112: Solid, actionable doc — nice addition.Clear, practical guidelines with concrete commands. This will help keep local/dev testing safe and fast. No blocking issues from me; see a few targeted refinements below.
14-15: Align AGENTS.md example with the actualstartsubcommand flags and use a<NODE_URL>placeholder
- Replace the hard-coded
https://test-nodewith<NODE_URL>to avoid accidental copy-paste against a real node.- Verify that the flags in your example match those exposed by the
startcommand inchainbench/main.py:
- The current code defines at least:
•--profile
•--users
•--headless
•--autoquit
•--target
•--run-id- If
--workersand--test-timeare not supported in your PR branch, either add those options to the CLI or update the example to use the correct flags for concurrency and duration.-poetry run chainbench start --profile evm.light --users 5 --workers 1 --test-time 30s --target https://test-node --headless --autoquit +poetry run chainbench start --profile evm.light --users 5 --workers 1 --test-time 30s --target <NODE_URL> --headless --autoquit
54-64: Clarify valid--clientsvalues in CLI docsIt’s not immediately obvious which client identifiers are supported by
chainbench discover --clients. To ensure users pick valid values, please update the snippet (and surrounding text) to:
- Show at least two real examples (e.g.
--clients geth,erigonor--clients nethermind,openethereum)- Or link to the definitive source of truth (
chainbench/tools/discovery/clients.jsonor call outRpcDiscovery.get_client_names())For example:
- chainbench discover https://test-node --clients geth + chainbench discover https://test-node --clients geth,erigon # (Valid values: geth, erigon, nethermind, openethereum, etc.; see chainbench/tools/discovery/clients.json)See the
validate_clientscallback andRpcDiscovery.get_client_names()for how client names are loaded.
a865823 to
23e5ce6
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
AGENTS.md (1)
28-33: Align Flake8 guidance with Black: add W503 to ignores.To be fully Black-compatible, recommend ignoring both E203 and W503 (the latter for line breaks around binary operators). Update the bullet to reflect this.
-* Follow Flake8 linting rules (E203 ignored for Black compatibility). +* Follow Flake8 linting rules (ignore E203 and W503 for Black compatibility).
🧹 Nitpick comments (6)
AGENTS.md (6)
21-25: Tighten Poetry workflow phrasing.poetry add already updates the lock; poetry lock --no-update is typically for metadata-only changes. Suggest clarifying when to use each.
-1. Use Poetry to manage dependencies: `poetry add <package>` or `poetry add --group dev <package>`. -2. Run `poetry lock --no-update` to update the lock file. +1. Use Poetry to manage dependencies: `poetry add <package>` (or `--group dev` for dev deps) — this updates the lock file. +2. If you only change metadata (no version bumps), run `poetry lock --no-update` to refresh the lock deterministically.
45-49: Pre-commit: add install step so hooks run on commit.Running hooks manually is fine, but most folks expect automatic checks on commit.
Or use pre-commit hooks: ```bash +poetry run pre-commit install poetry run pre-commit run --all-files--- `77-81`: **Grammar and repeatability nits; clarify when to use latest blocks.** ```diff -* Start with smallest data size (`--size XS`) for development. +* Start with the smallest data size (`--size XS`) for development. * Use `--use-latest-blocks` for nodes with limited history. -* Consider using `--ref-url` for test data generation from a reference node. +* Consider using `--ref-url` for test data generation from a reference node. +* For repeatable benchmarks across runs, prefer pinning a block range over `--use-latest-blocks`.
83-90: Add a quick test step to the workflow.If the repo has tests, include a minimal test invocation so contributors don’t skip it.
2. Run formatting: `poetry run black . && poetry run isort .` 3. Run linting: `poetry run flake8` 4. Run type checking: `poetry run mypy .` -5. Test with minimal profile first +5. Run tests: `poetry run pytest -q` # or project-specific test runner +6. Test with minimal profile first -6. Gradually increase complexity and load +7. Gradually increase complexity and loadIf the project doesn’t use pytest, I can tailor this to your test runner.
26-33: Optional: link to the authoritative configs so the doc stays in sync.A small “See also” tying to pyproject.toml sections for Black/isort/Flake8/MyPy helps readers map guidelines to enforcement.
* Use type hints where appropriate. * Keep MyPy checks passing. + +See also: the `[tool.black]`, `[tool.isort]`, `[tool.flake8]`, and `[tool.mypy]` sections in `pyproject.toml` (and `mypy.ini` if present).
12-15: Docs: Confirm CLI flags and add spawn‐rate in exampleVerified that the example flags in AGENTS.md match the current Chainbench CLI (all options below exist in chainbench/main.py, with
--test-timeas the correct flag and no--run-timealias). Since the--spawn-rateoption is supported and helps ensure a predictable ramp-up, please update the example command to include it:• File: AGENTS.md (around lines 12–15)
Update the example invocation to:- poetry run chainbench start --profile evm.light --users 5 --workers 1 --test-time 30s --target https://test-node --headless --autoquit + poetry run chainbench start --profile evm.light --users 5 --spawn-rate 1 --workers 1 --test-time 30s --target https://test-node --headless --autoquit
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
AGENTS.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
AGENTS.md
[grammar] ~38-~38: There might be a mistake here.
Context: ...ose | | ------------------------ | -----------...
(QB_NEW_EN)
[grammar] ~39-~39: There might be a mistake here.
Context: ...-------------------------------------- | | poetry run black . | Format code...
(QB_NEW_EN)
[grammar] ~40-~40: There might be a mistake here.
Context: ...mat code to project standards | | poetry run isort . | Sort import...
(QB_NEW_EN)
[grammar] ~41-~41: There might be a mistake here.
Context: ... imports | | poetry run flake8 | Run linting...
(QB_NEW_EN)
[grammar] ~42-~42: There might be a mistake here.
Context: ... linting checks | | poetry run mypy . | Run type ch...
(QB_NEW_EN)
[grammar] ~77-~77: There might be a mistake here.
Context: ... ## 7. Working with Test Data * Start with smallest data size (--size XS) for de...
(QB_NEW_EN)
[grammar] ~77-~77: There might be a mistake here.
Context: ...data size (--size XS) for development. * Use --use-latest-blocks for nodes with...
(QB_NEW_EN)
🔇 Additional comments (2)
AGENTS.md (2)
1-112: Overall: solid, practical doc.Well-structured and actionable. Clear guidance for safe, short, headless development runs and a tidy QA loop (format → lint → typecheck → smoke). Nice.
91-101: Useful commands list verifiedI’ve confirmed that all of the commands in the “Useful Commands Recap” section map directly to registered click subcommands in
chainbench/main.py:
chainbench list methodschainbench list profileschainbench list shapeschainbench discover <endpoint>(The
listgroup and its four subcommands—methods, profiles, shapes, and the client‐options listing—are all defined via@_list.commanddecorators.) No drift detected, so no updates are required here.
* Sort discovery methods case-insensitively --------- Co-authored-by: Erwin Wee <erwin.wee@chainstack.com>
…inbench into add-agents-md
Summary by CodeRabbit