Files
serumalt/docs/JANITOR_TASKS.md
biggy f69316a454 docs: record the round 2 janitor sweep
What changed in each phase, the verification evidence and its commands, the
findings that were reported rather than fixed, and the limits of what could be
checked on this machine. The graft graph was rebuilt after the source moves.
2026-10-06 09:18:37 +02:00

11 KiB

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 <dir> --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 <!-- graft:start --> 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.