Skip to content

fix: loadout name now comes exclusively from the file name stem - #1242

Merged
evanspearman merged 1 commit into
gominimal:mainfrom
evanspearman:loadoutname
Aug 18, 2026
Merged

fix: loadout name now comes exclusively from the file name stem#1242
evanspearman merged 1 commit into
gominimal:mainfrom
evanspearman:loadoutname

Conversation

@evanspearman

@evanspearman evanspearman commented Aug 18, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Member

fixes https://github.com/gominimal/inbox/issues/476

Summary

A loadout is now identified solely by its filename. The name field inside the file is accepted, warned about, and discarded — matching the filename or not, loading continues either way, so existing loadouts keep working. Previously a disagreement was a hard LoadError::NameMismatch.

Internals: LoadoutFile holds a file's contents unnamed; into_loadout(stem) pairs it with the filename, so Loadout still carries a non-optional name and no caller changed. Docs and fixtures drop name = ....

Testing

cargo test --workspace green (sessions 404 → 407 tests), just clippy and cargo fmt --check clean. Manually verified via min loadout list: both warnings fire, and a file declaring something-else lists as its filename.

Checklist

  • Docs updated if behavior changed
  • BREAKING CHANGE: footer present if this is a breaking change

Summary by CodeRabbit

  • New Features

    • Loadout names are now determined by their TOML filenames.
    • The in-file name field is optional and no longer overrides the filename.
    • Legacy or mismatched names are accepted with warnings.

  • Bug Fixes

    • Improved validation and error reporting for malformed loadout files.
    • Loadout listings now handle legacy name discrepancies without failing.

  • Documentation

    • Updated loadout examples and schema references to reflect filename-based naming.

@evanspearman
evanspearman requested a review from a team as a code owner August 18, 2026 16:16
@coderabbitai

coderabbitai Bot commented Aug 18, 2026 •
edited
Loading

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 61c40b1c-7e5e-4141-a603-e4902a7a373b

📥 Commits

Reviewing files that changed from the base of the PR and between 71c0999 and 9bf5c56.

📒 Files selected for processing (1)
  • docs/concepts/loadouts.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • docs/concepts/loadouts.md

Included review availability: 4 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour.

📝 Walkthrough

Walkthrough

Loadout identity now comes from the TOML filename. The internal name field is optional legacy metadata. Mismatches produce warnings instead of load failures. The loadout model, disk loader, tests, fixtures, and documentation reflect this behavior.

Changes

Loadout identity migration

Layer / File(s) Summary
Separate loadout identity from file contents
crates/sessions/src/core/loadout.rs		 LoadoutFile stores parsed content and optional legacy names. Loadout stores the resolved identity and updates builders, accessors, composition, and serialization.
Resolve names during disk loading
crates/sessions/src/client/disk.rs, crates/minimal/src/loadouts.rs, crates/minimal/tests/bug.rs		 Disk loading uses filename stems, warns for redundant or mismatched names, and accepts nameless files. Tests cover parsing, listing, and malformed schemas.
Update documented and generated loadouts
docs/concepts/loadouts.md, docs/reference/loadouts.md, scripts/session-e2e.sh		 Documentation and fixtures remove redundant name fields and describe filename-based identity.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: ⚪ Minimal · up to 9bf5c

The change makes loadout names derive from filename stems while preserving compatibility with existing files; no actionable merge-blocking risk remains after normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant Disk
  participant LoadoutFile
  participant Loadout
  Disk->>LoadoutFile: parse TOML content
  Disk->>LoadoutFile: inspect optional declared name
  Disk->>Loadout: provide filename stem
  LoadoutFile->>Loadout: create resolved loadout
  Loadout-->>Disk: return loadout or parse error
Loading

Possibly related PRs

Poem

A rabbit reads the filename bright,
And leaves old names as warning light.
TOML may skip the name today,
The file stem leads the way.
Hop, hop—tests and docs align!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely identifies the primary change: loadout names now come from filename stems.
Description check ✅ Passed The description includes the required Summary, Testing, and Checklist sections with relevant details and completed checklist items.
Linked Issues check ✅ Passed The changes satisfy issue #476 by making the filename authoritative and treating the internal name as optional metadata.
Out of Scope Changes check ✅ Passed The code, tests, documentation, and fixtures all directly support the filename-based loadout identity change.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@evanspearman
evanspearman merged commit be87789 into gominimal:main Aug 18, 2026
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@twitchyliquid64 twitchyliquid64 twitchyliquid64 approved these changes
@norrietaylor norrietaylor norrietaylor approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@evanspearman @twitchyliquid64 @norrietaylor