Skip to content

fix(store): preserve extensionless burnpack paths - #5494

Merged
laggui merged 1 commit into
tracel-ai:mainfrom
rioyu123:codex/fix-burnpack-exact-path
Aug 27, 2026
Merged

laggui merged 1 commit into
tracel-ai:mainfrom
rioyu123:codex/fix-burnpack-exact-path

Conversation

@rioyu123

Copy link
Copy Markdown
Contributor

Checklist

  • Confirmed that cargo run-checks has been executed.
  • Made sure the book is up to date with changes in this PR (no book changes required).

Related Issues/PRs

Fixes #5482.

Changes

BurnpackStore resolved its configured path first, but burn-pack applied its own extension fallback again. As a result, .auto_extension(false) still wrote to model.bpk, the overwrite guard checked a different path from the writer, and an exact load could fall back to the sibling .bpk file.

  • Preserve the existing default extension behavior.
  • Allow Writer to keep an extensionless path for both direct and atomic writes.
  • Add an exact-path Reader entry point so higher-level callers do not reapply fallback behavior.
  • Make BurnpackStore apply its extension policy exactly once for save, overwrite checks, and load.
  • Use the canonical extension constant and include the requested path in open errors.
  • Add regressions for default behavior, exact direct/atomic writes, exact loads, overwrite protection, and end-to-end store round trips.

Testing

  • cargo run-checks (Rust 1.96.0)
  • cargo test -p burn-pack -p burn-store
  • cargo check -p burn-store --no-default-features
  • cargo clippy -p burn-pack -p burn-store --all-targets -- -D warnings
  • cargo fmt --all -- --check

@rioyu123
rioyu123 force-pushed the codex/fix-burnpack-exact-path branch from 1d7ef24 to 09a0dd0 Compare August 26, 2026 16:58
@antimora
antimora requested a balanced review from Copilot August 27, 2026 13:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes extensionless Burnpack paths by applying extension policy exactly once across store and pack APIs.

Changes:

  • Adds exact-path reader and configurable writer extension behavior.
  • Aligns BurnpackStore save, load, and overwrite paths.
  • Adds regression coverage for direct, atomic, and store operations.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

Show a summary per file
File Description
crates/burn-store/tests/burnpack_store.rs Tests extensionless store behavior.
crates/burn-store/src/burnpack.rs Applies store path policy once.
crates/burn-pack/tests/round_trip.rs Tests exact direct writes and reads.
crates/burn-pack/tests/lazy_writer.rs Tests exact atomic writes.
crates/burn-pack/src/writer.rs Adds configurable auto-extension.
crates/burn-pack/src/reader.rs Adds exact-path loading.
crates/burn-pack/src/lib.rs Documents the new file API.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@laggui laggui left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@laggui
laggui merged commit 84a88b5 into tracel-ai:main Aug 27, 2026
14 of 15 checks passed
@codecov

codecov Bot commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.72%. Comparing base (6687586) to head (09a0dd0).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5494      +/-   ##
==========================================
+ Coverage   67.68%   67.72%   +0.04%     
==========================================
  Files        1257     1257              
  Lines      194831   194919      +88     
==========================================
+ Hits       131863   132012     +149     
+ Misses      62968    62907      -61     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

burn-store: BurnpackStore::auto_extension(false) is ignored on save

3 participants