fix(ci): run snap tests on CI for debug and codecov builds #6333
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
WalkthroughAdds a new forest-dev binary and dev CLI for fetching/caching RPC test snapshots, centralizes snapshot retrieval into fetch_rpc_test_snapshot, converts several mains to async, simplifies retry/timeout logic in utils, and updates CI workflows and coverage targets. Snapshot tests opt out via FOREST_RPC_SNAPSHOT_TEST_OPT_OUT. Changes
Sequence Diagram(s)mermaid Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ 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)
Comment |
hanabi1224
requested review from
LesnyRumcajs and
akaladarshi
and removed request for
a team
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/utils/mod.rs (1)
172-176: Scoped lint suppression looks fine; consider tightening scope further if possible.
#[allow(dead_code)]is narrowly applied and only under#[cfg(test)], so this is low-risk. Ifis_debug_build()isn’t intended to be part of a “test utils” public surface, consider making itpub(crate)or moving it into the test-only module to avoid needing the allow.src/tool/subcommands/api_cmd/test_snapshot.rs (1)
218-224: Timeout bump for download reliability is sound; atomic temp file pattern already handles cancellation safety.The CI=30s / local=300s timeout increase is good for reducing flaky snapshot downloads. The implementation is already cancellation-safe:
download_httpuses a temporary.frdownloadfile with atomicstd::fs::rename, so if the timeout fires mid-download, only the temp file is left and the cache is never corrupted.Consider extracting
30and300into named constants (e.g.,CI_DOWNLOAD_TIMEOUT_SECS = 30,LOCAL_DOWNLOAD_TIMEOUT_SECS = 300) for easier future tuning, though this is optional given the codebase's mixed approach to timeout constants.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/tool/subcommands/api_cmd/test_snapshot.rs(1 hunks)src/utils/mod.rs(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: Makefile:0-0
Timestamp: 2025-08-25T08:06:18.865Z
Learning: hanabi1224 intentionally uses multi-threading (instead of single-threaded execution) in RPC snapshot tests to speed up file downloading operations, prioritizing performance over potential test isolation benefits.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: .github/workflows/unit-tests.yml:86-96
Timestamp: 2025-08-25T14:11:10.790Z
Learning: hanabi1224's RPC snapshot tests include a checksum validation mechanism that compares against HTTP HEAD headers, automatically re-downloading snapshots if the cached version doesn't match, providing protection against corrupted cached files.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: .github/workflows/unit-tests.yml:72-76
Timestamp: 2025-08-25T09:53:37.443Z
Learning: hanabi1224 intentionally designs RPC snapshot test cache to be OS/arch-agnostic, sharing cached snapshot files across different platforms using enableCrossOsArchive: true and hash-based cache keys to improve cache efficiency and reduce redundant downloads.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5930
File: build.rs:64-77
Timestamp: 2025-08-13T09:43:20.301Z
Learning: hanabi1224 prefers hard compile-time errors in build scripts rather than runtime safeguards or collision detection, believing it's better to fail fast and fix root causes of issues like malformed snapshot names.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 6057
File: src/cli/subcommands/f3_cmd.rs:0-0
Timestamp: 2025-09-09T10:37:17.947Z
Learning: hanabi1224 prefers having default timeouts (like 10m for --no-progress-timeout) to prevent commands from hanging indefinitely, even when the timeout flag isn't explicitly provided by users. This fail-fast approach is preferred over requiring explicit flag usage.
📚 Learning: 2025-08-07T13:39:36.962Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5886
File: interop-tests/src/tests/go_app/gen.go:88-90
Timestamp: 2025-08-07T13:39:36.962Z
Learning: Skip reviewing auto-generated files marked with "Generated by rust2go. Please DO NOT edit this C part manually." as these should not be manually edited and any issues should be addressed in the code generation tool or source templates instead.
Applied to files:
src/utils/mod.rs
📚 Learning: 2025-08-25T13:35:24.230Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5969
File: src/tool/subcommands/snapshot_cmd.rs:412-412
Timestamp: 2025-08-25T13:35:24.230Z
Learning: In src/tool/subcommands/snapshot_cmd.rs, the +1 in `last_epoch = ts.epoch() - epochs as i64 + 1` fixes an off-by-1 bug where specifying --check-stateroots=N would validate N+1 epochs instead of N epochs, causing out-of-bounds errors when the snapshot contains only N recent state roots.
Applied to files:
src/tool/subcommands/api_cmd/test_snapshot.rs
📚 Learning: 2025-08-25T14:17:09.129Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: .github/workflows/unit-tests.yml:0-0
Timestamp: 2025-08-25T14:17:09.129Z
Learning: hanabi1224's download_file_with_cache function in src/utils/net/download_file.rs preserves URL path structure in local cache directories by using cache_dir.join(url.path().strip_prefix('/').unwrap_or_else(|| url.path())), so snapshots from https://forest-snapshots.fra1.cdn.digitaloceanspaces.com/rpc_test/ are cached locally at ~/.cache/forest/test/rpc-snapshots/rpc_test/ (including the rpc_test subdirectory from the URL path).
Applied to files:
src/tool/subcommands/api_cmd/test_snapshot.rs
⏰ 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: Coverage
- GitHub Check: Build forest binaries on Linux AMD64
- GitHub Check: tests-release
- GitHub Check: tests
- GitHub Check: All lint checks
- GitHub Check: Build Ubuntu
- GitHub Check: cargo-publish-dry-run
- GitHub Check: Build MacOS
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
.github/workflows/unit-tests.yml (2)
36-55: Consider snapshot cache restore/save for thetests(debug) job too.
Right now onlytests-releaserestores/saves/home/runner/.cache/forest/test/rpc-snapshots/rpc_test; the new debug fetch will likely re-download every run.
52-55: Consider adding--lockedto the newcargo runstep for CI reproducibility.The new step at line 54 (
cargo run --bin forest-dev --no-default-features -- fetch-rpc-tests) will work fine since theubuntu-24.04-armrunner includes Rust pre-installed. However, adding--lockedaligns with CI best practices for reproducibility, especially sinceCargo.lockis committed:- - run: cargo run --bin forest-dev --no-default-features -- fetch-rpc-tests + - run: cargo run --locked --bin forest-dev --no-default-features -- fetch-rpc-testssrc/dev/subcommands/mod.rs (1)
55-58: Consider bounding concurrency (semaphore) to avoid resource spikes / rate limits.
Spawning one task per snapshot can overwhelm CI runners or the DO bucket.src/lib.rs (1)
50-51:forest_dev_mainre-export is fine; confirm you’re OK making it part of the crate’s public surface.
If this is intended as “internal tooling only”, consider gating behind a feature (e.g.,dev-tools) to avoid extra surface/compile for library users.Also applies to: 128-129
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.github/workflows/coverage.yml(1 hunks).github/workflows/unit-tests.yml(1 hunks)src/bin/forest-dev.rs(1 hunks)src/bin/forest-tool.rs(1 hunks)src/bin/forest-wallet.rs(1 hunks)src/dev/main.rs(1 hunks)src/dev/mod.rs(1 hunks)src/dev/subcommands/mod.rs(1 hunks)src/lib.rs(2 hunks)src/tool/main.rs(2 hunks)src/tool/subcommands/api_cmd/test_snapshot.rs(2 hunks)src/utils/mod.rs(4 hunks)src/wallet/main.rs(2 hunks)
🧰 Additional context used
🧠 Learnings (15)
📓 Common learnings
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: .github/workflows/unit-tests.yml:0-0
Timestamp: 2025-08-25T14:17:09.129Z
Learning: hanabi1224's download_file_with_cache function in src/utils/net/download_file.rs preserves URL path structure in local cache directories by using cache_dir.join(url.path().strip_prefix('/').unwrap_or_else(|| url.path())), so snapshots from https://forest-snapshots.fra1.cdn.digitaloceanspaces.com/rpc_test/ are cached locally at ~/.cache/forest/test/rpc-snapshots/rpc_test/ (including the rpc_test subdirectory from the URL path).
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: .github/workflows/unit-tests.yml:72-76
Timestamp: 2025-08-25T09:53:37.443Z
Learning: hanabi1224 intentionally designs RPC snapshot test cache to be OS/arch-agnostic, sharing cached snapshot files across different platforms using enableCrossOsArchive: true and hash-based cache keys to improve cache efficiency and reduce redundant downloads.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: Makefile:0-0
Timestamp: 2025-08-25T08:06:18.865Z
Learning: hanabi1224 intentionally uses multi-threading (instead of single-threaded execution) in RPC snapshot tests to speed up file downloading operations, prioritizing performance over potential test isolation benefits.
Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 5923
File: src/tool/subcommands/api_cmd/test_snapshots.txt:206-252
Timestamp: 2025-09-02T14:23:53.808Z
Learning: In the Forest project, .rpcsnap.json.zst snapshot files listed in src/tool/subcommands/api_cmd/test_snapshots.txt are not stored in the repository itself but are downloaded from a DigitalOcean (DO) bucket when needed. The manifest file serves as an index/catalog of available snapshots.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5930
File: build.rs:64-77
Timestamp: 2025-08-13T09:43:20.301Z
Learning: hanabi1224 prefers hard compile-time errors in build scripts rather than runtime safeguards or collision detection, believing it's better to fail fast and fix root causes of issues like malformed snapshot names.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 6057
File: src/cli/subcommands/f3_cmd.rs:0-0
Timestamp: 2025-09-09T10:37:17.947Z
Learning: hanabi1224 prefers having default timeouts (like 10m for --no-progress-timeout) to prevent commands from hanging indefinitely, even when the timeout flag isn't explicitly provided by users. This fail-fast approach is preferred over requiring explicit flag usage.
📚 Learning: 2025-08-28T12:52:46.927Z
Learnt from: LesnyRumcajs
Repo: ChainSafe/forest PR: 6011
File: src/cli/main.rs:18-25
Timestamp: 2025-08-28T12:52:46.927Z
Learning: In Forest CLI (src/cli/main.rs), the early RPC network check before Cli::parse_from() does not block help/version commands because clap processes these internally before reaching the RPC call. LesnyRumcajs confirmed this implementation works correctly and that RPC call failures are acceptable in this context.
Applied to files:
src/dev/main.rssrc/lib.rssrc/dev/subcommands/mod.rssrc/bin/forest-tool.rssrc/tool/main.rssrc/wallet/main.rssrc/bin/forest-dev.rs
📚 Learning: 2025-08-08T12:10:45.218Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5867
File: src/ipld/util.rs:553-558
Timestamp: 2025-08-08T12:10:45.218Z
Learning: Forest project targets Rust stable >=1.89; features stabilized in 1.88 like let-chains are acceptable in this codebase.
Applied to files:
src/bin/forest-wallet.rssrc/lib.rssrc/bin/forest-tool.rssrc/bin/forest-dev.rs
📚 Learning: 2025-08-04T13:36:22.993Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5835
File: src/chain/tests.rs:58-76
Timestamp: 2025-08-04T13:36:22.993Z
Learning: In the Forest codebase, `Vec<u8>` can be used as an `AsyncWrite` implementation in test contexts. The user confirmed that tests using `&mut Vec<u8>` with `export` and `export_v2` functions compile and pass both locally and on CI.
Applied to files:
src/bin/forest-wallet.rssrc/bin/forest-tool.rs
📚 Learning: 2025-09-11T16:03:14.328Z
Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 6000
File: src/tool/subcommands/api_cmd/state_decode_params_tests/miner.rs:4-9
Timestamp: 2025-09-11T16:03:14.328Z
Learning: In the Forest codebase's state_decode_params_tests module, common imports like `to_vec`, `TokenAmount`, `Cid`, etc. are centralized in the mod.rs file and made available to submodules through `use super::*;`, eliminating the need for duplicate imports in individual test files.
Applied to files:
src/lib.rssrc/dev/subcommands/mod.rssrc/tool/subcommands/api_cmd/test_snapshot.rs
📚 Learning: 2025-08-11T12:42:42.848Z
Learnt from: elmattic
Repo: ChainSafe/forest PR: 5836
File: src/tool/subcommands/api_cmd/api_run_tests.rs:186-190
Timestamp: 2025-08-11T12:42:42.848Z
Learning: In the Forest project's test infrastructure (specifically in `src/tool/subcommands/api_cmd/api_run_tests.rs`), the team prefers to use only `ws` (non-secure WebSocket) connections for simplicity in their stateful API test scenarios, as security is not a concern in their testing context.
Applied to files:
src/dev/subcommands/mod.rssrc/tool/subcommands/api_cmd/test_snapshot.rs.github/workflows/unit-tests.ymlsrc/bin/forest-dev.rs
📚 Learning: 2025-08-25T14:17:09.129Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: .github/workflows/unit-tests.yml:0-0
Timestamp: 2025-08-25T14:17:09.129Z
Learning: hanabi1224's download_file_with_cache function in src/utils/net/download_file.rs preserves URL path structure in local cache directories by using cache_dir.join(url.path().strip_prefix('/').unwrap_or_else(|| url.path())), so snapshots from https://forest-snapshots.fra1.cdn.digitaloceanspaces.com/rpc_test/ are cached locally at ~/.cache/forest/test/rpc-snapshots/rpc_test/ (including the rpc_test subdirectory from the URL path).
Applied to files:
src/dev/subcommands/mod.rssrc/tool/subcommands/api_cmd/test_snapshot.rs
📚 Learning: 2025-08-07T13:39:15.107Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5886
File: interop-tests/src/tests/go_app/gen.go:29-29
Timestamp: 2025-08-07T13:39:15.107Z
Learning: Auto-generated files like those created by rust2go (indicated by "Generated by rust2go. Please DO NOT edit this C part manually." comment) in the Forest project should be skipped during code review as they are not intended for manual editing.
Applied to files:
src/bin/forest-tool.rssrc/bin/forest-dev.rs
📚 Learning: 2025-10-17T09:36:15.757Z
Learnt from: elmattic
Repo: ChainSafe/forest PR: 6128
File: src/ipld/util.rs:23-30
Timestamp: 2025-10-17T09:36:15.757Z
Learning: Always run `cargo check` or `cargo build` to verify actual compilation errors in the Forest codebase before flagging them as issues. Do not rely solely on documentation or assumptions about trait implementations.
Applied to files:
src/bin/forest-tool.rs.github/workflows/unit-tests.ymlsrc/bin/forest-dev.rs
📚 Learning: 2025-09-02T14:23:53.808Z
Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 5923
File: src/tool/subcommands/api_cmd/test_snapshots.txt:206-252
Timestamp: 2025-09-02T14:23:53.808Z
Learning: In the Forest project, .rpcsnap.json.zst snapshot files listed in src/tool/subcommands/api_cmd/test_snapshots.txt are not stored in the repository itself but are downloaded from a DigitalOcean (DO) bucket when needed. The manifest file serves as an index/catalog of available snapshots.
Applied to files:
src/tool/subcommands/api_cmd/test_snapshot.rs
📚 Learning: 2025-08-25T09:53:37.443Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: .github/workflows/unit-tests.yml:72-76
Timestamp: 2025-08-25T09:53:37.443Z
Learning: hanabi1224 intentionally designs RPC snapshot test cache to be OS/arch-agnostic, sharing cached snapshot files across different platforms using enableCrossOsArchive: true and hash-based cache keys to improve cache efficiency and reduce redundant downloads.
Applied to files:
src/tool/subcommands/api_cmd/test_snapshot.rs
📚 Learning: 2025-08-25T13:35:24.230Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5969
File: src/tool/subcommands/snapshot_cmd.rs:412-412
Timestamp: 2025-08-25T13:35:24.230Z
Learning: In src/tool/subcommands/snapshot_cmd.rs, the +1 in `last_epoch = ts.epoch() - epochs as i64 + 1` fixes an off-by-1 bug where specifying --check-stateroots=N would validate N+1 epochs instead of N epochs, causing out-of-bounds errors when the snapshot contains only N recent state roots.
Applied to files:
src/tool/subcommands/api_cmd/test_snapshot.rs
📚 Learning: 2025-08-25T14:11:10.790Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: .github/workflows/unit-tests.yml:86-96
Timestamp: 2025-08-25T14:11:10.790Z
Learning: hanabi1224's RPC snapshot tests include a checksum validation mechanism that compares against HTTP HEAD headers, automatically re-downloading snapshots if the cached version doesn't match, providing protection against corrupted cached files.
Applied to files:
src/tool/subcommands/api_cmd/test_snapshot.rs
📚 Learning: 2025-08-25T14:17:09.129Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: .github/workflows/unit-tests.yml:0-0
Timestamp: 2025-08-25T14:17:09.129Z
Learning: hanabi1224's download_file_with_cache function preserves URL path structure in local cache directories, so snapshots from https://forest-snapshots.fra1.cdn.digitaloceanspaces.com/rpc_test/ are cached locally at ~/.cache/forest/test/rpc-snapshots/rpc_test (including the rpc_test subdirectory from the URL path).
Applied to files:
src/tool/subcommands/api_cmd/test_snapshot.rs
📚 Learning: 2025-08-07T13:39:29.732Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5886
File: interop-tests/src/tests/go_app/gen.go:58-58
Timestamp: 2025-08-07T13:39:29.732Z
Learning: In the Forest project, auto-generated files (like those generated by rust2go tool) should be skipped during code review as they are not meant to be manually edited.
Applied to files:
src/bin/forest-dev.rs
🧬 Code graph analysis (9)
src/dev/mod.rs (1)
src/dev/main.rs (1)
main(9-18)
src/bin/forest-wallet.rs (3)
src/bin/forest-dev.rs (1)
main(5-7)src/bin/forest-tool.rs (1)
main(5-7)src/bin/forest-cli.rs (1)
main(5-7)
src/lib.rs (2)
src/bin/forest-dev.rs (1)
main(5-7)src/dev/main.rs (1)
main(9-18)
src/dev/subcommands/mod.rs (4)
src/utils/net/download_file.rs (1)
download_file_with_cache(30-89)src/utils/proofs_api/paramfetch.rs (1)
ensure_proof_params_downloaded(59-66)src/utils/proofs_api/parameters.rs (1)
maybe_set_proofs_parameter_cache_dir_env(114-118)src/utils/mod.rs (2)
retry(116-145)is_ci(171-174)
src/bin/forest-tool.rs (7)
src/bin/forest-dev.rs (1)
main(5-7)src/bin/forest-wallet.rs (1)
main(5-7)src/dev/main.rs (1)
main(9-18)src/tool/main.rs (1)
main(11-37)src/wallet/main.rs (1)
main(13-34)src/bin/forest-cli.rs (1)
main(5-7)src/bin/forest.rs (1)
main(4-6)
src/tool/subcommands/api_cmd/test_snapshot.rs (1)
src/dev/subcommands/mod.rs (1)
fetch_rpc_test_snapshot(63-88)
src/tool/main.rs (4)
src/dev/main.rs (1)
main(9-18)src/wallet/main.rs (1)
main(13-34)src/cli/main.rs (1)
main(14-50)src/daemon/main.rs (1)
main(26-71)
src/wallet/main.rs (7)
src/bin/forest-dev.rs (1)
main(5-7)src/bin/forest-wallet.rs (1)
main(5-7)src/dev/main.rs (1)
main(9-18)src/tool/main.rs (1)
main(11-37)src/cli/main.rs (1)
main(14-50)src/rpc/reflect/mod.rs (1)
call(354-363)src/rpc/client.rs (2)
call(91-152)client(115-115)
src/bin/forest-dev.rs (5)
src/bin/forest-tool.rs (1)
main(5-7)src/bin/forest-wallet.rs (1)
main(5-7)src/dev/main.rs (1)
main(9-18)src/tool/main.rs (1)
main(11-37)src/wallet/main.rs (1)
main(13-34)
⏰ 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: All lint checks
- GitHub Check: Coverage
- GitHub Check: tests
- GitHub Check: tests-release
- GitHub Check: Build forest binaries on Linux AMD64
- GitHub Check: Build Ubuntu
- GitHub Check: cargo-publish-dry-run
- GitHub Check: Build MacOS
🔇 Additional comments (12)
src/bin/forest-tool.rs (1)
4-7: Async Tokio entrypoint wiring looks correct.src/utils/mod.rs (3)
22-26: Import cleanup looks fine (Future+Duration).
116-145:retry()simplification is solid; confirmmax_retries=0semantics are acceptable.
0..max_retriesmeansmax_retries=0performs zero attempts and returnsRetriesExceededimmediately—just confirm that matches prior behavior/expectation.
176-231: Tests updated appropriately for the new timeout strategy.src/dev/mod.rs (1)
4-5: Module wiring looks good.src/bin/forest-dev.rs (1)
4-7: Newforest-deventrypoint is clean and consistent with the other async binaries..github/workflows/coverage.yml (1)
50-55: [rewritten comment]
[classification tag]src/bin/forest-wallet.rs (1)
4-7: LGTM! Consistent async conversion.The conversion to async main with multi-threaded Tokio runtime is consistent with the pattern applied across other binaries (forest-cli, forest-dev, forest-tool). The async execution model enables better integration with the async ecosystem and prepares the wallet for async operations.
src/tool/subcommands/api_cmd/test_snapshot.rs (1)
207-209: LGTM! Excellent refactoring to centralize snapshot fetching.Replacing the inline download logic with the shared
fetch_rpc_test_snapshothelper improves maintainability by:
- Eliminating code duplication
- Centralizing retry logic and timeout handling
- Providing consistent caching behavior across test contexts
The helper function preserves all original functionality including URL construction, cache directory structure, and checksum validation.
src/dev/main.rs (1)
9-18: LGTM! Clean async entry point for dev tools.The implementation follows the established pattern used across other main entry points (cli, wallet, tool). The minimal logger setup is appropriate for developer tooling, and the generic argument handling provides flexibility for testing and reuse.
src/wallet/main.rs (1)
13-33: LGTM! Clean async refactor with improved separation of concerns.The conversion removes boilerplate runtime management by making the function natively async. The runtime is now provided by the binary entry point (
#[tokio::main]), resulting in cleaner code separation. All original logic is preserved including network detection and CurrentNetwork configuration.src/tool/main.rs (1)
11-37: LGTM! Consistent async conversion with proper sync/async subcommand handling.The refactor removes runtime boilerplate by making the main function natively async, while correctly preserving the distinction between synchronous subcommands (Backup, Completion) and asynchronous ones. The runtime is now provided by the binary entry point, improving code organization.
Comment on lines
+43
to
+61
| async fn fetch_rpc_tests() -> anyhow::Result<()> { | ||
| crate::utils::proofs_api::maybe_set_proofs_parameter_cache_dir_env( | ||
| &crate::Config::default().client.data_dir, | ||
| ); | ||
| ensure_proof_params_downloaded().await?; | ||
| let tests = include_str!("../../tool/subcommands/api_cmd/test_snapshots.txt") | ||
| .lines() | ||
| .map(|i| { | ||
| // Remove comment | ||
| i.split("#").next().unwrap().trim().to_string() | ||
| }) | ||
| .filter(|l| !l.is_empty() && !l.starts_with('#')); | ||
| let mut joinset = JoinSet::new(); | ||
| for test in tests { | ||
| joinset.spawn(fetch_rpc_test_snapshot(test.into())); | ||
| } | ||
| joinset.join_all().await; | ||
| Ok(()) | ||
| } |
Contributor
There was a problem hiding this comment.
Don’t ignore JoinSet results—fail fast and surface which snapshot(s) failed.
Right now joinset.join_all().await; discards all task outputs/errors, making CI behavior flaky and hard to debug when a fetch fails.
let mut joinset = JoinSet::new();
for test in tests {
joinset.spawn(fetch_rpc_test_snapshot(test.into()));
}
- joinset.join_all().await;
+ while let Some(res) = joinset.join_next().await {
+ // JoinError (panic/cancel) OR fetch error should fail the command.
+ res??;
+ }
Ok(())
}🤖 Prompt for AI Agents
In src/dev/subcommands/mod.rs around lines 43 to 61, the JoinSet results are
currently ignored which hides task failures; change the code to await each
spawned task result and propagate errors with context so CI fails fast and
reports which snapshot failed. Specifically, when spawning, associate the
snapshot name (clone or move the `test` string into the task) and after spawning
iterate over the joinset results (e.g., loop with joinset.join_next().await or
process the JoinSet returned futures) checking each task's JoinError and inner
Result; if a task panics or returns Err, return an anyhow::Error that includes
the snapshot identifier and the underlying error so the function fails
immediately and surfaces which snapshot fetch failed.
Comment on lines
+63
to
+88
| pub async fn fetch_rpc_test_snapshot<'a>(name: Cow<'a, str>) -> anyhow::Result<PathBuf> { | ||
| let url: Url = | ||
| format!("https://forest-snapshots.fra1.cdn.digitaloceanspaces.com/rpc_test/{name}") | ||
| .parse() | ||
| .with_context(|| format!("Failed to parse URL for test: {name}"))?; | ||
| let project_dir = | ||
| ProjectDirs::from("com", "ChainSafe", "Forest").context("failed to get project dir")?; | ||
| let cache_dir = project_dir.cache_dir().join("test").join("rpc-snapshots"); | ||
| let path = crate::utils::retry( | ||
| crate::utils::RetryArgs { | ||
| timeout: Some(Duration::from_secs(if crate::utils::is_ci() { | ||
| 30 | ||
| } else { | ||
| 300 | ||
| })), | ||
| max_retries: Some(5), | ||
| ..Default::default() | ||
| }, | ||
| || async { | ||
| download_file_with_cache(&url, &cache_dir, DownloadFileOption::NonResumable).await | ||
| }, | ||
| ) | ||
| .await? | ||
| .path; | ||
| Ok(path) | ||
| } |
Contributor
There was a problem hiding this comment.
Validate name to prevent cache path traversal via crafted URL paths.
Because fetch_rpc_test_snapshot is pub and download_file_with_cache() joins cache_dir with url.path(), a name containing ../ or path separators can escape the intended cache directory.
pub async fn fetch_rpc_test_snapshot<'a>(name: Cow<'a, str>) -> anyhow::Result<PathBuf> {
+ // Prevent path traversal / unexpected subpaths.
+ if name.contains("..") || name.contains('/') || name.contains('\\') {
+ anyhow::bail!("invalid snapshot name: {name}");
+ }
let url: Url =
format!("https://forest-snapshots.fra1.cdn.digitaloceanspaces.com/rpc_test/{name}")
.parse()
.with_context(|| format!("Failed to parse URL for test: {name}"))?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| pub async fn fetch_rpc_test_snapshot<'a>(name: Cow<'a, str>) -> anyhow::Result<PathBuf> { | |
| let url: Url = | |
| format!("https://forest-snapshots.fra1.cdn.digitaloceanspaces.com/rpc_test/{name}") | |
| .parse() | |
| .with_context(|| format!("Failed to parse URL for test: {name}"))?; | |
| let project_dir = | |
| ProjectDirs::from("com", "ChainSafe", "Forest").context("failed to get project dir")?; | |
| let cache_dir = project_dir.cache_dir().join("test").join("rpc-snapshots"); | |
| let path = crate::utils::retry( | |
| crate::utils::RetryArgs { | |
| timeout: Some(Duration::from_secs(if crate::utils::is_ci() { | |
| 30 | |
| } else { | |
| 300 | |
| })), | |
| max_retries: Some(5), | |
| ..Default::default() | |
| }, | |
| || async { | |
| download_file_with_cache(&url, &cache_dir, DownloadFileOption::NonResumable).await | |
| }, | |
| ) | |
| .await? | |
| .path; | |
| Ok(path) | |
| } | |
| pub async fn fetch_rpc_test_snapshot<'a>(name: Cow<'a, str>) -> anyhow::Result<PathBuf> { | |
| // Prevent path traversal / unexpected subpaths. | |
| if name.contains("..") || name.contains('/') || name.contains('\\') { | |
| anyhow::bail!("invalid snapshot name: {name}"); | |
| } | |
| let url: Url = | |
| format!("https://forest-snapshots.fra1.cdn.digitaloceanspaces.com/rpc_test/{name}") | |
| .parse() | |
| .with_context(|| format!("Failed to parse URL for test: {name}"))?; | |
| let project_dir = | |
| ProjectDirs::from("com", "ChainSafe", "Forest").context("failed to get project dir")?; | |
| let cache_dir = project_dir.cache_dir().join("test").join("rpc-snapshots"); | |
| let path = crate::utils::retry( | |
| crate::utils::RetryArgs { | |
| timeout: Some(Duration::from_secs(if crate::utils::is_ci() { | |
| 30 | |
| } else { | |
| 300 | |
| })), | |
| max_retries: Some(5), | |
| ..Default::default() | |
| }, | |
| || async { | |
| download_file_with_cache(&url, &cache_dir, DownloadFileOption::NonResumable).await | |
| }, | |
| ) | |
| .await? | |
| .path; | |
| Ok(path) | |
| } |
🤖 Prompt for AI Agents
In src/dev/subcommands/mod.rs around lines 63 to 88, validate and sanitize the
incoming name to prevent cache path traversal: before constructing the URL,
reject or error on any name containing path separators or traversal patterns
(e.g. '/', '\\', "..") or percent-encoded equivalents ("%2F", "%5C", "%2E%2E"),
or constrain name to a safe whitelist (e.g. allow only alphanumeric, dots,
dashes and underscores); alternatively percent-encode the name so it cannot
produce extra path segments, and ensure any value joined into the cache path is
treated as a single filename (not a path) so download_file_with_cache cannot
escape cache_dir.
38ad14e to
36891fb
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/dev/subcommands/mod.rs (2)
43-60: Don’t ignoreJoinSetresults—propagate task failures (and include which snapshot failed).
Right nowjoinset.join_all().await;drops errors/panics, so CI may pass even if some downloads failed.async fn fetch_rpc_tests() -> anyhow::Result<()> { @@ let mut joinset = JoinSet::new(); for test in tests { - joinset.spawn(fetch_rpc_test_snapshot(test.into())); + let test: String = test; + joinset.spawn(async move { + fetch_rpc_test_snapshot(Cow::Owned(test.clone())) + .await + .with_context(|| format!("failed to fetch RPC test snapshot: {test}")) + }); } - joinset.join_all().await; + while let Some(res) = joinset.join_next().await { + res??; + } Ok(()) }
63-87: Validate/encodenameto prevent cache path traversal & unintended URL paths.
Because caching derives the on-disk path fromurl.path(), a craftednamecan escapecache_dirvia../separators. Prefer building the URL viapath_segments_mut()and enforce a safe filename-like allowlist.pub async fn fetch_rpc_test_snapshot<'a>(name: Cow<'a, str>) -> anyhow::Result<PathBuf> { - let url: Url = - format!("https://forest-snapshots.fra1.cdn.digitaloceanspaces.com/rpc_test/{name}") - .parse() - .with_context(|| format!("Failed to parse URL for test: {name}"))?; + // Treat snapshot name as a single path segment / file name. + // (Prevents cache path traversal and avoids accidental extra URL segments.) + let name_ref = name.as_ref(); + if name_ref.is_empty() + || name_ref.contains("..") + || name_ref.contains('/') + || name_ref.contains('\\') + { + anyhow::bail!("invalid snapshot name: {name_ref}"); + } + + let mut url: Url = "https://forest-snapshots.fra1.cdn.digitaloceanspaces.com/" + .parse() + .context("Failed to parse snapshots base URL")?; + url.path_segments_mut() + .map_err(|_| anyhow::anyhow!("base URL cannot be a base"))? + .extend(["rpc_test", name_ref]); @@ Ok(path) }
🧹 Nitpick comments (2)
src/dev/subcommands/mod.rs (1)
48-55: Simplify/avoid extra allocations when parsingtest_snapshots.txt.
The current split/trim/to_string + redundantstarts_with('#')check works, but can be made clearer and avoid allocating strings for comment/blank lines.- let tests = include_str!("../../tool/subcommands/api_cmd/test_snapshots.txt") - .lines() - .map(|i| { - // Remove comment - i.split("#").next().unwrap().trim().to_string() - }) - .filter(|l| !l.is_empty() && !l.starts_with('#')); + let tests = include_str!("../../tool/subcommands/api_cmd/test_snapshots.txt") + .lines() + .map(|l| l.split('#').next().unwrap_or("").trim()) + .filter(|l| !l.is_empty()); @@ - for test in tests { - joinset.spawn(fetch_rpc_test_snapshot(test.into())); + for test in tests { + joinset.spawn(fetch_rpc_test_snapshot(Cow::Borrowed(test))); }src/tool/main.rs (1)
11-36: Async main refactor looks good; consider lazily constructingclientonly for subcommands that need it.
Right nowClient::default_or_from_env(None)runs for all tool invocations even though only some branches use it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
.github/workflows/coverage.yml(1 hunks).github/workflows/unit-tests.yml(1 hunks)src/bin/forest-dev.rs(1 hunks)src/bin/forest-tool.rs(1 hunks)src/bin/forest-wallet.rs(1 hunks)src/dev/main.rs(1 hunks)src/dev/mod.rs(1 hunks)src/dev/subcommands/mod.rs(1 hunks)src/lib.rs(2 hunks)src/tool/main.rs(2 hunks)src/tool/subcommands/api_cmd/test_snapshot.rs(2 hunks)src/utils/mod.rs(4 hunks)src/wallet/main.rs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (7)
- .github/workflows/coverage.yml
- .github/workflows/unit-tests.yml
- src/dev/mod.rs
- src/tool/subcommands/api_cmd/test_snapshot.rs
- src/lib.rs
- src/dev/main.rs
- src/bin/forest-tool.rs
🧰 Additional context used
🧠 Learnings (17)
📓 Common learnings
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: .github/workflows/unit-tests.yml:0-0
Timestamp: 2025-08-25T14:17:09.129Z
Learning: hanabi1224's download_file_with_cache function in src/utils/net/download_file.rs preserves URL path structure in local cache directories by using cache_dir.join(url.path().strip_prefix('/').unwrap_or_else(|| url.path())), so snapshots from https://forest-snapshots.fra1.cdn.digitaloceanspaces.com/rpc_test/ are cached locally at ~/.cache/forest/test/rpc-snapshots/rpc_test/ (including the rpc_test subdirectory from the URL path).
Learnt from: LesnyRumcajs
Repo: ChainSafe/forest PR: 6011
File: src/cli/main.rs:18-25
Timestamp: 2025-08-28T12:52:46.927Z
Learning: In Forest CLI (src/cli/main.rs), the early RPC network check before Cli::parse_from() does not block help/version commands because clap processes these internally before reaching the RPC call. LesnyRumcajs confirmed this implementation works correctly and that RPC call failures are acceptable in this context.
Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 5923
File: src/tool/subcommands/api_cmd/test_snapshots.txt:206-252
Timestamp: 2025-09-02T14:23:53.808Z
Learning: In the Forest project, .rpcsnap.json.zst snapshot files listed in src/tool/subcommands/api_cmd/test_snapshots.txt are not stored in the repository itself but are downloaded from a DigitalOcean (DO) bucket when needed. The manifest file serves as an index/catalog of available snapshots.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5867
File: src/ipld/util.rs:553-558
Timestamp: 2025-08-08T12:10:45.218Z
Learning: Forest project targets Rust stable >=1.89; features stabilized in 1.88 like let-chains are acceptable in this codebase.
Learnt from: elmattic
Repo: ChainSafe/forest PR: 5836
File: src/tool/subcommands/api_cmd/api_run_tests.rs:186-190
Timestamp: 2025-08-11T12:42:42.848Z
Learning: In the Forest project's test infrastructure (specifically in `src/tool/subcommands/api_cmd/api_run_tests.rs`), the team prefers to use only `ws` (non-secure WebSocket) connections for simplicity in their stateful API test scenarios, as security is not a concern in their testing context.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5835
File: src/chain/tests.rs:58-76
Timestamp: 2025-08-04T13:36:22.993Z
Learning: In the Forest codebase, `Vec<u8>` can be used as an `AsyncWrite` implementation in test contexts. The user confirmed that tests using `&mut Vec<u8>` with `export` and `export_v2` functions compile and pass both locally and on CI.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5930
File: build.rs:64-77
Timestamp: 2025-08-13T09:43:20.301Z
Learning: hanabi1224 prefers hard compile-time errors in build scripts rather than runtime safeguards or collision detection, believing it's better to fail fast and fix root causes of issues like malformed snapshot names.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 6057
File: src/cli/subcommands/f3_cmd.rs:0-0
Timestamp: 2025-09-09T10:37:17.947Z
Learning: hanabi1224 prefers having default timeouts (like 10m for --no-progress-timeout) to prevent commands from hanging indefinitely, even when the timeout flag isn't explicitly provided by users. This fail-fast approach is preferred over requiring explicit flag usage.
📚 Learning: 2025-08-08T12:10:45.218Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5867
File: src/ipld/util.rs:553-558
Timestamp: 2025-08-08T12:10:45.218Z
Learning: Forest project targets Rust stable >=1.89; features stabilized in 1.88 like let-chains are acceptable in this codebase.
Applied to files:
src/bin/forest-dev.rssrc/bin/forest-wallet.rs
📚 Learning: 2025-08-07T13:39:15.107Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5886
File: interop-tests/src/tests/go_app/gen.go:29-29
Timestamp: 2025-08-07T13:39:15.107Z
Learning: Auto-generated files like those created by rust2go (indicated by "Generated by rust2go. Please DO NOT edit this C part manually." comment) in the Forest project should be skipped during code review as they are not intended for manual editing.
Applied to files:
src/bin/forest-dev.rs
📚 Learning: 2025-08-07T13:39:29.732Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5886
File: interop-tests/src/tests/go_app/gen.go:58-58
Timestamp: 2025-08-07T13:39:29.732Z
Learning: In the Forest project, auto-generated files (like those generated by rust2go tool) should be skipped during code review as they are not meant to be manually edited.
Applied to files:
src/bin/forest-dev.rs
📚 Learning: 2025-10-17T09:36:15.757Z
Learnt from: elmattic
Repo: ChainSafe/forest PR: 6128
File: src/ipld/util.rs:23-30
Timestamp: 2025-10-17T09:36:15.757Z
Learning: Always run `cargo check` or `cargo build` to verify actual compilation errors in the Forest codebase before flagging them as issues. Do not rely solely on documentation or assumptions about trait implementations.
Applied to files:
src/bin/forest-dev.rs
📚 Learning: 2025-08-08T12:10:45.218Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5867
File: src/ipld/util.rs:553-558
Timestamp: 2025-08-08T12:10:45.218Z
Learning: Forest pins Rust toolchain to 1.89.0 via rust-toolchain.toml; features stabilized in 1.88 (e.g., let-chains) are acceptable in this codebase.
Applied to files:
src/bin/forest-dev.rs
📚 Learning: 2025-08-28T12:52:46.927Z
Learnt from: LesnyRumcajs
Repo: ChainSafe/forest PR: 6011
File: src/cli/main.rs:18-25
Timestamp: 2025-08-28T12:52:46.927Z
Learning: In Forest CLI (src/cli/main.rs), the early RPC network check before Cli::parse_from() does not block help/version commands because clap processes these internally before reaching the RPC call. LesnyRumcajs confirmed this implementation works correctly and that RPC call failures are acceptable in this context.
Applied to files:
src/bin/forest-dev.rssrc/wallet/main.rssrc/tool/main.rssrc/dev/subcommands/mod.rs
📚 Learning: 2025-08-11T12:42:42.848Z
Learnt from: elmattic
Repo: ChainSafe/forest PR: 5836
File: src/tool/subcommands/api_cmd/api_run_tests.rs:186-190
Timestamp: 2025-08-11T12:42:42.848Z
Learning: In the Forest project's test infrastructure (specifically in `src/tool/subcommands/api_cmd/api_run_tests.rs`), the team prefers to use only `ws` (non-secure WebSocket) connections for simplicity in their stateful API test scenarios, as security is not a concern in their testing context.
Applied to files:
src/bin/forest-dev.rssrc/dev/subcommands/mod.rs
📚 Learning: 2025-08-04T13:36:22.993Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5835
File: src/chain/tests.rs:58-76
Timestamp: 2025-08-04T13:36:22.993Z
Learning: In the Forest codebase, `Vec<u8>` can be used as an `AsyncWrite` implementation in test contexts. The user confirmed that tests using `&mut Vec<u8>` with `export` and `export_v2` functions compile and pass both locally and on CI.
Applied to files:
src/bin/forest-wallet.rs
📚 Learning: 2025-08-25T14:17:09.129Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: .github/workflows/unit-tests.yml:0-0
Timestamp: 2025-08-25T14:17:09.129Z
Learning: hanabi1224's download_file_with_cache function in src/utils/net/download_file.rs preserves URL path structure in local cache directories by using cache_dir.join(url.path().strip_prefix('/').unwrap_or_else(|| url.path())), so snapshots from https://forest-snapshots.fra1.cdn.digitaloceanspaces.com/rpc_test/ are cached locally at ~/.cache/forest/test/rpc-snapshots/rpc_test/ (including the rpc_test subdirectory from the URL path).
Applied to files:
src/dev/subcommands/mod.rs
📚 Learning: 2025-09-11T16:03:14.328Z
Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 6000
File: src/tool/subcommands/api_cmd/state_decode_params_tests/miner.rs:4-9
Timestamp: 2025-09-11T16:03:14.328Z
Learning: In the Forest codebase's state_decode_params_tests module, common imports like `to_vec`, `TokenAmount`, `Cid`, etc. are centralized in the mod.rs file and made available to submodules through `use super::*;`, eliminating the need for duplicate imports in individual test files.
Applied to files:
src/dev/subcommands/mod.rs
📚 Learning: 2025-09-02T14:23:53.808Z
Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 5923
File: src/tool/subcommands/api_cmd/test_snapshots.txt:206-252
Timestamp: 2025-09-02T14:23:53.808Z
Learning: In the Forest project, .rpcsnap.json.zst snapshot files listed in src/tool/subcommands/api_cmd/test_snapshots.txt are not stored in the repository itself but are downloaded from a DigitalOcean (DO) bucket when needed. The manifest file serves as an index/catalog of available snapshots.
Applied to files:
src/dev/subcommands/mod.rs
📚 Learning: 2025-08-11T14:00:47.060Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5886
File: src/daemon/db_util.rs:236-259
Timestamp: 2025-08-11T14:00:47.060Z
Learning: In Forest's snapshot import (`src/daemon/db_util.rs`), when F3 CID is present in snapshot metadata but the actual F3 data is missing, this should cause a hard error as it indicates snapshot corruption. Only errors during the F3 import process itself (after data is successfully retrieved) should be non-fatal and logged.
Applied to files:
src/dev/subcommands/mod.rs
📚 Learning: 2025-08-25T14:17:09.129Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: .github/workflows/unit-tests.yml:0-0
Timestamp: 2025-08-25T14:17:09.129Z
Learning: hanabi1224's download_file_with_cache function preserves URL path structure in local cache directories, so snapshots from https://forest-snapshots.fra1.cdn.digitaloceanspaces.com/rpc_test/ are cached locally at ~/.cache/forest/test/rpc-snapshots/rpc_test (including the rpc_test subdirectory from the URL path).
Applied to files:
src/dev/subcommands/mod.rs
📚 Learning: 2025-08-25T09:53:37.443Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: .github/workflows/unit-tests.yml:72-76
Timestamp: 2025-08-25T09:53:37.443Z
Learning: hanabi1224 intentionally designs RPC snapshot test cache to be OS/arch-agnostic, sharing cached snapshot files across different platforms using enableCrossOsArchive: true and hash-based cache keys to improve cache efficiency and reduce redundant downloads.
Applied to files:
src/dev/subcommands/mod.rs
📚 Learning: 2025-08-25T14:11:10.790Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: .github/workflows/unit-tests.yml:86-96
Timestamp: 2025-08-25T14:11:10.790Z
Learning: hanabi1224's RPC snapshot tests include a checksum validation mechanism that compares against HTTP HEAD headers, automatically re-downloading snapshots if the cached version doesn't match, providing protection against corrupted cached files.
Applied to files:
src/dev/subcommands/mod.rs
📚 Learning: 2025-08-25T09:57:27.412Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: .github/workflows/unit-tests.yml:72-76
Timestamp: 2025-08-25T09:57:27.412Z
Learning: hanabi1224's strategy for RPC snapshot cache versioning: branch isolation is not needed currently, but if snapshot generation logic changes in the future, a version string can be added to the cache key to handle the evolution.
Applied to files:
src/dev/subcommands/mod.rs
🧬 Code graph analysis (5)
src/bin/forest-dev.rs (5)
src/bin/forest-tool.rs (1)
main(5-7)src/bin/forest-wallet.rs (1)
main(5-7)src/dev/main.rs (1)
main(9-18)src/tool/main.rs (1)
main(11-37)src/wallet/main.rs (1)
main(13-34)
src/bin/forest-wallet.rs (3)
src/bin/forest-dev.rs (1)
main(5-7)src/bin/forest-tool.rs (1)
main(5-7)src/bin/forest-cli.rs (1)
main(5-7)
src/wallet/main.rs (3)
src/bin/forest-wallet.rs (1)
main(5-7)src/cli/main.rs (1)
main(14-50)src/shim/address.rs (1)
set_global(66-69)
src/tool/main.rs (3)
src/dev/main.rs (1)
main(9-18)src/cli/main.rs (1)
main(14-50)src/daemon/main.rs (1)
main(26-71)
src/dev/subcommands/mod.rs (4)
src/utils/net/download_file.rs (1)
download_file_with_cache(30-89)src/utils/proofs_api/paramfetch.rs (1)
ensure_proof_params_downloaded(59-66)src/utils/proofs_api/parameters.rs (1)
maybe_set_proofs_parameter_cache_dir_env(114-118)src/utils/mod.rs (2)
retry(116-145)is_ci(171-174)
⏰ 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: cargo-publish-dry-run
- GitHub Check: Build Ubuntu
- GitHub Check: Build MacOS
- GitHub Check: Build forest binaries on Linux AMD64
- GitHub Check: Coverage
- GitHub Check: tests-release
- GitHub Check: tests
- GitHub Check: All lint checks
🔇 Additional comments (4)
src/bin/forest-dev.rs (1)
4-7: Tokio async entrypoint looks consistent with the rest of the repo.src/bin/forest-wallet.rs (1)
4-7: Async#[tokio::main]wrapper is consistent and minimal.src/wallet/main.rs (1)
13-34: Async conversion is clean; please sanity-check RPC requirement for all wallet commands.
SinceStateNetworkName::callis unconditional (for any non-help parse), just confirm that’s intended for every wallet subcommand (vs lazily only when needed).src/utils/mod.rs (1)
116-145:retry()refactor is much simpler; please verify timeout semantics match callers’ expectations.
tokio::time::timeout(timeout, task)times out the whole retry loop (including delays), not a single attempt—worth double-checking any callsites that expect “per try” timeouts.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Makefile(1 hunks)src/tool/subcommands/api_cmd/test_snapshot.rs(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/tool/subcommands/api_cmd/test_snapshot.rs
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: LesnyRumcajs
Repo: ChainSafe/forest PR: 6011
File: src/cli/main.rs:18-25
Timestamp: 2025-08-28T12:52:46.927Z
Learning: In Forest CLI (src/cli/main.rs), the early RPC network check before Cli::parse_from() does not block help/version commands because clap processes these internally before reaching the RPC call. LesnyRumcajs confirmed this implementation works correctly and that RPC call failures are acceptable in this context.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: .github/workflows/unit-tests.yml:0-0
Timestamp: 2025-08-25T14:17:09.129Z
Learning: hanabi1224's download_file_with_cache function in src/utils/net/download_file.rs preserves URL path structure in local cache directories by using cache_dir.join(url.path().strip_prefix('/').unwrap_or_else(|| url.path())), so snapshots from https://forest-snapshots.fra1.cdn.digitaloceanspaces.com/rpc_test/ are cached locally at ~/.cache/forest/test/rpc-snapshots/rpc_test/ (including the rpc_test subdirectory from the URL path).
Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 5923
File: src/tool/subcommands/api_cmd/test_snapshots.txt:206-252
Timestamp: 2025-09-02T14:23:53.808Z
Learning: In the Forest project, .rpcsnap.json.zst snapshot files listed in src/tool/subcommands/api_cmd/test_snapshots.txt are not stored in the repository itself but are downloaded from a DigitalOcean (DO) bucket when needed. The manifest file serves as an index/catalog of available snapshots.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5867
File: src/ipld/util.rs:553-558
Timestamp: 2025-08-08T12:10:45.218Z
Learning: Forest project targets Rust stable >=1.89; features stabilized in 1.88 like let-chains are acceptable in this codebase.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: .github/workflows/unit-tests.yml:72-76
Timestamp: 2025-08-25T09:53:37.443Z
Learning: hanabi1224 intentionally designs RPC snapshot test cache to be OS/arch-agnostic, sharing cached snapshot files across different platforms using enableCrossOsArchive: true and hash-based cache keys to improve cache efficiency and reduce redundant downloads.
Learnt from: elmattic
Repo: ChainSafe/forest PR: 5836
File: src/tool/subcommands/api_cmd/api_run_tests.rs:186-190
Timestamp: 2025-08-11T12:42:42.848Z
Learning: In the Forest project's test infrastructure (specifically in `src/tool/subcommands/api_cmd/api_run_tests.rs`), the team prefers to use only `ws` (non-secure WebSocket) connections for simplicity in their stateful API test scenarios, as security is not a concern in their testing context.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5930
File: build.rs:64-77
Timestamp: 2025-08-13T09:43:20.301Z
Learning: hanabi1224 prefers hard compile-time errors in build scripts rather than runtime safeguards or collision detection, believing it's better to fail fast and fix root causes of issues like malformed snapshot names.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 6057
File: src/cli/subcommands/f3_cmd.rs:0-0
Timestamp: 2025-09-09T10:37:17.947Z
Learning: hanabi1224 prefers having default timeouts (like 10m for --no-progress-timeout) to prevent commands from hanging indefinitely, even when the timeout flag isn't explicitly provided by users. This fail-fast approach is preferred over requiring explicit flag usage.
📚 Learning: 2025-08-07T13:39:15.107Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5886
File: interop-tests/src/tests/go_app/gen.go:29-29
Timestamp: 2025-08-07T13:39:15.107Z
Learning: Auto-generated files like those created by rust2go (indicated by "Generated by rust2go. Please DO NOT edit this C part manually." comment) in the Forest project should be skipped during code review as they are not intended for manual editing.
Applied to files:
Makefile
📚 Learning: 2025-10-17T09:36:15.757Z
Learnt from: elmattic
Repo: ChainSafe/forest PR: 6128
File: src/ipld/util.rs:23-30
Timestamp: 2025-10-17T09:36:15.757Z
Learning: Always run `cargo check` or `cargo build` to verify actual compilation errors in the Forest codebase before flagging them as issues. Do not rely solely on documentation or assumptions about trait implementations.
Applied to files:
Makefile
📚 Learning: 2025-08-07T13:39:29.732Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5886
File: interop-tests/src/tests/go_app/gen.go:58-58
Timestamp: 2025-08-07T13:39:29.732Z
Learning: In the Forest project, auto-generated files (like those generated by rust2go tool) should be skipped during code review as they are not meant to be manually edited.
Applied to files:
Makefile
📚 Learning: 2025-08-11T12:42:42.848Z
Learnt from: elmattic
Repo: ChainSafe/forest PR: 5836
File: src/tool/subcommands/api_cmd/api_run_tests.rs:186-190
Timestamp: 2025-08-11T12:42:42.848Z
Learning: In the Forest project's test infrastructure (specifically in `src/tool/subcommands/api_cmd/api_run_tests.rs`), the team prefers to use only `ws` (non-secure WebSocket) connections for simplicity in their stateful API test scenarios, as security is not a concern in their testing context.
Applied to files:
Makefile
📚 Learning: 2025-09-02T14:23:53.808Z
Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 5923
File: src/tool/subcommands/api_cmd/test_snapshots.txt:206-252
Timestamp: 2025-09-02T14:23:53.808Z
Learning: In the Forest project, .rpcsnap.json.zst snapshot files listed in src/tool/subcommands/api_cmd/test_snapshots.txt are not stored in the repository itself but are downloaded from a DigitalOcean (DO) bucket when needed. The manifest file serves as an index/catalog of available snapshots.
Applied to files:
Makefile
📚 Learning: 2025-08-08T12:10:45.218Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5867
File: src/ipld/util.rs:553-558
Timestamp: 2025-08-08T12:10:45.218Z
Learning: Forest project targets Rust stable >=1.89; features stabilized in 1.88 like let-chains are acceptable in this codebase.
Applied to files:
Makefile
📚 Learning: 2025-08-25T14:17:09.129Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: .github/workflows/unit-tests.yml:0-0
Timestamp: 2025-08-25T14:17:09.129Z
Learning: hanabi1224's download_file_with_cache function in src/utils/net/download_file.rs preserves URL path structure in local cache directories by using cache_dir.join(url.path().strip_prefix('/').unwrap_or_else(|| url.path())), so snapshots from https://forest-snapshots.fra1.cdn.digitaloceanspaces.com/rpc_test/ are cached locally at ~/.cache/forest/test/rpc-snapshots/rpc_test/ (including the rpc_test subdirectory from the URL path).
Applied to files:
Makefile
⏰ 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). (7)
- GitHub Check: tests-release
- GitHub Check: tests
- GitHub Check: Coverage
- GitHub Check: Build forest binaries on Linux AMD64
- GitHub Check: Build Ubuntu
- GitHub Check: cargo-publish-dry-run
- GitHub Check: All lint checks
Comment on lines
101
to
105
| test: | ||
| # Skip RPC snapshot tests for debug build | ||
| export FOREST_RPC_SNAPSHOT_TEST_OPT_OUT=1 | ||
| cargo nextest run --workspace --no-fail-fast | ||
|
|
Contributor
There was a problem hiding this comment.
export ... won’t affect the next recipe line (each line is a separate shell)
Line 103 exports FOREST_RPC_SNAPSHOT_TEST_OPT_OUT in a different shell than Line 104, so cargo nextest run ... likely won’t see it and the opt-out won’t apply.
Suggested fix (attach env var to the command(s) that need it):
test:
# Skip RPC snapshot tests for debug build
- export FOREST_RPC_SNAPSHOT_TEST_OPT_OUT=1
- cargo nextest run --workspace --no-fail-fast
+ FOREST_RPC_SNAPSHOT_TEST_OPT_OUT=1 cargo nextest run --workspace --no-fail-fast
# nextest doesn't run doctests https://github.com/nextest-rs/nextest/issues/16
# see also lib.rs::doctest_private
- cargo test --doc --features doctest-private
+ FOREST_RPC_SNAPSHOT_TEST_OPT_OUT=1 cargo test --doc --features doctest-privateIf you only want the opt-out for nextest (not doctests), drop the second env prefix.
🤖 Prompt for AI Agents
In Makefile lines 101-105 the `export FOREST_RPC_SNAPSHOT_TEST_OPT_OUT=1` is in
a separate shell from the `cargo nextest run` line, so the env var won't be
visible; change the recipe to attach the env var to the command (e.g. prefix the
cargo invocation with `FOREST_RPC_SNAPSHOT_TEST_OPT_OUT=1 cargo nextest run
...`) or combine into one shell with `export ... && cargo ...`; if the opt-out
should only apply to nextest and not doctests, only prefix the `cargo nextest
run` command.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Makefile (1)
101-108: Environment variable export won't be visible to cargo commands (unresolved from prior review).In Makefiles, each recipe line runs in a separate shell instance. The
exporton line 103 has no effect on thecargo nextest runon line 104 (or thecargo test --docon line 108).To apply the env var to the commands that need it, prefix the command itself:
test: # Skip RPC snapshot tests for debug build - export FOREST_RPC_SNAPSHOT_TEST_OPT_OUT=1 - cargo nextest run --workspace --no-fail-fast + FOREST_RPC_SNAPSHOT_TEST_OPT_OUT=1 cargo nextest run --workspace --no-fail-fast # nextest doesn't run doctests https://github.com/nextest-rs/nextest/issues/16 # see also lib.rs::doctest_private - cargo test --doc --features doctest-private + FOREST_RPC_SNAPSHOT_TEST_OPT_OUT=1 cargo test --doc --features doctest-private
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/coverage.yml(1 hunks)Makefile(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/coverage.yml
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: LesnyRumcajs
Repo: ChainSafe/forest PR: 6011
File: src/cli/main.rs:18-25
Timestamp: 2025-08-28T12:52:46.927Z
Learning: In Forest CLI (src/cli/main.rs), the early RPC network check before Cli::parse_from() does not block help/version commands because clap processes these internally before reaching the RPC call. LesnyRumcajs confirmed this implementation works correctly and that RPC call failures are acceptable in this context.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: .github/workflows/unit-tests.yml:0-0
Timestamp: 2025-08-25T14:17:09.129Z
Learning: hanabi1224's download_file_with_cache function in src/utils/net/download_file.rs preserves URL path structure in local cache directories by using cache_dir.join(url.path().strip_prefix('/').unwrap_or_else(|| url.path())), so snapshots from https://forest-snapshots.fra1.cdn.digitaloceanspaces.com/rpc_test/ are cached locally at ~/.cache/forest/test/rpc-snapshots/rpc_test/ (including the rpc_test subdirectory from the URL path).
Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 5923
File: src/tool/subcommands/api_cmd/test_snapshots.txt:206-252
Timestamp: 2025-09-02T14:23:53.808Z
Learning: In the Forest project, .rpcsnap.json.zst snapshot files listed in src/tool/subcommands/api_cmd/test_snapshots.txt are not stored in the repository itself but are downloaded from a DigitalOcean (DO) bucket when needed. The manifest file serves as an index/catalog of available snapshots.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5867
File: src/ipld/util.rs:553-558
Timestamp: 2025-08-08T12:10:45.218Z
Learning: Forest project targets Rust stable >=1.89; features stabilized in 1.88 like let-chains are acceptable in this codebase.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: .github/workflows/unit-tests.yml:72-76
Timestamp: 2025-08-25T09:53:37.443Z
Learning: hanabi1224 intentionally designs RPC snapshot test cache to be OS/arch-agnostic, sharing cached snapshot files across different platforms using enableCrossOsArchive: true and hash-based cache keys to improve cache efficiency and reduce redundant downloads.
Learnt from: elmattic
Repo: ChainSafe/forest PR: 5836
File: src/tool/subcommands/api_cmd/api_run_tests.rs:186-190
Timestamp: 2025-08-11T12:42:42.848Z
Learning: In the Forest project's test infrastructure (specifically in `src/tool/subcommands/api_cmd/api_run_tests.rs`), the team prefers to use only `ws` (non-secure WebSocket) connections for simplicity in their stateful API test scenarios, as security is not a concern in their testing context.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: .github/workflows/unit-tests.yml:0-0
Timestamp: 2025-08-25T14:17:09.129Z
Learning: hanabi1224's download_file_with_cache function preserves URL path structure in local cache directories, so snapshots from https://forest-snapshots.fra1.cdn.digitaloceanspaces.com/rpc_test/ are cached locally at ~/.cache/forest/test/rpc-snapshots/rpc_test (including the rpc_test subdirectory from the URL path).
Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 6000
File: src/tool/subcommands/api_cmd/state_decode_params_tests/miner.rs:4-9
Timestamp: 2025-09-11T16:03:14.328Z
Learning: In the Forest codebase's state_decode_params_tests module, common imports like `to_vec`, `TokenAmount`, `Cid`, etc. are centralized in the mod.rs file and made available to submodules through `use super::*;`, eliminating the need for duplicate imports in individual test files.
Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 6000
File: src/tool/subcommands/api_cmd/state_decode_params_tests/miner.rs:4-9
Timestamp: 2025-09-11T16:03:14.328Z
Learning: In the Forest codebase's state_decode_params_tests module, common imports like `to_vec`, `TokenAmount`, `Cid`, etc. are centralized in the mod.rs file and made available to submodules through `use super::*;`, eliminating the need for duplicate imports in individual test files.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: Makefile:0-0
Timestamp: 2025-08-25T08:06:18.865Z
Learning: hanabi1224 intentionally uses multi-threading (instead of single-threaded execution) in RPC snapshot tests to speed up file downloading operations, prioritizing performance over potential test isolation benefits.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5930
File: build.rs:64-77
Timestamp: 2025-08-13T09:43:20.301Z
Learning: hanabi1224 prefers hard compile-time errors in build scripts rather than runtime safeguards or collision detection, believing it's better to fail fast and fix root causes of issues like malformed snapshot names.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 6057
File: src/cli/subcommands/f3_cmd.rs:0-0
Timestamp: 2025-09-09T10:37:17.947Z
Learning: hanabi1224 prefers having default timeouts (like 10m for --no-progress-timeout) to prevent commands from hanging indefinitely, even when the timeout flag isn't explicitly provided by users. This fail-fast approach is preferred over requiring explicit flag usage.
📚 Learning: 2025-08-07T13:39:15.107Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5886
File: interop-tests/src/tests/go_app/gen.go:29-29
Timestamp: 2025-08-07T13:39:15.107Z
Learning: Auto-generated files like those created by rust2go (indicated by "Generated by rust2go. Please DO NOT edit this C part manually." comment) in the Forest project should be skipped during code review as they are not intended for manual editing.
Applied to files:
Makefile
📚 Learning: 2025-08-07T13:39:29.732Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5886
File: interop-tests/src/tests/go_app/gen.go:58-58
Timestamp: 2025-08-07T13:39:29.732Z
Learning: In the Forest project, auto-generated files (like those generated by rust2go tool) should be skipped during code review as they are not meant to be manually edited.
Applied to files:
Makefile
📚 Learning: 2025-10-17T09:36:15.757Z
Learnt from: elmattic
Repo: ChainSafe/forest PR: 6128
File: src/ipld/util.rs:23-30
Timestamp: 2025-10-17T09:36:15.757Z
Learning: Always run `cargo check` or `cargo build` to verify actual compilation errors in the Forest codebase before flagging them as issues. Do not rely solely on documentation or assumptions about trait implementations.
Applied to files:
Makefile
📚 Learning: 2025-09-16T12:55:26.955Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 6079
File: src/chain_sync/metrics.rs:6-7
Timestamp: 2025-09-16T12:55:26.955Z
Learning: HEAD_EPOCH references in shell scripts (like scripts/tests/calibnet_eth_mapping_check.sh) that extract data from `forest-cli info show` are unrelated to Rust metrics constants with the same name and should not be flagged when metrics cleanup is performed.
Applied to files:
Makefile
📚 Learning: 2025-10-17T14:24:47.046Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 6167
File: src/tool/subcommands/state_compute_cmd.rs:89-91
Timestamp: 2025-10-17T14:24:47.046Z
Learning: In `src/tool/subcommands/state_compute_cmd.rs`, when using `ReadOpsTrackingStore` to generate minimal snapshots, `HEAD_KEY` should be written to `db.tracker` (not `db` itself) before calling `export_forest_car()`, because the export reads from the tracker MemoryDB which accumulates only the accessed data during computation.
Applied to files:
Makefile
📚 Learning: 2025-08-11T12:42:42.848Z
Learnt from: elmattic
Repo: ChainSafe/forest PR: 5836
File: src/tool/subcommands/api_cmd/api_run_tests.rs:186-190
Timestamp: 2025-08-11T12:42:42.848Z
Learning: In the Forest project's test infrastructure (specifically in `src/tool/subcommands/api_cmd/api_run_tests.rs`), the team prefers to use only `ws` (non-secure WebSocket) connections for simplicity in their stateful API test scenarios, as security is not a concern in their testing context.
Applied to files:
Makefile
📚 Learning: 2025-09-02T14:23:53.808Z
Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 5923
File: src/tool/subcommands/api_cmd/test_snapshots.txt:206-252
Timestamp: 2025-09-02T14:23:53.808Z
Learning: In the Forest project, .rpcsnap.json.zst snapshot files listed in src/tool/subcommands/api_cmd/test_snapshots.txt are not stored in the repository itself but are downloaded from a DigitalOcean (DO) bucket when needed. The manifest file serves as an index/catalog of available snapshots.
Applied to files:
Makefile
📚 Learning: 2025-08-08T12:10:45.218Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5867
File: src/ipld/util.rs:553-558
Timestamp: 2025-08-08T12:10:45.218Z
Learning: Forest project targets Rust stable >=1.89; features stabilized in 1.88 like let-chains are acceptable in this codebase.
Applied to files:
Makefile
📚 Learning: 2025-08-25T14:17:09.129Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: .github/workflows/unit-tests.yml:0-0
Timestamp: 2025-08-25T14:17:09.129Z
Learning: hanabi1224's download_file_with_cache function in src/utils/net/download_file.rs preserves URL path structure in local cache directories by using cache_dir.join(url.path().strip_prefix('/').unwrap_or_else(|| url.path())), so snapshots from https://forest-snapshots.fra1.cdn.digitaloceanspaces.com/rpc_test/ are cached locally at ~/.cache/forest/test/rpc-snapshots/rpc_test/ (including the rpc_test subdirectory from the URL path).
Applied to files:
Makefile
⏰ 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). (9)
- GitHub Check: Build Ubuntu
- GitHub Check: Build MacOS
- GitHub Check: cargo-publish-dry-run
- GitHub Check: Coverage
- GitHub Check: Build forest binaries on Linux AMD64
- GitHub Check: tests-release
- GitHub Check: tests
- GitHub Check: All lint checks
- GitHub Check: Go lint checks
Makefile
Outdated
Comment on lines
115
to
114
| codecov: | ||
| cargo llvm-cov --workspace --profile quick --codecov --output-path lcov.info test |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
cargo llvm-cov correct syntax test subcommand documentation
💡 Result:
Correct syntax (from the official cargo-llvm-cov docs):
Usage:
cargo llvm-cov test [OPTIONS] [-- ...]
Meaning:
- "test" is the subcommand that runs tests with coverage instrumentation.
- Any options for cargo-llvm-cov go before the "--".
- Anything after "--" is passed as arguments to the test binary (same as cargo test).
Common options (examples):
--lcov export coverage in lcov format.
--json export coverage in JSON.
--output-path write report to file (otherwise printed to stdout).
--open open HTML report (available for report subcommand; see docs).
(See full option list in the docs.)
Examples:
cargo llvm-cov test
cargo llvm-cov test -- --nocapture
cargo llvm-cov test --workspace
source <(cargo llvm-cov show-env --export-prefix); cargo llvm-cov clean --workspace; cargo build; cargo llvm-cov report --lcov
Sources: cargo-llvm-cov documentation and README. (docs.rs)
Citations:
Move test subcommand to correct position and replace --codecov with --lcov.
The test subcommand must come immediately after cargo llvm-cov, not at the end. Additionally, the correct flag for LCOV format output is --lcov, not --codecov. The command should be:
cargo llvm-cov test --workspace --profile quick --lcov --output-path lcov.info
🤖 Prompt for AI Agents
In Makefile around lines 115 to 116, the `cargo llvm-cov` invocation places the
`test` subcommand at the end and uses the wrong flag `--codecov`; move the
`test` subcommand immediately after `cargo llvm-cov` and replace `--codecov`
with `--lcov` so the command reads with `test` in the correct position and uses
the LCOV flag and the same `--output-path lcov.info`.
7df13a0 to
9ff8027
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/coverage.yml (1)
65-69: Upgrade Codecov action to v5 and handle token for fork PRs.
codecov/codecov-action@v3is stale; v5 is the current major version. More critically, onpull_requestevents from forks, GitHub withholdssecrets.CODECOV_TOKEN, causing upload failures. Combined withfail_ci_if_error: true, this hard-fails CI even when coverage generation succeeds.Consider upgrading to
@v5and switching to OIDC authentication (use_oidc: truewithid-token: writepermission), which authenticates via GitHub's built-in OIDC token instead of a repo secret. Alternatively, usepull_request_targetorworkflow_runto access secrets in a trusted context, or enable tokenless uploads for public repositories.
🧹 Nitpick comments (2)
.github/workflows/coverage.yml (2)
52-54: Good: prefetching RPC snapshots; consider adding cache restore/save to avoid re-downloading each run.Line 52 makes coverage runs more deterministic, but you’ll likely want the same snapshot cache pattern used elsewhere (restore before, save after) for
~/.cache/forest/test/rpc-snapshots/rpc_testto reduce runtime and external dependency.If you want, I can sketch the exact YAML block mirroring
unit-tests.yml’sactions/cacheusage. (Based on learnings about the project’s snapshot cache behavior.)
50-55: Verify tool installation matchesmake codecov(avoid redundant installs).Since Line 54 switched from direct
cargo llvm-cov … nextestusage tomake codecov, please verify whethertaiki-e/install-action@cargo-llvm-cov(Line 50) and/or@nextest(Line 51) are still required. If not, dropping them will speed up CI and reduce failure surface.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/coverage.yml(1 hunks).github/workflows/unit-tests.yml(1 hunks)Makefile(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- Makefile
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: .github/workflows/unit-tests.yml:0-0
Timestamp: 2025-08-25T14:17:09.129Z
Learning: hanabi1224's download_file_with_cache function in src/utils/net/download_file.rs preserves URL path structure in local cache directories by using cache_dir.join(url.path().strip_prefix('/').unwrap_or_else(|| url.path())), so snapshots from https://forest-snapshots.fra1.cdn.digitaloceanspaces.com/rpc_test/ are cached locally at ~/.cache/forest/test/rpc-snapshots/rpc_test/ (including the rpc_test subdirectory from the URL path).
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: .github/workflows/unit-tests.yml:72-76
Timestamp: 2025-08-25T09:53:37.443Z
Learning: hanabi1224 intentionally designs RPC snapshot test cache to be OS/arch-agnostic, sharing cached snapshot files across different platforms using enableCrossOsArchive: true and hash-based cache keys to improve cache efficiency and reduce redundant downloads.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: .github/workflows/unit-tests.yml:86-96
Timestamp: 2025-08-25T14:11:10.790Z
Learning: hanabi1224's RPC snapshot tests include a checksum validation mechanism that compares against HTTP HEAD headers, automatically re-downloading snapshots if the cached version doesn't match, providing protection against corrupted cached files.
Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 5923
File: src/tool/subcommands/api_cmd/test_snapshots.txt:206-252
Timestamp: 2025-09-02T14:23:53.808Z
Learning: In the Forest project, .rpcsnap.json.zst snapshot files listed in src/tool/subcommands/api_cmd/test_snapshots.txt are not stored in the repository itself but are downloaded from a DigitalOcean (DO) bucket when needed. The manifest file serves as an index/catalog of available snapshots.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: Makefile:0-0
Timestamp: 2025-08-25T08:06:18.865Z
Learning: hanabi1224 intentionally uses multi-threading (instead of single-threaded execution) in RPC snapshot tests to speed up file downloading operations, prioritizing performance over potential test isolation benefits.
Learnt from: LesnyRumcajs
Repo: ChainSafe/forest PR: 6011
File: src/cli/main.rs:18-25
Timestamp: 2025-08-28T12:52:46.927Z
Learning: In Forest CLI (src/cli/main.rs), the early RPC network check before Cli::parse_from() does not block help/version commands because clap processes these internally before reaching the RPC call. LesnyRumcajs confirmed this implementation works correctly and that RPC call failures are acceptable in this context.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5930
File: build.rs:64-77
Timestamp: 2025-08-13T09:43:20.301Z
Learning: hanabi1224 prefers hard compile-time errors in build scripts rather than runtime safeguards or collision detection, believing it's better to fail fast and fix root causes of issues like malformed snapshot names.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 6057
File: src/cli/subcommands/f3_cmd.rs:0-0
Timestamp: 2025-09-09T10:37:17.947Z
Learning: hanabi1224 prefers having default timeouts (like 10m for --no-progress-timeout) to prevent commands from hanging indefinitely, even when the timeout flag isn't explicitly provided by users. This fail-fast approach is preferred over requiring explicit flag usage.
📚 Learning: 2025-09-02T14:23:53.808Z
Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 5923
File: src/tool/subcommands/api_cmd/test_snapshots.txt:206-252
Timestamp: 2025-09-02T14:23:53.808Z
Learning: In the Forest project, .rpcsnap.json.zst snapshot files listed in src/tool/subcommands/api_cmd/test_snapshots.txt are not stored in the repository itself but are downloaded from a DigitalOcean (DO) bucket when needed. The manifest file serves as an index/catalog of available snapshots.
Applied to files:
.github/workflows/unit-tests.yml
📚 Learning: 2025-08-07T13:39:15.107Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5886
File: interop-tests/src/tests/go_app/gen.go:29-29
Timestamp: 2025-08-07T13:39:15.107Z
Learning: Auto-generated files like those created by rust2go (indicated by "Generated by rust2go. Please DO NOT edit this C part manually." comment) in the Forest project should be skipped during code review as they are not intended for manual editing.
Applied to files:
.github/workflows/coverage.yml
📚 Learning: 2025-10-17T09:36:15.757Z
Learnt from: elmattic
Repo: ChainSafe/forest PR: 6128
File: src/ipld/util.rs:23-30
Timestamp: 2025-10-17T09:36:15.757Z
Learning: Always run `cargo check` or `cargo build` to verify actual compilation errors in the Forest codebase before flagging them as issues. Do not rely solely on documentation or assumptions about trait implementations.
Applied to files:
.github/workflows/coverage.yml
⏰ 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: cargo-publish-dry-run
- GitHub Check: Build MacOS
- GitHub Check: Build Ubuntu
- GitHub Check: Coverage
- GitHub Check: Build forest binaries on Linux AMD64
- GitHub Check: tests-release
- GitHub Check: tests
- GitHub Check: All lint checks
Comment on lines
54
to
+57
| - run: make test | ||
| env: | ||
| # Skip RPC snapshot tests for debug build | ||
| FOREST_RPC_SNAPSHOT_TEST_OPT_OUT: 1 |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for FOREST_RPC_SNAPSHOT_TEST_OPT_OUT usage in the codebase
rg "FOREST_RPC_SNAPSHOT_TEST_OPT_OUT" -A 3 -B 3Repository: ChainSafe/forest
Length of output: 601
🏁 Script executed:
# Check the full unit-tests.yml workflow file to understand context
cat -n .github/workflows/unit-tests.ymlRepository: ChainSafe/forest
Length of output: 4070
🏁 Script executed:
# Look for references to snapshot tests to understand the intent
rg "snapshot" .github/workflows/ -i -A 2 -B 2Repository: ChainSafe/forest
Length of output: 5360
🏁 Script executed:
# Check git log to see if there's context about this change
git log --oneline -20 -- .github/workflows/unit-tests.yml 2>/dev/null | head -20Repository: ChainSafe/forest
Length of output: 75
🏁 Script executed:
# Check if there are recent PR files or commit messages that show the PR intent
git log --all --oneline -30 --grep="snap\|snapshot" 2>/dev/null | head -20Repository: ChainSafe/forest
Length of output: 1474
🏁 Script executed:
# Check the current state of the repository and find any PR context
git log --oneline -1Repository: ChainSafe/forest
Length of output: 75
🏁 Script executed:
# Search for codecov references to understand if that job exists
rg "codecov\|coverage" .github/workflows/ -i -B 2 -A 2Repository: ChainSafe/forest
Length of output: 42
🏁 Script executed:
# Search for any coverage or codecov configuration elsewhere
fd "codecov\|coverage" .github/ -type f 2>/dev/nullRepository: ChainSafe/forest
Length of output: 42
🏁 Script executed:
# Check if there's a codecov configuration file
ls -la | grep -i codecovRepository: ChainSafe/forest
Length of output: 42
🏁 Script executed:
# Look at the git diff or show what was changed
git show 9ff8027 --stat 2>/dev/null | head -30Repository: ChainSafe/forest
Length of output: 1707
This change opts out of snapshot tests in the debug CI, contradicting the PR objective.
Line 57 sets FOREST_RPC_SNAPSHOT_TEST_OPT_OUT: 1 while the PR is titled "run snap tests on CI for debug and codecov builds". When FOREST_RPC_SNAPSHOT_TEST_OPT_OUT is truthy, the snapshot test early-returns and skips execution (see src/tool/subcommands/api_cmd/test_snapshot.rs). This does the opposite of the stated goal.
Pick one:
- If debug CI should run snapshot tests: remove the opt-out (and consider adding the same snapshot prefetch/cache restore as in the release job to reduce network flakiness).
- - run: make test
- env:
- # Skip RPC snapshot tests for debug build
- FOREST_RPC_SNAPSHOT_TEST_OPT_OUT: 1
+ - run: make test- If debug CI should not run snapshot tests: rename/reword the PR title and description to match the behavior (and add a rationale).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| - run: make test | |
| env: | |
| # Skip RPC snapshot tests for debug build | |
| FOREST_RPC_SNAPSHOT_TEST_OPT_OUT: 1 | |
| - run: make test |
🤖 Prompt for AI Agents
.github/workflows/unit-tests.yml around lines 54 to 57: the workflow sets
FOREST_RPC_SNAPSHOT_TEST_OPT_OUT: 1 which causes snapshot tests to be skipped
and contradicts the PR goal of running snapshot tests on debug CI; to fix,
either remove that environment variable line (so debug CI runs snapshot tests)
and add the same snapshot prefetch/cache restore steps used in the release job
to reduce network flakiness, or if you intend to keep skipping snapshots, update
the PR title and description to state that debug CI will opt out and include a
brief rationale for the choice.
5c28eb8 to
8cd3ede
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
Makefile (1)
113-115:cargo llvm-covinvocation likely needs thetestsubcommand / correct output flag
This matches an already-raised concern (subcommand placement +--codecovvs--lcov). Please align the command with thecargo-llvm-covversion used in this repo/CI.#!/bin/bash set -euo pipefail # Inspect the installed cargo-llvm-cov help in CI/dev to confirm the correct syntax/flags. cargo llvm-cov --help | sed -n '1,120p' cargo llvm-cov test --help | sed -n '1,160p'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/coverage.yml(2 hunks).github/workflows/unit-tests.yml(1 hunks)Makefile(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/unit-tests.yml
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: .github/workflows/unit-tests.yml:0-0
Timestamp: 2025-08-25T14:17:09.129Z
Learning: hanabi1224's download_file_with_cache function in src/utils/net/download_file.rs preserves URL path structure in local cache directories by using cache_dir.join(url.path().strip_prefix('/').unwrap_or_else(|| url.path())), so snapshots from https://forest-snapshots.fra1.cdn.digitaloceanspaces.com/rpc_test/ are cached locally at ~/.cache/forest/test/rpc-snapshots/rpc_test/ (including the rpc_test subdirectory from the URL path).
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: .github/workflows/unit-tests.yml:72-76
Timestamp: 2025-08-25T09:53:37.443Z
Learning: hanabi1224 intentionally designs RPC snapshot test cache to be OS/arch-agnostic, sharing cached snapshot files across different platforms using enableCrossOsArchive: true and hash-based cache keys to improve cache efficiency and reduce redundant downloads.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: .github/workflows/unit-tests.yml:86-96
Timestamp: 2025-08-25T14:11:10.790Z
Learning: hanabi1224's RPC snapshot tests include a checksum validation mechanism that compares against HTTP HEAD headers, automatically re-downloading snapshots if the cached version doesn't match, providing protection against corrupted cached files.
Learnt from: LesnyRumcajs
Repo: ChainSafe/forest PR: 6011
File: src/cli/main.rs:18-25
Timestamp: 2025-08-28T12:52:46.927Z
Learning: In Forest CLI (src/cli/main.rs), the early RPC network check before Cli::parse_from() does not block help/version commands because clap processes these internally before reaching the RPC call. LesnyRumcajs confirmed this implementation works correctly and that RPC call failures are acceptable in this context.
Learnt from: akaladarshi
Repo: ChainSafe/forest PR: 5923
File: src/tool/subcommands/api_cmd/test_snapshots.txt:206-252
Timestamp: 2025-09-02T14:23:53.808Z
Learning: In the Forest project, .rpcsnap.json.zst snapshot files listed in src/tool/subcommands/api_cmd/test_snapshots.txt are not stored in the repository itself but are downloaded from a DigitalOcean (DO) bucket when needed. The manifest file serves as an index/catalog of available snapshots.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5978
File: Makefile:0-0
Timestamp: 2025-08-25T08:06:18.865Z
Learning: hanabi1224 intentionally uses multi-threading (instead of single-threaded execution) in RPC snapshot tests to speed up file downloading operations, prioritizing performance over potential test isolation benefits.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5930
File: build.rs:64-77
Timestamp: 2025-08-13T09:43:20.301Z
Learning: hanabi1224 prefers hard compile-time errors in build scripts rather than runtime safeguards or collision detection, believing it's better to fail fast and fix root causes of issues like malformed snapshot names.
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 6057
File: src/cli/subcommands/f3_cmd.rs:0-0
Timestamp: 2025-09-09T10:37:17.947Z
Learning: hanabi1224 prefers having default timeouts (like 10m for --no-progress-timeout) to prevent commands from hanging indefinitely, even when the timeout flag isn't explicitly provided by users. This fail-fast approach is preferred over requiring explicit flag usage.
📚 Learning: 2025-10-17T09:36:15.757Z
Learnt from: elmattic
Repo: ChainSafe/forest PR: 6128
File: src/ipld/util.rs:23-30
Timestamp: 2025-10-17T09:36:15.757Z
Learning: Always run `cargo check` or `cargo build` to verify actual compilation errors in the Forest codebase before flagging them as issues. Do not rely solely on documentation or assumptions about trait implementations.
Applied to files:
Makefile.github/workflows/coverage.yml
📚 Learning: 2025-09-16T12:55:26.955Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 6079
File: src/chain_sync/metrics.rs:6-7
Timestamp: 2025-09-16T12:55:26.955Z
Learning: HEAD_EPOCH references in shell scripts (like scripts/tests/calibnet_eth_mapping_check.sh) that extract data from `forest-cli info show` are unrelated to Rust metrics constants with the same name and should not be flagged when metrics cleanup is performed.
Applied to files:
Makefile
📚 Learning: 2025-10-17T14:24:47.046Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 6167
File: src/tool/subcommands/state_compute_cmd.rs:89-91
Timestamp: 2025-10-17T14:24:47.046Z
Learning: In `src/tool/subcommands/state_compute_cmd.rs`, when using `ReadOpsTrackingStore` to generate minimal snapshots, `HEAD_KEY` should be written to `db.tracker` (not `db` itself) before calling `export_forest_car()`, because the export reads from the tracker MemoryDB which accumulates only the accessed data during computation.
Applied to files:
Makefile
📚 Learning: 2025-08-07T13:39:15.107Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5886
File: interop-tests/src/tests/go_app/gen.go:29-29
Timestamp: 2025-08-07T13:39:15.107Z
Learning: Auto-generated files like those created by rust2go (indicated by "Generated by rust2go. Please DO NOT edit this C part manually." comment) in the Forest project should be skipped during code review as they are not intended for manual editing.
Applied to files:
.github/workflows/coverage.yml
📚 Learning: 2025-08-07T13:39:29.732Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5886
File: interop-tests/src/tests/go_app/gen.go:58-58
Timestamp: 2025-08-07T13:39:29.732Z
Learning: In the Forest project, auto-generated files (like those generated by rust2go tool) should be skipped during code review as they are not meant to be manually edited.
Applied to files:
.github/workflows/coverage.yml
📚 Learning: 2025-08-08T12:10:45.218Z
Learnt from: hanabi1224
Repo: ChainSafe/forest PR: 5867
File: src/ipld/util.rs:553-558
Timestamp: 2025-08-08T12:10:45.218Z
Learning: Forest pins Rust toolchain to 1.89.0 via rust-toolchain.toml; features stabilized in 1.88 (e.g., let-chains) are acceptable in this codebase.
Applied to files:
.github/workflows/coverage.yml
🪛 GitHub Actions: Script linters
.github/workflows/coverage.yml
[warning] 1-1: Code style issues found in .github/workflows/coverage.yml. Run 'Prettier --write' to fix.
[error] 1-1: Prettier formatting check failed for .github/workflows/coverage.yml. Run 'Prettier --write' to fix code style issues.
⏰ 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: cargo-publish-dry-run
- GitHub Check: Build Ubuntu
- GitHub Check: Build MacOS
- GitHub Check: Coverage
- GitHub Check: All lint checks
- GitHub Check: tests-release
- GitHub Check: tests
- GitHub Check: Build forest binaries on Linux AMD64
🔇 Additional comments (1)
.github/workflows/coverage.yml (1)
57-58: Coverage flow looks coherent (fetch RPC tests →make codecov)
Assuming theMakefilecodecovtarget is correct for yourcargo-llvm-covversion, this orchestration is a nice simplification.
hanabi1224
force-pushed
the
hm/ci-snap-tests
branch
3 times, most recently
from
d3c6b1f to
74efa8f
Compare
hanabi1224
force-pushed
the
hm/ci-snap-tests
branch
from
74efa8f to
5e01e0e
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files
... and 127 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Cargo.toml
Outdated
| [profile.codecov] | ||
| inherits = "test" | ||
| opt-level = 1 | ||
| lto = "off" |
Member
There was a problem hiding this comment.
any reason to disable LTO?
Contributor
Author
There was a problem hiding this comment.
This profile inherits test where lto is false. I can just remove this line.
Member
There was a problem hiding this comment.
It's orthogonal to the codecov issue, no? If so, let's have a separate PR for this change.
LesnyRumcajs
requested changes
Dec 12, 2025
Member
LesnyRumcajs
left a comment
LesnyRumcajs
left a comment
There was a problem hiding this comment.
There's lots of spam/debug logs in the fetch command. Perhaps you need to setup the logger?
Contributor
Author
@LesnyRumcajs I think it's OK to keep them for troubleshooting as this is a dev tool |
Contributor
Author
|
Superseded by #6340 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary of changes
RPC module coverage:
PR: 55.27% : https://app.codecov.io/github/ChainSafe/forest/commit/5e01e0e55f539816f31e9e65613949bf6edad336/tree/src?dropdown=coverage
main: 31% : https://app.codecov.io/github/ChainSafe/forest/tree/main/src
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes
Other information and links
Change checklist
Summary by CodeRabbit
New Features
Chores
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.