[yongyin-leon]

Summary

project.saveAs changes the project's in-memory name before attempting its atomic write. Because it is a view command, a failed write does not automatically roll back that name, leaving the project renamed even though it was not saved.

Retain the previous name and restore it when the existing write path returns an error. Only the name string is retained; there is no project/dataset clone or new save API. Successful saves still use the original write path, append the normal extension when needed, clear dirty/edit counts, update the saved path and preserve undo/redo history.

Add one integration regression covering saved/unsaved and clean/dirty projects, missing-parent and occupied-directory failures, project contents/path/edit counts/history preservation, actual undo/redo after failure, and a successful retry reopened from disk. This PR changes only crates/engine/src/cmd/project.rs and the new crates/engine/tests/project_save.rs, with no dependency, manifest, configuration or workflow changes.

Testing

Validation below is reused from the unchanged source snapshot committed as 1d872c2; submission did not rerun builds or tests.

• Windows/MSVC: cargo test --locked --offline -j 2 -p rustgis-engine --test project_save passed 1 integration test, covering the state/failure/retry combinations above.
• macOS ARM64 and Linux ARM64, Rust 1.95.0, ordinary users: the same integration test passed 1 test per platform, with exact nonzero test counts and zero ignored tests.
• The Unix runs restored the upstream io.rs before testing this patch, confirming that it is independent of the staging-collision fix.
• Windows: cargo clippy --locked --offline -j 2 -p rustgis-engine --lib --test project_save -- -D warnings passed. Formatting and diff checks passed.
• Source digests were checked across the validated snapshots and final working tree.

Full-workspace CI and Linux x86_64 runtime tests were not rerun locally; new PR checks are separate evidence. Disk exhaustion and power-loss behavior were not tested.

Review follow-up (c9e7d91)

Added Rustdoc for Save As and its regression, and clarified why this non-undoable command restores the name explicitly. This update changes comments only; the implementation and test assertions are unchanged. Prior Windows/macOS ARM64/Linux ARM64 regression results and the successful CI, Python and docs runs at 1d872c2 remain applicable. Only formatting and diff checks were repeated; no build or test rerun was requested.

Related issue

None.

Summary by CodeRabbit

• Bug Fixes
  • Saving a project under a new name now updates the project name to match the destination filename after a successful save.
  • If saving fails, the existing project name and session state are preserved, including the project path, unsaved changes, and undo/redo history. This helps ensure a failed save does not alter the current project or overwrite a previously saved project.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 950a57e5-e787-4b89-8a9f-621ee88d703d

📥 Commits

Reviewing files that changed from the base of the PR and between 1d872c2 and c9e7d91.

📒 Files selected for processing: 2

• crates/engine/src/cmd/project.rs
• crates/engine/tests/project_save.rs

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 1 remain after this review.

📝 Walkthrough

save_as sets the project name from the destination filename stem before writing. If writing fails, it restores the previous name. An integration test checks session state after failed and successful saves.

Changes: saveAs project state

Layer / File(s)	Summary
Set and restore project name crates/engine/src/cmd/project.rs, crates/engine/tests/project_save.rs	save_as sets the project name from the destination filename stem and restores the previous name if writing fails. The integration test checks state preservation after failures and behavior after a successful save.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: giswqs

Merge Risk: ⚪ Minimal · up to c9e7d

The Save As rollback change is narrowly implemented and covered by the added integration test; no merge-blocking risk remains.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly and concisely describes the primary change: restoring the project name when Save As fails.
Description check		The description includes all required sections. It explains the change, documents testing and limitations, and identifies the related issue as none.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{A filename lends a name to save,
A failed write leaves the old one safe.
The tests check paths and history,
Then save again successfully.
A rabbit thumps beside the page.}

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Code review

PR #703 makes project.saveAs restore the old project name when the write fails. It changes crates/engine/src/cmd/project.rs and adds crates/engine/tests/project_save.rs. I have no Bash tool, so I couldn't build the code or run the tests. I read the diff and write() in project.rs.

Bugs

• None found. write() only changes dirty, edit_counts and path after project.save succeeds, so restoring project.name on error fully undoes the failed Save As. (high confidence)

Quality

• crates/engine/src/cmd/project.rs:142 – The one-line map(|stem| std::mem::replace(...)) hides a side effect inside a combinator and is long for the surrounding style. An if let that saves the old name and assigns the new one would be easier to read. Nit; medium confidence.
• crates/engine/src/cmd/project.rs:13 – The comment "Save As is a view command: a failed write does not get an automatic project rollback" is unclear. Something like "Session only rolls back on undoable commands; restore the name by hand when the write fails" would say why the manual restore is needed. Nit; low confidence.
• save_as still doesn't trim, cap at 200 characters, or skip empty stems the way name_from_file (project.rs:126) does. This was already the case before the PR, so it's out of scope. Low confidence.
• crates/engine/tests/project_save.rs – The test is thorough: it covers saved and unsaved projects, dirty and clean state, undo/redo history, edit_counts, and the destination contents. It is one long loop. If it fails, it won't be clear which combination broke, so assert! messages or splitting it into helpers would help. It also builds a temp dir by hand and calls remove_dir_all only on the success path, so a failed assertion leaves the directory behind. Nit; medium confidence.

AGENTS.md

• No unwrap, expect, panic! or unsafe in the non-test change. The test uses ? and map_err, which fits the repo style. (high confidence)
• The PR adds a regression test for the fix, as the guidelines ask. It does not add the failing-save case to hostile_params_never_panic, but that test is about panics and is not required here. (low confidence)
• I could not confirm that cargo xtask ci passes. The long lines in project.rs and the test may need cargo fmt if the repo's rustfmt config wraps them. (low confidence)

Security

• No issues. The test writes only under std::env::temp_dir() in a uniquely named directory.

Performance

• No issues.

Change size

Compared: the whole PR against main (merge base 8b4f78a to PR head 1d872c2)

Metric	Value
Files changed	2 (1 added, 1 modified, 0 removed, 0 renamed)
Size before	13.9 KB
Size after	17.5 KB
Total size change	+3.6 KB

File	Before	After	Change
crates/engine/tests/project_save.rs	0 B	3.4 KB	+3.4 KB
crates/engine/src/cmd/project.rs	13.9 KB	14.2 KB	+245 B

[github-actions]

📖 Docs preview

Item	Value
Preview	https://preview.rustgis.app/pr-703/
Web app	https://preview.rustgis.app/pr-703/app/
Commit	c9e7d91

GitHub Pages takes a minute or two to publish a new commit.

[github-actions]

Nit (low confidence, pre-existing behaviour): name_from_file just below trims the stem, skips empty stems and caps the name at 200 chars. This inline replace does none of that, so Save As and project.open/openText can name the same file differently. You could take the previous name first (let previous_name = s.project.name.clone();), call name_from_file(s, &path), and restore on error. That would also drop the Option plumbing. It's optional and would change Save As naming slightly, so it may be better as a follow-up.

[github-actions]

Code review

The fix is correct and small. save_as now remembers the old project name and puts it back if write returns an error. On success nothing changes, because write still clears the dirty flag and edit counts, sets the path, and leaves undo history alone. I found no bugs, security issues or performance problems. I read the diff and the surrounding write, save and name_from_file code. I did not build or run the tests.

Bugs

• None found. write only changes dirty, edit_counts and path after project.save succeeds. Restoring the name alone is therefore enough to undo a failed Save As.

Security

• None found. No new dependencies, and the new test only writes inside a temp directory it creates and removes.

Performance

• None found. The only extra cost is one retained String.

Quality

• Name handling differs from name_from_file (low confidence, optional). Save As takes the raw file stem. name_from_file, used by open, trims it, skips empty stems and caps it at 200 characters. This was already the case before the PR. Reusing the helper would remove the Option handling, but it would change Save As naming slightly. It is posted as an inline comment on crates/engine/src/cmd/project.rs:143.
• Test cleanup (low confidence). crates/engine/tests/project_save.rs removes its temp directory only on success. A failed assertion or an early ? leaves it behind under temp_dir(). A small drop guard would fix that. The directory name already includes the process id and a timestamp, so there is no collision risk.
• Test coverage is thorough. It covers saved and unsaved projects, clean and dirty state, a missing parent directory, a destination that is a directory, undo/redo history after failure, and a retry that succeeds. I have no concerns there.

AGENTS.md

• No unwrap, expect or panic in non-test code, and the new test returns Result rather than unwrapping.
• No new dependencies and no UI strings, so the licence, notices and translation rules don't apply.
• The PR description says cargo xtask ci and the full-workspace CI were not rerun for this head. Please check that CI is green before merging.

Change size

Compared: the whole PR against main (merge base 8b4f78a to PR head c9e7d91)

Metric	Value
Files changed	2 (1 added, 1 modified, 0 removed, 0 renamed)
Size before	13.9 KB
Size after	17.7 KB
Total size change	+3.8 KB

File	Before	After	Change
crates/engine/tests/project_save.rs	0 B	3.4 KB	+3.4 KB
crates/engine/src/cmd/project.rs	13.9 KB	14.3 KB	+332 B