From 23a2cda1d8205be1544ef9b278610683e6dcf9fb Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 18 Aug 2026 02:54:29 +0000 Subject: [PATCH 1/2] Make a release re-runnable, and write down the two traps that caught us Three releases in a row -- 1.0.1, 1.1.0 and 1.2.0 -- are tags with no GitHub Release. Each publish step failed, and the release job depends on publish, so the notes were never created even though the artifacts are on PyPI. The publish step now passes skip-existing. Without it a release whose files reached PyPI any other way can never be re-run: the upload aborts on "file already exists" and everything after it is unreachable. With it, a re-run skips the upload and goes on to create the release, which is what re-running is for. docs/releasing.md gains the two traps, both of which cost real time today: Tag the commit that carries the bump, not whatever master was when you started. v1.2.0 was created on the commit before the bump merged, so the tree there still said 1.1.0. The workflow's tag check refused it -- exactly what that guard is for -- and a local build in the same clone quietly produced 1.1.0 artifacts that PyPI rejected as duplicates. A one-line `grep __version__` before tagging catches it, and the recovery is written out, including that a --depth 1 clone cannot delete and re-push a tag. And what skip-existing is protecting against, so nobody removes it as noise. The workflow was parsed to confirm the key landed where intended rather than trusting the diff. 667 tests, ruff clean, 2,499 sources checked. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PBAjxy7cD6DzJ72NX8TJEc --- .github/workflows/release.yml | 7 +++++++ docs/releasing.md | 30 +++++++++++++++++++++++++++++- 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index fe610daa..448639d7 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -74,10 +74,17 @@ jobs: name: dist path: dist/ + # skip-existing makes a re-run idempotent. Without it, a release that got + # its files onto PyPI some other way -- a manual twine upload, or a first + # attempt that uploaded and then failed later in the job -- can never be + # re-run, because the upload aborts on "file already exists". That took + # 1.0.1, 1.1.0 and 1.2.0 with it: the publish step failed, the release + # job below never ran, and the repository has tags with no releases. - name: Publish to PyPI uses: pypa/gh-action-pypi-publish@release/v1 with: packages-dir: dist/ + skip-existing: true release: name: Create GitHub Release diff --git a/docs/releasing.md b/docs/releasing.md index d5b52006..9a39c5ec 100644 --- a/docs/releasing.md +++ b/docs/releasing.md @@ -18,11 +18,32 @@ python3 -m unittest discover -s tests python3 -m plainsong spec python3 -m plainsong check docs examples plainsong/songbook README.md -# 4. Tag and push. +# 4. Tag the commit that carries the bump -- not whatever master happened to be +# when you started. Merge the version bump FIRST, then pull, then tag. +git pull +grep __version__ plainsong/version.py # must be the version you are tagging git tag -a v1.1.0 -m "1.1.0" git push origin v1.1.0 ``` +**Tag the right commit.** `v1.2.0` was once created on the commit before the +version bump merged. The tree there still said 1.1.0, so the workflow's tag +check refused it -- correctly, and that is the guard doing its job -- and a +local `python -m build` in the same clone quietly produced `1.1.0` artifacts +that PyPI then rejected as duplicates. The one-line `grep` above is what catches +it. If you have already pushed a tag to the wrong commit: + +```bash +git tag -d v1.2.0 +git push origin :refs/tags/v1.2.0 # delete it from the remote +git pull # get the real bump +git tag -a v1.2.0 -m "1.2.0" +git push origin v1.2.0 +``` + +Do not use a shallow (`--depth 1`) clone for that: deleting and re-pushing a tag +needs the real history. + ## What the workflow then does `.github/workflows/release.yml` triggers on any tag matching `v*`: @@ -39,6 +60,13 @@ git push origin v1.1.0 [Trusted Publishing](https://docs.pypi.org/trusted-publishers/). There is no API token in the repository and there should never be one: the job authenticates with a short-lived OIDC identity, which is why it declares `id-token: write`. + It passes `skip-existing`, so a version already on PyPI is skipped rather than + failing the job. That matters more than it sounds: without it, a release whose + files reached PyPI some other way -- a manual `twine upload`, or an attempt + that uploaded and then failed later -- can never be re-run, because the upload + aborts on "file already exists" and the **release** step below never gets to + run. That is exactly how 1.0.1, 1.1.0 and 1.2.0 ended up as tags with no + GitHub Release. 4. **release** — creates the GitHub Release with the artifacts attached and generated notes. This needs `contents: write`; without it the step fails at the very end, after a successful publish, which is the worst place to find out. From 2caff83bb216d5131b52994a3cac48cdec2dcdc9 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 18 Aug 2026 03:22:57 +0000 Subject: [PATCH 2/2] Say something when a header field cannot be read, and fix the PyPI page A fresh agent installed 1.2.0 from PyPI and typed the mistakes a beginner makes. Two findings, both real. Header typos were swallowed in silence. `tempo: banana` compiled at 100, `time: 3-4` at 4/4, and `key: Zz` sounded in C while keeping "Zz" for display, so a chart would print one key over music in another. Nothing said anything -- not compile, not check, and not `info --verbose`, which documents itself as showing every diagnostic. Each still falls back to a default, which is right; falling back without saying so is the same fault as Xm9 compiling to a silent bar. The parsers do not raise on nonsense, so this could not be done by catching an exception. theory.KEY_RE and Meter.readable name each rule once and serve both the parse and the question "will parsing work", instead of a second copy of each pattern drifting from the first. The key warning names the sounding key rather than the text, because "using Zz major" would have been no help. It found a real one on the first run over the corpus: the Hungarian Rhapsody carries `time: 2/4 (Lassan) then 4/4 (Friska)`, a human annotation the metre field cannot express, silently becoming 4/4. The file is left alone -- changing the metre would change the music -- and the warning now says what was ignored. And the README is the PyPI page. pyproject sets readme = "README.md", so forty-four relative links and one relative image resolved on GitHub and 404'd on PyPI, which is where a pip user reads them: the page told people to read a dozen documents none of which they could reach, and to compile examples/ files the wheel does not ship. All absolute now, images through raw.githubusercontent.com because a blob URL serves HTML rather than an image, and the examples section says where the examples actually are. tests/test_readme_links.py buys back what absolute links lose: every URL into this repository is mapped to a path and checked to exist, images may not use blob, and the version the README claims must match the tree. It said "Version 1.0" while PyPI shipped 1.2.0. One process note. The first attempt at this changelog entry was a str.replace on "## Unreleased", which no longer exists since 1.2.0 was cut. It matched nothing and wrote the file unchanged -- a silent no-op of exactly the kind this commit is about. Caught because git status did not list the file. The second attempt asserts its anchor. 671 tests, 7 specs, ruff clean, 6,321 files compiling to exactly the music they did. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PBAjxy7cD6DzJ72NX8TJEc --- CHANGELOG.md | 52 ++++++++++++++++++ CLAUDE.md | 26 ++++++--- README.md | 102 +++++++++++++++++++---------------- plainsong/notation/ir.py | 25 +++++++-- plainsong/notation/parser.py | 48 ++++++++++++++++- plainsong/notation/theory.py | 8 ++- tests/test_readme_links.py | 77 ++++++++++++++++++++++++++ 7 files changed, 278 insertions(+), 60 deletions(-) create mode 100644 tests/test_readme_links.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a3c0869..ad2584ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,58 @@ Notable changes, newest first. Dates are ISO 8601. +## Unreleased + +### Header typos were swallowed in silence + +`tempo: banana` compiled happily at 100. `time: 3-4` compiled at 4/4. `key: Zz` +sounded in C while a chart printed `Zz` over it. None of them said anything — +not on `compile`, not on `check`, and not on `info --verbose`, which documents +itself as showing every diagnostic. + +Each still falls back to a default, which is right: a typo in a header should +not cost you the piece. What was wrong is falling back **without saying so**, +the same fault as `Xm9` compiling to a silent bar. Now: + +``` +warning: tempo 'banana' is not a number; using 100 +warning: time '3-4' is not a metre; using 4/4 +warning: key 'Zz' is not a key; sounding in C major +``` + +The key message names the *sounding* key rather than the text, because the +unreadable text is kept for display — so "using Zz major" would have been no +help at all. + +The parsers do not raise on nonsense, so these could not simply catch an +exception. `theory.KEY_RE` and `Meter.readable` name each rule once and serve +both the parse and the question "will parsing work", rather than a second copy +of each pattern drifting from the first. + +Found by a fresh agent installing from PyPI and typing plausible mistakes — a +test this project cannot run on itself, having long since stopped making +beginner errors. It immediately turned up a real one in the bundled songbook: +`time: 2/4 (Lassan) then 4/4 (Friska)` on the Hungarian Rhapsody, a human +annotation the metre field cannot express, silently becoming 4/4. The file is +left alone — changing the metre would change the music — and the warning now +says what was ignored. + +### The README is the PyPI page, and its links did not work there + +`pyproject.toml` sets `readme = "README.md"`, so that file *is* the project page. +Forty-four relative links and one relative image resolved on GitHub and 404'd on +PyPI, which is where a `pip install` user reads them. The page told people to +read a dozen documents none of which they could reach. + +All absolute now, images through `raw.githubusercontent.com` because a `blob` +URL serves HTML rather than an image. `tests/test_readme_links.py` buys back the +guarantee absolute links lose: every URL into this repository is mapped to a path +and checked to exist, images must not use `blob`, and the version the README +claims must match the tree — it said "Version 1.0" while PyPI shipped 1.2.0. + +The examples section also told pip users to compile `examples/…`, which the wheel +does not ship. It now says so, and points at the bundled library, which it does. + ## 1.2.0 — 2026-08-18 A minor rather than a patch: `notation/merge.py`, `notation/lyrics.py` and diff --git a/CLAUDE.md b/CLAUDE.md index 9898e3ca..5c612491 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -296,13 +296,25 @@ famous for came from those rows and are now 2. - The built-in synthesiser is a preview renderer; timbres are approximations. Audio is mono. - The host bridge cannot stream and reports no token usage. -- **`plainsong/mcp/` also exists in `SuperInstance/plainsong-mcp`.** This is the - one open violation of "one of everything" and it is deliberate but temporary: - the extraction happened while this branch was in review. The intended end - state is that the MCP server lives only in the sibling repository and this - package stops carrying it. Until that lands, a change to one copy must be - made to the other or they will drift — which is precisely the failure mode - the rule exists to prevent. Do not build anything new on this copy. +- **`plainsong/mcp/` also exists in `SuperInstance/plainsong-mcp`.** The one + open violation of "one of everything". Do not build anything new on this copy. + + **This has now cost something real, so it is no longer a theoretical rule.** + The two copies were measured: 240 lines of difference across seven of eight + files. Most is mechanical — relative versus absolute imports, unavoidable when + the same code lives inside and outside a package. One was not. The HTTP + transport's DNS-rebinding guard (`_host_is_local`, requiring a loopback + `Host` because Origin-against-Host alone is defeated by rebinding) existed + **here and not in the sibling**, which is the copy people `pip install` for + MCP. Fixed there, with tests, but it sat open for months with nothing in + either repository able to notice. + + The injection machinery in `mcp/tools.py` and `mcp/resources.py` — the + `ensemble=` parameter and `_default_ensemble()` — is **unused**. It was + written so the sibling could import this `tools.py` and pass its own ensemble; + the sibling never does, having its own copy bound by relative import. Do not + extend it: the end state is that MCP lives only in the sibling and this + directory goes. - The MCP server has now been driven by a third-party client: the official `mcp` Python SDK 2.0.0, over stdio. `initialize`, `tools/list`, `resources/list`, `prompts/list`, `tools/call` and `resources/read` all diff --git a/README.md b/README.md index 8f35ad0f..9aae4b1e 100644 --- a/README.md +++ b/README.md @@ -16,14 +16,14 @@ file any editor, any diff tool and any language model can read. **If you read music but not code**, you already understand most of that block — it is a lead sheet with the bars drawn in. Start with -[Your first song](docs/tutorial-first-song.md); it assumes nothing about +[Your first song](https://github.com/SuperInstance/plainsong/blob/master/docs/tutorial-first-song.md); it assumes nothing about programming and gets you to something you can hear. **If you write code but not music**, the thing to know is that you never declare durations. You write how many events happen in a bar and the bar divides -itself. Start with [the notation reference](docs/notation.md). +itself. Start with [the notation reference](https://github.com/SuperInstance/plainsong/blob/master/docs/notation.md). -**If you are an agent**, read [AGENTS.md](AGENTS.md) first. It is short, and +**If you are an agent**, read [AGENTS.md](https://github.com/SuperInstance/plainsong/blob/master/AGENTS.md) first. It is short, and most of it is the mistakes other agents have already made here. ## Try it without installing anything @@ -148,7 +148,7 @@ plainsong lyrics song.song # which note each syllable really lands on Set `core.lyrics = "bound"` and each syllable is sung on the note it is written under, with the barline resyncing so a miscount costs one bar instead of the rest of the song. The default leaves existing files alone. See -[docs/lyrics.md](docs/lyrics.md). +[docs/lyrics.md](https://github.com/SuperInstance/plainsong/blob/master/docs/lyrics.md). ### Two rows of the same kind run in sequence @@ -246,7 +246,7 @@ against the standard library. **Audio.** The built-in synthesiser works with nothing installed. For higher quality, install fluidsynth and a General MIDI soundfont — Plainsong will use -them automatically. See [docs/audio.md](docs/audio.md) for installation and +them automatically. See [docs/audio.md](https://github.com/SuperInstance/plainsong/blob/master/docs/audio.md) for installation and configuration. Optional extras — NumPy for faster synthesis, ffmpeg for format conversion, mido @@ -258,7 +258,7 @@ for hardware MIDI — are detected when present and never required. plainsong chart song.song -o chart.svg ``` -![a chord chart rendered from one of the bundled examples](docs/img/creatures-of-interval.svg) +![a chord chart rendered from one of the bundled examples](https://raw.githubusercontent.com/SuperInstance/plainsong/master/docs/img/creatures-of-interval.svg) That image is the SVG above, committed to this repository and embedded with an `` tag — which is the only way a chart appears in markdown on a platform @@ -270,7 +270,7 @@ fonts it names. Bars are as wide as their contents need, worked out from the same measurements. It reads `prefers-color-scheme`, so it is legible in a dark README as well as a light one. -See [docs/chart.md](docs/chart.md), including what it deliberately is not: a +See [docs/chart.md](https://github.com/SuperInstance/plainsong/blob/master/docs/chart.md), including what it deliberately is not: a chord chart rather than an engraver, with no noteheads, staff or beaming. ## Timing that models the room @@ -300,7 +300,7 @@ what conductor hears, against the written beat The organist's key goes down 241 ms early so the pipe speaks on the beat. At the podium everything lands together; at any player's desk it does not, which is why an orchestra watches rather than listens. See -[docs/performance.md](docs/performance.md). +[docs/performance.md](https://github.com/SuperInstance/plainsong/blob/master/docs/performance.md). Without a `[Stage]` block none of this applies and written times are taken at face value, exactly as before. @@ -321,7 +321,7 @@ Groq, Mistral, Together, Fireworks, Cerebras, Azure), with local servers (Ollama LM Studio, vLLM, llama.cpp), and with **no key at all** when you are already running inside another agent such as Claude Code — it borrows the model that is already there. Providers are catalogue entries rather than code, so adding one is -a JSON file. See [docs/providers.md](docs/providers.md). +a JSON file. See [docs/providers.md](https://github.com/SuperInstance/plainsong/blob/master/docs/providers.md). Try it with no key and no network at all: @@ -380,21 +380,33 @@ Every command takes `--json`. Use it when parsing output. Eight complete pieces ship in the repository, written to be read rather than generated. Each one is a template: copy it, change the chords, recompile. +These live in the repository rather than the wheel, so a `pip install` does not +put them on your disk. Clone, or read them at the links below: + +```bash +git clone https://github.com/SuperInstance/plainsong +plainsong compile plainsong/examples/plainsong-1-creatures-of-interval.song --audio out.wav +``` + +With only `pip install`, the bundled library is the equivalent — several thousand +charts that *are* shipped: + ```bash -plainsong compile examples/plainsong-1-creatures-of-interval.song --audio out.wav +plainsong library "waltz" # search what came with the package +plainsong play stand-by-me # render and play one by name ``` | | | |---|---| -| [Creatures of Interval](examples/plainsong-1-creatures-of-interval.song) | Four voices, plain 4/4 — the simplest complete piece | -| [The Room Is Safe](examples/plainsong-2-the-room-is-safe.song) | Lyrics and melody together | -| [Hermes Blues](examples/plainsong-3-hermes-blues.song) | Twelve-bar blues with swing | -| [Closing Time](examples/plainsong-4-closing-time.song) | The relative dialect — roman numerals and scale degrees | -| [Deck Work](examples/plainsong-5-deck-work.song) | Several named players | -| [Spacing: melody](examples/plainsong-6-spacing-melody.song) · [chords](examples/plainsong-7-spacing-chords.song) · [dashes](examples/plainsong-8-spacing-dashes.song) | How duration-by-spacing behaves | +| [Creatures of Interval](https://github.com/SuperInstance/plainsong/blob/master/examples/plainsong-1-creatures-of-interval.song) | Four voices, plain 4/4 — the simplest complete piece | +| [The Room Is Safe](https://github.com/SuperInstance/plainsong/blob/master/examples/plainsong-2-the-room-is-safe.song) | Lyrics and melody together | +| [Hermes Blues](https://github.com/SuperInstance/plainsong/blob/master/examples/plainsong-3-hermes-blues.song) | Twelve-bar blues with swing | +| [Closing Time](https://github.com/SuperInstance/plainsong/blob/master/examples/plainsong-4-closing-time.song) | The relative dialect — roman numerals and scale degrees | +| [Deck Work](https://github.com/SuperInstance/plainsong/blob/master/examples/plainsong-5-deck-work.song) | Several named players | +| [Spacing: melody](https://github.com/SuperInstance/plainsong/blob/master/examples/plainsong-6-spacing-melody.song) · [chords](https://github.com/SuperInstance/plainsong/blob/master/examples/plainsong-7-spacing-chords.song) · [dashes](https://github.com/SuperInstance/plainsong/blob/master/examples/plainsong-8-spacing-dashes.song) | How duration-by-spacing behaves | And five deliberately awkward files in -[`examples/edge-cases/`](examples/edge-cases/) — polyrhythm, tempo shifts, dense +[`examples/edge-cases/`](https://github.com/SuperInstance/plainsong/blob/master/examples/edge-cases/) — polyrhythm, tempo shifts, dense chords, players with no chord row, and a kitchen sink. They exist to be compiled, not imitated. @@ -406,36 +418,36 @@ plainsong library --collections # what is in there plainsong play stand-by-me # render and play by name ``` -See [the songbook](docs/songbook.md) for what is in it and why it is chord +See [the songbook](https://github.com/SuperInstance/plainsong/blob/master/docs/songbook.md) for what is in it and why it is chord charts only. ## Documentation | | | |---|---| -| [For agents](AGENTS.md) | **Read this first if you are one.** The contract, and the mistakes agents make here | -| [Your first song](docs/tutorial-first-song.md) | Thirty minutes from install to a piece you wrote | -| [Arranging](docs/tutorial-arranging.md) | Several players, time signatures, and the stage model | -| [Integration](docs/integration.md) | Driving Plainsong from other software | -| [Getting started](docs/getting-started.md) | From clone to a finished piece | -| [Notation reference](docs/notation.md) | The whole language | -| [Performance timing](docs/performance.md) | Stages, arrival times, conductor directives | -| [Audio](docs/audio.md) | Rendering to WAV, MP3, and other formats; fluidsynth with soundfonts | -| [MCP server](docs/mcp.md) | Driving the system from any MCP client | -| [Ensemble sessions](docs/ensemble.md) | Several agents co-authoring one score | -| [Providers](docs/providers.md) | Connecting a model, adding your own | -| [Host bridge](docs/host-bridge.md) | Running under another agent, with no key | -| [Agents](docs/agents.md) | The composer and build agents, and their tools | -| [Chords](docs/chords.md) | Which symbols are understood, and the rules that derive the notes | -| [Voicing](docs/voicing.md) | Which notes sound when a chord names more than fit, and how that was measured | -| [Lyrics](docs/lyrics.md) | Binding syllables to the notes they are sung on, and why padding is not melisma | -| [Charts](docs/chart.md) | Drawing a chord chart as SVG, and the measurements behind it | -| [The songbook](docs/songbook.md) | The bundled charts, and the copyright policy they follow | -| [Connectors](docs/connectors.md) | Getting notation and audio into other systems | -| [Architecture](docs/architecture.md) | How it fits together, and why | -| [Specs](docs/specs.md) | The checks the system runs against itself | -| [Releasing](docs/releasing.md) | How a version is cut | -| [Contributing](CONTRIBUTING.md) | Getting involved | +| [For agents](https://github.com/SuperInstance/plainsong/blob/master/AGENTS.md) | **Read this first if you are one.** The contract, and the mistakes agents make here | +| [Your first song](https://github.com/SuperInstance/plainsong/blob/master/docs/tutorial-first-song.md) | Thirty minutes from install to a piece you wrote | +| [Arranging](https://github.com/SuperInstance/plainsong/blob/master/docs/tutorial-arranging.md) | Several players, time signatures, and the stage model | +| [Integration](https://github.com/SuperInstance/plainsong/blob/master/docs/integration.md) | Driving Plainsong from other software | +| [Getting started](https://github.com/SuperInstance/plainsong/blob/master/docs/getting-started.md) | From clone to a finished piece | +| [Notation reference](https://github.com/SuperInstance/plainsong/blob/master/docs/notation.md) | The whole language | +| [Performance timing](https://github.com/SuperInstance/plainsong/blob/master/docs/performance.md) | Stages, arrival times, conductor directives | +| [Audio](https://github.com/SuperInstance/plainsong/blob/master/docs/audio.md) | Rendering to WAV, MP3, and other formats; fluidsynth with soundfonts | +| [MCP server](https://github.com/SuperInstance/plainsong/blob/master/docs/mcp.md) | Driving the system from any MCP client | +| [Ensemble sessions](https://github.com/SuperInstance/plainsong/blob/master/docs/ensemble.md) | Several agents co-authoring one score | +| [Providers](https://github.com/SuperInstance/plainsong/blob/master/docs/providers.md) | Connecting a model, adding your own | +| [Host bridge](https://github.com/SuperInstance/plainsong/blob/master/docs/host-bridge.md) | Running under another agent, with no key | +| [Agents](https://github.com/SuperInstance/plainsong/blob/master/docs/agents.md) | The composer and build agents, and their tools | +| [Chords](https://github.com/SuperInstance/plainsong/blob/master/docs/chords.md) | Which symbols are understood, and the rules that derive the notes | +| [Voicing](https://github.com/SuperInstance/plainsong/blob/master/docs/voicing.md) | Which notes sound when a chord names more than fit, and how that was measured | +| [Lyrics](https://github.com/SuperInstance/plainsong/blob/master/docs/lyrics.md) | Binding syllables to the notes they are sung on, and why padding is not melisma | +| [Charts](https://github.com/SuperInstance/plainsong/blob/master/docs/chart.md) | Drawing a chord chart as SVG, and the measurements behind it | +| [The songbook](https://github.com/SuperInstance/plainsong/blob/master/docs/songbook.md) | The bundled charts, and the copyright policy they follow | +| [Connectors](https://github.com/SuperInstance/plainsong/blob/master/docs/connectors.md) | Getting notation and audio into other systems | +| [Architecture](https://github.com/SuperInstance/plainsong/blob/master/docs/architecture.md) | How it fits together, and why | +| [Specs](https://github.com/SuperInstance/plainsong/blob/master/docs/specs.md) | The checks the system runs against itself | +| [Releasing](https://github.com/SuperInstance/plainsong/blob/master/docs/releasing.md) | How a version is cut | +| [Contributing](https://github.com/SuperInstance/plainsong/blob/master/CONTRIBUTING.md) | Getting involved | ## Testing @@ -461,7 +473,7 @@ in the documentation that stopped compiling would fail the build. ## Status -Version 1.0. The notation, the CLI surface and the provider catalogue format are +Version 1.2. The notation, the CLI surface and the provider catalogue format are stable; changes to them will go through a deprecation cycle. This release is a rebuild rather than an increment. The previous engine assumed a @@ -481,7 +493,7 @@ Known limits, in the open: - The built-in synthesiser produces mono audio with synthetic timbres. It works immediately with no dependencies. For real instrument samples, install fluidsynth and a soundfont (five minutes on any OS). See - [docs/audio.md](docs/audio.md). + [docs/audio.md](https://github.com/SuperInstance/plainsong/blob/master/docs/audio.md). - The host bridge cannot stream and reports no token usage. - The TUI needs `curses`, which stock Python on Windows does not ship. - No third-party MCP client has connected to the server yet; its protocol @@ -489,9 +501,9 @@ Known limits, in the open: - The bundled songbook (`plainsong/songbook/`) is **chord charts only** — melody and lyric rows were removed from all 6,309 files. A chord progression is not protectable expression; a tune and its words are, and nothing in those generated files recorded provenance. - See [docs/songbook.md](docs/songbook.md). + See [docs/songbook.md](https://github.com/SuperInstance/plainsong/blob/master/docs/songbook.md). - The two earlier engines are in `legacy/`, unmaintained. Nothing imports them. ## Licence -MIT. See [LICENSE](LICENSE). +MIT. See [LICENSE](https://github.com/SuperInstance/plainsong/blob/master/LICENSE). diff --git a/plainsong/notation/ir.py b/plainsong/notation/ir.py index e1f3b3f7..5010b975 100644 --- a/plainsong/notation/ir.py +++ b/plainsong/notation/ir.py @@ -138,15 +138,30 @@ def __str__(self) -> str: return f"{self.numerator}/{self.denominator}" @classmethod - def parse(cls, text: str) -> Meter: + def readable(cls, text: str) -> bool: + """Whether `parse` will get a metre out of this rather than defaulting. + + The parser needs to tell "you wrote 4/4" from "you wrote nonsense and + got 4/4", and it cannot do that from the result alone. + """ + try: + numerator, slash, denominator = text.strip().partition("/") + except AttributeError: + return False + if not slash and not numerator.strip().isdigit(): + return False try: - numerator, _, denominator = text.strip().partition("/") meter = cls(int(numerator), int(denominator or 4)) - if meter.numerator < 1 or meter.denominator not in (1, 2, 4, 8, 16, 32): - return cls() - return meter except (ValueError, AttributeError): + return False + return meter.numerator >= 1 and meter.denominator in (1, 2, 4, 8, 16, 32) + + @classmethod + def parse(cls, text: str) -> Meter: + if not cls.readable(text): return cls() + numerator, _, denominator = text.strip().partition("/") + return cls(int(numerator), int(denominator or 4)) @dataclass diff --git a/plainsong/notation/parser.py b/plainsong/notation/parser.py index 8b809cb1..9ca46f85 100644 --- a/plainsong/notation/parser.py +++ b/plainsong/notation/parser.py @@ -346,27 +346,71 @@ def _handle_metadata(self, line: str, index: int) -> None: if not value: continue try: - self._apply_metadata(key, value) + self._apply_metadata(key, value, index) except Exception as exc: # never let a header field abort a parse self._note("warning", f"could not read {key} = {value!r} ({exc})", index) - def _apply_metadata(self, key: str, value: str) -> None: + def _apply_metadata(self, key: str, value: str, index: int = 0) -> None: + """Read one header field, and say so when it cannot be read. + + Every one of these falls back to a default rather than failing, which is + right -- a typo in a header should not cost you the piece. What was + wrong is that it fell back in *silence*: `tempo: banna` compiled happily + at 100, `time: 3-4` at 4/4, and nothing anywhere said why the piece came + out at the wrong speed or in the wrong metre. The parsers below do not + raise on nonsense, so the check has to be here. + """ if key == "key": self.meta.key = theory.parse_key(value) + if not theory.KEY_RE.match(value.strip()): + # `parse_key` keeps the text it was given, so an unreadable key + # is displayed on a chart while the music sounds in C. + # Report what will *sound*, not the text that was kept. The + # unreadable text is carried through for display, so a chart + # would print `Zz` over music in C and the warning saying "using + # Zz major" would be no help at all. + sounding = theory.NOTE_NAMES_SHARP[self.meta.key.tonic_pc % 12] + self._note( + "warning", + f"key {value!r} is not a key; sounding in {sounding} {self.meta.key.mode}", + index, + hint="a key looks like C, Am, F#m, Bb, or a mode: `key: D dorian`", + ) elif key in {"tempo", "bpm"}: number = re.search(r"-?\d+(?:\.\d+)?", value) if number: tempo = float(number.group(0)) self.meta.tempo = min(max(tempo, 20.0), 400.0) + else: + self._note( + "warning", + f"tempo {value!r} is not a number; using {self.meta.tempo:g}", + index, + hint="tempo is beats per minute, as in `tempo: 96`", + ) elif key == "swing": number = re.search(r"-?\d+(?:\.\d+)?", value) if number: swing = float(number.group(0)) self.meta.swing = swing / 100.0 if swing > 1 else swing + else: + self._note( + "warning", + f"swing {value!r} is not a number; using {self.meta.swing:g}", + index, + hint="swing is a percentage, as in `swing: 62%`", + ) elif key == "subdivision": self.meta.subdivision = value elif key in {"time", "meter"}: self.meta.meter = Meter.parse(value) + if not Meter.readable(value): + self._note( + "warning", + f"time {value!r} is not a metre; using {self.meta.meter}", + index, + hint="a metre is two numbers over a slash, as in `time: 6/8`", + ) else: self.meta.extra[key] = value diff --git a/plainsong/notation/theory.py b/plainsong/notation/theory.py index 9b7e2b18..2e60a1e0 100644 --- a/plainsong/notation/theory.py +++ b/plainsong/notation/theory.py @@ -451,12 +451,18 @@ def __eq__(self, other: object) -> bool: } +KEY_RE = re.compile(r"^([A-Ga-g])([#b♯♭]*)\s*(.*)$") +"""What a key has to start with. Named because the parser also needs to ask +whether a header value *is* a key, and asking with a second copy of this +pattern is how the two answers drift apart.""" + + def parse_key(text: str) -> Key: """Parse ``Am``, ``A minor``, ``F# dorian`` or ``Bb`` into a :class:`Key`.""" cleaned = text.strip() if not cleaned: return Key(0, "major") - match = re.match(r"^([A-Ga-g])([#b♯♭]*)\s*(.*)$", cleaned) + match = KEY_RE.match(cleaned) if not match: return Key(0, "major", text=cleaned) letter, accidentals, remainder = match.groups() diff --git a/tests/test_readme_links.py b/tests/test_readme_links.py new file mode 100644 index 00000000..8dbbb519 --- /dev/null +++ b/tests/test_readme_links.py @@ -0,0 +1,77 @@ +"""The README is the PyPI project page, so its links have to work from there. + +`pyproject.toml` sets `readme = "README.md"`, which means this file is rendered +verbatim as the long description on PyPI. A relative link like +`[notation](docs/notation.md)` resolves on GitHub and 404s on PyPI, and a +relative image does not render at all — which is how the published page came to +tell people to read a dozen documents none of which they could reach, and to run +example files that a `pip install` does not put on disk. + +So the links are absolute. The cost of that is losing the guarantee that they +point at something real, since nothing local resolves them any more. This test +buys it back: every GitHub URL into this repository is mapped back to a path and +checked to exist. +""" + +from __future__ import annotations + +import re +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent +README = ROOT / "README.md" + +BLOB = "https://github.com/SuperInstance/plainsong/blob/master/" +RAW = "https://raw.githubusercontent.com/SuperInstance/plainsong/master/" + +LINK_RE = re.compile(r"!?\[[^\]]*\]\(([^)\s]+)\)") + + +def targets() -> list[str]: + return LINK_RE.findall(README.read_text(encoding="utf-8")) + + +class TestTheReadmeWorksOnPyPI(unittest.TestCase): + def test_no_relative_links_remain(self): + """A relative link is invisible on PyPI, silently.""" + relative = [ + t + for t in targets() + if not t.startswith(("http://", "https://", "#", "mailto:")) + ] + self.assertEqual(relative, [], f"these 404 on the PyPI page: {relative}") + + def test_every_link_into_this_repo_points_at_a_real_file(self): + missing = [] + for target in targets(): + for prefix in (BLOB, RAW): + if target.startswith(prefix): + path = target[len(prefix) :].split("#")[0] + if not (ROOT / path).exists(): + missing.append(target) + self.assertEqual(missing, [], f"link points at a file that is not here: {missing}") + + def test_images_use_raw_rather_than_blob(self): + """A `blob` URL serves an HTML page, so an pointing at one is a + broken image on PyPI and in any other renderer.""" + images = re.findall(r"!\[[^\]]*\]\(([^)\s]+)\)", README.read_text(encoding="utf-8")) + wrong = [i for i in images if i.startswith(BLOB)] + self.assertEqual(wrong, [], f"image must use raw.githubusercontent.com: {wrong}") + + def test_the_version_it_claims_is_the_version_in_the_tree(self): + """The status line said "Version 1.0" while PyPI shipped 1.2.0.""" + from plainsong.version import __version__ + + claimed = re.search(r"(?m)^Version (\d+\.\d+)\.", README.read_text(encoding="utf-8")) + self.assertIsNotNone(claimed, "the README no longer states a version") + major_minor = ".".join(__version__.split(".")[:2]) + self.assertEqual( + claimed.group(1), + major_minor, + f"README says {claimed.group(1)}, the tree is {__version__}", + ) + + +if __name__ == "__main__": + unittest.main()