Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ Four facts do most of the work:

## Ask the tool; do not infer

Every command takes `--json`. Use it.
Every command takes `--json`, and it works before or after the subcommand --
`plainsong --json info song.song` and `plainsong info song.song --json` are the
same command. Use it.

```bash
plainsong chord "G7alt" --explain # what a symbol means, degree by degree
Expand Down
53 changes: 53 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,59 @@ Notable changes, newest first. Dates are ISO 8601.

## Unreleased

### `--help` now shows the shape of the notation

Every small model observed driving this CLI cold ran `--help` first. The ones
that reached `plainsong new` were fine -- it writes a correct file and they read
the format off it, and one reached for it even when told not to. The ones that
could not, because they were adding a file to an existing project, guessed. They
guessed `Title:`, which is not the notation, and got a dropped title and a
phantom section for it.

Six lines of `--help` remove the guess. Because those lines are now a claim
about the language, they are held to the same bar as a fenced block in the
prose: `tests/test_runtime.py::TestTheHelpTeachesRealNotation` compiles the
sample, requires it to sound, and requires it to produce no diagnostics. Putting
`Title:` in the help fails two of those three.

### Global flags now work in either position

`plainsong info song.song --json` was refused outright -- `unrecognized
arguments: --json` -- because argparse hangs global options off the top-level
parser only. It is what almost everyone types, and it is not a small failure:
the command does not run at all.

This was documented rather than fixed, and documenting it did not work.
`tools/verify_release.py` in this repository was written against the
documentation and reported two false failures. To find out whether that
generalises, four Haiku-class agents were pointed at the CLI cold, with the
docs withheld. One hit the flag position three times in a single session -- on
`--json`, on `-v` and on `--quiet` -- and named it first when asked what it
wished the tool had said up front.

So the trap is removed instead of explained. All 24 subcommands now accept
`--json`, `-v`/`--verbose` and `-q`/`--quiet` themselves, via
`argparse.SUPPRESS` so the global position is not silently clobbered by a
subparser default -- which would have traded a loud failure for a quiet one.
A subcommand defining its own flag keeps it: `info --verbose` is untouched.

### `--verbose` now shows info-level diagnostics, which nothing showed before

An entire severity was write-only. `_diagnostics` filtered to errors and
warnings, so info-level diagnostics were produced by the parser and displayed
by no command at all -- including `info --verbose`, which documents itself as
showing every diagnostic available.

It matters because `Title: My Song` is the natural way to name a piece and is
not the notation. The parser says so at info level, so the title was dropped
silently, the stray row silently became a section, and the only trace was
`(untitled)` and a section count one too high. Another of the cold agents hit
exactly that and had to read the parser's source to work out why.

The default output is unchanged; only `--verbose` grows. The corpus produces no
info-level diagnostics, so nothing in the songbook gets noisier.


### CI was red, and one of its checks could not go red at all

Four jobs were failing on master and had been since 19 August. The cause of
Expand Down
14 changes: 9 additions & 5 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,15 @@ python3 -m plainsong setup # connect a model
python3 -m plainsong build # tailor this install to the machine
```

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.
Every command takes `--json`, `-v` and `-q`, **in either position**:
`plainsong --json info song.song` and `plainsong info song.song --json` are the
same command. The second used to be refused outright with `unrecognized
arguments: --json` -- it is what almost everyone types, it is what
`tools/verify_release.py` was written against, and documenting the constraint
did not stop anyone falling into it, so the constraint was removed instead.

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
Expand Down
11 changes: 7 additions & 4 deletions docs/integration.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,11 @@ Every command takes `--json` and prints one JSON object on stdout.
plainsong --json info song.song
```

**The flag goes before the subcommand.** `plainsong --json info song.song`
works; `plainsong info --json song.song` does not. It is a global option, and
this catches nearly everyone once.
**The flag works in either position.** `plainsong --json info song.song` and
`plainsong info song.song --json` are the same command. The second used to be
refused, which caught nearly everyone once -- including this project's own
release checker -- so it is now accepted rather than explained. The same goes
for `-v` and `-q`.

| Command | Top-level keys |
|---|---|
Expand Down Expand Up @@ -173,7 +175,8 @@ a client should install it from.

Collected because every one of these cost somebody time:

1. **`--json` goes before the subcommand.**
1. **`--json` works before or after the subcommand.** It did not always;
if you are on 1.4.0 or older it must come first.
2. **JSON and HTTP APIs use `content`**; the CLI takes a file path positionally.
3. **MCP tool names differ from CLI command names** (`compile_score` vs
`compile`).
Expand Down
106 changes: 99 additions & 7 deletions plainsong/interfaces/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,29 @@
"""


# Every agent observed driving this CLI cold ran `--help` first, and the ones
# that could not reach `plainsong new` -- writing a file into an existing
# project rather than scaffolding one -- had to guess the shape of the notation.
# They guessed `Title:`, which is not it, and got a dropped title and a phantom
# section for it. Six lines here cost nothing and remove the guess.
NOTATION_AT_A_GLANCE = """\
The notation, in one section:

**TRACK: Title** <- the title. `Title:` is not it.
[MetaData]
key: Am | tempo: 96 | time: 4/4

[V1] (Verse - 2 Bars) <- a section
Chords: | Am . . . | F . . . |
Melody: | A4 . C5 E5 | F4 . A4 C5 |

A bar is one bar long and its tokens divide it, so three tokens are triplets and
you never write durations. Rows of different kinds sound together.

Start with: plainsong new, then plainsong compile <file> --play
Flags (--json, -v, -q) work before or after the subcommand."""


# --------------------------------------------------------------------------
# output helpers
# --------------------------------------------------------------------------
Expand Down Expand Up @@ -91,14 +114,32 @@ def table(self, rows: list[tuple[str, ...]], gap: str = " ") -> None:
self.say(" " + line.rstrip())


def _diagnostics(out: Out, diagnostics: list, path: str = "", limit: int = 20) -> None:
shown = [diag for diag in diagnostics if diag.severity in ("error", "warning")]
def _diagnostics(
out: Out, diagnostics: list, path: str = "", limit: int = 20, verbose: bool = False
) -> None:
"""Show the diagnostics. `verbose` adds the info-level ones.

Info-level diagnostics used to be produced and then displayed by nothing at
all -- not by `compile`, not by `check`, and not by `info --verbose`, which
documents itself as showing every diagnostic available. An entire severity
was write-only.

That is not a cosmetic gap. `Title: My Song` is the most natural way to name
a piece and is not the notation; the parser says so at info level, so the
title was silently dropped, the stray row silently became a section, and the
only visible trace was `(untitled)` and a section count one too high. A
small model driving this cold hit exactly that.
"""
levels = ("error", "warning", "info") if verbose else ("error", "warning")
shown = [diag for diag in diagnostics if diag.severity in levels]
if not shown:
return
out.say()
painters = {"error": out.fail, "warning": out.warn}
for diag in shown[:limit]:
painter = out.fail if diag.severity == "error" else out.warn
painter(diag.format(path))
# Info keeps its own voice: painting it "warn" would say the file has a
# problem when what it has is a remark.
painters.get(diag.severity, out.dim)(diag.format(path))
if len(shown) > limit:
out.dim(f" ... and {len(shown) - limit} more")

Expand Down Expand Up @@ -147,7 +188,13 @@ def cmd_compile(args: argparse.Namespace, config: Config, out: Out) -> int:
for message in result.messages:
out.warn(message)
if not args.quiet:
_diagnostics(out, result.diagnostics, str(source), limit=8 if not args.verbose else 100)
_diagnostics(
out,
result.diagnostics,
str(source),
limit=8 if not args.verbose else 100,
verbose=args.verbose,
)

if args.play and result.audio_path:
from ..render.backends import play_audio
Expand Down Expand Up @@ -234,7 +281,7 @@ def cmd_info(args: argparse.Namespace, config: Config, out: Out) -> int:
)
for d in summary.get("diagnostics", [])
]
_diagnostics(out, reported, str(source), limit=100 if args.verbose else 10)
_diagnostics(out, reported, str(source), limit=100 if args.verbose else 10, verbose=args.verbose)
return 0


Expand Down Expand Up @@ -1216,7 +1263,7 @@ def build_parser() -> argparse.ArgumentParser:
parser = argparse.ArgumentParser(
prog="plainsong",
description="Plain-text music notation that compiles to MIDI and audio.",
epilog="Start with: plainsong new, then plainsong compile <file> --play",
epilog=NOTATION_AT_A_GLANCE,
formatter_class=argparse.RawDescriptionHelpFormatter,
)
parser.add_argument("--version", action="version", version=f"plainsong {__version__}")
Expand Down Expand Up @@ -1424,9 +1471,54 @@ def build_parser() -> argparse.ArgumentParser:
bridge_parser.add_argument("--text", default="", help="the reply, for `answer`")
bridge_parser.set_defaults(func=cmd_bridge)

_accept_global_flags_after_the_subcommand(subparsers)
return parser


# `plainsong info song.song --json` is what almost everyone types, because that
# is where flags go in nearly every other CLI. argparse hangs global options off
# the top-level parser only, so that invocation was refused outright --
# `unrecognized arguments: --json` -- and the refusal is total: the command does
# not run, it errors. This is not hypothetical. tools/verify_release.py in this
# repository was written against it and reported two false failures, and a small
# model driving this CLI cold hit it three times in one session, on --json, on
# -v and on --quiet, then named flag position as the single thing it most wished
# the tool had told it.
#
# Documenting the constraint was the old fix and it does not work: the sentence
# "Every command takes --json" was in AGENTS.md, and following it literally is
# what produced the bug above. So the trap is removed rather than described.
#
# argparse.SUPPRESS is the load-bearing detail. Without it the subparser's own
# default overwrites whatever the global flag set, so `plainsong --json info x`
# would parse and then silently lose its --json -- trading a loud failure for a
# quiet one, which is the worse trade.
GLOBAL_FLAGS_AFTER_SUBCOMMAND = (
(("-q", "--quiet"), "quiet", "only print what was asked for"),
(("-v", "--verbose"), "verbose", "show more detail"),
(("--json",), "json_mode", "machine-readable output"),
)


def _accept_global_flags_after_the_subcommand(subparsers: argparse._SubParsersAction) -> None:
"""Let every subcommand take the global flags in the natural position too."""
for sub in subparsers.choices.values():
for options, dest, help_text in GLOBAL_FLAGS_AFTER_SUBCOMMAND:
# A subcommand that already defines one of these keeps its own --
# `info` has its own `--verbose`, and overriding it here would be a
# behaviour change rather than an addition.
free = [o for o in options if o not in sub._option_string_actions]
if not free:
continue
sub.add_argument(
*free,
dest=dest,
action="store_true",
default=argparse.SUPPRESS,
help=help_text,
)


def main(argv: list[str] | None = None) -> int:
parser = build_parser()
args = parser.parse_args(argv)
Expand Down
Loading
Loading