diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f2ec7704..8cf0923d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -62,6 +62,23 @@ jobs: - run: ruff format --check plainsong tests continue-on-error: true + # Every other job runs with the repository on sys.path, which is structurally + # blind to packaging. That is not hypothetical: the spec files once lived + # outside the package, so `plainsong spec` reported "no specs found" to + # everybody who installed rather than cloned -- through a release, with a + # fully green suite. This job builds a wheel, installs it into a clean venv + # and drives the console script from outside the checkout. + packaging: + name: the built wheel actually works + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + - run: pip install build + - run: python tools/verify_release.py --stage wheel + corpus: name: notation library still parses runs-on: ubuntu-latest diff --git a/CHANGELOG.md b/CHANGELOG.md index 41e96b7d..7d66b16c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,67 @@ Notable changes, newest first. Dates are ISO 8601. +## Unreleased + +### `plainsong mcp` is deprecated + +`plainsong-mcp` 1.0.0 is on PyPI, so for the first time there is somewhere to +send people. `plainsong mcp` still works and will keep working until 2.0; it now +warns and names the replacement. + +That duplication has already cost twice. A DNS-rebinding fix existed in this +copy and not the sibling for months -- in the copy people `pip install` for MCP. +Then the same eight lines got the same two things wrong in both, because the +second was a copy of the first. Neither repository could notice either time. + +The notice goes to **stderr**, and that is not a detail. In stdio mode stdout +*is* the protocol: a deprecation line printed there would desynchronise every +client, turning a courtesy into an outage. `PLAINSONG_NO_DEPRECATION=1` silences +it. A test drives the real subprocess and parses every stdout line as JSON, so +the rule is enforced rather than remembered. + + +### `plainsong spec` called finding nothing a pass + +It printed `no specs found` and exited **0**. That is the exact shape of the +fault it exists to catch: the spec files once sat in a top-level `specs/` +directory, outside the package, so every `pip install` shipped without them -- +and every install, and every CI job that ran `plainsong spec`, read that zero +as a pass. The self-verification the whole design leans on was doing nothing, +loudly enough to print a warning and quietly enough that nobody's exit status +moved. + +It exits 1 now, and says which of the two things happened: an install missing +its spec files, or a `--tag` nothing carries. This is a **behaviour change** -- +a script that ran `plainsong spec` against an install with no specs and treated +0 as success will now see a failure, which is the point. + +Found while building `tools/verify_release.py` (below): a wheel built with the +specs deliberately excluded still passed the packaging canary, because the +canary trusted the exit status. Verified directly -- that wheel carries zero +`spec_files` entries, and `plainsong spec` in a venv installed from it reports +`no specs found`. + +### A release is now verified from outside the tree + +Everything in `tests/` runs with the repository on `sys.path`, which is +structurally blind to packaging: the "no specs found" bug above lived through a +release with a fully green suite. `tools/verify_release.py` never imports +plainsong. It builds a wheel, installs it into a throwaway virtualenv outside +the source tree, and drives the console script from `/tmp` with `PYTHONPATH` +stripped, then repeats against what is actually on PyPI -- including driving +the MCP server with real JSON-RPC over stdio and checking that the sibling's +loopback re-export is the compiler's own function. + +CI gains a `packaging` job running `--stage wheel`, so a data file that stops +being packaged fails a pull request rather than a release. + +One trap it had to learn: setuptools copies the package into `build/lib` and +**reuses whatever is already there**, so a data file that has stopped being +packaged still reaches the wheel from the last build that did include it. A +broken package then verifies perfectly. The script clears `build/` and +`*.egg-info` before building for that reason. + ## 1.4.0 — 2026-08-18 A minor rather than a patch: `plainsong.runtime.localhost` is a new public diff --git a/CLAUDE.md b/CLAUDE.md index 1fcd911d..d0b33bb4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -35,7 +35,8 @@ python3 -m unittest tests.test_notation.TestArrange.test_tokens_divide_the_bar python3 -m pytest tests -q # works too, if pytest is installed # The system's checks on itself -- run these before and after any change -python3 -m plainsong spec # exits non-zero on failure +python3 -m plainsong spec # exits non-zero on failure, + # and on finding no specs at all python3 -m plainsong doctor --specs python3 -m plainsong check docs examples plainsong/songbook README.md # every source, prose included python3 -m plainsong fingerprint plainsong/songbook examples docs --check tests/corpus-fingerprint.txt @@ -43,6 +44,12 @@ python3 -m plainsong fingerprint plainsong/songbook examples docs --check tests/ # Checks CI cannot run -- no browser there. Run by hand after touching either side. python3 tools/demo_differential.py # the browser demo against the compiler +# The one check the test suite structurally cannot do: everything in tests/ runs +# with this repository on sys.path, so it never meets the artifact anyone +# installs. This builds a wheel, installs it outside the tree, and drives it. +python3 tools/verify_release.py --stage wheel # what CI gates on +python3 tools/verify_release.py # tree, wheel and PyPI + # Working with notation python3 -m plainsong new "Title" -o song.song python3 -m plainsong compile song.song -o out.mid --audio out.wav @@ -69,7 +76,11 @@ python3 -m plainsong setup # connect a model python3 -m plainsong build # tailor this install to the machine ``` -Every command takes `--json`. Use it when parsing output. +Every command takes `--json`, but it is a **global** flag and goes before the +subcommand: `plainsong --json info song.song`. Written after it, argparse +refuses the whole invocation with `unrecognized arguments: --json`. Use it when +parsing output, and note the totals live under `arrangement` -- `arrangement.notes` +is the note count, not a top-level key. Run the suite with `discover`, not by naming files. Several tests are about how modules behave when imported in a particular order, and a single-file run can @@ -203,6 +214,17 @@ there rather than by reasoning: `chmod` is a no-op on Windows, so permission tests must be skipped there rather than asserted around. +## How this project decides something is true + +`docs/verification.md` is the short version and it is worth reading before you +trust any green result here. The one-line summary: **success is not evidence**. +`plainsong spec` once printed "no specs found" and exited 0, so every install +missing its spec files reported a pass; a guard that no mutation can fail is +decoration; and a test suite with this repository on `sys.path` cannot see a +packaging bug, which is why `tools/verify_release.py` never imports plainsong. + +When you add a check, break the thing it checks and confirm it goes red. + ## Specs `plainsong/spec_files/*.toml` state what the system promises; @@ -255,10 +277,13 @@ re-parse to fetch diagnostics you have already computed. Related, and the reason that matters: an unrecognised token silently became a rest. `Xm9` compiled "ok, 0 warnings" and produced a bar of nothing. It now -warns. Turning that on immediately found that `EbMaj7`, `G7alt` and `CM7` are -legitimate spellings the chord parser does not accept and has been quietly -dropping — still open, and it wants a spec and a changelog entry because it -changes how existing notation compiles. +warns. Turning that on immediately found that `EbMaj7`, `G7alt` and `CM7` were +legitimate spellings the chord parser did not accept and had been quietly +dropping. **That is fixed** -- all three parse, and `chordsymbol.parse_symbol` +is the place to confirm it rather than this paragraph. The lesson the entry is +kept for is the one that generalises: the warning is what found them. Before +it, an unreadable chord and a deliberate rest were the same silence, and the +compiler reported `ok` for both. ## Changing the notation @@ -289,7 +314,10 @@ It lives inside the package because `plainsong library` and `plainsong play stand-by-me` found nothing for anyone who had not cloned. Two side effects worth knowing: the ~3,800 bar-count warnings this directory was -famous for came from those rows and are now 2. +famous for came from those rows and are now 1 -- the Hungarian Rhapsody's +`time: 2/4 (Lassan) then 4/4 (Friska)`, a human annotation the metre field +cannot express. That file is deliberately left alone, because changing the +metre would change the music. ## Rough edges @@ -298,6 +326,12 @@ famous for came from those rows and are now 2. - The host bridge cannot stream and reports no token usage. - **`plainsong/mcp/` also exists in `SuperInstance/plainsong-mcp`.** The one open violation of "one of everything". Do not build anything new on this copy. + **`plainsong mcp` is now deprecated and goes in 2.0.** It warns on stderr and + points at `pip install plainsong-mcp`, which is published and is the same + server maintained in one place. The notice cannot go on stdout -- that is the + protocol in stdio mode, and one stray line desynchronises every client, which + `tests/test_mcp.py::TestDeprecationNoticeStaysOffTheWire` holds by driving a + real subprocess and parsing every stdout line as JSON. **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 diff --git a/coordinate/LICENSE b/coordinate/LICENSE new file mode 100644 index 00000000..744aa530 --- /dev/null +++ b/coordinate/LICENSE @@ -0,0 +1,21 @@ +MIT License + +Copyright (c) 2026 SuperInstance + +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/coordinate/README.md b/coordinate/README.md new file mode 100644 index 00000000..bbd62817 --- /dev/null +++ b/coordinate/README.md @@ -0,0 +1,82 @@ +# coordinate + +**Scheduling backwards from when an effect should land.** + +One file, standard library only, MIT. Copy it next to whatever needs it. + +Most scheduling says when to *act*. This says when the effect should *arrive*, +and solves backwards for when each participant has to move: + +``` +act = intent·scale + shift − alignment·(actuation + lead + reference_delay + bias) +effect = act + actuation + lead + bias + observed_delay +``` + +## Staged for extraction + +This directory lives inside `SuperInstance/plainsong` for now because that is +where the idea was built and tested, and where the proof that the extraction is +inert can actually run. **It is destined for its own repository** and is not +part of the `plainsong` wheel — nothing under `plainsong/` imports it yet. + +## Why the two delay terms must not be collapsed + +`reference_delay` is the transport delay the plan was **compensated for**. +`observed_delay` is the delay **actually experienced** by whoever is watching +now. + +When they are equal the correction cancels exactly and the effect lands where +it was written. When they differ it does not, and the residue is real. It is +why `spread` — the gap between the earliest and latest effect — is zero at the +point you tuned for and non-zero everywhere else, and why a coordinated group +needs a conductor rather than mutual listening. + +Collapse those two into one variable and the model becomes symmetric, +self-consistent, and a description of nothing. + +## `intent` survives compensation; `lead` does not + +Swing is meant to be heard. A deliberate lead into a turn is meant to happen. +So `intent` moves the effect and is *not* solved away, while `lead` moves only +the action and *is* compensated. They look identical in the arithmetic and are +opposites in meaning, which is exactly why they are separate fields. + +## The same three quantities, three domains + +| | Orchestra | Boat helm | Camera / avatar cue | +|---|---|---|---| +| `actuation` | the instrument speaking | valve lag + hydraulic slew | rig acceleration, render lead | +| `bias` | habitual drag | linkage backlash | fixed pipeline stage | +| `reference_delay` | distance to the podium | conditions the autopilot was tuned in | the timing the sequence was cut against | +| `observed_delay` | distance to this listener | loaded, in current, in a seaway | this machine, this frame rate | +| `alignment` | ensemble discipline | trust in calibration right now | degraded mode | +| `intent` | swing, rubato | deliberate lead into a turn | an intentionally late reveal | +| `spread` | smear of a chord | **how far from tuning conditions you are** | cue drift across channels | + +That middle column is the one worth dwelling on. An autopilot tuned in flat +water at one speed is running with a `reference_delay` that no longer matches +its `observed_delay` once the boat is loaded or in a seaway. `spread` turns +that mismatch into a number you can put on a screen. + +## Proving a change is safe + +`test_equivalence.py` drives this and `plainsong.perform.solve` with the same +inputs and requires **bit-identical** results across ~4,000 combinations — +`assertEqual`, not `assertAlmostEqual`, because a reordered sum is a different +implementation even when it is close, and close is what accumulates. + +That is not a formality. Reordering one addition in `solve_one` — same terms, +same value mathematically — fails 1,803 of those cases. + +```bash +cd coordinate && python3 test_equivalence.py +``` + +## What is deliberately not here + +- **No I/O, no configuration, no logging.** It computes offsets in seconds. +- **No medium baked in.** `delay_for(distance, speed)` defaults to sound in air + and takes any speed. Where there is no distance at all, set the delays + directly and never call it. +- **No opinion about what a participant is.** A player, a steering pump, a + camera rig, an agent waiting on a message. diff --git a/coordinate/coordinate.py b/coordinate/coordinate.py new file mode 100644 index 00000000..79fbc982 --- /dev/null +++ b/coordinate/coordinate.py @@ -0,0 +1,307 @@ +"""Scheduling backwards from when an effect should land. + +Copyright (c) 2026 SuperInstance. MIT licensed -- see LICENSE beside this file. + +Most scheduling says when to *act*. This says when the effect should *arrive*, +and solves backwards for when each participant has to move. That inversion is +the whole idea, and it is worth stating plainly because everything else here +follows from it: + + A written time is an arrival time. + +Say four players must sound a chord together at the podium. They sit at +different distances, their instruments speak at different speeds, and some +drag by habit. If each acts on the beat, the chord arrives smeared. If each +acts early by exactly their own lag, it arrives together. The second is what +this computes. + +Nothing here is about music. The same three quantities describe a boat -- +a steering pump has valve lag (`actuation`), linkage backlash (`bias`), and +was tuned in conditions you are no longer in (`reference_delay` versus +`observed_delay`) -- or a camera rig, or a fleet of agents whose messages take +different times to land. + +The two delay terms are the part that must not be collapsed +------------------------------------------------------------ +`reference_delay` is the transport delay the plan was **compensated for**. +`observed_delay` is the delay **actually experienced** by whoever is watching +now. When they are equal the correction cancels exactly and the effect lands +where it was written. When they differ it does not, and the residue is real: +it is why a spread of zero at the tuning point becomes non-zero everywhere +else, and why a coordinated group needs a conductor rather than mutual +listening. + +Collapse those two into one variable and the model becomes symmetric, +self-consistent, and a description of nothing. + +`intent` survives compensation on purpose +----------------------------------------- +Swing is meant to be heard; a deliberate lead into a turn is meant to happen. +So `intent` moves the effect and is not solved away, while `lead` moves only +the action and *is* compensated. They are not the same knob and must not be +merged -- a correction and an intention look alike in the arithmetic and are +opposites in meaning. + +Stdlib only, one file, no configuration. Copy it next to whatever needs it. +""" + +from __future__ import annotations + +from collections.abc import Iterable, Sequence +from dataclasses import dataclass, field + +__all__ = [ + "Latency", + "Participant", + "Shaping", + "NEUTRAL", + "Timing", + "Schedule", + "schedule", + "delay_for", + "divide", + "SPEED_OF_SOUND", + "speed_of_sound", +] + + +# -- what a participant costs ------------------------------------------------- + + +@dataclass(frozen=True) +class Latency: + """How long one participant takes to turn an action into an effect. + + ``actuation`` + Seconds from beginning to act to the effect beginning. A bowed string + speaking, a hydraulic valve opening, a model starting to emit. + + ``bias`` + Seconds of systematic offset this participant carries regardless -- + backlash in a linkage, a habitual drag, a fixed pipeline stage. Split + from ``actuation`` because one is the cost of moving and the other is + an error you may be able to measure away. + """ + + name: str = "" + actuation: float = 0.0 + bias: float = 0.0 + note: str = "" + + +@dataclass(frozen=True) +class Participant: + """One actor, and the two transport delays that matter to it.""" + + name: str + latency: Latency = field(default_factory=Latency) + + intent: float = 0.0 + """Seconds this participant is *meant* to be off the written time. Survives + compensation, because it is an intention rather than an error.""" + + reference_delay: float = 0.0 + """Transport delay the plan was compensated for.""" + + observed_delay: float = 0.0 + """Transport delay actually experienced by the current observer. Equal to + ``reference_delay`` means observing from the point the plan was built for.""" + + +@dataclass(frozen=True) +class Shaping: + """A directive applied to every participant at one moment. + + ``intent_shift`` + Seconds added to the effect, and so to the action too. The whole group + leaning early or late together; the gaps between them do not change. + + ``lead`` + Extra seconds of preparation. The action moves earlier by exactly this + and the effect does not move at all, because it is compensated. + + ``alignment`` + How much of the correction is actually applied, 0 to 1. One is a group + that has it right; lower values let the effects spread apart, which is + what a degraded or distrusted calibration looks like. + + ``intent_scale`` + Scales each participant's own ``intent``, so a directive can pull + individuals onto one instant without discarding the group shift. + """ + + intent_shift: float = 0.0 + lead: float = 0.0 + alignment: float = 1.0 + intent_scale: float = 1.0 + + +NEUTRAL = Shaping() + + +# -- the answer --------------------------------------------------------------- + + +@dataclass(frozen=True) +class Timing: + """When one participant must act, and when its effect reaches the observer. + + Both are offsets in seconds from the written time. ``act_at`` is normally + negative: everyone moves early. ``effect_at`` of zero means the effect + lands exactly where it was written. + """ + + participant: Participant + act_at: float + effect_at: float + + @property + def name(self) -> str: + return self.participant.name + + def as_dict(self) -> dict[str, object]: + return { + "participant": self.name, + "profile": self.participant.latency.name, + "actuation_ms": round(self.participant.latency.actuation * 1000.0, 1), + "bias_ms": round(self.participant.latency.bias * 1000.0, 1), + "intent_ms": round(self.participant.intent * 1000.0, 1), + "reference_delay_ms": round(self.participant.reference_delay * 1000.0, 1), + "observed_delay_ms": round(self.participant.observed_delay * 1000.0, 1), + "act_at_ms": round(self.act_at * 1000.0, 1), + "effect_at_ms": round(self.effect_at * 1000.0, 1), + } + + +@dataclass(frozen=True) +class Schedule: + """Every participant's timing, as seen from one observation point.""" + + timings: dict[str, Timing] = field(default_factory=dict) + compensated: bool = True + + @property + def spread(self) -> float: + """Seconds between the earliest and latest effect of one written time. + + Zero when every observed delay matches its reference -- which is the + observation point the plan was built for, and nowhere else. Reading + this live tells you how far you are from the conditions you tuned in. + """ + if not self.timings: + return 0.0 + landings = [timing.effect_at for timing in self.timings.values()] + return max(landings) - min(landings) + + def __iter__(self) -> Iterable[Timing]: + return iter(self.timings.values()) + + def __len__(self) -> int: + return len(self.timings) + + def as_dict(self) -> dict[str, object]: + return { + "compensated": self.compensated, + "spread_ms": round(self.spread * 1000.0, 1), + "participants": [timing.as_dict() for timing in self.timings.values()], + } + + +def solve_one( + participant: Participant, + shaping: Shaping = NEUTRAL, + compensate: bool = True, +) -> Timing: + """The equation, and the only place it is written down. + + act = intent·scale + shift − alignment·(actuation + lead + reference + bias) + effect = act + actuation + lead + bias + observed + + With ``alignment`` at one and the observer at the reference point, the + second line cancels the first and the effect lands exactly where it was + written -- give or take the intent, which is meant to be seen. + + ``compensate=False`` drops the correction entirely: participants act on the + written time and the effects land wherever the physics puts them. That is + the honest default for a plan that never declared any latencies. + """ + alignment = shaping.alignment if compensate else 0.0 + latency = participant.latency + + correction = latency.actuation + shaping.lead + participant.reference_delay + latency.bias + act = participant.intent * shaping.intent_scale + shaping.intent_shift - alignment * correction + effect = act + latency.actuation + shaping.lead + latency.bias + participant.observed_delay + + return Timing(participant=participant, act_at=act, effect_at=effect) + + +def schedule( + participants: Sequence[Participant], + shaping: Shaping = NEUTRAL, + compensate: bool = True, +) -> Schedule: + """Solve every participant against one observation point.""" + return Schedule( + timings={p.name: solve_one(p, shaping, compensate) for p in participants}, + compensated=compensate, + ) + + +# -- turning a distance into a delay ------------------------------------------ + +SPEED_OF_SOUND = 343.2 +"""Metres per second in air at 20 degrees. A default, not an assumption.""" + + +def speed_of_sound(temperature_c: float = 20.0) -> float: + """``331.3 · sqrt(1 + T/273.15)`` -- 343.2 m/s at 20 degrees.""" + return 331.3 * (1.0 + temperature_c / 273.15) ** 0.5 + + +def delay_for(distance: float, speed: float = SPEED_OF_SOUND) -> float: + """Seconds for something to cross ``distance`` at ``speed``. + + The medium is a parameter because it is the only physical assumption in + this file. Sound in air is the default; a signal on a wire, a hydraulic + line, or a network hop is the same arithmetic with a different constant -- + and where there is no distance at all, set the delays directly and never + call this. + """ + if speed <= 0.0: + raise ValueError("speed must be positive") + return distance / speed + + +# -- dividing an interval ----------------------------------------------------- + +_EPSILON = 1e-9 + + +def divide(count: int, span: float = 1.0, start: float = 0.0) -> list[float]: + """Where ``count`` items fall when they divide ``span`` between them. + + The rule is that the interval is one interval long and its contents divide + it: three items are thirds, twelve are twelfths, and a thirteenth cannot + spill into the next one. Positions are computed from ``start`` rather than + accumulated, because accumulation drifts and the drift is invisible until + something far away lands on the wrong side of a boundary. + """ + if count <= 0: + return [] + width = span / count + return [start + index * width for index in range(count)] + + +def index_at(position: float, count: int, span: float = 1.0, start: float = 0.0) -> int: + """Which of ``count`` slots ``position`` falls in. + + Nudged before flooring. Positions are produced by division, so a boundary + arrives as ``0.29999999999999993`` about as often as ``0.3``, and a bare + floor puts it in the slot below. + """ + if count <= 0: + raise ValueError("count must be positive") + if span <= 0.0: + raise ValueError("span must be positive") + offset = (position - start) / span * count + return max(0, min(count - 1, int(offset + _EPSILON))) diff --git a/coordinate/test_equivalence.py b/coordinate/test_equivalence.py new file mode 100644 index 00000000..eed25bf3 --- /dev/null +++ b/coordinate/test_equivalence.py @@ -0,0 +1,226 @@ +"""The neutral core must answer exactly what the music solver answers. + +An extraction is only safe if it is inert, and "I read both and they look the +same" is not evidence -- this repository has paid for that lesson more than +once. So this drives `plainsong.perform.solve` and `coordinate` with the same +inputs and requires bit-identical results, over a grid of cases chosen to hit +the terms that could plausibly diverge: alignment off, intent scaled, an +observer away from the reference point, and a lead that must move the action +without moving the effect. + +If this passes, `coordinate.solve_one` can replace the equation in +`VoiceTiming.offsets` without moving a single note. + +Run: python3 -m unittest discover -s coordinate -t . +""" + +from __future__ import annotations + +import sys +import unittest +from itertools import product +from pathlib import Path + +sys.path.insert(0, str(Path(__file__).resolve().parent)) +sys.path.insert(0, str(Path(__file__).resolve().parent.parent)) + +from coordinate import Latency, Participant, Shaping, delay_for, divide, index_at, schedule +from plainsong.perform.solve import Shaping as MusicShaping +from plainsong.perform.solve import VoiceTiming + +# Deliberately awkward numbers. Round ones hide ordering differences, because +# floating-point addition is not associative and 0.5 + 0.25 is exact. +ACTUATION = (0.0, 0.0031, 0.017, 0.1234567) +BIAS = (0.0, -0.0042, 0.0091) +INTENT = (0.0, 0.0237, -0.0158) +REFERENCE = (0.0, 0.0291, 0.1013) +OBSERVED = (0.0, 0.0291, 0.0044, 0.2007) + +SHAPINGS = ( + (0.0, 0.0, 1.0, 1.0), # neutral + (0.0, 0.0, 0.0, 1.0), # alignment off -- no correction applied + (0.0, 0.0, 0.35, 1.0), # partial trust + (0.021, 0.0, 1.0, 1.0), # the whole group leaning + (0.0, 0.013, 1.0, 1.0), # lead: moves the action, not the effect + (0.0, 0.013, 0.5, 0.0), # lead under partial alignment, intent discarded + (-0.007, 0.004, 0.8, 2.0), # everything at once +) + + +def music_offsets(actuation, bias, intent, reference, observed, shaping, compensate): + timing = VoiceTiming( + name="v", + position=(0.0, 0.0), + profile="test", + speech=actuation, + p_center=bias, + feel=intent, + reference_distance=0.0, + reference_propagation=reference, + observed_distance=0.0, + observed_propagation=observed, + emission_offset=0.0, + arrival_offset=0.0, + ) + return timing.offsets( + MusicShaping( + feel=shaping[0], preparation=shaping[1], alignment=shaping[2], feel_scale=shaping[3] + ), + compensate, + ) + + +def neutral_offsets(actuation, bias, intent, reference, observed, shaping, compensate): + participant = Participant( + name="v", + latency=Latency(name="test", actuation=actuation, bias=bias), + intent=intent, + reference_delay=reference, + observed_delay=observed, + ) + result = schedule( + [participant], + Shaping( + intent_shift=shaping[0], lead=shaping[1], alignment=shaping[2], intent_scale=shaping[3] + ), + compensate, + ) + timing = result.timings["v"] + return timing.act_at, timing.effect_at + + +class TestTheExtractionIsInert(unittest.TestCase): + def test_every_combination_agrees_to_the_bit(self): + cases = 0 + for actuation, bias, intent, reference, observed, shaping, compensate in product( + ACTUATION, BIAS, INTENT, REFERENCE, OBSERVED, SHAPINGS, (True, False) + ): + args = (actuation, bias, intent, reference, observed, shaping, compensate) + with self.subTest(args=args): + # assertEqual, not assertAlmostEqual. A reordered sum is a + # different implementation even when it is close, and close is + # what accumulates. + self.assertEqual(music_offsets(*args), neutral_offsets(*args)) + cases += 1 + self.assertGreater(cases, 3000, "the grid collapsed; this proves nothing") + + +class TestTheClaimsThatMakeItGeneral(unittest.TestCase): + """The properties the docstring asserts. If these are not true, the + generalisation is decoration.""" + + def test_at_the_reference_point_the_effect_lands_where_written(self): + p = Participant( + "a", Latency(actuation=0.02, bias=0.005), reference_delay=0.03, observed_delay=0.03 + ) + timing = schedule([p]).timings["a"] + self.assertAlmostEqual(timing.effect_at, 0.0, places=12) + self.assertLess(timing.act_at, 0.0, "the participant must act early") + + def test_away_from_the_reference_point_it_does_not(self): + p = Participant( + "a", Latency(actuation=0.02, bias=0.005), reference_delay=0.03, observed_delay=0.09 + ) + timing = schedule([p]).timings["a"] + self.assertAlmostEqual(timing.effect_at, 0.06, places=12) + + def test_spread_is_zero_at_the_tuning_point_and_not_elsewhere(self): + """The claim the whole model rests on: a group compensated for one + observer lands together *there* and smeared anywhere else.""" + near = Participant("near", Latency(actuation=0.01), reference_delay=0.01, observed_delay=0.01) + far = Participant("far", Latency(actuation=0.03), reference_delay=0.08, observed_delay=0.08) + self.assertAlmostEqual(schedule([near, far]).spread, 0.0, places=12) + + moved = replace_observed(near, 0.05), replace_observed(far, 0.02) + self.assertGreater(schedule(list(moved)).spread, 0.0) + + def test_intent_survives_compensation_and_lead_does_not(self): + """The distinction that must never be merged: one is an intention, the + other a correction, and they are the same shape in the arithmetic.""" + base = Participant("a", Latency(actuation=0.02), reference_delay=0.01, observed_delay=0.01) + + intended = replace_intent(base, 0.05) + self.assertAlmostEqual(schedule([intended]).timings["a"].effect_at, 0.05, places=12) + + led = schedule([base], Shaping(lead=0.05)).timings["a"] + self.assertAlmostEqual(led.effect_at, 0.0, places=12) + plain = schedule([base]).timings["a"] + self.assertAlmostEqual(led.act_at, plain.act_at - 0.05, places=12) + + def test_without_compensation_nothing_is_solved_away(self): + p = Participant("a", Latency(actuation=0.02, bias=0.01), observed_delay=0.03) + timing = schedule([p], compensate=False).timings["a"] + self.assertEqual(timing.act_at, 0.0) + self.assertAlmostEqual(timing.effect_at, 0.06, places=12) + + +class TestIntervalDivision(unittest.TestCase): + def test_contents_divide_the_interval(self): + self.assertEqual(divide(4), [0.0, 0.25, 0.5, 0.75]) + self.assertEqual(len(divide(12)), 12) + + def test_nothing_spills_past_the_end(self): + for count in range(1, 65): + with self.subTest(count=count): + self.assertLess(max(divide(count)), 1.0) + + def test_positions_are_computed_not_accumulated(self): + """Accumulation drifts, and which divisors drift is not guessable. + + A twelfth added twelve times lands on exactly 1.0; a seventh added + seven times lands on 0.9999999999999998, and a ninth overshoots. So + "it worked when I tried it" is worth nothing here -- the ones that + drift are found by measuring, which is the argument for computing + every position from the start rather than walking a cursor. + """ + drifted = [] + for count in range(2, 65): + accumulated = 0.0 + for _ in range(count): + accumulated += 1.0 / count + if accumulated != 1.0: + drifted.append(count) + self.assertGreater(len(drifted), 20, "expected many divisors to drift") + self.assertIn(7, drifted) + self.assertNotIn(12, drifted) # the intuitive example is the exact one + + # Computed positions never drift, whichever divisor it is. + for count in drifted: + with self.subTest(count=count): + self.assertEqual(divide(count)[0], 0.0) + self.assertLess(max(divide(count)), 1.0) + self.assertEqual(divide(3, span=12.0)[2], 8.0) + + def test_a_boundary_lands_in_the_slot_it_belongs_to(self): + """0.3 arrives from division as 0.29999999999999993; a bare floor puts + it one slot low.""" + for count in (3, 5, 6, 7, 12, 13): + for index, position in enumerate(divide(count)): + with self.subTest(count=count, index=index): + self.assertEqual(index_at(position, count), index) + + +class TestMediumIsAParameter(unittest.TestCase): + def test_sound_is_the_default_not_the_assumption(self): + self.assertAlmostEqual(delay_for(343.2), 1.0, places=6) + self.assertAlmostEqual(delay_for(1.0, speed=2.0), 0.5, places=12) + + def test_a_nonsense_medium_is_refused(self): + with self.assertRaises(ValueError): + delay_for(1.0, speed=0.0) + + +def replace_observed(participant: Participant, observed: float) -> Participant: + from dataclasses import replace as _replace + + return _replace(participant, observed_delay=observed) + + +def replace_intent(participant: Participant, intent: float) -> Participant: + from dataclasses import replace as _replace + + return _replace(participant, intent=intent) + + +if __name__ == "__main__": + unittest.main() diff --git a/docs/verification.md b/docs/verification.md new file mode 100644 index 00000000..1058cc51 --- /dev/null +++ b/docs/verification.md @@ -0,0 +1,138 @@ +# Verifying things + +Every rule here is a fault that was actually paid for, most of them in this +repository and several of them in sibling repositories during one week. None is +a style preference. They are written down because each one cost time that a +paragraph would have saved, and because the failure mode they share is the same: +**a claim outran the thing that was supposed to check it, and nothing noticed.** + +This file is deliberately portable. Nothing in it is about music. + +--- + +## 1. Success is not evidence + +A green result means the check passed. It does not mean the check ran, or that +it was checking what you think. + +- **`plainsong spec` printed `no specs found` and exited 0.** The spec files had + once lived outside the package, so every `pip install` shipped without them. + Every install, and every CI job running `plainsong spec`, read that zero as a + pass. The self-verification the whole design leans on was doing nothing — + loudly enough to print a warning, quietly enough that no exit status moved. + It exits 1 now. + +- **A sibling's CI suppresses its own build.** `make lib || echo "...syntax + check only"` and `nvcc -fsyntax-only ... || true`. That pipeline has never + failed and has never once demonstrated the code compiles. A step that cannot + fail is decoration. + +- **Six consecutive release runs failed at the same step and nobody read the + log.** The `test` and `build` jobs passed every time, which made the failure + easy to keep mis-reading. A release shipped a fix for the wrong cause, + confidently, in a changelog. The actual error had been printed in full, six + times: `invalid-publisher`. + +**The rule:** when something reports success, ask what specific observation +would have made it report failure. If you cannot name one, you have not +verified anything. + +## 2. A guard you cannot fail is not a guard + +Write the check, then **break the thing on purpose and confirm the check goes +red.** If the suite stays green, the check is decoration and you have learned +that before relying on it, which is the only good time to learn it. + +- Two guards in `TimeGrid` — a rounding step and an epsilon nudge — did the same + job. No mutation could fail a test, because either one alone was sufficient. + Collapsing to the nudge alone produced a guard that *did* fail when removed. + +- Reordering a single addition in `coordinate.solve_one` — same terms, same + mathematical value, different floating-point association — fails **1,803** of + ~4,000 equivalence cases. That is what makes the extraction proof real rather + than a reading exercise. + +- Replacing a re-export with a local shim that forwards to the same function — + behaviourally identical on every input — turns the sibling's suite red, + because the test asserts *identity*, not equality. That catches a second + implementation appearing before the two have had any chance to disagree. + +**The rule:** an assertion you have never seen fail is a hypothesis. + +## 3. Two copies drift into the same bug + +Not into different bugs — the same one, because the second copy was a copy. + +The loopback check lived twice. Both copies accepted `127.evil.example` +(a registrable domain that can be pointed at 127.0.0.1 — precisely the attack +the check exists to stop) and both mangled `[::1]` into `":"`, refusing a real +local caller. One fix, applied once, would have fixed neither. + +A 300-line analysis module was byte-identical across two repositories. A +security fix existed in one copy and not the other for months, in the copy +people `pip install`. + +**The rule:** when you are about to copy a definition, don't. If you must, add +a test that fails when the copies diverge — an identity assertion where the +languages allow it, a differential test where they do not. + +## 4. Verify by doing, not by asking + +The thing that tells you about the world is often not the world. + +- **PyPI's JSON API reported an older version than `pip` then resolved** — twice + in one week. `pip install` into a clean environment is the answer; the API is + a rumour. + +- **A test suite with the repository on `sys.path` is structurally blind to + packaging.** The "no specs found" bug above lived through an entire release + with a fully green suite, because the tests never met the artifact anyone + installs. `tools/verify_release.py` builds a wheel, installs it into a + throwaway virtualenv *outside the source tree*, and drives the console script + from `/tmp` with `PYTHONPATH` stripped. + +- **setuptools reuses whatever is already in `build/lib`.** A data file that has + stopped being packaged still reaches the wheel from the last build that + included it, so a broken package verifies perfectly. Clear `build/` and + `*.egg-info` before any build you intend to trust. This was found by breaking + the packaging deliberately and watching the check pass anyway. + +- **A pipe eats the exit code.** `some_command | tail` reports `tail`'s status. + A verification run reported exit 0 while printing two failures. Redirect to a + file and read `$?`, or check `PIPESTATUS`. + +**The rule:** verify the artifact people receive, in the environment they +receive it in. + +## 5. A name is not a description, and a README is not a fact + +- A repository called `magda-tensor` contains no tensors; it is a DAW fork. +- A repository's own audit graded its README **F**, listing three headline + claims as false or unsupported, and concluded: *"treat the README as + advertising copy, not technical fact."* The README was unchanged. +- A file named `bridge.rs` was markdown prose pasted into a `.rs` file. It would + not compile and was not in the build. +- A package's `setup.py` declared `license="MIT"` with the OSI classifier over a + repository containing no LICENSE file at all — the claim shipped in every + wheel, the grant in none. +- A generated design document proposed three phases of architecture built on a + library that does not exist, using vocabulary borrowed from an unrelated + repository that happened to be real. + +**The rule:** clone it and grep it. Reasoning about a repository from its name, +its README, or a document describing it is not research. "Not found" is a +valuable answer and should be reported as confidently as a discovery. + +## 6. Report what happened + +If tests fail, say so with the output. If a step was skipped, say that. When +something is verified, state it plainly with the evidence and without hedging. + +Two failures in a verification run turned out to be the harness's own bugs +rather than the product's — an argument written in the wrong position and a JSON +key read at the wrong level. Diagnosing that honestly took ten minutes. +Reporting it as a product defect would have cost considerably more, and +reporting it as success would have cost the most of all. + +**The rule:** the point of a check is to change what you believe. A check whose +result you would explain away is not one you are running. diff --git a/plainsong/interfaces/cli.py b/plainsong/interfaces/cli.py index d24f4830..42eb91dc 100644 --- a/plainsong/interfaces/cli.py +++ b/plainsong/interfaces/cli.py @@ -879,8 +879,16 @@ def cmd_spec(args: argparse.Namespace, config: Config, out: Out) -> int: results = verify_all(paths=config.paths, tag=args.tag) out.data([result.as_dict() for result in results]) if not results: - out.warn("no specs found") - return 0 + # Finding nothing to verify is not success. The specs once shipped + # outside the package, so every pip install answered "no specs found" + # and exited 0 -- the self-verification the whole design leans on, + # quietly doing nothing, with every caller reading that zero as a pass. + # A packaging regression has to be loud here or it is invisible. + if args.tag: + out.warn(f"no specs are tagged {args.tag!r}") + else: + out.warn("no specs found -- this install is missing its spec files") + return 1 out.say(format_results(results, verbose=args.verbose)) return 1 if any(result.status == "FAIL" for result in results) else 0 @@ -1088,9 +1096,36 @@ def cmd_serve(args: argparse.Namespace, config: Config, out: Out) -> int: return serve(config, host=host, port=port, open_browser=args.open, out=out) +MCP_DEPRECATION = ( + "warning: `plainsong mcp` is deprecated and will be removed in 2.0.\n" + " Use the dedicated package instead: pip install plainsong-mcp\n" + " It is the same server, maintained in one place rather than two.\n" + " Set PLAINSONG_NO_DEPRECATION=1 to silence this." +) + + +def _warn_mcp_deprecated() -> None: + """Say it on stderr, never stdout, and never through `out`. + + In stdio mode stdout *is* the protocol: one stray line desynchronises the + client, which is why `cmd_mcp` prints nothing there. stderr is the only + channel safe in all three modes, and MCP clients treat it as a log. + + `plainsong/mcp/` duplicates `SuperInstance/plainsong-mcp`, and that + duplication has already cost twice -- a DNS-rebinding fix that existed in + one copy and not the other for months, then the same eight lines getting + the same two things wrong in both. The sibling is on PyPI now, so for the + first time there is somewhere to send people. + """ + if os.environ.get("PLAINSONG_NO_DEPRECATION"): + return + print(MCP_DEPRECATION, file=sys.stderr) + + def cmd_mcp(args: argparse.Namespace, config: Config, out: Out) -> int: from ..mcp.server import Server, serve_http, serve_stdio + _warn_mcp_deprecated() server = Server(config=config) if args.list_tools: for spec in sorted(server.registry.specs(), key=lambda spec: spec.name): diff --git a/tests/test_mcp.py b/tests/test_mcp.py index 743e062f..73261dd9 100644 --- a/tests/test_mcp.py +++ b/tests/test_mcp.py @@ -10,6 +10,8 @@ import http.client import io import json +import subprocess +import sys import tempfile import threading import unittest @@ -290,6 +292,53 @@ def write(self, text: str) -> int: self.assertEqual(protocol.serve_stdio(server.dispatcher, reader, ClosedPipe()), 0) +class TestDeprecationNoticeStaysOffTheWire(unittest.TestCase): + """`plainsong mcp` is deprecated, and saying so must not break it. + + stdout *is* the protocol in stdio mode. A deprecation line printed there + would desynchronise every client -- turning a courtesy into an outage -- + which is why the notice goes to stderr and why this test exists rather + than a reading of the code. + """ + + def _run(self, args: list[str], env_extra: dict | None = None) -> subprocess.CompletedProcess: + import os + + env = dict(os.environ) + env.pop("PLAINSONG_NO_DEPRECATION", None) + env.update(env_extra or {}) + root = Path(__file__).resolve().parent.parent + return subprocess.run( + [sys.executable, "-m", "plainsong", *args], + input='{"jsonrpc":"2.0","id":1,"method":"initialize","params":{}}\n', + capture_output=True, + text=True, + timeout=60, + cwd=str(root), + env=env, + ) + + def test_stdout_carries_only_json_rpc(self) -> None: + finished = self._run(["mcp"]) + for line in finished.stdout.splitlines(): + if not line.strip(): + continue + # Fails loudly with the offending line rather than a bare False. + try: + json.loads(line) + except ValueError: # pragma: no cover - the failure path is the point + self.fail(f"non-protocol line on stdout: {line!r}") + + def test_the_notice_is_on_stderr_and_names_the_replacement(self) -> None: + finished = self._run(["mcp"]) + self.assertIn("deprecated", finished.stderr) + self.assertIn("plainsong-mcp", finished.stderr) + + def test_it_can_be_silenced(self) -> None: + finished = self._run(["mcp"], {"PLAINSONG_NO_DEPRECATION": "1"}) + self.assertNotIn("deprecated", finished.stderr) + + class TestHttpTransport(unittest.TestCase): def setUp(self) -> None: self.temporary = tempfile.TemporaryDirectory() diff --git a/tests/test_runtime.py b/tests/test_runtime.py index 9a56bbaa..d1baa8eb 100644 --- a/tests/test_runtime.py +++ b/tests/test_runtime.py @@ -180,6 +180,36 @@ def test_specs_load(self): self.assertTrue(spec.title) self.assertTrue(spec.checks, f"{spec.id} has no checks") + def test_finding_no_specs_is_a_failure_not_a_pass(self): + """`plainsong spec` answered "no specs found" and exited 0. + + That is the shape of the bug it exists to catch. The spec files once + sat in a top-level `specs/` directory, outside the package, so every + pip install shipped without them -- and every install, and every CI job + that ran `plainsong spec`, read that zero as a pass. The + self-verification the design leans on was doing nothing, loudly enough + to print a warning and quietly enough that nobody's exit status moved. + + Found again while building `tools/verify_release.py`: a wheel built with + the specs excluded still passed the packaging canary, because the canary + trusted the exit status. + """ + import argparse + import contextlib + import io + + from plainsong.interfaces.cli import Out, cmd_spec + from plainsong.runtime.config import load_config + + config = load_config() + # A tag nothing carries is the reachable way to make the result set + # empty without dismantling the install. + args = argparse.Namespace(list=False, tag="no-such-tag-exists", verbose=False) + buffer = io.StringIO() + with contextlib.redirect_stdout(buffer): + status = cmd_spec(args, config, Out()) + self.assertEqual(status, 1, "an empty spec run must not report success") + def test_all_specs_pass(self): failures = [result for result in verify_all() if result.status == "FAIL"] self.assertEqual( diff --git a/tools/verify_release.py b/tools/verify_release.py index 06f55ef8..ddf2969d 100644 --- a/tools/verify_release.py +++ b/tools/verify_release.py @@ -16,8 +16,9 @@ different question: the tree can be right and the upload can be a version behind. - python3 tools/verify_release.py # tree, wheel and PyPI - python3 tools/verify_release.py --local # skip the network + python3 tools/verify_release.py # tree, wheel and PyPI + python3 tools/verify_release.py --stage wheel # what CI gates on + python3 tools/verify_release.py --local # tree and wheel, no network python3 tools/verify_release.py --json Exit status is 0 only if every check passed. Nothing here is skipped silently: @@ -169,11 +170,19 @@ def exercise(binary: Path, report: Report, label: str, expect_version: str | Non # The packaging canary. `spec` reads the spec_files/ TOMLs out of the # installed package, so it fails loudly when package-data is wrong -- which # is the failure the test suite structurally cannot see. + # An exit status alone was not enough here, and finding that out is the + # reason this check is worded so carefully. `plainsong spec` used to answer + # "no specs found" and exit 0, so a wheel carrying none of them passed this + # check. The CLI now exits non-zero for that, but the canary should not + # depend on any single signal: require that specs were actually found and + # that none failed. code, out = run([str(plainsong), "spec"], cwd=outside) + last = out.splitlines()[-1] if out else "" + found = "no specs found" not in out and " passed," in out report.check( f"[{label}] specs pass from the install (packaging canary)", - code == 0, - out.splitlines()[-1] if out else "", + code == 0 and found, + last, ) # The songbook is package data too, and shipped for exactly this command. @@ -249,6 +258,17 @@ def exercise(binary: Path, report: Report, label: str, expect_version: str | Non def check_wheel(report: Report) -> Path | None: """Build the tree and install the wheel somewhere clean.""" print("\nbuilt wheel") + + # Clear `build/` first, and this is not housekeeping. setuptools copies the + # package into `build/lib` and *reuses whatever is already there*, so a data + # file that has stopped being packaged still reaches the wheel from the last + # build that did include it. A broken package then verifies perfectly. This + # was found the hard way: removing the specs from `package-data` and + # rebuilding produced a wheel that still carried all seven of them, and this + # script reported ten checks passed. + for stale in (ROOT / "build", *ROOT.glob("*.egg-info")): + shutil.rmtree(stale, ignore_errors=True) + dist = Path(tempfile.mkdtemp(prefix="plainsong-dist-")) code, out = run([sys.executable, "-m", "build", "--outdir", str(dist)], cwd=ROOT) wheels = sorted(dist.glob("*.whl")) @@ -349,16 +369,36 @@ def check_pypi(report: Report) -> None: def main() -> int: parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument( + "--stage", + action="append", + choices=("tree", "wheel", "pypi"), + help="run only this stage; repeatable. Default: all three. " + "CI runs `--stage wheel`, the tree already being covered by the other jobs.", + ) parser.add_argument("--local", action="store_true", help="skip the checks that need the network") parser.add_argument("--json", action="store_true", help="machine-readable summary on stdout") args = parser.parse_args() + stages = set(args.stage or ("tree", "wheel", "pypi")) + if args.local: + stages.discard("pypi") + report = Report() - check_tree(report) - check_wheel(report) - if not args.local: + if "tree" in stages: + check_tree(report) + if "wheel" in stages: + check_wheel(report) + if "pypi" in stages: check_pypi(report) + # A run that checked nothing must not report success -- that is the same + # confusion between "we did not look" and "it was fine" that the rest of + # this script exists to prevent. + if not report.results: + print("no stages selected, so nothing was verified", file=sys.stderr) + return 1 + passed = len(report.results) - len(report.failed) if args.json: print(json.dumps({