mpckit/pull-requests/atomic-edits/all-or-nothing-edits.md

All-or-nothing edits

Branch: atomic-edits → main

PR 6 of the review fixes plan (docs/review-fixes-plan.md), part of the 0.5.3 series; the version does not change in this PR.

What changed

Each edit checks first, then changes the project (M3). An edit that fails now leaves the project exactly as it was, and only an edit that succeeds marks it as having unsaved changes.

• AssignSample works out everything before touching the project: which pool entry to reuse, which entries replacing the pad frees, the sample limit, the new file name, and the new pad, built in full from the blank pad. Only then does it replace the pad and add the pool entry. None of those last steps can fail.
  • The name of a sample freed by the replacement can still be reused, as before: replacing kick.wav with a different kick.wav keeps the name instead of making it kick001.wav. A new freedBy helper works this out, and also handles a file used by two layers of the same pad.
• SetTune checks the pad's first layer is an object before writing. Before, if it was not, the project was marked changed anyway.
• SetMuteGroup marks the project changed only if the write worked.
• SetSequence looks up and checks every clip before writing anything. Before, if a clip had no value, the new name, BPM and length were already half written when it failed. The length write is split into lengthClips (check) and writeLength (write); setLength, used by AddSequence, uses both.
• AddSequence and RemoveSequence set dirty after the change rather than before. RemoveSequence picks the next current sequence before removing anything.

Project fields are written only by project (M4).

• Dirty and Sources are now the unexported dirty and sources.
• New methods replace them: Dirty() reads the flag, MarkExported() clears it after an export, and Source(file) returns a pool file's source and whether one is recorded.
• app and export use these. The CLI and tools did not use either field.

Tests.

• TestFailedEditsChangeNothing: each of these edits fails on a loaded DemoFile and leaves the encoded project, the sources and Dirty() unchanged:
  • assigning a missing file;
  • assigning a file that is not a WAV;
  • tuning a pad whose layer is not an object;
  • setting the mute group of a pad that is not an object;
  • editing a sequence with a broken clip;
  • a bad slot;
  • assigning a 129th sample.
    

Against the old ordering, the tune, mute-group and sequence cases fail.

• TestAssignReplacesBrokenPad: the plan's example, a pad whose layersv is empty. AssignSample does not fail on it: it starts from the blank pad, so the broken pad is replaced whole.
• TestAssignReusesFreedName: the freed-name behaviour above.
• Export tests: the two that set Sources directly now rename a pool entry in the tree, or call the name check and the zip writer directly.

Notes

• The plan's example does not fail (see the test above). AssignSample replaces the pad with the blank pad before writing, so a pad with no layers is simply replaced. The failures that can actually happen are now all covered.
• The second-line name check in export. Only project can record a source now, so Folder and Zip can no longer be handed a bad pool name from outside, and their check is reached only if project itself has a bug. The test calls the check and the zip writer directly. internal/export coverage is 85.9%, still above PR 3's 85%.
• One test still writes a field from outside project: export_test.go sets Name to a/b to test the export name check, since SetName refuses such a name. No non-test code outside project writes a project field. Name and SourceDir stay exported, as the plan only names Dirty and Sources.
• Nothing changes in the files MPCkit writes. The golden tests (MpckitT1, T3, Demo02) and the round-trip tests pass unchanged.

Testing

On the server, every command with GOTOOLCHAIN=go1.27.4:

• go test ./... passes, including the golden and round-trip tests.
• go vet ./... (also with GOOS=windows) and staticcheck ./... are clean.
• govulncheck ./...: "No vulnerabilities found."
• npm install, npm run check (0 errors, 0 warnings) and npm run build pass. The lockfile was not committed. The bindings did not change.
• wails3 build builds, and the app starts under Xvfb.

On the laptop (PowerShell, in the repo):

1. git fetch; git checkout atomic-edits
2. go test ./...: all ok.
3. wails3 dev: load testdata\device\DemoFile.xpj. The footer shows no unsaved changes.
4. Assign a file that is not a WAV to pad A1, for example by renaming a text file to .wav. An error shows, A1 keeps its sample, and the footer still shows no unsaved changes.
5. Assign a real WAV to A1: it replaces the sample, and the footer shows unsaved changes.
6. Change a tune, a mute group, and a sequence's name, BPM and length. Each works as before. Export to folder, then load the export: the changes are there.