From 426f82e5f6d3729536a942d82fe3e4b021428b06 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 21:29:18 +0000 Subject: [PATCH 1/6] Stop treating "nothing to verify" as verified `plainsong spec` printed "no specs found" and exited 0. That is the exact shape of the fault it exists to catch: the spec files once sat outside the package, so every pip install shipped without them, and every install and 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 warn and quietly enough that no exit status moved. It exits 1 now and says which of the two cases happened. Found while writing tools/verify_release.py, which exists because everything in tests/ runs with the repository on sys.path and is therefore blind to packaging. It builds a wheel, installs it into a throwaway venv outside the source tree, drives the console script from /tmp with PYTHONPATH stripped, then does the same against PyPI -- including a real JSON-RPC stdio session against the MCP server and an identity check that the sibling's loopback re-export is the compiler's own function. Two things it had to learn, both by being wrong first. setuptools reuses whatever is already in build/lib, so a data file that has stopped being packaged still reaches the wheel from the last build that included it; the script clears build/ and *.egg-info first. And the packaging canary cannot trust an exit status alone, so it now requires that specs were found as well as that none failed. CI gains a packaging job running --stage wheel, so a data file that stops being packaged fails a pull request rather than a release. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PBAjxy7cD6DzJ72NX8TJEc --- .github/workflows/ci.yml | 17 ++++++++++++ CHANGELOG.md | 43 +++++++++++++++++++++++++++++ plainsong/interfaces/cli.py | 12 +++++++-- tests/test_runtime.py | 30 +++++++++++++++++++++ tools/verify_release.py | 54 ++++++++++++++++++++++++++++++++----- 5 files changed, 147 insertions(+), 9 deletions(-) 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..2d7d18b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,49 @@ Notable changes, newest first. Dates are ISO 8601. +## Unreleased + +### `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/plainsong/interfaces/cli.py b/plainsong/interfaces/cli.py index d24f4830..2567a5fc 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 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({ From 3aeb06b670ca2083b57060d03a574a29d8c83ba2 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 22:01:36 +0000 Subject: [PATCH 2/6] Extract the arrival-time solver as a domain-neutral core A written time is an arrival time, and the solver works backwards from it to when each participant must act. That idea is the one genuinely novel thing in this repository and none of it is about music: the equation is six lines of float arithmetic wearing instrument names. `speech` is actuation latency, `p_center` is systematic bias, and the reference/observed propagation split is "the conditions this was tuned for" against "the conditions it is in". coordinate/ is that equation with the music vocabulary removed, plus interval division and a medium that is a parameter rather than an assumption. One file, standard library only, Apache-2.0 so it can sit beside the robotics ecosystem, which is Apache-2.0 throughout. It is staged here because this is where the proof can run; it belongs in its own repository and nothing under plainsong/ imports it. The proof is what matters. test_equivalence.py drives coordinate and plainsong.perform.solve with the same inputs over ~4,000 combinations and requires bit-identical results -- assertEqual, not assertAlmostEqual, because a reordered sum is a different implementation even when it is close, and close is what accumulates. Reordering a single addition in solve_one, same terms and same mathematical value, fails 1,803 of those cases. So the extraction is inert by measurement rather than by reading. It also pins the four claims that make the thing general rather than decorative: the effect lands where written at the reference point and does not away from it, spread is zero at the tuning point and positive elsewhere, and intent survives compensation while lead does not. One test assertion was wrong on the first run and is worth keeping the lesson from: accumulating a twelfth twelve times lands on exactly 1.0, while a seventh does not. Which divisors drift is not guessable, so the test now measures them instead of asserting a favourite example. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PBAjxy7cD6DzJ72NX8TJEc --- coordinate/README.md | 82 +++++++++ coordinate/coordinate.py | 307 +++++++++++++++++++++++++++++++++ coordinate/test_equivalence.py | 226 ++++++++++++++++++++++++ 3 files changed, 615 insertions(+) create mode 100644 coordinate/README.md create mode 100644 coordinate/coordinate.py create mode 100644 coordinate/test_equivalence.py diff --git a/coordinate/README.md b/coordinate/README.md new file mode 100644 index 00000000..05e190cc --- /dev/null +++ b/coordinate/README.md @@ -0,0 +1,82 @@ +# coordinate + +**Scheduling backwards from when an effect should land.** + +One file, standard library only, Apache-2.0. 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..a39ad5f0 --- /dev/null +++ b/coordinate/coordinate.py @@ -0,0 +1,307 @@ +"""Scheduling backwards from when an effect should land. + +Copyright 2026 SuperInstance. Licensed under the Apache License, Version 2.0. + +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() From 212de4f789b9572e597c5390db3c07a5b164c2f8 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 14:06:45 +0000 Subject: [PATCH 3/6] Correct three claims in CLAUDE.md that had stopped being true This file says its rules are faults that were actually paid for, which only works if the file is true. Each of these was checked by running the thing, not by reading around it. `EbMaj7`, `G7alt` and `CM7` were described as spellings the chord parser does not accept, "still open". All three parse. The entry is kept, because the lesson generalises and the fix does not: the warning on an unrecognised token is what found them, and before it an unreadable chord and a deliberate rest were the same silence with the compiler reporting ok for both. The songbook's bar-count warnings were given as 2. There is 1 -- the Hungarian Rhapsody's `time: 2/4 (Lassan) then 4/4 (Friska)`, a human annotation the metre field cannot express, left alone deliberately because changing the metre would change the music. Naming it is more useful than counting it. "Every command takes --json" is true only for the global position. Written after the subcommand, argparse refuses the whole invocation with `unrecognized arguments: --json`. A reader following that sentence literally gets an error, which is how tools/verify_release.py came to report two false failures. The totals also live under `arrangement`, so the note count is `arrangement.notes` rather than a top-level key. Verified unchanged: songbook 3,824 charts, fakebook archive 2,484, 27 MCP tools. 693 tests pass, check reports 6,336 files, and the corpus fingerprint is untouched. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PBAjxy7cD6DzJ72NX8TJEc --- CLAUDE.md | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 1fcd911d..ba10f24e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -69,7 +69,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 @@ -255,10 +259,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 +296,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 From 00818861e72a48b2a83b0124aa91007b878e11de Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 14:38:20 +0000 Subject: [PATCH 4/6] Licence coordinate MIT, not Apache-2.0 I chose Apache-2.0 for the patent grant and to match the robotics ecosystem. The stated priority is distribution over credit, and that inverts the trade. Apache-2.0's obligations run against exactly this file's purpose. It is one file meant to be copied next to whatever needs it -- onto a Pi beside a steering pump, with no packaging step -- and Apache-2.0 asks that vendored copy to carry a NOTICE and to state that files were changed. MIT asks it to keep one paragraph. When the goal is that the code travels, the licence with fewer conditions on travelling is the right one. It also makes the stack one licence rather than two. plainsong is MIT; a single licence across the ecosystem means no compatibility matrix for anyone assembling pieces, and MIT is the text every legal review already has a stamp for. Apache-2.0 and GPL-3.0 can both still consume MIT, so nothing downstream is foreclosed -- including MAGDA. The patent grant is the real thing given up. It is worth less here than it looks: this is arithmetic rather than a mechanism, and Apache's grant protects users from *contributors'* patents, which matters most in projects with many corporate contributors. This has none yet. If attribution should be dropped entirely, 0BSD is the next step and is a one-file change from here. MIT is the more conservative choice and the more widely recognised one, so it is where this starts. --- coordinate/LICENSE | 21 +++++++++++++++++++++ coordinate/README.md | 2 +- coordinate/coordinate.py | 2 +- 3 files changed, 23 insertions(+), 2 deletions(-) create mode 100644 coordinate/LICENSE 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 index 05e190cc..bbd62817 100644 --- a/coordinate/README.md +++ b/coordinate/README.md @@ -2,7 +2,7 @@ **Scheduling backwards from when an effect should land.** -One file, standard library only, Apache-2.0. Copy it next to whatever needs it. +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: diff --git a/coordinate/coordinate.py b/coordinate/coordinate.py index a39ad5f0..79fbc982 100644 --- a/coordinate/coordinate.py +++ b/coordinate/coordinate.py @@ -1,6 +1,6 @@ """Scheduling backwards from when an effect should land. -Copyright 2026 SuperInstance. Licensed under the Apache License, Version 2.0. +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 From 6d2b5e9a0eb83a2ccf5f8e8618a8300bc641d29c Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 14:59:19 +0000 Subject: [PATCH 5/6] Write down how this project decides something is true The lessons from this week were living in commit messages, which the next person does not read. docs/verification.md collects them, deliberately written so nothing in it is about music -- the same faults turned up in five sibling repositories and the file should be portable to any of them. Six rules, each with the incident that paid for it. Success is not evidence: `plainsong spec` printed "no specs found" and exited 0, and a sibling's CI suppresses its own build with `|| true`, so neither has ever demonstrated anything. A guard you cannot fail is not a guard: two TimeGrid guards were redundant and no mutation could fail a test until one was removed. Two copies drift into the same bug, not different ones -- both loopback copies accepted `127.evil.example` and both mangled `[::1]`. Verify by doing: PyPI's API reported a stale version twice, a suite on sys.path cannot see a packaging bug, setuptools reuses build/lib so a broken package verifies perfectly, and a pipe eats the exit code. A name is not a description and a README is not a fact. Report what happened, including when the two failures in your own verification run turn out to be your harness rather than the product. CLAUDE.md points at it from a new section and its command list now carries verify_release.py beside the demo differential, both being checks the test suite structurally cannot perform. --- CLAUDE.md | 20 ++++++- docs/verification.md | 138 +++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 157 insertions(+), 1 deletion(-) create mode 100644 docs/verification.md diff --git a/CLAUDE.md b/CLAUDE.md index ba10f24e..31e2028c 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 @@ -207,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; 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. From f6bf34e311eb9acad0448609a7ef517fefaaa40b Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 15:21:42 +0000 Subject: [PATCH 6/6] Deprecate `plainsong mcp`, now that the sibling is published plainsong-mcp 1.0.0 is on PyPI, so for the first time there is somewhere to send people. The command still works and will until 2.0; it now warns and names the replacement. The 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 for anyone scripting against the old command. The rule is enforced rather than remembered. TestDeprecationNoticeStaysOffTheWire drives a real subprocess and parses every line of its stdout as JSON, so a future edit that prints the notice to the wrong stream fails the suite instead of breaking clients quietly. --- CHANGELOG.md | 18 ++++++++++++++ CLAUDE.md | 6 +++++ plainsong/interfaces/cli.py | 27 ++++++++++++++++++++ tests/test_mcp.py | 49 +++++++++++++++++++++++++++++++++++++ 4 files changed, 100 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d7d18b2..7d66b16c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,24 @@ 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 diff --git a/CLAUDE.md b/CLAUDE.md index 31e2028c..d0b33bb4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -326,6 +326,12 @@ metre would change the music. - 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/plainsong/interfaces/cli.py b/plainsong/interfaces/cli.py index 2567a5fc..42eb91dc 100644 --- a/plainsong/interfaces/cli.py +++ b/plainsong/interfaces/cli.py @@ -1096,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()