Repository navigation
fix: preserve flame graphs when export fails - #412
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Unix exports reset report permissions, potentially making reports unreadable to other users or services.
Review effort: Balanced
Findings: 1
What changed in this PR
Protects existing flame graph reports from failed exports by following the snapshot export’s temporary-file pattern.
Changes:
- Render, flush, and sync a temporary file before atomically replacing the output.
- Add regression tests for preservation, cleanup, and successful replacement.
| File | Description |
|---|---|
| tests/stack_commands.rs | Tests failed exports and successful report replacement. |
| src/main.rs | Stages flame graph output before installation. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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
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.
Running
dua flamegraph --output usage.svg empty-directoryreturnsNo stack counts found, but currently overwrites an existingusage.svgbefore reporting that error. This change renders to a temporary file in the destination directory, flushes and syncs it, and atomically replaces the output only after rendering succeeds, following the existing snapshot-export pattern.Regression tests cover preserving a previously generated report on failure, leaving no partial report or temporary files when the output does not exist, and successfully replacing an existing report. Both failure regressions were confirmed against the original implementation before applying the fix. No dependencies were added.
Validation on Linux with Rust 1.98.0:
make check, using--locked.--no-default-features --features trash-move.cargo fmt --all -- --checkandcargo clippy --locked -- -D warningspassed.AI assistance: Codex. Model attribution requested by the author: GPT-6 Astra (exact model identifier unverified).