Harden for agent users: spec validation, key-sig accidentals, N64 chord qualities
- make_tune: validate_spec() reports every spec problem in one run (missing keys, unknown style, bad qualities, bpm<=0, out-of-range loop/fills, melody longer than chart, unsafe name) — no raw KeyErrors - make_tune: key_sig_for handles accidentals in minor and major keys, any case (Bb minor, F# minor, bb major); real spellings kept (C# major) - make_tune: n64 pad uses QUALITIES table (dim/aug/sus now correct) - make_tune: WARNING on stderr for fields a style can't use (counter for non-DOS, fills_at for n64) instead of silent drops - make_tune: name sanitised ([A-Za-z0-9][A-Za-z0-9._-]*), no traversal - make_tune: render errors actionable (timidity presence check, stderr surfaced, sys.executable instead of bare python3) - songkit: parse_score lints the FINAL bar too (was unchecked); this caught a real 5-beat bar in 01_service_station_2am COUNTER_A (fixed) - build_all.sh: dependency + soundfont checks, removes stale midi/wav outputs with no current source, fails if nothing was produced - smoke_test: runs every spec, checks loop marker tick positions, feeds malformed bars to the linter, covers validation + key sigs - AGENTS.md: documents 0-based bars and exclusive end_bar, validation behaviour; SKILL.md added so the repo loads as an agent skill
This commit is contained in:
1 parent
f5bd7ae13c
commit
810f127d01
7 files changed
+482
-50
No files matched your search
+68
-7
@@ -4,18 +4,71 @@
|
||||
#
|
||||
# ./tools/build_all.sh
|
||||
#
|
||||
# Requires: timidity (with a GM soundfont, e.g. FluidR3_GM.sf2).
|
||||
# Requires: python3, timidity (with a GM soundfont, e.g. FluidR3_GM.sf2).
|
||||
# WAVs land in build/ (gitignored); MIDI files land in midi/.
|
||||
#
|
||||
# Stale outputs are removed: any midi/*.mid or build/*.wav that no longer
|
||||
# has a corresponding source (tunes/*.py or tunes/specs/*.json) is deleted,
|
||||
# so "ALL OK" can never refer to a tune whose source has gone.
|
||||
set -euo pipefail
|
||||
cd "$(dirname "$0")/.."
|
||||
mkdir -p midi build
|
||||
|
||||
for t in tunes/*.py; do
|
||||
# ---- dependency checks (fail fast with an actionable message) ----
|
||||
missing=0
|
||||
for cmd in python3 timidity; do
|
||||
if ! command -v "$cmd" >/dev/null 2>&1; then
|
||||
echo "ERROR: '$cmd' not found on PATH." >&2
|
||||
missing=1
|
||||
fi
|
||||
done
|
||||
if [ "$missing" -eq 1 ]; then
|
||||
echo "Debian/Ubuntu: apt install python3 timidity fluid-soundfont-gm" >&2
|
||||
exit 1
|
||||
fi
|
||||
# soundfont: timidity needs one; check its config or the usual FluidR3 path
|
||||
if ! timidity -iA --help >/dev/null 2>&1; then :; fi # presence already checked
|
||||
if [ ! -f /usr/share/sounds/sf2/FluidR3_GM.sf2 ] && \
|
||||
! grep -qsE '^[[:space:]]*dir[[:space:]]|^[[:space:]]*file[[:space:]]' /etc/timidity/timidity.cfg 2>/dev/null; then
|
||||
echo "WARNING: no obvious GM soundfont found (checked /usr/share/sounds/sf2/FluidR3_GM.sf2 and /etc/timidity/timidity.cfg)." >&2
|
||||
echo " WAV rendering may fail or sound wrong. apt install fluid-soundfont-gm" >&2
|
||||
fi
|
||||
|
||||
# ---- collect sources and the set of names they produce ----
|
||||
sources_py=(tunes/*.py)
|
||||
sources_json=(tunes/specs/*.json)
|
||||
expected=()
|
||||
for t in "${sources_py[@]}"; do
|
||||
[ -e "$t" ] && expected+=("$(basename "${t%.py}")")
|
||||
done
|
||||
for s in "${sources_json[@]}"; do
|
||||
[ -e "$s" ] && expected+=("$(python3 -c "import json,sys; print(json.load(open(sys.argv[1]))['name'])" "$s")")
|
||||
done
|
||||
|
||||
# ---- remove stale outputs (no current source) ----
|
||||
shopt -s nullglob
|
||||
for f in midi/*.mid; do
|
||||
n="$(basename "${f%.mid}")"
|
||||
keep=0
|
||||
for e in "${expected[@]}"; do [ "$e" = "$n" ] && keep=1 && break; done
|
||||
if [ "$keep" -eq 0 ]; then
|
||||
echo "stale: removing $f (no source)"
|
||||
rm -f "$f" "build/$n.wav"
|
||||
fi
|
||||
done
|
||||
for f in build/*.wav; do
|
||||
[ -e "midi/$(basename "${f%.wav}").mid" ] || { echo "stale: removing $f"; rm -f "$f"; }
|
||||
done
|
||||
|
||||
# ---- build ----
|
||||
for t in "${sources_py[@]}"; do
|
||||
[ -e "$t" ] || continue
|
||||
echo "== $t"
|
||||
python3 "$t"
|
||||
done
|
||||
|
||||
for s in tunes/specs/*.json; do
|
||||
for s in "${sources_json[@]}"; do
|
||||
[ -e "$s" ] || continue
|
||||
echo "== $s"
|
||||
python3 tools/make_tune.py "$s"
|
||||
done
|
||||
@@ -25,7 +78,9 @@ python3 - <<'EOF'
|
||||
import sys, glob
|
||||
sys.path.insert(0, "lib")
|
||||
from midikit import verify
|
||||
for f in sorted(glob.glob("midi/*.mid")):
|
||||
files = sorted(glob.glob("midi/*.mid"))
|
||||
assert files, "no MIDI files were produced — nothing to verify"
|
||||
for f in files:
|
||||
d = verify(f)
|
||||
print(" %-40s %6.1fs %4d notes %d tracks" %
|
||||
(f, d["duration_sec"], sum(len(t["notes"]) for t in d["tracks"]),
|
||||
@@ -33,9 +88,15 @@ for f in sorted(glob.glob("midi/*.mid")):
|
||||
EOF
|
||||
|
||||
echo "== rendering WAV (timidity)"
|
||||
for f in midi/*.mid; do
|
||||
shopt -s nullglob
|
||||
mids=(midi/*.mid)
|
||||
[ "${#mids[@]}" -gt 0 ] || { echo "ERROR: no MIDI to render" >&2; exit 1; }
|
||||
for f in "${mids[@]}"; do
|
||||
out="build/$(basename "${f%.mid}").wav"
|
||||
timidity -Ow -s 44 "$f" -o "$out" 2>/dev/null
|
||||
if ! timidity -Ow -s 44 "$f" -o "$out"; then
|
||||
echo "ERROR: timidity failed on $f" >&2
|
||||
exit 1
|
||||
fi
|
||||
python3 tools/check_wav.py "$out"
|
||||
done
|
||||
echo "ALL OK"
|
||||
echo "ALL OK (${#mids[@]} tunes)"
|
||||
+211
-27
@@ -20,13 +20,23 @@ Spec format (see tunes/specs/ for working examples):
|
||||
"melody_start_bar": 4,
|
||||
"melody": "A4-8 C5-8 E5-4 A5-2 | ...", # bar-grid, '|' separators (linted 4/4)
|
||||
"loop": {"start_bar": 4, "end_bar": 20}, # optional; LOOP_* markers
|
||||
"counter": "...", # optional 2nd melody line (style-dependent)
|
||||
"counter": "...", # optional 2nd melody line (dos_adventure only)
|
||||
"fills_at": [11, 19] # optional: drum fill at end of these bars
|
||||
}
|
||||
|
||||
Bar indexing convention: bars are 0-BASED throughout. A loop's end_bar is
|
||||
EXCLUSIVE — {"start_bar": 4, "end_bar": 20} loops bars 4..19 (16 bars).
|
||||
The melody must fit the chord chart: melody_start_bar + (number of melody
|
||||
bars) must be <= len(chords).
|
||||
|
||||
Chord qualities: maj m dim aug 7 maj7 m7 m7b5 6 m6 9 add9 sus2 sus4 sus
|
||||
|
||||
Validation: the spec is checked up front and every problem is reported
|
||||
together (one run tells you everything wrong), instead of a raw KeyError
|
||||
halfway through. Fields a style cannot use (e.g. counter for n64_ambient)
|
||||
produce a warning, never silence.
|
||||
"""
|
||||
import argparse, json, os, sys
|
||||
import argparse, json, os, re, shutil, subprocess, sys
|
||||
|
||||
HERE = os.path.dirname(os.path.abspath(__file__))
|
||||
sys.path.insert(0, os.path.join(HERE, "..", "lib"))
|
||||
@@ -35,21 +45,157 @@ from midikit import MidiFile, Track, MPQN, verify, note as N
|
||||
from songkit import (bar, melody, arp, bass_drive, drums, vibrato, crash,
|
||||
parse_score, QUALITIES, TPQ)
|
||||
|
||||
KEY_PC = {"C": 0, "D": 2, "E": 4, "F": 5, "G": 7, "A": 9, "B": 11}
|
||||
LETTER_PC = {"C": 0, "D": 2, "E": 4, "F": 5, "G": 7, "A": 9, "B": 11}
|
||||
# circle of fifths lookup for key signature meta (flats negative, sharps positive)
|
||||
MAJOR_KEYS = {"C": 0, "G": 1, "D": 2, "A": 3, "E": 4, "B": 5, "F#": 6, "C#": 7,
|
||||
"F": -1, "Bb": -2, "Eb": -3, "Ab": -4, "Db": -5, "Gb": -6}
|
||||
"F": -1, "Bb": -2, "Eb": -3, "Ab": -4, "Db": -5, "Gb": -6, "Cb": -7}
|
||||
# canonical spelling per pitch class, preferring names present in MAJOR_KEYS
|
||||
PC_TO_KEY = {0: "C", 1: "Db", 2: "D", 3: "Eb", 4: "E", 5: "F", 6: "F#",
|
||||
7: "G", 8: "Ab", 9: "A", 10: "Bb", 11: "B"}
|
||||
|
||||
NAME_RE = re.compile(r"^[A-Za-z0-9][A-Za-z0-9._-]*$")
|
||||
TONIC_RE = re.compile(r"^([A-Ga-g])([#bf]?)$")
|
||||
|
||||
|
||||
class SpecError(ValueError):
|
||||
"""Raised with the full list of problems found in a spec."""
|
||||
|
||||
|
||||
def parse_tonic(tonic):
|
||||
"""'Bb', 'f#', 'A' -> pitch class, or None if unparseable."""
|
||||
m = TONIC_RE.match(str(tonic).strip())
|
||||
if not m:
|
||||
return None
|
||||
pc = LETTER_PC[m.group(1).upper()]
|
||||
pc += m.group(2).count("#") + m.group(2).count("s")
|
||||
pc -= m.group(2).count("b") + m.group(2).count("f")
|
||||
return pc % 12
|
||||
|
||||
|
||||
def key_sig_for(tonic, mode):
|
||||
if mode in ("minor", "m"):
|
||||
# relative major is 3 semitones up
|
||||
pc = (KEY_PC[tonic.upper()] + 3) % 12
|
||||
name = {0: "C", 2: "D", 4: "E", 5: "F", 7: "G", 9: "A", 11: "B"}.get(pc)
|
||||
if name is None: # sharp-side minor keys
|
||||
name = {1: "Db", 3: "Eb", 6: "F#", 8: "Ab", 10: "Bb"}[pc]
|
||||
"""(accidentals, is_minor) for the key-signature meta event.
|
||||
|
||||
Accepts accidentals in either case: 'Bb minor', 'F# minor', 'bb major'.
|
||||
A tonic whose own spelling is a real key (C# major, Db major) keeps its
|
||||
own signature; otherwise the canonical enharmonic spelling is used.
|
||||
"""
|
||||
pc = parse_tonic(tonic)
|
||||
if pc is None:
|
||||
raise SpecError("unparseable key tonic %r (expected letter + optional #/b)" % tonic)
|
||||
t = str(tonic).strip()
|
||||
spelling = t[0].upper() + t[1:]
|
||||
if str(mode).lower() in ("minor", "m"):
|
||||
name = PC_TO_KEY[(pc + 3) % 12] # relative major is 3 semitones up
|
||||
return MAJOR_KEYS[name], True
|
||||
return MAJOR_KEYS[tonic.capitalize() if len(tonic) == 1 else tonic], False
|
||||
name = spelling if spelling in MAJOR_KEYS else PC_TO_KEY[pc]
|
||||
return MAJOR_KEYS[name], False
|
||||
|
||||
|
||||
def _melody_bar_count(score):
|
||||
"""Number of '|'-separated bars in a score (0 if no separators)."""
|
||||
segs = [s for s in score.split("|") if s.strip()]
|
||||
return len(segs) if "|" in score else 0
|
||||
|
||||
|
||||
def validate_spec(spec):
|
||||
"""Return (warnings, errors). Errors block the build; warnings don't."""
|
||||
errors, warnings = [], []
|
||||
|
||||
def need(key, typ=(str,)):
|
||||
if key not in spec:
|
||||
errors.append("missing required key %r" % key)
|
||||
return None
|
||||
v = spec[key]
|
||||
if typ and not isinstance(v, typ):
|
||||
errors.append("%r must be %s, got %r" % (key, "/".join(t.__name__ for t in typ), v))
|
||||
return None
|
||||
return v
|
||||
|
||||
name = need("name", (str,))
|
||||
if name is not None:
|
||||
if not NAME_RE.match(name) or ".." in name or "/" in name or os.sep in name:
|
||||
errors.append("name %r is not a safe file name (letters, digits, '.', '_', '-' only; "
|
||||
"no path separators or '..')" % name)
|
||||
|
||||
style = need("style", (str,))
|
||||
if style is not None and style not in STYLES:
|
||||
errors.append("unknown style %r (choose: %s)" % (style, ", ".join(sorted(STYLES))))
|
||||
|
||||
bpm = need("bpm", (int, float))
|
||||
if bpm is not None and not (1 <= bpm <= 300):
|
||||
errors.append("bpm must be 1..300, got %r" % bpm)
|
||||
|
||||
if "key" in spec:
|
||||
key = spec["key"]
|
||||
if not (isinstance(key, list) and len(key) == 2):
|
||||
errors.append("key must be [tonic, mode], got %r" % (key,))
|
||||
else:
|
||||
if parse_tonic(key[0]) is None:
|
||||
errors.append("key tonic %r is not a note letter with optional #/b" % key[0])
|
||||
if str(key[1]).lower() not in ("major", "minor", "m"):
|
||||
errors.append("key mode %r must be 'major' or 'minor'" % key[1])
|
||||
|
||||
chords = need("chords", (list,))
|
||||
n_chords = 0
|
||||
if chords is not None:
|
||||
if not chords:
|
||||
errors.append("chords must be a non-empty list")
|
||||
for i, c in enumerate(chords):
|
||||
if not (isinstance(c, list) and len(c) == 2):
|
||||
errors.append("chords[%d] must be [root, quality], got %r" % (i, c))
|
||||
continue
|
||||
try:
|
||||
N(str(c[0]) + "4")
|
||||
except (KeyError, ValueError, IndexError):
|
||||
errors.append("chords[%d] root %r is not a note letter with optional #/b" % (i, c[0]))
|
||||
if c[1] not in QUALITIES:
|
||||
errors.append("chords[%d] quality %r unknown (choose: %s)"
|
||||
% (i, c[1], " ".join(sorted(QUALITIES))))
|
||||
n_chords = len(chords)
|
||||
|
||||
start = spec.get("melody_start_bar", 0)
|
||||
if not isinstance(start, int) or start < 0:
|
||||
errors.append("melody_start_bar must be a non-negative int, got %r" % start)
|
||||
start = 0
|
||||
|
||||
if spec.get("melody"):
|
||||
try:
|
||||
parse_score(spec["melody"]) # bar-length lint raises here
|
||||
except (ValueError, KeyError) as e:
|
||||
errors.append("melody: %s" % e)
|
||||
else:
|
||||
nb = _melody_bar_count(spec["melody"])
|
||||
if nb and n_chords and start + nb > n_chords:
|
||||
errors.append("melody runs past the chord chart: starts at bar %d, %d bars "
|
||||
"long, but the chart has only %d bars" % (start, nb, n_chords))
|
||||
|
||||
if spec.get("counter"):
|
||||
try:
|
||||
parse_score(spec["counter"])
|
||||
except (ValueError, KeyError) as e:
|
||||
errors.append("counter: %s" % e)
|
||||
if style is not None and style != "dos_adventure":
|
||||
warnings.append("style %r has no counter line — 'counter' will be ignored" % style)
|
||||
|
||||
if "loop" in spec:
|
||||
loop = spec["loop"]
|
||||
if not (isinstance(loop, dict) and isinstance(loop.get("start_bar"), int)
|
||||
and isinstance(loop.get("end_bar"), int)):
|
||||
errors.append("loop must be {\"start_bar\": int, \"end_bar\": int}, got %r" % (loop,))
|
||||
else:
|
||||
s, e = loop["start_bar"], loop["end_bar"]
|
||||
if not (0 <= s < e <= n_chords):
|
||||
errors.append("loop bars out of range: need 0 <= start(%d) < end(%d) <= "
|
||||
"len(chords)(%d) [bars are 0-based, end_bar is EXCLUSIVE]"
|
||||
% (s, e, n_chords))
|
||||
|
||||
for b in spec.get("fills_at", []):
|
||||
if not isinstance(b, int) or not (0 <= b < n_chords):
|
||||
errors.append("fills_at bar %r out of range 0..%d" % (b, n_chords - 1))
|
||||
if spec.get("fills_at") and style == "n64_ambient":
|
||||
warnings.append("style 'n64_ambient' has no drums — 'fills_at' will be ignored")
|
||||
|
||||
return warnings, errors
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@@ -80,10 +226,10 @@ def style_n64_ambient(mf, spec, chords):
|
||||
bass = Track("Cello", channel=3)
|
||||
bass.program(0, "cello"); bass.cc(0, 7, 78); bass.cc(0, 10, 34)
|
||||
mf.add_track(lead); mf.add_track(pad); mf.add_track(harp); mf.add_track(bass)
|
||||
# whole-note pads + root bass
|
||||
# whole-note pads + root bass; the pad honours the chord quality
|
||||
# (dim/aug/sus included) via the shared QUALITIES table
|
||||
for i, (root, qual) in enumerate(chords):
|
||||
third = 3 if qual.startswith("m") or qual.startswith("dim") else 4
|
||||
for iv in (0, third, 7):
|
||||
for iv in QUALITIES[qual][:3]:
|
||||
pad.note(bar(i), N(root + "3") + iv, TPQ * 4, 55)
|
||||
bass.note(bar(i), N(root + "2"), TPQ * 4, 70)
|
||||
return dict(lead=lead, bass=bass, acomp=harp, drums=None,
|
||||
@@ -114,7 +260,16 @@ STYLES = {"dos_adventure": style_dos_adventure,
|
||||
"chiptune_racing": style_chiptune_racing}
|
||||
|
||||
|
||||
def build(spec):
|
||||
def build(spec, warn=True):
|
||||
"""Build a MidiFile from a validated spec. Raises SpecError listing every
|
||||
problem; style-incompatible fields produce warnings on stderr."""
|
||||
warnings, errors = validate_spec(spec)
|
||||
if errors:
|
||||
raise SpecError("spec invalid:\n - " + "\n - ".join(errors))
|
||||
if warn:
|
||||
for w in warnings:
|
||||
print("WARNING:", w, file=sys.stderr)
|
||||
|
||||
chords = [(r, q) for r, q in spec["chords"]]
|
||||
n = len(chords)
|
||||
mf = MidiFile(tpq=TPQ)
|
||||
@@ -158,34 +313,63 @@ def build(spec):
|
||||
return mf
|
||||
|
||||
|
||||
def midi_path_for(name):
|
||||
"""midi/<name>.mid under the repo root; name must already be validated."""
|
||||
return os.path.join(os.path.dirname(HERE), "midi", name + ".mid")
|
||||
|
||||
|
||||
def render_wav(mid_path, name):
|
||||
"""Render WAV via timidity and run the audio checks. Clear errors,
|
||||
stderr surfaced, interpreter-agnostic (sys.executable, not bare python3)."""
|
||||
if shutil.which("timidity") is None:
|
||||
raise SystemExit("ERROR: timidity not found on PATH. "
|
||||
"Debian/Ubuntu: apt install timidity fluid-soundfont-gm")
|
||||
wav = os.path.join(os.path.dirname(HERE), "build", name + ".wav")
|
||||
os.makedirs(os.path.dirname(wav), exist_ok=True)
|
||||
try:
|
||||
subprocess.run(["timidity", "-Ow", "-s", "44", mid_path, "-o", wav],
|
||||
check=True, capture_output=True, text=True)
|
||||
except subprocess.CalledProcessError as e:
|
||||
raise SystemExit("ERROR: timidity failed (exit %d):\n%s\n%s"
|
||||
% (e.returncode, e.stderr or "", e.stdout or ""))
|
||||
subprocess.run([sys.executable, os.path.join(HERE, "check_wav.py"), wav], check=True)
|
||||
return wav
|
||||
|
||||
|
||||
def main():
|
||||
ap = argparse.ArgumentParser(description=__doc__)
|
||||
ap = argparse.ArgumentParser(description=__doc__,
|
||||
formatter_class=argparse.RawDescriptionHelpFormatter)
|
||||
ap.add_argument("spec", help="path to tune spec JSON")
|
||||
ap.add_argument("--render", action="store_true", help="also render WAV via timidity")
|
||||
args = ap.parse_args()
|
||||
|
||||
spec = json.load(open(args.spec))
|
||||
mf = build(spec)
|
||||
out = os.path.join(os.path.dirname(HERE), "midi", spec["name"] + ".mid")
|
||||
try:
|
||||
with open(args.spec) as f:
|
||||
spec = json.load(f)
|
||||
except json.JSONDecodeError as e:
|
||||
raise SystemExit("ERROR: %s is not valid JSON: %s" % (args.spec, e))
|
||||
|
||||
try:
|
||||
mf = build(spec)
|
||||
except SpecError as e:
|
||||
raise SystemExit("ERROR: %s: %s" % (args.spec, e))
|
||||
|
||||
out = midi_path_for(spec["name"])
|
||||
os.makedirs(os.path.dirname(out), exist_ok=True)
|
||||
mf.write(out)
|
||||
d = verify(out)
|
||||
markers = [m for t in d["tracks"] for m in t["markers"]]
|
||||
loop = spec.get("loop")
|
||||
print("OK:", out)
|
||||
print(" %.1fs | %d notes | %d tracks | loop: %s" %
|
||||
(d["duration_sec"], sum(len(t["notes"]) for t in d["tracks"]),
|
||||
len(d["tracks"]),
|
||||
"bars %d-%d" % (spec["loop"]["start_bar"], spec["loop"]["end_bar"])
|
||||
if spec.get("loop") else "none"))
|
||||
"bars %d-%d" % (int(loop["start_bar"]), int(loop["end_bar"]))
|
||||
if loop else "none"))
|
||||
if markers:
|
||||
print(" markers:", ", ".join(sorted(set(m[1] for m in markers))))
|
||||
if args.render:
|
||||
import subprocess
|
||||
wav = os.path.join(os.path.dirname(HERE), "build", spec["name"] + ".wav")
|
||||
os.makedirs(os.path.dirname(wav), exist_ok=True)
|
||||
subprocess.run(["timidity", "-Ow", "-s", "44", out, "-o", wav],
|
||||
check=True, capture_output=True)
|
||||
subprocess.run(["python3", os.path.join(HERE, "check_wav.py"), wav], check=True)
|
||||
render_wav(out, spec["name"])
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
|
||||
Reference in new issue
Block a user