Repository navigation
Version every product's saved state, and run pluginval's state restoration test again - #25
Merged
Merged
Conversation
- libs/ui session/state_schema: an integer schema_version on the root of each
saved state (sessions and presets), a {from, apply} migration table run from
the saved version up, and the policy for a state from a newer build: load
each parameter this build has by ID (and what the product reads by name, the
Leveler's session part), drop the rest, and save as this build's version.
No version is version 0, version 1's layout, so older states and ZL's own
load unchanged. Every product is at 1 with no migrations.
- Each processor writes and reads it; generated factory presets carry it.
- Restoring a state now puts each switch (AudioParameterBool) exactly on 0 or
1. A switch keeps the raw value a host wrote (0.48 is off) and replaceState()
skips a parameter whose plain value already matches, so the host went on
reading 0.48 after an "off" state was restored (EQ, Compressor, Limiter).
Done in our wrappers; the vendored engines are untouched.
- Leveler: the rider fader is saved as the value its normalised position gives,
as the parameter tree stores its parameters. Its default, 0, restored as
-5.4e-7 (a step's rounding), so the state didn't save back identically.
- Tests per product: a round trip, a state without a version (0.1.0 layout),
one from a newer version, factory and user presets, and pluginval's state
restoration check (each parameter moved, the state set back, the same bytes
read back), which failed on EQ, Compressor, Limiter and Leveler before.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
It was disabled from the first commit, copied from ZL's workflows. Every product passes it now (strictness 10, ZL-Audio pluginval v1.0.4-temp). The Linux Thread Sanitizer job still skips Background thread state. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
pumfleet
force-pushed
the
state-schema-version
branch
from
November 2, 2026 18:35
8f9199b to
bdfdec2
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Every product's saved state now carries a schema version, and pluginval's "Plugin state restoration" test runs again for every product, after fixing what made a restored state differ from the saved one.
(a) State schema version
libs/ui/source/altvane_ui/session/state_schema.{hpp,cpp}: the shared part. libs/ui doesn't own the state envelope (each processor writes its own: an APVTS tree for De-Esser, Leveler and Dialogue, ZL'sParaStatefor EQ, Compressor and Limiter), so each product calls these helpers itself:write/read: an integerschema_versionattribute on the state's root element. A missing attribute reads as 0.upgrade(state, current, migrations): the migration hook. It takes a{from, apply}table and runs each step from the saved version up tocurrent. Version 0 (states saved before versions existed, including ZL's own releases) has version 1's layout, so it loads unchanged and the table starts at 1. Every product's table is empty for now.knownParameters: used for newer states (below).PluginProcessor::kStateVersion = 1. It writes the version ingetStateInformationand reads and upgrades it insetStateInformation. Generated factory presets (Compressor, Limiter, De-Esser, Leveler, Dialogue) carry the version too. A generator updated for a later layout must not write states labelled 0, which a migration would then rewrite again. Factory presets already on disk have no version, so they load as version 0.state_schema.hpp.setStateInformation. User presets keep the attribute through each codec: ZL's JSON keeps root properties, andxmlAsJsonkeeps attributes.(b) pluginval "Plugin state restoration"
Baseline: I built each product's VST3 from
mainand ran CI's pluginval (ZL-Audiov1.0.4-temp, strictness 10, repeat 2) with the test enabled. All six passed on Linux. The test was disabled in the very first commit (feat: init), copied from ZL's workflows, not because of a known failure here. Under pluginval's VST3 path the random parameter writes didn't produce the failures below. I didn't confirm why.So I added the same check to each product's tests (
state_tests.cpp, "the state restores exactly after every parameter moves, as pluginval checks"). It reads the state, moves each automatable parameter except bypass to a random value, sets the state back and expects the parameter restored, then expects the state read again to be byte-identical. The tolerance is 1e-6 instead of pluginval's 0.1. It runs from defaults and from a randomised start. It failed on four products for two real causes:AudioParameterBool) were not restored: EQ (dynamic on/learn/bypass/relative, side link, bypass, ...), Compressor (External Side, Side EQ Bypass, RMS ON), Limiter (DC Filter, Delta, ...). A switch keeps the raw value a host writes (0.48 is "off"), unlike every other JUCE parameter type, which stores its snapped plain value.AudioProcessorValueTreeState::replaceState()skips a parameter whose plain value already equals the saved one. So after restoring an "off" state, the switch kept reporting 0.48 to the host. That is an error of 0.48, above pluginval's 0.1. A host that writes parameters straight into the plug-in (AU does) would see the same. Fix:state_schema::settleSwitches(*this)afterreplaceState()in every product puts each switch exactly on 0 or 1 when it isn't already. It runs in our wrappers; nothing in the vendored engines changed (the parameters are declared in the vendoredzlp_definitions.hpp).ride_->get(). At its default that is exactly 0.0, but restoring 0.0 through its stepped range gives -24 + 2400 × 0.01f = -5.36e-7, so the next save differed. It is now saved as the value its normalised position gives, as APVTS stores its own parameters, and that value restores to itself.De-Esser and Dialogue passed as they were.
Re-enabled:
cmake_full_test.ymlandcmake_heavy_test.yml(PLUGINVAL_DISABLEDis now empty), andsanitizer.yml(macOS ASan/TSan and Linux ASan are now empty; Linux TSan keeps only "Background thread state", which I didn't investigate).cmake_release.ymldoesn't run pluginval. Nothing stays disabled for any product. The workflow edits only touch those disabled lists.Testing
products/*/tests/state_tests.cpp(EQ: added to its existingstate_tests.cpp). Per product:state_tests.cpp/compat_tests.cppstill pass unchanged);[state]tests pass.ctestpasses. The one exception is Dialogue's "models load under a locale with a decimal comma", which is SKIPPED because the box has no such locale and ctest counts a skip as a failure. Dialogue was built here with a localPLATFORMSedit that isn't committed.v1.0.4-temp, strictness 10, repeat 2, nothing disabled, on all six VST3s, with three seeds: all 18 runs pass (seeds 2611021100, 2611021234, 0x5eed1234). At baseline (main) one seed also passed for all six (see (b)).scripts/check_upstream.shpasses for eq, compressor and limiter.main, so I can't say whether AU failed before the switch fix.Review
codex exec review --base origin/main: no findings, both before and after rebasing onto the merged Move code copied between products into libs/testing and libs/ui #22 (shared helpers). The rebase only conflicted where Move code copied between products into libs/testing and libs/ui #22 removedstorePeakfrom the De-Esser and Dialogue processors, next to the new migration table.settleSwitchescallssetValueNotifyingHostduringsetStateInformation. It only fires for a switch the host left off-grid, and the semantic value doesn't change. The VST3 wrapper ignores it during a state load (inSetState).0.0to-5.36e-7at its default. Sessions saved before load the same.sanitizer.ymlandcmake_heavy_test.ymltoo).🤖 Generated with Claude Code