fix: save snapshot on mismatch in forest-dev state compute#7123
Conversation
Walkthrough
ChangesSnapshot export write relocation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Comment |
hanabi1224
requested review from
LesnyRumcajs and
sudo-shashank
and removed request for
a team
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/dev/subcommands/state_cmd.rs (1)
122-124: ⚡ Quick winAdd error context to snapshot file writes.
On Line 123,
std::fs::write(...)should include.with_context(...)so failures include the output path.Suggested change
if let Some(export_db_to) = export_db_to { - std::fs::write(export_db_to, db_snapshot)?; + std::fs::write(&export_db_to, db_snapshot).with_context(|| { + format!("failed to write db snapshot CAR to {}", export_db_to.display()) + })?; }As per coding guidelines: “Use
anyhow::Result<T>for most operations and add context with.context()when errors occur”.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/dev/subcommands/state_cmd.rs` around lines 122 - 124, The file writes DB snapshots using std::fs::write(export_db_to, db_snapshot)? without context; change this to propagate errors with context by calling .with_context(|| format!("Failed to write DB snapshot to {}", export_db_to)) (from anyhow::Context) on the write Result before the ? so failures include the output path; ensure the module imports anyhow::Context if not already and reference the export_db_to and db_snapshot variables and the std::fs::write call when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/dev/subcommands/state_cmd.rs`:
- Around line 122-124: The file writes DB snapshots using
std::fs::write(export_db_to, db_snapshot)? without context; change this to
propagate errors with context by calling .with_context(|| format!("Failed to
write DB snapshot to {}", export_db_to)) (from anyhow::Context) on the write
Result before the ? so failures include the output path; ensure the module
imports anyhow::Context if not already and reference the export_db_to and
db_snapshot variables and the std::fs::write call when making the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: bb04e694-8845-4c43-9dd2-0d0e78b1ea94
📒 Files selected for processing (1)
src/dev/subcommands/state_cmd.rs
Codecov Report❌ Patch coverage is
Additional details and impacted files
... and 13 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
2 participants
Summary of changes
Changes introduced in this pull request:
Reference issue to close (if applicable)
Closes
Other information and links
Change checklist
Outside contributions
Summary by CodeRabbit