[jayzed80]

Fixes #3975

Root cause

crates/engine/src/io.rs read_plain (the "Text named .xlsx is read as text" branch, ~line 302): bytes that sniff as text but carry an Excel package name skip the XLSX reader and fall to the CSV arm (_ =>, ~line 349), which returned no warning. So echo kein zip > broken.xlsx opened as a one-cell workbook with nothing said, and gridcraft-cli info/convert exited 0 silently.

Fix

In that CSV arm, when the name is an Excel package name (FileKind::Xlsx: xlsx/xlsm/xltx/xltm), the import still happens but a first warning is added: "'' isn't a valid .xlsx package (it is not a ZIP file), so it was read as plain text." The CLI already prints import warnings to stderr; the app shows them in its open warnings. Real CSV/TSV/TXT files are unchanged (no warning). I chose a warning rather than an error, which the issue allows ("or at least imported with a warning"), to keep existing opening behaviour; an error is a one-line follow-up if maintainers prefer it.

Tests

New io::tests::text_named_xlsx_opens_with_a_warning_but_real_csv_does_not (crates/engine/src/io/tests.rs).

• Without the fix: panicked at crates\engine\src\io\tests.rs:890:5: [] (no warnings), 1 failed.
• With the fix: test io::tests::text_named_xlsx_opens_with_a_warning_but_real_csv_does_not ... ok.

Checks run

• cargo fmt -p gridcraft-engine -- --check: clean
• cargo clippy -p gridcraft-engine --all-targets -j 4 -- -D warnings: clean
• cargo test -p gridcraft-engine -j 4: 959 + 120 + 7 passed, 0 failed

Not run

cargo xtask ci (layers, wasm, assets), the CLI binary by hand, and the UI. The message is an engine string like its neighbours in io.rs (those are not routed through a translation macro).

Summary by CodeRabbit

• Bug Fixes
  • When a text file with an .xlsx extension is opened as plain text, a warning now clarifies that it isn’t a valid XLSX file. CSV files retain their existing warning behavior.

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

The CSV fallback now prepends a warning when it reads non-ZIP text named with an .xlsx extension. A test checks this warning and confirms that CSV input named with a .csv extension has no warning.

Changes: XLSX text warning

Layer / File(s)	Summary
CSV fallback warning crates/engine/src/io.rs, crates/engine/src/io/tests.rs	The fallback prepends a warning when the filename resolves to FileKind::Xlsx. The test checks that non-ZIP text produces a warning and that CSV input does not.

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

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: dylanmtaylor, echelon

Merge Risk: Minimal · up to 21339

Text-like files named .xlsx, .xlsm, .xltx or .xltm now open as plain text with a visible warning instead of silently, which addresses the linked issue. Real CSV files behave as before. The change is small and reversible, and no merge-blocking risk was found.

Pre-merge checks: 4

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title names the main change: a warning is now emitted when a .xlsx file is not a ZIP package and is read as text. The raw summary confirms the io.rs change prepends that warning for text under an …
Linked Issues check		Issue #3975 allows either rejection with an error or import with a warning for a .xlsx/.xlsm file that is not a ZIP package. The PR takes the warning option. In crates/engine/src/io.rs (`read_pl…
Out of Scope Changes check		The whole-PR diff against the merge base changes only crates/engine/src/io.rs (+7/-1, the warning in the CSV arm of read_plain) and adds one test in crates/engine/src/io/tests.rs. Both changes a…

Full details: Title check

Explanation

The title names the main change: a warning is now emitted when a .xlsx file is not a ZIP package and is read as text. The raw summary confirms the io.rs change prepends that warning for text under an .xlsx name, and the added test checks it. The PR objectives describe the same behavior, so the title matches the changeset. The issue number in the title is a reference, not a problem.

Full details: Linked Issues check

Explanation

Issue #3975 allows either rejection with an error or import with a warning for a .xlsx/.xlsm file that is not a ZIP package. The PR takes the warning option. In crates/engine/src/io.rs (read_plain, CSV/text arm), when kind == Some(FileKind::Xlsx), the diff inserts the warning "'<name>' isn't a valid .xlsx package (it is not a ZIP file), so it was read as plain text." at the front of the import warnings. The file still imports as plain text, as the issue permits for explicit reads. Real CSV/TSV/TXT names get no new warning. The added test io::tests::text_named_xlsx_opens_with_a_warning_but_real_csv_does_not covers the engine-level behavior. The PR reports that this test failed before the change and passed after. Warnings reach stderr for the CLI info and convert commands through existing plumbing. I did not confirm that path by running the CLI. The tests.rs diff could not be read in this review, so the test content rests on the PR summary and the AI-generated summary.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff against the merge base changes only crates/engine/src/io.rs (+7/-1, the warning in the CSV arm of read_plain) and adds one test in crates/engine/src/io/tests.rs. Both changes address the issue's requirement that a misnamed Excel package produce a warning. The inline comment ties the change to #3975. No unrelated code is changed. The test file diff could not be read directly, so its scope is inferred from the PR summary and AI summary, which describe a single test case.

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

Recent review info

Run configuration

• Source mode: Auto
• Repository: storytold/gridcraft
• Branch: main
• File: .coderabbit.yaml
• Commit: c0ee120
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: cb3dcaf9-d881-460b-95e3-7f67e311319b

Commits

Reviewing files that changed from the base of the PR and between c0ee120 and 21339e4.

Files selected for processing: 2

• crates/engine/src/io.rs
• crates/engine/src/io/tests.rs

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