diff --git a/docs/JANITOR_TASKS.md b/docs/JANITOR_TASKS.md new file mode 100644 index 0000000..bf8c516 --- /dev/null +++ b/docs/JANITOR_TASKS.md @@ -0,0 +1,204 @@ +# Janitor round 2: structure, build separation, scripts + +Executed on branch `main` from the round 1 checkpoint. Round 1 had left all of its +work uncommitted and `HEAD` did not compile without the `FXProcessor` repair, so +this round begins by committing that work and then lands two phases of its own. +Round 1's own task list is archived at `docs/archive/JANITOR_TASKS_2026-05.md`. + +The user confirmed the scope before any file changed. The full set of decisions is +in the commit messages; this file records what changed, the evidence, and what was +left alone. + +## Commits + +| Commit | Contents | +| --- | --- | +| `e4cea30` | Round 1 checkpoint: the uncommitted `FXProcessor` repair, `ExactCompare.h`, `KnobLookAndFeel`, the `AGENT.md` to `AGENTS.md` renames, and graft integration. | +| `facd5c5` | Phase 1: `scripts/`, the toolchain file in `cmake/`, build output under `build/`, `clean.sh`, `docs/`, the Windows manifest generated from CMake, and three code fixes. | +| `4b0db50` | Phase 2: 36 source files into their module directories, and five directories renamed. | + +This file and the refreshed graft graph land in a fourth commit. + +## Phase 1: repository root, build output, scripts + +### Layout + +- `scripts/` holds `build_linux.sh`, `build_macos.sh`, `build_windows.sh`, + `build_harness.sh` and `clean.sh`. Each resolves the repository root from its own + location, so the documented `./scripts/build_harness.sh --run` works from any + directory. +- `cmake/` holds `mingw64-toolchain.cmake` beside `Plugin.cmake` and `Harness.cmake`. +- `docs/` holds `AUDIT_REPORT.md` and `UI_UX_AUDIT.md`; `docs/archive/` holds the + finished round 1 list. +- The repository root now holds `CMakeLists.txt`, `README.md`, `AGENTS.md`, + `.gitignore`, the tool configuration files, and four directories: `Source/`, + `cmake/`, `scripts/`, `docs/`. + +### Build output + +Each script defaults to a directory under `build/`: `build/linux`, `build/macos`, +`build/windows` and `build/harness`. `BUILD_DIR` still overrides any of them, and a +relative override resolves from the repository root. `SerumAlt_Windows/` remains the +staging directory for the shippable Windows bundle, and `OUTPUT_DIR` still overrides +it. + +`scripts/clean.sh` removes generated output: + +| Invocation | Removes | +| --- | --- | +| `./scripts/clean.sh` | the trees under `build/` | +| `--staging` | adds `SerumAlt_Windows/` | +| `--legacy` | adds `build_*` and `cmake-build-*` left in the root by earlier script versions | +| `--all` | both of the above | +| `--dry-run` | nothing; prints each path with its size | + +The script refuses a path outside the repository, a path under `Source/`, +`third_party/`, `.git`, `scripts/`, `cmake/`, `docs/` or `graft/`, a path tracked by +git, and refuses to run at all when `BUILD_DIR` or `OUTPUT_DIR` resolves outside the +repository. Exit status 2 marks a refusal. + +902 MB of stale trees were removed after the new layout built successfully: +`build/`'s original in-place tree, `build_harness`, `build_windows`, +`build_windows_fresh` and the three `build_verify_*` trees. `build_linux/` and +`SerumAlt_Windows/` were left in place; `./scripts/clean.sh --legacy --staging` +removes them when you no longer need the artifacts in them. + +### Windows manifest + +`build_windows.sh` used to carry a 60-line `moduleinfo.json` heredoc with the +version, vendor and class IDs written out by hand. The version also lives in +`project(SerumAlt VERSION 1.0.0)`, so a version bump would have shipped a manifest +that disagreed with the binary. + +`cmake/Plugin.cmake` now defines the plugin metadata once, passes it to +`juce_add_plugin`, and configures `cmake/moduleinfo.json.in` into the build +directory. The three VST3 class IDs are built from `PLUGIN_MANUFACTURER_CODE` and +`PLUGIN_CODE` the same way JUCE builds them, so they cannot drift from the codes +either. The script copies the configured file into the bundle and fails when it is +missing. + +### Code fixes + +- `Source/PluginProcessor.h` gained `using juce::AudioProcessor::processBlock;`. The + float override hid the base class double overload, which produced three warning + lines from three translation units. +- `Source/GUI/Slider.{h,cpp}` were deleted. Nothing constructed `serum::Slider`; it + was reachable only through one include in `PluginEditor.h`. `SERUMALT_INCLUDE_DIRS` + and `GUI/AGENTS.md` were updated to match, and `KnobLookAndFeel` was added to the + owned-files list, where it had been missing. +- Prose in the five shell scripts, the two `.claude/helpers/*.cjs` files and the + CMake files was tightened. The scripts had no em dashes and no banned vocabulary + left from round 1, so what changed was structure: numbered step headings, a + paragraph explaining what a load failure would look like, and the manifest + heredoc, all gone. + +### Unslop scope + +The five shell scripts, `.claude/helpers/graft-hooks.cjs`, +`.claude/helpers/graft-statusline.cjs`, `CMakeLists.txt`, `cmake/Plugin.cmake` and +`cmake/Harness.cmake`. `README.md` and `AGENTS.md` keep their prose and received +path corrections only, so the layout changes stay reviewable. + +## Phase 2: source modules + +The five directories `engine/`, `modulation/`, `params/`, `plugin/` and `synth/` +held nothing but guidance pointing at root-level implementations. Every module doc +already claimed its files, so the move only had to honour those claims. + +| Module | Files moved in | +| --- | --- | +| `Engine/` | `Engine.{h,cpp}` | +| `Synth/` | `Wavetable`, `Oscillator`, `SubOscillator`, `NoiseOscillator`, `Envelope`, `LFO`, `Filter`, `FilterBank`, `SynthVoice` (18 files) | +| `Modulation/` | `ModulationMatrix`, `MacroControls`, `RAVEButton` (6 files) | +| `Params/` | `Params.{h,cpp}` | +| `Plugin/` | `PluginProcessor.{h,cpp}` | +| `GUI/` | `PluginEditor.{h,cpp}` | +| `EffectUnits/` | `FXProcessor.{h,cpp}` | +| `Resources/` | `Resources.{h,cpp}` | + +36 files moved, not the 31 stated when the plan was agreed. The five directories +were renamed to `Engine/`, `Modulation/`, `Params/`, `Plugin/` and `Synth/` so that +every subdirectory of `Source/` uses one casing rule. + +`Source/ExactCompare.h` stays at the `Source/` root: five modules include it, and a +`Common/` directory for one header would cost more than it explains. + +`SERUMALT_INCLUDE_DIRS` now lists `Source/` and one directory per module, which let +all 31 parent-relative includes become plain names such as `Params.h`. No header +basename is duplicated in the tree, so that resolution cannot pick the wrong file. + +The module docs list owned files instead of describing a move that had not happened. +`README.md` and the `Current source layout` section of `AGENTS.md` describe the tree +that exists. + +## Verification + +Every command below was run after the phase it verifies. Build directories are under +`build/`, which is git-ignored. + +- `for s in scripts/*.sh; do bash -n "$s"; done` passes for all five scripts. +- `./scripts/build_harness.sh --run` reaches `100% tests passed out of 1` from a + clean directory, in both phases. +- `./scripts/build_linux.sh` links `SerumAlt.vst3` and the `SerumAlt` Standalone. +- `./scripts/build_windows.sh` links the cross-compiled VST3 and passes its own + checks: the module is x86-64, imports none of `libgcc_s_seh-1.dll`, + `libstdc++-6.dll` or `libwinpthread-1.dll`, and exports `GetPluginFactory`, + `InitDll` and `ExitDll`. +- The generated manifest is identical to the previously hand-written one. Compared + with `python3 -m json.tool` on both, after taking the old one from + `git show HEAD:build_windows.sh`. +- Target hygiene, read from `cmake --build --target help`: the harness + configure offers `SerumAltTest` and no plugin target; the Linux and Windows + configures offer `SerumAlt_VST3` and no `SerumAltTest`. +- First-party warning lines: 3 before the `processBlock` fix, 0 after. The four + warning lines that remain in a full run all name files under `third_party/JUCE` + (a `-Wpedantic` note about `::main` in the Standalone, two MinGW `#warning` lines + in `juce_graphics`, and a deprecated `tzname` in `juce_Time`). +- A full build leaves the source tree unchanged: `git status --porcelain -- Source + third_party/JUCE` before and after a three-target build is identical, so no build + product lands in the source tree. +- `./scripts/clean.sh --dry-run --all` prints each path with its size and removes + nothing. Run before the stale trees were deleted it listed nine paths; it now + lists the three that remain: `build/`, `SerumAlt_Windows/` and `build_linux/`. + `--staging`, `--legacy` and the outside-repository refusal were each run. +- `graft build` was re-run after the moves. The graph now mirrors the new paths, for + example `graft/Source/Engine/Engine.md`. + +## Kept, and why + +- `MacroControls::removeAssignment` is still defined and never called. No control in + the Macro tab would call it, so deleting it would remove the only path to a + feature rather than clean up dead code. +- `Source/GUI/PluginEditor.cpp` is still 1147 lines. It is the largest remaining + readability problem in the tree, and it is a GUI refactor with no headless way to + check the pixels. +- The two `.claude/helpers/*.cjs` files remain 66-line copies that differ in one + line. Each is a self-contained hook that `graft init` can regenerate; merging their + shared body into a third file would break on regeneration. Only their comments + changed. +- `opencode.json` and `.mcp.json` each declare the same graft MCP server for a + different host. Both hosts are configured this way deliberately, so removing + either would break that host. +- `third_party/JUCE/Projucer` and `third_party/JUCE/DemoRunner` are 22 MB of + upstream Linux binaries that ship with the vendored copy. They are not built here + and were not deleted. `.gitignore` now covers them, so they no longer appear as + untracked files. +- `docs/UI_UX_AUDIT.md` and `docs/archive/JANITOR_TASKS_2026-05.md` still contain the + old `./build_harness.sh` commands. They record what was run at the time, and + rewriting them would misreport that. `README.md` and `AGENTS.md` are the + maintained sources for commands. +- Nine em dashes remain inside the generated `` block in + `AGENTS.md`. They come from the installed `@nanonets/graft` template and return on + the next `graft init`; removing them needs a change to that package. + +## Limits + +- The macOS build could not be run, because it needs a Mac and the Apple SDK. + `scripts/build_macos.sh` was syntax checked and reviewed, and nothing else. +- No change in this round touched audio or GUI behaviour except deleting an unused + class, so the harness result is a regression check, not sound or visual proof. + There is still no headless way to verify a pixel. +- The staged cross-build for this round is at + `build/windows-staged-check/SerumAlt.vst3`. It is inside `build/`, so + `./scripts/clean.sh` removes it. `SerumAlt_Windows/SerumAlt.vst3` is the older + bundle and was not overwritten.