Commit 3b518774 by PLN (Algolia)

preload: a bank named 808bd killed the whole plan, and the checker said ok

Last night's WORD fix made five banks visible for the first time — 808bd,
808cy, 808hc, 808sd, 90s_synatm — and the emitter wrote them as `\808bd`.
A SuperCollider bare symbol cannot start with a digit, so preload.scd died
at parse time and warmed 0 banks where it had warmed 132 the day before:

    ERROR: syntax error, unexpected NAME, expecting ']'
              [ \808bd, 25, ".../Dirt-Samples/808bd" ],
    ERROR: Command line parse failed

One bad row costs the whole file, and nothing in the rig says so — with
doNotReadYet restored to true, all 132 banks just fall back to lazy reads.
It was only visible in the boot journal.

Quoting is the fix: '808bd' and \808bd are the same symbol, and only names
that need quoting get one, so an existing plan diffs by exactly the broken
rows. Now 137/137 banks warm in 3.8 s.

Fixing it exposed the second half. The row shape had four readers and
writers — emit_sc, check-preload's union_plan, and its banks()/bank_counts()
shell greps for `[ \name`, which cannot match a quoted row. So the checker
reported ok on a plan genuinely short by five banks: want came out 132
instead of 137. A checker that cannot read what the emitter writes is worse
than no checker, so all four now go through plan_row / plan_row_name /
plan_rows, and the shape is written down once.

43 new tests, and they were watched failing: with the quoting reverted, 21
of them break and check-preload goes blind again.
parent 05db016f
......@@ -118,11 +118,31 @@ fi
# reports ok. LC_ALL=C is set per command, not exported: exporting it would also set
# LC_CTYPE and make the generator's em-dashes an encoding error, and `export LC_COLLATE=C`
# is ignored whenever LC_ALL is already in the environment.
banks() { grep -oE '\[ \\[A-Za-z0-9_]+' "$1" 2>/dev/null | sed 's/.*\\//' | LC_ALL=C sort -u; }
bank_counts() {
grep -oE '\[ \\[A-Za-z0-9_]+, [0-9]+' "$1" 2>/dev/null \
| sed -E 's/\[ \\([A-Za-z0-9_]+), ([0-9]+)/\1 \2/' | LC_ALL=C sort -k1,1
# The plan's row shape has ONE definition, in setlist_samples (plan_rows). These
# two helpers used to grep for `[ \name` in shell, which cannot see the QUOTED rows
# that digit-leading bank names require -- `'808bd'`, because a SuperCollider bare
# symbol may not start with a digit. On 2026-09-24 that blindness made the checker
# report `ok` on a plan that was genuinely short by five banks: the emitter had
# grown them, the grep could not see them, so `want` came out 132 instead of 137.
# A checker that cannot read what the emitter writes is worse than no checker.
plan_read() {
python3 - "$1" "$2" <<'PLANROWS'
import importlib.util, sys
from pathlib import Path
spec = importlib.util.spec_from_file_location('ss', 'tools/setlist_samples.py')
ss = importlib.util.module_from_spec(spec); spec.loader.exec_module(ss)
path, what = sys.argv[1], sys.argv[2]
try:
text = Path(path).read_text()
except OSError:
sys.exit(0) # no plan file -> no rows, as the old grep did
for name, count, _folder in ss.plan_rows(text):
print(name if what == 'name' else f'{name} {count}')
PLANROWS
}
banks() { plan_read "$1" name | LC_ALL=C sort -u; }
bank_counts() { plan_read "$1" pair | LC_ALL=C sort -k1,1; }
nlines() { printf '%s\n' "$1" | grep -c . || true; }
# disk_counts BANK... -> `name count` per line, sorted, read from DISK right now.
......@@ -170,16 +190,16 @@ spec.loader.exec_module(ss)
lines = Path(sys.argv[1]).read_text().split('\n')
carry = sys.argv[2:]
ROW = re.compile(r'^\t\[ \\([A-Za-z0-9_]+), (\d+), "(.*)" \],$')
# The row shape lives in setlist_samples (plan_row / plan_row_name), never here.
start = lines.index('~pvPreload = [')
end = next(i for i in range(start + 1, len(lines)) if lines[i] == '];')
rows = {}
for ln in lines[start + 1:end]:
m = ROW.match(ln)
if not m:
nm = ss.plan_row_name(ln)
if nm is None:
sys.exit(f'! unrecognised preload row, refusing to merge: {ln!r}')
rows[m.group(1)] = ln
rows[nm] = ln
idx = ss.build_index()
kept, lost = [], []
......@@ -188,7 +208,7 @@ for name in carry:
if folder is None or not folder.is_dir():
lost.append(name) # a bank whose folder is GONE. Dropping it is the
continue # only option, so SAY SO — never in silence.
rows[name] = f'\t[ \\{name}, {ss.bank_file_count(folder)}, "{folder}" ],'
rows[name] = ss.plan_row(name, ss.bank_file_count(folder), folder)
kept.append(name)
body = [rows[n] for n in sorted(rows)]
......
......@@ -83,6 +83,55 @@ def last_n_tracks(n):
AUDIO_EXT = {'.wav', '.aiff', '.aif', '.flac', '.ogg'}
# ── the preload plan's row shape, written and read in ONE place ──────────────
#
# 2026-09-24, gig morning: the WORD fix made `808bd`/`808cy`/`808hc`/`808sd` and
# `90s_synatm` visible for the first time, and this emitter wrote them as `\808bd`.
# A SuperCollider bare symbol CANNOT START WITH A DIGIT, so the whole generated
# file died at parse time — `ERROR: syntax error, unexpected NAME, expecting ']'`
# then `Command line parse failed` — and preload.scd warmed **0** banks instead of
# the 132 it had warmed the day before. One bad row costs the entire file, which is
# why this is a syntax question and not a cosmetic one.
#
# Quoting is the whole fix: `'808bd'` and `\808bd` denote the SAME symbol, so the
# bank keys SuperDirt registers are unchanged. Only names that need quoting get it,
# so an existing plan diffs by exactly the rows that were broken.
#
# The row shape had TWO writers (this emitter and check-preload.sh's union_plan)
# and a third reader parsing it with its own regexp. They now all come through
# here — the same "one vocabulary" rule the repo applies to its nouns.
SC_BARE_SYMBOL = re.compile(r'^[A-Za-z_][A-Za-z0-9_]*$')
def sc_symbol(name):
"""A SuperCollider symbol literal for a bank name, quoted only when it must be."""
return f'\\{name}' if SC_BARE_SYMBOL.match(name) else f"'{name}'"
PLAN_ROW_RE = re.compile(
r'^\t\[ (?:\\([A-Za-z_][A-Za-z0-9_]*)|\'([^\']+)\'), (\d+), "(.*)" \],$')
def plan_row(name, count, folder):
"""One `~pvPreload` row. The only place this line is written."""
return f'\t[ {sc_symbol(name)}, {count}, "{folder}" ],'
def plan_rows(text):
"""Every `(name, count, folder)` a generated plan asserts, in file order."""
for ln in text.split('\n'):
m = PLAN_ROW_RE.match(ln)
if m:
yield (m.group(1) or m.group(2)), int(m.group(3)), m.group(4)
def plan_row_name(line):
"""The bank name in a `~pvPreload` row, or None if the line is not one."""
m = PLAN_ROW_RE.match(line)
return (m.group(1) or m.group(2)) if m else None
def bank_file_count(folder):
"""How many files SuperDirt will register for this folder.
......@@ -202,7 +251,7 @@ def emit_sc(resolved, unresolved, tracks):
print('~pvPreload = [')
for name in sorted(resolved):
folder = resolved[name]
print(f'\t[ \\{name}, {bank_file_count(folder)}, "{folder}" ],')
print(plan_row(name, bank_file_count(folder), folder))
print('];')
print("""
~dirt.doNotReadYet = false; // read AUDIO now, not just WAV headers
......
#!/usr/bin/env python3
"""Can SuperCollider parse the plan we generate, and can the checker read it back?
2026-09-24, gig morning, 10:23. The WORD fix landed the night before made five banks
visible for the first time — `808bd`, `808cy`, `808hc`, `808sd`, `90s_synatm` — and
`--fix` duly wrote them into `preload.scd` as `\\808bd`. **A SuperCollider bare symbol
cannot start with a digit.** The file died at parse time:
ERROR: syntax error, unexpected NAME, expecting ']'
[ \\808bd, 25, ".../Dirt-Samples/808bd" ],
ERROR: Command line parse failed
One bad row costs the WHOLE file, so the plan warmed **0** banks where the day before
it had warmed 132 — a regression dressed as a fix, and invisible unless someone read
the boot journal. Nothing in the rig says "your preload did nothing": `doNotReadYet`
is restored to true, so every bank silently falls back to lazy-on-demand. Exactly the
failure mode the repo's own ethos warns about — *"an artifact nobody compares against
reality is worse than none"*.
Then the fix exposed a second bug, one layer out: `check-preload.sh` read the plan
with its own shell grep for `[ \\name`, which cannot match a quoted row. So the
checker reported **ok** on a plan genuinely short by five banks — `want` came out 132
instead of 137. A checker that cannot read what the emitter writes is worse than no
checker, and that is why both now go through `setlist_samples.plan_row*`.
So this file tests the two halves of one contract, in both directions:
WRITE every row the emitter produces is parseable SuperCollider
READ every row the emitter produces is visible to the checker
python3 tools/tests/test_preload_plan.py
"""
from __future__ import annotations
import importlib.util
import pathlib
import sys
import pytest
ROOT = pathlib.Path(__file__).resolve().parent.parent.parent
spec = importlib.util.spec_from_file_location("ss", ROOT / "tools" / "setlist_samples.py")
SS = importlib.util.module_from_spec(spec)
sys.modules["ss"] = SS
spec.loader.exec_module(SS)
# The names that broke it, and their letter-leading neighbours that never did.
DIGIT_LED = ["808bd", "808cy", "808hc", "808sd", "90s_synatm"]
LETTER_LED = ["jbk_kick", "bd", "amencutup", "_odd", "rampleS13"]
# ── WRITE: nothing we emit can break SuperCollider's parser ──────────────────
@pytest.mark.parametrize("name", DIGIT_LED)
def test_digit_leading_names_are_quoted(name):
"""`\\808bd` is a syntax error; `'808bd'` is the same symbol, legally spelled."""
assert SS.sc_symbol(name) == f"'{name}'"
@pytest.mark.parametrize("name", LETTER_LED)
def test_letter_leading_names_stay_bare(name):
"""Quoting everything would churn 130-odd rows for no reason. Only quote what must be."""
assert SS.sc_symbol(name) == f"\\{name}"
@pytest.mark.parametrize("name", DIGIT_LED + LETTER_LED)
def test_no_emitted_row_starts_a_bare_symbol_with_a_digit(name):
"""The mechanical form of the bug: a backslash immediately followed by a digit."""
row = SS.plan_row(name, 25, f"/samples/{name}")
assert "[ \\" not in row or not row.split("[ \\", 1)[1][0].isdigit(), row
# ── READ: every row we emit is visible to the checker that judges it ─────────
@pytest.mark.parametrize("name", DIGIT_LED + LETTER_LED)
def test_row_roundtrips_through_the_reader(name):
"""plan_row -> plan_row_name must be the identity, or the checker undercounts."""
row = SS.plan_row(name, 7, f"/samples/{name}")
assert SS.plan_row_name(row) == name
@pytest.mark.parametrize("name", DIGIT_LED + LETTER_LED)
def test_row_roundtrips_with_its_count_and_folder(name):
"""plan_rows is what banks()/bank_counts() now read — counts included."""
folder = f"/samples/{name}"
(got_name, got_count, got_folder), = SS.plan_rows(SS.plan_row(name, 42, folder))
assert (got_name, got_count, got_folder) == (name, 42, folder)
def test_a_mixed_plan_is_read_whole():
"""The regression that made the checker say ok: quoted rows skipped, count short."""
plan = "\n".join(
["~pvPreload = ["]
+ [SS.plan_row(n, 3, f"/samples/{n}") for n in DIGIT_LED + LETTER_LED]
+ ["];"]
)
assert [n for n, _, _ in SS.plan_rows(plan)] == DIGIT_LED + LETTER_LED
def test_non_rows_are_not_mistaken_for_rows():
"""plan_rows walks the whole file, so it must ignore everything that is not a row."""
for line in ["];", "~pvPreload = [", "// 17 track(s) scanned", "", "var ok = 0;"]:
assert SS.plan_row_name(line) is None, line
# ── The live artifact, if it exists: it must be parseable RIGHT NOW ──────────
def test_the_checked_in_plan_has_no_unparseable_row():
"""preload.scd is gitignored and load-bearing. If it is here, it must be valid."""
plan = ROOT / "preload.scd"
if not plan.exists():
pytest.skip("no preload.scd in this tree (it is generated and gitignored)")
lines = plan.read_text().split("\n")
start = lines.index("~pvPreload = [")
end = next(i for i in range(start + 1, len(lines)) if lines[i] == "];")
bad = [ln for ln in lines[start + 1:end] if SS.plan_row_name(ln) is None]
assert not bad, f"rows SuperCollider would reject, killing the whole file: {bad}"
if __name__ == "__main__":
sys.exit(pytest.main([__file__, "-v"]))
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment