docs: finish the audit, and stop --out from silently working - #3
Merged
Conversation
Asked whether ALL documentation was updated, the honest answer was no. The docs/ tree and README were rewritten; the surfaces outside them were not, and two of them contradicted the change: - PULL_REQUEST_TEMPLATE.md told contributors the palette is an input "the user supplies from their own installed copy, never something this repo ships". It ships. That checklist item would have failed a PR for doing the right thing. It also said "neither gate" where there are now three. - SECURITY.md described the tool as reading "a MIDI file you give it, plus two data files you point it at". There are no files to point at, and the threat model was missing the surface that replaced them -- the shipped palette, which decides what every note resolves to. - CODEOWNERS guarded the CI workflows and the dependency declaration as "supply-chain-critical" but not the shipped data, which is now exactly that: identifiers rather than code, so nothing can execute, but a tampered entry silently changes which sound every note plays and no test can tell that from an intentional edit. tools/ joins it, since that is what regenerates the data. Two behaviours were also undocumented anywhere. architecture.md now explains why reading a pitch from a name is ambiguous (`b` is both a note and a flat marker, so `play_fluteb4` parses two ways and only the instrument stem settles it), and why the blank stage carries sixteen persistent integers. **The audit found a live bug, not just stale prose.** Checking that every documented flag exists turned up `--out` still working: argparse abbreviates unambiguous prefixes, so `--out song.json` bound to `--out-dir` and wrote `song.json/rawmap.json` -- creating a DIRECTORY named after the file the user meant to write, silently, exit 0. Anyone working from muscle memory or an old note got that. `--out` now refuses with the migration it needs, and allow_abbrev is off so no other flag binds that way either (`--but` was reaching `--button`). README gains real install instructions. It assumed `pip install` into a system Python, which fails outright on any PEP 668 distribution and pollutes the interpreter everywhere else. There is now a short path and an isolated path, with the activation line per shell, the PowerShell execution-policy fix, and the point people actually trip on -- that activation does not survive closing the terminal. Every step was run verbatim from an empty directory before being written down. Every Python snippet in the docs is now executed against the installed package, every documented flag checked against --help, and every internal link and anchor resolved. 130 passed, 4 skipped.
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.
Follow-up to #2, prompted by the question "did you update ALL documentation?" — the honest
answer was no.
What was stale
docs/andREADME.mdwere rewritten in #2. The surfaces outside them were not, and twoactively contradicted the change:
PULL_REQUEST_TEMPLATE.mdPULL_REQUEST_TEMPLATE.mdSECURITY.mdCODEOWNERSpyproject.tomlas supply-chain-criticalSECURITY.mdalso gained the threat that replaced the two it lost: the shipped palette holdsidentifiers rather than code, so nothing can execute, but a tampered entry silently changes
which sound every note plays and no test can tell that from an intentional edit.
tools/joins CODEOWNERS for the same reason — it is what regenerates that data.
Two behaviours were undocumented anywhere
architecture.mdnow explains why reading a pitch out of a name is ambiguous —bis both anote and a flat marker, so
play_fluteb4parses two ways and only the instrument stem settlesit — and why the blank stage carries sixteen persistent integers.
The audit found a live bug
Checking that every documented flag actually exists turned up
--outstill working. argparseabbreviates unambiguous prefixes, so
--out song.jsonbound to--out-dirand wrotesong.json/rawmap.json— creating a directory named after the file the user meant towrite. Silent, exit 0. Anyone working from muscle memory or an old note got that.
--outnow refuses with the migration it needs, andallow_abbrev=Falsestops any other flagbinding that way (
--butwas reaching--button).README install instructions
It assumed
pip installinto a system Python — which fails outright on any PEP 668distribution and pollutes the interpreter everywhere else. There is now a short path and an
isolated path: which folder to
cdinto, the activation line for each shell, the PowerShellexecution-policy fix, and the thing people actually trip on — activation does not survive
closing the terminal.
Every step was run verbatim from an empty directory before being written down.
How this was checked, not asserted
--help130 passed, 4 skipped.