Reviewed the change against the codebase — the diagnosis and fix both hold up.

Root cause confirmed. seedBuiltinDefaults() is called before readScopeFromJson() on every load (ThemeManager.cpp:638) and seeds applet/{tx,rx,comp} via scopeOrCreate (lines 516/522/528), which materializes the top-level applet scope. The old code's into->children.emplace(name, …) is a no-op when the key already exists, so the saved applet/* subtree was parsed into a raw node that was created but never linked — every per-applet override was silently dropped. spectrum isn't seeded, which is exactly why spectrum/vfo round-tripped fine. The find-or-recurse rewrite is the right shape.

Merge semantics are correct. Recursing into the existing node funnels saved tokens through flattenTokens into into->tokens, which uses QHash::insert (overwrite) — so saved overrides win over seeded defaults, and untouched seeded tokens are preserved. The reused node keeps its name/path/parent, all consistent with how scopeOrCreate computes them, so no divergence there.

Tests are well-chosen. The four scenarios pin the regression (applet/rx font), guard the previously-working path (spectrum/vfo), prove generality (applet/comp color), and the sparse-theme case confirms seeded per-applet defaults survive when a theme doesn't mention applet/* — that last one is the case the merge logic could most plausibly have broken, so good to see it covered.

Conventions look right: no flat-key AppSettings, RAII via unique_ptr preserved, localized to one function. Nice clean-room bug report and fix — thanks @mvanhorn.

_{🤖 aethersdr-agent · cost: $1.7704 · model: claude-opus-4-8}

Thanks for this, @mvanhorn — clear write-up and a well-targeted fix.

I traced the root cause and it matches your diagnosis exactly. seedBuiltinDefaults() calls scopeOrCreate("applet/tx" | "applet/rx" | "applet/comp") (ThemeManager.cpp:515-532) on every load, which materializes the intermediate applet node as a child of root before the theme JSON is read. So in the old readScopeFromJson, the applet key already existed and into->children.emplace(name, …) was a no-op — the saved applet/* subtree was recursed into the freshly-constructed raw duplicate that was never linked into the tree (and then discarded at end of loop), silently dropping every per-applet override. spectrum isn't pre-seeded, which is why spectrum/vfo round-tripped fine. The find-then-merge rewrite is the right shape and stays localized to the one function.

A few things I checked that hold up:

• Merge semantics are correct. Recursing into the existing seeded node means flattenTokens overwrites only the keys present in the JSON and leaves the seeded floor intact — consistent with the "seeds are the floor, JSON wins where defined" contract documented at loadThemeFromPath (line 633-634). Your sparse-theme test pins this nicely.
• No leak / dangling concern in either the old or new path — the old code discarded data but didn't leak; the new path links what it creates.
• m_scopeByPath stays consistent — loadThemeFromPath calls rebuildScopePathIndex() after readScopeFromJson (line 669), so the merged-in nodes get indexed regardless.

Conventions look good: no flat-key AppSettings, RAII unique_ptr ownership preserved, C++20 idioms intact. Tests are thorough — regression lock on applet/rx, the working-path spectrum/vfo control, a general applet/comp color case, and the sparse-theme seeded-defaults case. All six CI checks (build, macOS, Windows, CodeQL/analyze) are green.

Nothing blocking from me. Nice, surgical bug fix.

_{🤖 aethersdr-agent · cost: $1.9535 · model: claude-opus-4-8}

Approving — accurate root-cause fix and thorough tests.

The bug: readScopeFromJson always did make_unique + std::map::emplace, but emplace no-ops when the key exists — and seedBuiltinDefaults() pre-creates the applet scope, so saved applet/* overrides were read into an orphaned node never linked into the tree (while non-pre-seeded spectrum/* loaded fine). The fix correctly recurses into the existing child and merges, rather than orphaning a duplicate. Minimal and localized.

The four new scenarios genuinely lock it (applet/rx round-trip = the regression lock; spectrum/vfo guards the working path; applet/comp color proves it's general; sparse-theme confirms seeded defaults survive) — built and passed 1/1 locally. Bug fix with a clear root cause, Principle V honored. Nice work @mvanhorn.