feat(nam): cab detection, model metadata, and folder UX by OpenSauce · Pull Request #286 · OpenSauce/rustortion

OpenSauce · 2026-07-27T19:22:54Z

Three UX gaps around NAM models, plus what fell out of investigating them.

Does a model include a cab?

nam-rs did not expose this. Its Metadata typed only the three calibration
numbers, exactly matching what NAMCore's C++ ModelMetadata parses; gear_type
was reachable only as a raw JSON blob. nam-rs 0.4.0 adds
Metadata::includes_cab(), and this branch consumes it.

gear_type stays a String rather than an enum because there is no single
vocabulary: NAM's trainer and TONE3000 use overlapping-but-different sets, and
TONE3000 has already deprecated values (full-rig, ir) that exist in files on
disk. An enum over either set would reject real files. Cross-referencing the
TONE3000 API also confirmed full-rig is documented as an alias for amp-cab,
so that classification is theirs rather than our inference.

Unrecognised values return "unknown" and change nothing — NAM's undocumented
studio and TONE3000's experimental included. A wrong answer here routes
audio through one cab too many, or none at all.

The `amp` problem

98% of models in circulation say gear_type: "amp". A field that never varies is
a default, not a choice — and Boosted-6505+-A2-Chugs-FullRig.nam proves it:
gear_type: "amp" with T3K-Null in every field the uploader could have filled,
and the truth recorded only in the name.

So a model's name can override gear_type, but narrowly: only when gear_type
is absent or every descriptive field is a placeholder, only ever upgrading to
"has a cab", and matching only "full rig" and a standalone cab/cabs/cabinet
token. Measured over 233 real models it reclassifies exactly the two 6505 copies
and nothing else. Inferred conclusions are marked (from name) in the UI so a
guess never reads as a fact.

What the card shows

The NAM stage reports the cab/IR pairing at all times, colouring the two
combinations that are actually wrong:

model	IR	line	colour
cab	bypassed	`Cab included · IR bypassed`	subtle
cab	active	`Cab included · IR also active: two cabs in series`	warning
no cab	active	`No cab · IR active`	subtle
no cab	bypassed	`No cab · IR recommended`	warning
unknown	either	`IR active` / `IR bypassed`	subtle

A button toggles the IR inline. Nothing is changed automatically. An earlier
revision auto-bypassed the IR, and that was backed out: set_ir_bypass reaches
the host through raw_begin_set_parameter/raw_end_set_parameter, which is
exactly the gesture a DAW reads as "the user is touching this control" — so
selecting a model would have written automation on IR Bypass that the user never
performed, and fought any automation already there. Every parameter write on this
branch is a real user gesture.

gear_type itself is never displayed, only the conclusion drawn from it.

Model metadata

Filenames in the wild are cryptic (S-[AMP] 100W VARC CLEAN Rock-Crunch #01)
while the metadata names the actual amp, so a labelled block sits at the bottom
of the card:

Metadata:
      Gear:        Mesa Boogie Badlander Rectifier 100W
      Tone type:   hi_gain
      By:          2dor
      Loudness:    -10.4 LUFS

Loudness earns its place: it spans more than 20 dB across models in circulation,
which is exactly the volume jump you hear when switching between them. It is
shown as dBFS, not LUFS -- NAM's loudness is plain RMS
(20*log10(sqrt(mean(y^2)))) against a standardized input, with no K-weighting
and no gating. nam-rs documented it as LUFS through 0.2.0/0.3.0 and corrected
that in 0.4.0; this branch had inherited the wrong unit and now names the field
loudness_dbfs. Display-only here -- nothing gain-stages against it, which is
the failure mode that mislabel invites.

Roughly half of all models carry TONE3000's literal tz-make/tz-model/
T3K-Null sentinels where real values should be; those are filtered rather than
displayed. metadata_typed() re-parses the whole JSON including the trainer
blob, so summaries are extracted once at load into nam::ModelInfo and cached in
the registry — never per GUI frame.

Folder handling

The NAM directory is created when missing instead of merely warned about,
so the path the UI tells users to drop models into actually exists (mirroring
the IR loader). Failure to create it is still not fatal.
An Open Folder button reveals it in the file manager, via a plain process
spawn rather than a dialog crate — rfd pulls in gtk3, which breaks CI. Spawn
and reap happen on a detached thread, so a slow file manager can't stall the
GUI and no zombie is left per click.

Testing

make lint clean, 10 suites green. New coverage for the metadata summary and
placeholder filtering, the name-override rule and its narrowness, NAM directory
creation (including the un-creatable case), and the opener command.

Verified end-to-end against 233 real .nam files: all parse, none lose their
calibration numbers, and the cab-inclusive captures are exactly the ones flagged.

Not verified visually — the GUI needs JACK and a display, so the card layout
and colour balance want a human eye before merge.

i18n keys added to both EN and ZH_CN.

… folder Three UX gaps around NAM models. **Cab detection.** A capture whose `gear_type` says it already contains a speaker (`amp_cab`, `full-rig`, …) followed by the IR cabinet is two cabs in series. The stage card now says whether the selected model includes a cab, and selecting one bypasses the IR automatically. The bypass is reversible and narrowly scoped. Switching back to a cab-less model restores the IR, but only if *we* were the ones who bypassed it — tracked by `SharedApp::ir_auto_bypassed`, cleared the moment the user touches the control themselves. Without that, switching models would switch on an IR the user had deliberately turned off. Models whose metadata doesn't say change nothing: no evidence, no action. The rule lives in the pure `ir_bypass_decision`, so it is tested directly rather than through a backend. Automatic changes travel as `Message::IrAutoBypassed`, distinct from the manual `Message::IrBypassed` precisely so the two can be told apart. The standalone persists both — otherwise the IR silently returns on the next launch. Only an explicit model pick reconciles; a preset load carries its own IR state. **Model metadata.** Filenames in the wild are often unreadable ("S-[AMP] 100W VARC CLEAN Rock-Crunch #1") while the metadata names the actual amp, so the card now shows gear, tone, who captured it, and output loudness. Loudness earns its space: it spans >20 dB across models in circulation, which is exactly the volume jump you hear when switching between them. Roughly half of all models carry TONE3000's literal `tz-make`/`tz-model` placeholders where the gear name should be; those are dropped rather than displayed, as is `T3K-Null` for tone. `metadata_typed()` re-parses the whole JSON including the trainer blob, so summaries are extracted once at load into `nam::ModelInfo` and cached in the registry — never per GUI frame. **Folder handling.** The NAM directory is now created when missing instead of merely warned about, so the path the UI tells users to drop models into actually exists (mirroring the IR loader). Failure to create it still isn't fatal. An "Open Folder" button reveals it in the file manager, via a plain process spawn rather than a dialog crate — `rfd` pulls in gtk3, which breaks CI. The spawn and its reap happen on a detached thread, so a slow file manager can't stall the GUI and no zombie is left behind. Verified end-to-end against real captures: cab-inclusive models flag and bypass, sparse metadata degrades to "unknown" without touching the chain, and placeholder gear names are filtered out.

Follow-up to the NAM metadata card. **One labelled row per field.** Gear, tone type, author and loudness each get their own "Label: value" row instead of being run together on two lines, and "By" is capitalised like the rest. **`T3K-Null` is filtered as gear, not just as tone.** It is TONE3000's literal "nothing entered" sentinel, and it lands in `gear_make`/`gear_model` as readily as in `tone_type` — a model with an unfilled form was showing its gear as "T3K-Null T3K-Null". The three known sentinels are now one list applied to every descriptive field. **A model's name can override `gear_type`, but only when nothing was filled in.** `Boosted-6505+-A2-Chugs-FullRig` reports `gear_type: "amp"` while every other field is `T3K-Null`: the uploader filled in nothing, so the `amp` is a form default rather than a statement, and the name is the better evidence. Trusting `amp` there tells the user to add an IR to a capture that already has a cab — the exact double-cab this feature exists to prevent. The rule is deliberately narrow. It only ever upgrades to "has a cab", never down; it applies only when `gear_type` is absent or the descriptive fields are all placeholders, so a filled-in `amp` still wins over a cab-sounding name; and it matches only "full rig" and a standalone cab/cabs/cabinet token, because a false positive costs the user their IR. Measured over 233 real models it reclassifies exactly the two Boosted-6505 copies and nothing else. Inferred conclusions are marked "(from name)" in the UI via `ModelInfo::cab_from_name`, so a guess is never displayed as though the file had stated it. `gear_type` itself is still never shown — only the conclusion and what was done about it.

`missing_path_does_not_panic` called `open_directory` for real, so every `cargo test` spawned `xdg-open` on a path that doesn't exist — popping a file manager window and an error dialog on the developer's desktop, and flaky on a headless CI box besides. Split the command construction out as `open_command` so the test can assert what *would* run without running it: the program is the platform opener and the path is its single argument. Also covers that a path with spaces or shell metacharacters is passed as one argument rather than through a shell, which the old test never checked. No behaviour change to the button itself.

The descriptive fields were interleaved with the controls near the top of the stage card, competing with the pick-list and sliders for attention. Group them under a "Metadata:" heading at the bottom, behind a rule, indented and set in the small subtle text style. Labels are column-aligned so the values line up. The block is omitted entirely when the file carries nothing. The cab/IR status line stays with the controls: it reports a change made to the live signal chain, which is not a description of the file. Also trims the comments added with this feature — several had grown into multi-paragraph essays that buried the point.

The cab line only spoke up when the model claimed a cab, and never said what the IR was actually doing. Selecting a cab-less model while the IR was bypassed read "No cab — IR recommended" with no hint that the IR was already off, which looks broken. Show the pairing at all times: what the model has, what the IR is doing, and a button to change it. Warning colour marks the two combinations that are actually wrong — a cab-inclusive model with the IR live (two cabs in series), and a cab-less model with the IR bypassed (no cab at all). The matching combinations stay in the subtle style so the card is quiet when nothing needs attention. With no cab information the IR state is still reported, just without a verdict. The button emits `Message::IrBypassed`, the manual path, so clicking it clears `ir_auto_bypassed` and we stop moving the control once the user has taken over. That keeps a user-set bypass from being overridden while making it one click to fix, rather than forcing a choice between silently changing their tone and leaving them stuck.

Two tests walking the state machine, because "when does the manual override reset?" was not answerable from reading it. `a_manual_toggle_does_not_disable_auto_bypass_for_good` traces the full cycle: auto-bypass fires, the user overrides, and auto-bypass still fires on the next cab model — re-arming the restore. The auto-bypass arm was never gated on `auto_bypassed`, only the restore arm is, so a manual click is not a permanent opt-out. `a_user_set_bypass_survives_cycling_through_models` pins the one case that does stay sticky by design: a bypass the user set is never lifted for them, whatever they select afterwards. The card's warning line and IR toggle are the way out.

The NAM card now reports the cab/IR pairing and offers a button; it never moves the IR itself. Automatic changes were the wrong trade in a plugin. `set_ir_bypass` reaches the host through `raw_begin_set_parameter` / `raw_end_set_parameter`, and that gesture pair is precisely what a host reads as "the user is touching this control" — so selecting a NAM model would write automation on IR Bypass that the user never performed, and fight any automation already there. Suppressing the gesture would have kept the parameter and the automation lane fighting over the same value anyway. Removes `Message::IrAutoBypassed`, `SharedApp::ir_auto_bypassed`, `ParamUpdate::NamModelSelected`, `reconcile_ir_for_nam_cab`, `ir_bypass_decision` and its tests. Model selection is a plain `NeedsStageRebuild` again, and the standalone persists only the manual toggle. What survives is the part that was always the point: the card says whether the model has a cab, what the IR is doing, colours the two combinations that are actually wrong, and puts the fix one click away. Every parameter write is now a real user gesture.

Swaps the temporary local path dep for `nam-rs = "0.4.0"` from crates.io, so CI can resolve it. Also corrects the unit. NAM's `loudness` is dBFS RMS measured against a standardized input — the trainer computes `20·log10(sqrt(mean(y²)))` — with no K-weighting and no gating. nam-rs documented it as LUFS through 0.2.0 and 0.3.0 and corrected that in 0.4.0; this branch had inherited the wrong unit, naming the field `loudness_lufs` and rendering "-19.4 LUFS" on the card. Renamed to `loudness_dbfs` and displayed as dBFS. No audio impact: the value is display-only here. It is stored, checked for presence by `is_empty`, and printed — nothing gain-stages or loudness-matches against it, which is the failure mode this mislabel invites elsewhere.

…n-and-folder-ux

Copilot

Pull request overview

This PR improves the NAM model UX across the shared GUI and core model-loading path by adding cab-detection-driven UI messaging, cached model metadata summaries, and better NAM folder handling (creation + “Open Folder”).

Changes:

Add cached ModelInfo extraction at NAM load time (cab-inclusion inference + metadata summary) and expose it via the NAM registry for cheap GUI access.
Enhance the NAM stage card to show cab/IR status with warnings, add an IR toggle button, and display metadata rows (gear, tone type, author, loudness).
Improve NAM folder UX: create the directory when missing and add an “Open Folder” action via a small cross-platform opener shim.

Reviewed changes

Copilot reviewed 13 out of 14 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
rustortion-ui/src/stages/nam.rs	Adds cab/IR status line + IR toggle, displays cached metadata, and adds Open Folder action in the NAM stage UI.
rustortion-ui/src/stages/mod.rs	Introduces `ParamUpdate::OpenNamModelsDir` for wiring the Open Folder action.
rustortion-ui/src/platform.rs	Adds cross-platform `open_directory()` helper to reveal a folder via the OS file manager.
rustortion-ui/src/lib.rs	Exposes the new `platform` module.
rustortion-ui/src/i18n/mod.rs	Adds i18n keys/strings for metadata + cab/IR messaging + folder button label.
rustortion-ui/src/components/widgets/common.rs	Extends `StageViewState` with `ir_bypassed` for NAM cab/IR messaging.
rustortion-ui/src/app.rs	Wires `OpenNamModelsDir` to `platform::open_directory` and supplies `ir_bypassed` in `StageViewState`.
rustortion-standalone/src/gui/app.rs	Refactors IR bypass persistence detection logic (same behavior, clearer flow).
rustortion-core/src/nam/registry.rs	Adds a cached `ModelInfo` registry alongside the parsed model registry.
rustortion-core/src/nam/mod.rs	Adds `info` module and re-exports `ModelInfo`.
rustortion-core/src/nam/loader.rs	Creates NAM directory if missing and extracts/caches `ModelInfo` during scanning.
rustortion-core/src/nam/info.rs	New `ModelInfo` implementation: placeholder filtering, cab inference, and metadata summary extraction (+ tests).
rustortion-core/Cargo.toml	Bumps `nam-rs` dependency to `0.4.0`.
Cargo.lock	Updates lockfile for `nam-rs 0.4.0`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+/// already displayed next to the button.
+pub fn open_directory(path: &Path) {
+    let path: PathBuf = path.to_path_buf();
+    std::thread::spawn(move || {


        let folder_row = row![
            text(dir_text).width(Length::Fill),
+            button(text(tr!(nam_open_folder))).on_press(Message::Stage(
+                idx,
+                StageMessage::Nam(NamMessage::OpenFolder)
+            )),
            button(text(tr!(nam_rescan_models)))
                .on_press(Message::Stage(idx, StageMessage::Nam(NamMessage::Rescan))),
        ]


…no dir Review findings on the cab heuristic and the folder row. **Negated names were reported as cab-inclusive.** `name_suggests_cab` matched a bare `cab` token with no regard for what preceded it, so `5150 (no cab)`, `Marshall cab-less`, `Amp only - without cab` and `Bogner - cab sim off` all returned "cab included" — whereupon the card advises bypassing the IR and the signal ends up with no cab at all, the exact opposite of the intent. The `-`/`_` to space normalization made it worse rather than better: it is what turns `cab-less` into a bare `cab` token in the first place. A negation word anywhere in the name now suppresses the name signal entirely and falls back to `gear_type`. Suppressing broadly beats parsing the negation, because the errors are not symmetric: over-suppressing costs nothing (we use the metadata we would have used anyway) while a missed negation costs the user their cab. The narrowness test previously only covered substring traps ("Cabernet"), which a whole-word tokenizer passes for free; it now covers the realistic class, and fails without this change. **Open Folder is disabled when there is no directory** rather than offering a click whose only effect is a log line (Copilot). Unreachable today — both backends always return `Some` — so this is hardening. Also corrects three docs that described behaviour the code doesn't have: the `cab_from_name` gate gets at `gear`/`tone` only (not "no metadata at all", since the exporter fills `modeled_by` and `loudness` in regardless), the test named for that case is renamed to match what it actually asserts, and the module doc no longer claims `metadata_typed` re-parses the whole file — it deep-clones the metadata subtree. Removes a comment referring to the auto-bypass flag deleted in 3d1dca5.

`IrCabinetControl::default()` is `new(false, 0.1)`, and nothing in the plugin ever seeded it from `params.ir_bypass` / `params.ir_gain`. The engine reads the truth independently in `process()`, so the GUI and the audio disagreed from the moment the editor opened. The standalone already does this correctly (`gui/app.rs`, seeded from settings and preset). That was survivable while the value only drove an unlabeled checkbox. The NAM card now renders it as a coloured verdict about the signal chain, which turns it into an active lie: reopen a project saved with IR Bypass on plus a cab-inclusive NAM model, and the card claims warning-orange "two cabs in series" while the IR is in fact bypassed. Worse, the button offered to fix it would send a begin/end-gesture write of `ir_bypass = true` — the value it already held — so the host records an automation touch the audio never needed and the false warning survives the click. Read both params before `flags.params` moves into the backend and construct the control from them. Does not address drift *while* the editor is open: nothing polls the host, so automation moving `ir_bypass` mid-session still leaves the card stale. That is pre-existing and wants a subscription, not a constructor change.

OpenSauce added 8 commits July 27, 2026 18:43

OpenSauce marked this pull request as ready for review July 27, 2026 19:58

Copilot AI review requested due to automatic review settings July 27, 2026 19:58

Copilot started reviewing on behalf of OpenSauce July 27, 2026 19:58 View session

Merge remote-tracking branch 'origin/main' into feat/nam-cab-detectio…

9b36b43

…n-and-folder-ux

Copilot AI reviewed Jul 27, 2026

View reviewed changes

OpenSauce added 2 commits July 27, 2026 21:23

OpenSauce merged commit b7f98ee into main Jul 27, 2026
10 checks passed

OpenSauce deleted the feat/nam-cab-detection-and-folder-ux branch July 27, 2026 20:58

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(nam): cab detection, model metadata, and folder UX - #286

feat(nam): cab detection, model metadata, and folder UX#286
OpenSauce merged 11 commits into
mainfrom
feat/nam-cab-detection-and-folder-ux

OpenSauce commented Jul 27, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

OpenSauce commented Jul 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Does a model include a cab?

The amp problem

What the card shows

Model metadata

Folder handling

Testing

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

OpenSauce commented Jul 27, 2026 •

edited

Loading

The `amp` problem