Commit 5089eec0 by PLN (Algolia)

fix(surface): a button's ROLE decides its row — the remap had been assigning cells in reading order

PLN found this by ear, twice in ten minutes, while recording OPAL takes:

  "techno drum mask is inverted on d8, should ALWAYS be the mask midiOff on ^60
   and the ply or other multiplier/nassim button effect on the push-release 92"
  "drums in revolution seem way too fast ... yea def an inversion"

## The bug I shipped

Phase 2 of the surface remap (5910aacc) column-aligned every button reference, and
assigned each orbit's two cells in FIRST-APPEARANCE ORDER. That is role-blind, and
tracks conventionally write the gesture line above the gate line:

    $ midiOn  "^92" (ply "1 <2!3 4>")      -- momentary flourish
    $ midiOff "^60" (mask "t(4,8,1)")      -- latched gate

so the first control encountered — the gesture — took the LATCHING cell, and the
gate took the momentary one. Measured on the diff itself rather than on corpus
history: of the button lines that commit moved, **32 were inverted**, 11 happened
to be repaired, 10 were already wrong, 2 were right both times. 13 files, nearly
all of them in the OPAL setlist.

"Way too fast" is the audible signature: a `ply` on a latching button stays
multiplied after one press. The mirror image is a `mask` that only gates while a
finger holds it down. Neither errors. Neither is silent. Only the ear catches it —
which is precisely the class of bug that needs a machine check.

## The convention, now written down once

    row E (41-44, 57-60), latching   -> GATES    (mask, struct)
    row F (73-76, 89-92), momentary  -> GESTURES (ply, fast, stut, chop, slice...)

A real limit of the grid, found while encoding it: **d1-d3 own only ONE button.**
Their row-F cells (73,74,75) are the per-family mutes gMute1/2/3, so a gesture on
^41/^42/^43 is not an inversion — it is the only cell that exists. Every tool here
skips those columns rather than inventing a slot.

## tools/button_roles.py — one judgement, three consumers

migrate-columns.py ASSIGNS cells, fix-button-roles.py REPAIRS them, pvlint PV010
REPORTS them. Three copies of a musical rule is three chances to drift (the #97
lesson, applied before it could bite). Two parsing rules were earned in the space
of one afternoon, and both were wrong in my first cut:

1. **A gate beats a gesture at the SAME level.** `mask "t(8,16,1)" . chop 16` is a
   latched break-gate that happens to chop while open. Treating it as ambiguous is
   what hid revolution's d8 pair from the first scan — the very pair PLN heard.

2. **But only at the same level.** `superimpose (struct "t . t(3,8)" . arpeggiate
   . (|+ note 12))` is a HELD FLOURISH; the `struct` builds the added layer's
   rhythm two levels down and says nothing about the button. A plain substring
   search called phunk's d6 a gate and moved it to the latching row — the same
   inversion, one level deeper. So the role belongs to the top-level chain, with
   nested arguments stripped and string literals skipped (`mask "t(4,8,1)"` would
   otherwise look like it opened two parens and swallowed the rest).

3. **The body is not the rest of the line.** `midiOn "^91" ( -- SLICE!` puts the
   function on the FOLLOWING lines. A same-line regex returns "unknown", and an
   unknown control gets a positional fallback — re-creating the inversion. The
   classifier now follows the parentheses, capped at 8 lines.

## Validation

- 214 tests pass (24 new: 14 for the classifier, 10 for PV010/PV011).
- The classifier is CONVERGENT: applying the repair twice yields 0 further
  rewrites. That is what caught mistakes 2 and 3 — each fix made the tool disagree
  with its own previous pass, and the disagreement was the bug report.
- pvlint on the setlist: 0 errors, and PV010's 12 unfixable cases are reported as
  `info`, not warnings, because an orbit with two gestures genuinely cannot put
  both on the momentary row. A lint that nags about the impossible gets ignored,
  and this one has to stay trustworthy enough to gate a gig.

## PV011 — one FX bus slot shared by two orbits

PLN's ask, right after PV004 surprised him with a shared cut group: "putain a
unexpected cut shared, well done! like shared busses they screw things. can we
lint and flag these? diff bus shared button is ok, but to be detected too". A
`<fx>bus N` is a SLOT in SuperDirt, so two orbits naming the same number share one
instance and the last event to land sets the amount for both. It immediately found
take_5_drops putting d4's bass and d7's choir on `crushbus 41` — the standing
suspect for "d7 feels way more attacky than before". Different effects on the same
number are fine and are not flagged: the namespace is per effect, exactly as he
said.
parent 2cd4108a
#!/usr/bin/env python3
"""button_roles — is this control a GATE or a GESTURE? One answer, three consumers.
THE DISTINCTION
---------------
GATE state you latch and leave mask, struct, gMute
GESTURE a flourish you hold ply, fast, stut, chop, slice, arp, ...
PLN's convention, stated twice in one session and confirmed by ear both times:
*"should ALWAYS be the mask midiOff on ^60 and the ply or other
multiplier/nassim button effect on the push-release 92"*
row E (41-44, 57-60), latching -> GATES
row F (73-76, 89-92), momentary -> GESTURES
WHY THIS IS ITS OWN MODULE
--------------------------
Three tools need the same judgement and must never disagree:
tools/migrate-columns.py assigns button cells when remapping
tools/fix-button-roles.py repairs cells that are already wrong
tools/pvlint (PV010) reports cells that are still wrong
The phase-2 remap (5910aac) shipped 32 inverted pairs because it had no notion of
role at all — it handed out cells in first-appearance order, and tracks
conventionally write the gesture line above the gate line. PLN heard it before any
check did: *"drums in revolution seem way too fast ... yea def an inversion"*. A
`ply` on the latching row stays multiplied after one press; a `mask` on the
momentary row only gates while a finger holds it down. Neither errors, neither is
silent, so only the ear catches it — which is exactly the kind of rule that must
live in ONE place. Three copies of a musical judgement is three chances to drift,
the same lesson as #97 (one generated grid, three consumers).
TWO HARD-WON PARSING RULES
--------------------------
1. **A gate beats a gesture when both appear.** `mask "t(8,16,1)" . chop 16` is a
latched break-gate that happens to chop while it is open. Treating it as
ambiguous is what hid the_revolution_will_be_sampled's d8 pair from the first
scan — the very pair PLN heard.
2. **The body is not the rest of the line.** `midiOn "^91" ( -- SLICE!` puts the
whole function on the FOLLOWING lines, so a same-line regex sees `(` and
concludes "no role". That silence is dangerous, not neutral: an unclassified
control gets a positional fallback, which can re-create the very inversion
being fixed. `site_body` follows the parentheses.
"""
from __future__ import annotations
import re
GATE = re.compile(r"\b(mask|struct|gMute)\b")
GESTURE = re.compile(
r"\b(ply|stut|striate|chop|echo|jux|arp|hurry|fast|loopAt|slice|superimpose)\b")
# The simple, rewritable form. Arithmetic (`midiOn ("^89" - "^53")`) combines two
# controls; its role is genuinely ambiguous and callers must leave it alone.
SIMPLE_CALL = re.compile(r'\bmidi(?:On|Off)\s+"\^(\d+)"')
GATE_ROLE = "gate" # lcxl_grid role name for the latching cell (row E)
GESTURE_ROLE = "gate2" # lcxl_grid role name for the momentary cell (row F)
MAX_BODY_LINES = 8 # a button gesture that spans more than this is not one
def strip_comment(line: str) -> str:
"""Drop a Haskell `--` comment. Deliberately naive: no `--` inside a string
literal appears in this corpus, and a false strip can only LOSE a role (safe,
reported as unknown), never invent one."""
return line.split("--")[0]
def site_body(lines: list[str], idx: int, after: int) -> str:
"""The text a `midiOn "^NN"` actually applies, following unbalanced parens.
`lines` is the whole file (or block); `idx` the 0-based line of the call;
`after` the column just past the `"^NN"` literal. Returns the remainder of
that line plus as many following lines as are needed to balance the
parentheses opened on it (capped at MAX_BODY_LINES).
"""
head = strip_comment(lines[idx])[after:]
depth = head.count("(") - head.count(")")
if depth <= 0:
return head
out = [head]
for j in range(idx + 1, min(idx + 1 + MAX_BODY_LINES, len(lines))):
nxt = strip_comment(lines[j])
out.append(nxt)
depth += nxt.count("(") - nxt.count(")")
if depth <= 0:
break
return " ".join(out)
def top_level(body: str) -> str:
"""The body's OUTERMOST composition chain, with nested arguments removed.
Depth is counted from the `(` that opens the midiOn argument, so the chain
lives at depth 1 and anything deeper is a nested function's own business.
String literals are skipped, because `mask "t(4,8,1)"` would otherwise look
like it opened two parens and hide everything after it.
WHY THIS IS NOT OPTIONAL: `midiOn "^58" (superimpose (struct "t . t(3,8)" .
arpeggiate . (|+ note 12)))` is a HELD FLOURISH — it adds an arpeggiated
layer. A plain substring search sees `struct`, calls it a gate, and moves it
to the latching row: the exact inversion this module exists to prevent, one
level down. The role belongs to the head of the chain, not to any word in it.
"""
out, depth, i, quote = [], 0, 0, False
while i < len(body):
c = body[i]
if quote:
if c == "\\":
i += 2
continue
if c == '"':
quote = False
i += 1
continue
if c == '"':
quote = True
elif c == "(":
depth += 1
elif c == ")":
depth -= 1
elif depth <= 1:
out.append(c)
i += 1
return "".join(out)
def classify(body: str) -> str | None:
"""-> "gate" | "gate2" | None (unknown — callers must not guess).
A GATE beats a GESTURE **at the same level**: `mask "t(8,16,1)" . chop 16` is
a latched break-gate that happens to chop while it is open. Reading that as
ambiguous is what hid the_revolution_will_be_sampled's d8 inversion, the very
pair PLN heard as "drums way too fast".
"""
chain = top_level(body)
if GATE.search(chain):
return GATE_ROLE
if GESTURE.search(chain):
return GESTURE_ROLE
return None
def sites(lines: list[str], pool: set[int] | None = None):
"""Yield (line_idx, cc, col, end, role) for each simple button call.
`col`/`end` bound the `"^NN"` literal itself, so a caller can rewrite exactly
that token and nothing else. `role` may be None.
"""
for i, raw in enumerate(lines):
code = strip_comment(raw)
for m in SIMPLE_CALL.finditer(code):
cc = int(m.group(1))
if pool is not None and cc not in pool:
continue
lit = code.index(f'"^{cc}"', m.start())
yield i, cc, lit, lit + len(f'"^{cc}"'), classify(site_body(lines, i, m.end()))
def describe(role: str | None) -> str:
return {GATE_ROLE: "gate", GESTURE_ROLE: "gesture"}.get(role, "unknown")
#!/usr/bin/env python3
"""fix-button-roles — put GATES back on the latching row and GESTURES on the momentary one.
WHY THIS EXISTS
---------------
The phase-2 surface remap (5910aac, #94) column-aligned every button reference:
each orbit's buttons moved onto its own column. It assigned the two available
slots in **encounter order** — first button reference in the block got row E,
second got row F. That is role-blind, and tracks conventionally write the
gesture line ABOVE the gate line:
$ midiOn "^92" (ply "1 <2!3 4>") -- gesture, momentary
$ midiOff "^60" (mask "t(4,8,1)") -- gate, latching
so the remap systematically swapped them. PLN found it by ear on two tracks
within minutes of each other:
*"techno drum mask is inverted on d8, should ALWAYS be the mask midiOff on ^60
and the ply or other multiplier/nassim button effect on the push-release 92"*
*"drums in revolution seem way too fast ... yea def an inversion"*
"Way too fast" is the audible signature: a `ply` gesture landed on the LATCHING
button, so a single press left the drums permanently multiplied. The reverse is
just as bad — a `mask` on the momentary button means the gate only holds while
a finger holds it down.
THE CONVENTION
--------------
row E (41-44, 57-60), latching -> GATES (mask/struct, usually via midiOff)
row F (73-76, 89-92), momentary -> GESTURES (ply/fast/stut/chop/slice, midiOn)
A REAL LIMIT OF THE GRID, worth knowing before reading the report: **orbits 1-3
have only ONE button slot.** Their row-F cells (73,74,75) are the per-family
mutes gMute1/2/3, so d1-d3 own a row-E button and nothing else. A gesture on
^41/^42/^43 is therefore not an inversion — it is the only cell that exists.
This tool skips those columns rather than inventing a slot, and says so.
SAFETY
------
Every button CC seeds to 0 at boot (BootTidal.hs `_seed`), so permuting CCs
*between two calls in the same block* cannot change either call's default branch:
`midiOn` stays never-applied and `midiOff` stays always-applied. The edit is
event-neutral at boot BY CONSTRUCTION — verify it anyway with
`tools/silent-eval.py --seeded` before and after and diff the verdicts.
USAGE
python3 tools/fix-button-roles.py # dry run, whole corpus
python3 tools/fix-button-roles.py --setlist # dry run, OPAL set only
python3 tools/fix-button-roles.py --apply # rewrite in place
"""
from __future__ import annotations
import argparse
import collections
import pathlib
import re
import sys
sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent))
import lcxl_grid # noqa: E402
import button_roles # noqa: E402
ROOT = pathlib.Path(__file__).resolve().parent.parent
# Role classification lives in tools/button_roles.py — ONE judgement, shared with
# migrate-columns.py (which assigns cells) and pvlint PV010 (which reports them).
# Three copies of a musical rule is three chances to drift.
ANY_CC = re.compile(r'"\^(\d+)"')
ORBIT = re.compile(r"^d(\d+)\b")
def blocks(lines):
"""Yield (orbit, start_index, end_index) for each dN block. Column 0 only."""
starts = [(i, int(m.group(1))) for i, l in enumerate(lines) if (m := ORBIT.match(l))]
for n, (i, orbit) in enumerate(starts):
end = starts[n + 1][0] if n + 1 < len(starts) else len(lines)
yield orbit, i, end
def plan_file(path: pathlib.Path):
"""-> (edits, skips). An edit is (line_idx, col_start, col_end, old_cc, new_cc, why)."""
lines = path.read_text().splitlines()
edits, skips = [], []
for orbit, start, end in blocks(lines):
home = lcxl_grid.orbit_home(orbit)
e_cc, f_cc = home.get("gate"), home.get("gate2")
if e_cc is None or f_cc is None:
continue # d1-d3 (and non-button columns): only one slot exists
block = lines[start:end]
occupied = set() # every button CC referenced in this block, movable or not
for raw in block:
for m in ANY_CC.finditer(button_roles.strip_comment(raw)):
occupied.add(int(m.group(1)))
# Rewritable, role-classified sites. `col`/`end` bound the "^NN" literal
# itself so the rewrite touches that token and nothing else.
sites = [{"line": start + i, "cc": cc, "role": role, "col": col, "end": end_}
for i, cc, col, end_, role in button_roles.sites(block, {e_cc, f_cc})
if role is not None]
if not sites:
continue
gates = [s for s in sites if s["role"] == button_roles.GATE_ROLE]
gests = [s for s in sites if s["role"] == button_roles.GESTURE_ROLE]
wrong = [s for s in sites if s["cc"] != home[s["role"]]]
if not wrong:
continue
tag = f"{path.relative_to(ROOT)}:d{orbit}"
# Case 1 — a clean swap: one gate and one gesture, each on the other's cell.
if len(gates) == 1 and len(gests) == 1 and gates[0]["cc"] == f_cc and gests[0]["cc"] == e_cc:
edits.append((gates[0], e_cc, "swap: gate -> latching row E (was F)"))
edits.append((gests[0], f_cc, "swap: gesture -> momentary row F (was E)"))
continue
# Case 2 — a lone control on the wrong cell, with the right cell free.
if len(sites) == 1:
s = sites[0]
target = home[s["role"]]
noun = button_roles.describe(s["role"])
if target not in occupied:
edits.append((s, target, f"move: {noun} -> its own row"))
else:
skips.append((tag, f"{noun} on ^{s['cc']} wants ^{target}, which is "
f"taken — migrate-columns.py can permute it"))
continue
skips.append((tag, f"{len(gates)} gate(s) + {len(gests)} gesture(s) on "
f"{sorted(s['cc'] for s in sites)} — not a clean swap, needs a human"))
return lines, edits, skips
def apply_edits(lines, edits):
"""Rewrite right-to-left per line so earlier columns stay valid."""
by_line = collections.defaultdict(list)
for site, new_cc, _ in edits:
by_line[site["line"]].append((site, new_cc))
for i, items in by_line.items():
for site, new_cc in sorted(items, key=lambda it: -it[0]["col"]):
l = lines[i]
lines[i] = l[:site["col"]] + f'"^{new_cc}"' + l[site["end"]:]
return lines
def main() -> int:
ap = argparse.ArgumentParser()
ap.add_argument("--apply", action="store_true", help="rewrite files in place")
ap.add_argument("--setlist", action="store_true", help="only the OPAL setlist")
a = ap.parse_args()
files = sorted((ROOT / "live").rglob("*.tidal"))
if a.setlist:
sl = ROOT / "tools" / "setlist.txt"
if not sl.exists():
print(f"no setlist at {sl}", file=sys.stderr)
return 2
names = {l.strip() for l in sl.read_text().splitlines() if l.strip() and not l.startswith("#")}
files = [f for f in files if f.name in names or str(f.relative_to(ROOT)) in names]
total, touched, all_skips = 0, [], []
for f in files:
try:
lines, edits, skips = plan_file(f)
except Exception as exc: # a malformed file must not stop the sweep
print(f" !! {f.relative_to(ROOT)}: {exc}", file=sys.stderr)
continue
all_skips += skips
if not edits:
continue
total += len(edits)
touched.append(f)
print(f"\n{f.relative_to(ROOT)}")
for site, new_cc, why in edits:
noun = button_roles.describe(site["role"])
print(f" L{site['line']+1:>4} ^{site['cc']} -> ^{new_cc:<3} {noun:<7} {why}")
if a.apply:
f.write_text("\n".join(apply_edits(lines, edits)) + "\n")
print(f"\n=== {total} rewrites in {len(touched)} files "
f"({'APPLIED' if a.apply else 'dry run'}) ===")
if all_skips:
print(f"\n=== {len(all_skips)} sites left for a human ===")
for tag, why in all_skips:
print(f" {tag}: {why}")
return 0
if __name__ == "__main__":
sys.exit(main())
...@@ -77,6 +77,7 @@ CC_REF = re.compile(r'"\^(\d+)"') ...@@ -77,6 +77,7 @@ CC_REF = re.compile(r'"\^(\d+)"')
# CC ranges AND the destination arithmetic (`28 + orbit`, `48 + orbit`), which is # CC ranges AND the destination arithmetic (`28 + orbit`, `48 + orbit`), which is
# exactly the duplication that let the HUD's copy go stale after the remap. # exactly the duplication that let the HUD's copy go stale after the remap.
import lcxl_grid # noqa: E402 import lcxl_grid # noqa: E402
import button_roles # noqa: E402
KNOB_A = set(lcxl_grid.ROW_CCS["A"]) KNOB_A = set(lcxl_grid.ROW_CCS["A"])
KNOB_B = set(lcxl_grid.ROW_CCS["B"]) KNOB_B = set(lcxl_grid.ROW_CCS["B"])
...@@ -156,6 +157,29 @@ def _seen_ccs(blocks: list, pool: set[int], helpers: set[int]) -> list[tuple[int ...@@ -156,6 +157,29 @@ def _seen_ccs(blocks: list, pool: set[int], helpers: set[int]) -> list[tuple[int
return seen return seen
def _button_roles(blocks: list) -> dict[int, str]:
"""cc -> "gate" | "gate2", read from what the control DOES (button_roles.py).
THE BUG THIS EXISTS TO PREVENT (#94, found by ear the same day it shipped):
slots used to be handed out in first-appearance order. Tracks conventionally
write the gesture line above the gate line —
$ midiOn "^92" (ply "1 <2!3 4>") -- momentary flourish
$ midiOff "^60" (mask "t(4,8,1)") -- latched gate
— so positional assignment gave the gesture the LATCHING cell and the gate the
momentary one, inverting 32 pairs across 13 files. PLN heard it before any
check did: *"drums in revolution seem way too fast ... yea def an inversion"*.
Encounter order is a fine tiebreak between two gestures; it is not a role.
"""
roles: dict[int, str] = {}
for orb in blocks:
for _i, cc, _c, _e, role in button_roles.sites(orb.lines):
if role is not None:
roles.setdefault(cc, role)
return roles
def segment(lines: list[str], i: int, last: int) -> list[int]: def segment(lines: list[str], i: int, last: int) -> list[int]:
"""The whole `$`-chain segment starting at 1-indexed line i. """The whole `$`-chain segment starting at 1-indexed line i.
...@@ -207,9 +231,24 @@ def plan_track(path: Path, helpers: set[int]) -> dict: ...@@ -207,9 +231,24 @@ def plan_track(path: Path, helpers: set[int]) -> dict:
pairs: list[tuple[int, int, int]] = [] pairs: list[tuple[int, int, int]] = []
for pool, slots in ((KNOBS, knob_slots(number)), for pool, slots in ((KNOBS, knob_slots(number)),
(BUTTONS, button_slots(number))): (BUTTONS, button_slots(number))):
for i, (cc, line) in enumerate(_seen_ccs(orbs, pool, helpers)): sites = _seen_ccs(orbs, pool, helpers)
if i < len(slots): # Buttons are claimed BY ROLE first (see _button_roles); whatever is
pairs.append((cc, line, slots[i])) # left falls back to encounter order over the remaining cells.
free = list(slots)
claimed: dict[int, int] = {}
if pool is BUTTONS:
home = lcxl_grid.orbit_home(number)
roles = _button_roles(orbs)
for cc, _line in sites:
want = home.get(roles.get(cc, ""))
if want is not None and want in free:
claimed[cc] = want
free.remove(want)
for cc, line in sites:
if cc in claimed:
pairs.append((cc, line, claimed[cc]))
elif free:
pairs.append((cc, line, free.pop(0)))
else: else:
overflow.append((number, cc, line)) overflow.append((number, cc, line))
wants[number] = pairs wants[number] = pairs
......
...@@ -542,6 +542,144 @@ def pv009_gain_multiplied_by_zero_seed(track: Track) -> Iterable[Finding]: ...@@ -542,6 +542,144 @@ def pv009_gain_multiplied_by_zero_seed(track: Track) -> Iterable[Finding]:
) )
# --------------------------------------------------------------------------
# PV010 — a GATE on the momentary button row, or a GESTURE on the latching one
# --------------------------------------------------------------------------
@rule
def pv010_button_role_on_wrong_row(track: Track) -> Iterable[Finding]:
"""`midiOn "^60" (ply ...)` — a multiplier latched ON until you press again.
PLN's convention, stated twice in one session and confirmed by ear both times:
*"should ALWAYS be the mask midiOff on ^60 and the ply or other
multiplier/nassim button effect on the push-release 92"*
row E (41-44, 57-60), latching -> GATES (mask/struct)
row F (73-76, 89-92), momentary -> GESTURES (ply/fast/stut/chop/slice)
Earned the hard way: the phase-2 remap (5910aac) assigned each orbit's two
button cells in ENCOUNTER ORDER, and tracks conventionally write the gesture
line above the gate line, so it swapped 32 pairs across 13 files. PLN heard it
before any check did — *"drums in revolution seem way too fast ... yea def an
inversion"*. "Way too fast" is the signature of a `ply` on the latching row:
one press and the drums stay multiplied. The mirror image is a `mask` that
only gates while a finger holds it down.
Neither case errors, and neither is silent — which is why it needs a rule.
NOT flagged, deliberately: orbits d1-d3. Their row-F cells (73,74,75) are the
per-family mutes gMute1/2/3, so those columns own exactly ONE button. A
gesture on ^41/^42/^43 is not an inversion, it is the only cell that exists.
The role judgement is `tools/button_roles.py`, shared with the two tools that
ASSIGN cells (migrate-columns.py) and REPAIR them (fix-button-roles.py) — one
rule, three consumers, no chance to drift.
"""
try:
import lcxl_grid as _g
import button_roles as _br
except ImportError:
return
for orb in track.orbits():
home = _g.orbit_home(orb.number)
gate_cc, gest_cc = home.get(_br.GATE_ROLE), home.get(_br.GESTURE_ROLE)
if gate_cc is None or gest_cc is None:
continue # single-slot column: the convention cannot apply
found = list(_br.sites(orb.lines, {gate_cc, gest_cc}))
roles = {cc: role for _i, cc, _c, _e, role in found if role is not None}
for ln_off, cc, _c, _e, role in found:
if role is None or cc == home[role]:
continue
# Two controls of the SAME role cannot both have their own cell: an
# orbit owns one latching and one momentary button, full stop. That is
# a budget limit, not a mistake, and must not read as one — a lint that
# nags about the impossible is a lint that gets ignored, and this one
# has to stay trustworthy enough to gate a gig.
if roles.get(home[role]) == role:
yield Finding(
rule="PV010",
severity="info",
line=orb.start + ln_off,
message=f"d{orb.number}: two {_br.describe(role)}s share one "
f"column (^{cc} and ^{home[role]}) — only one can be "
f"on the right row",
detail="An orbit owns one latching button and one momentary "
"one, so a second gesture (or a second gate) has to sit "
"on the other row. Not fixable by renumbering: either "
"accept it, or fold the pair onto one control once gSel "
"lands (#54).",
)
continue
row = "momentary row F" if cc == gest_cc else "latching row E"
cure = ("a latched multiplier: one press and it stays on"
if role == _br.GESTURE_ROLE else
"a gate that only holds while you hold the button")
yield Finding(
rule="PV010",
severity="warning",
line=orb.start + ln_off,
message=f"d{orb.number}: {_br.describe(role)} on ^{cc} ({row}) "
f"— wants ^{home[role]}",
detail=f"This gives you {cure}. Gates latch on row E, "
f"gestures are held on row F. "
f"Fix: python3 tools/fix-button-roles.py --apply",
)
# --------------------------------------------------------------------------
# PV011 — one global FX bus slot shared by two orbits
# --------------------------------------------------------------------------
FXBUS_RE = re.compile(r"#\s*(\w+)bus\s+(\d+)")
@rule
def pv011_fx_bus_shared(track: Track) -> Iterable[Finding]:
"""`# crushbus 41` on d4 AND d7 — one effect instance, last writer wins.
PLN's ask, right after PV004 caught a shared cut group he had not expected:
*"putain a unexpected cut shared, well done! like shared busses they screw
things. can we lint and flag these? diff bus shared button is ok, but to be
detected too"*.
A `<fx>bus N` is a SLOT in SuperDirt, not a per-orbit effect. Two orbits
naming the same number get one shared instance, so whichever event lands last
sets the parameter for both — a knob meant for the choir also crushes the
bass, and the amount you hear depends on event density rather than on the
knob. Different effects on the same NUMBER (`crushbus 41` vs `djfbus 41`) are
fine and are not flagged: the slot namespace is per effect.
A warning, not an error: sharing a bus is a legitimate way to glue two orbits
into one colour. The point is that it should be a decision, not a surprise.
"""
owners: dict[tuple[str, int], list[tuple[int, int]]] = {}
for orb in track.orbits():
for ln_off, ln in enumerate(orb.lines):
for m in FXBUS_RE.finditer(strip_comment(ln)):
key = (m.group(1), int(m.group(2)))
owners.setdefault(key, []).append((orb.number, orb.start + ln_off))
for (fx, num), uses in sorted(owners.items()):
orbs = sorted({o for o, _ in uses})
if len(orbs) < 2:
continue
where = ", ".join(f"d{o}" for o in orbs)
first_line_of: dict[int, int] = {}
for o, line in uses:
first_line_of.setdefault(o, line)
for _, line in sorted(first_line_of.items()):
yield Finding(
rule="PV011",
severity="warning",
line=line,
message=f"{fx}bus slot {num} is shared by {where}",
detail=f"One {fx} instance serves both orbits, so the last event "
f"to land sets the amount for both — the knob you turn for "
f"one is heard on the other. Give each orbit its own slot "
f"number, or say in a comment that the glue is deliberate.",
)
def check(track: Track, enabled: set[str] | None = None) -> list[Finding]: def check(track: Track, enabled: set[str] | None = None) -> list[Finding]:
out: list[Finding] = [] out: list[Finding] = []
for fn in RULES: for fn in RULES:
......
...@@ -310,3 +310,87 @@ def test_pv009_ignores_the_djf_knobs_which_seed_to_centre(): ...@@ -310,3 +310,87 @@ def test_pv009_ignores_the_djf_knobs_which_seed_to_centre():
def test_pv009_ignores_commented_lines(): def test_pv009_ignores_commented_lines():
assert lint('d9 $ n "0"\n -- $ (|* gain "^17")\n', "PV009") == [] assert lint('d9 $ n "0"\n -- $ (|* gain "^17")\n', "PV009") == []
# ------------------------------------------------------------------- PV010
def test_pv010_flags_a_gesture_latched_on_row_e():
"""The revolution bug PLN heard: a `ply` on the latching button.
One press and the drums stay multiplied — *"drums in revolution seem way too
fast ... yea def an inversion"*.
"""
f = lint('d8 $ midiOn "^60" (ply "<2 <4 [4 8]>>")\n', "PV010")
assert len(f) == 1
assert f[0].severity == "warning"
assert "^60" in f[0].message and "^92" in f[0].message
assert "latching" in f[0].message
def test_pv010_flags_a_gate_on_the_momentary_row():
"""The mirror image: a mask that only holds while a finger holds it."""
f = lint('d8 $ midiOff "^92" (mask "t(4,8,1)")\n', "PV010")
assert len(f) == 1
assert "^92" in f[0].message and "^60" in f[0].message
def test_pv010_accepts_the_convention():
src = ('d8 $ midiOn "^92" (ply "1 <2!3 4>")\n'
' $ midiOff "^60" (mask "t(4,8,1)")\n')
assert lint(src, "PV010") == []
def test_pv010_a_gate_composed_with_a_gesture_is_a_gate():
"""`mask "t(8,16,1)" . chop 16` is a latched break-gate that chops.
Reading it as ambiguous is what hid revolution's d8 pair from the first scan.
"""
assert lint('d8 $ midiOff "^60" (mask "t(8,16,1)" . chop 16)\n', "PV010") == []
f = lint('d8 $ midiOff "^92" (mask "t(8,16,1)" . chop 16)\n', "PV010")
assert len(f) == 1 and "gate" in f[0].message
def test_pv010_exempts_d1_to_d3_which_own_only_one_button():
"""Row F for d1-d3 is gMute1/2/3, so those columns have ONE cell.
A gesture on ^41 is not an inversion — there is nowhere else to put it.
"""
assert lint('d1 $ midiOn "^41" (ply 2)\n', "PV010") == []
assert lint('d3 $ midiOn "^43" (fast 2)\n', "PV010") == []
def test_pv010_ignores_arithmetic_and_comments():
"""Two-control forms have no single role; commented lines are not code."""
assert lint('d5 $ midiOn ("^89" - "^53") (ply 2)\n', "PV010") == []
assert lint('d8 $ n "0"\n -- $ midiOn "^60" (ply 4)\n', "PV010") == []
# ------------------------------------------------------------------- PV011
def test_pv011_flags_one_fx_slot_shared_by_two_orbits():
"""take_5_drops really does put d4's bass and d7's choir on crushbus 41."""
src = ('d4 $ n "0" # crushbus 41 (range 16 3.5 "^52")\n\n'
'd7 $ n "0" # crushbus 41 (range 16 3.5 "^55")\n')
f = lint(src, "PV011")
assert len(f) == 2 # one per orbit needing an edit
assert all(x.severity == "warning" for x in f)
assert "crushbus slot 41" in f[0].message and "d4, d7" in f[0].message
def test_pv011_different_effects_may_reuse_a_number():
"""The slot namespace is per effect — PLN: "diff bus shared button is ok"."""
src = ('d4 $ n "0" # crushbus 41 (range 16 3.5 "^52")\n\n'
'd7 $ n "0" # djfbus 41 (range 0 1 "^55")\n')
assert lint(src, "PV011") == []
def test_pv011_one_orbit_may_name_its_own_slot_twice():
src = ('d4 $ n "0" # crushbus 41 (range 16 3.5 "^52")\n'
' # crushbus 41 (range 16 3.5 "^52")\n')
assert lint(src, "PV011") == []
def test_pv011_ignores_commented_lines():
src = ('d4 $ n "0" # crushbus 41 (range 16 3.5 "^52")\n\n'
'd7 $ n "0" -- # crushbus 41 (range 16 3.5 "^55")\n')
assert lint(src, "PV011") == []
"""button_roles — the GATE/GESTURE judgement three tools depend on.
If this drifts, migrate-columns assigns the wrong cell, fix-button-roles
"repairs" it to the wrong cell, and PV010 blesses it. So it is tested here rather
than only through its consumers.
"""
import pathlib
import sys
sys.path.insert(0, str(pathlib.Path(__file__).resolve().parent.parent))
import button_roles as br # noqa: E402
# ------------------------------------------------------------------ classify
def test_a_mask_is_a_gate_and_a_ply_is_a_gesture():
assert br.classify('(mask "t(4,8,1)")') == br.GATE_ROLE
assert br.classify('(ply "1 <2!3 4>")') == br.GESTURE_ROLE
def test_a_gate_composed_with_a_gesture_is_a_gate():
"""`mask "t(8,16,1)" . chop 16` is a latched break-gate that chops while open.
Calling this ambiguous is what hid the_revolution_will_be_sampled's d8
inversion from the first scan — the very pair PLN heard as "way too fast".
"""
assert br.classify('(mask "t(8,16,1)" . chop 16)') == br.GATE_ROLE
assert br.classify('(chop 16 . mask "t(8,16,1)")') == br.GATE_ROLE
def test_an_unknown_body_stays_unknown():
"""Never guess. A wrong guess gets ACTED on; None makes the caller fall back."""
assert br.classify('(# n "102")') is None
assert br.classify("(") is None
# ----------------------------------------------------------------- site_body
def test_body_follows_a_gesture_opened_on_the_next_line():
"""The real you_my_sunshine d7 shape: the paren opens, then a comment, then
the function. A same-line regex sees `(` and says "unknown" — and an unknown
control gets a positional fallback, which can re-create the inversion."""
lines = [' $ midiOn "^91" ( -- SLICE!',
' slice 4 "0 1 2 3"',
' )']
sites = list(br.sites(lines))
assert len(sites) == 1
assert sites[0][1] == 91
assert sites[0][4] == br.GESTURE_ROLE
def test_body_stops_at_the_closing_paren_and_does_not_bleed():
"""A gate on one line must not absorb the NEXT line's gesture."""
lines = [' $ midiOff "^60" (mask "t(4,8,1)")',
' $ midiOn "^92" (ply 4)']
roles = {cc: role for _i, cc, _c, _e, role in br.sites(lines)}
assert roles == {60: br.GATE_ROLE, 92: br.GESTURE_ROLE}
def test_a_runaway_body_is_capped():
"""An unbalanced paren must not swallow the rest of the file."""
lines = [' $ midiOn "^92" ('] + [' x'] * 50 + [' mask "t"']
sites = list(br.sites(lines))
assert sites[0][4] is None # the mask is past the cap, so: unknown
# --------------------------------------------------------------------- sites
def test_arithmetic_forms_are_not_reported():
"""`midiOn ("^89" - "^53")` combines two controls; its role is ambiguous and
it must never be moved."""
assert list(br.sites([' $ midiOn ("^89" - "^53") (ply 2)'])) == []
def test_comments_are_not_code():
assert list(br.sites([' -- $ midiOn "^60" (ply 4)'])) == []
def test_the_pool_filters_and_the_span_bounds_only_the_literal():
lines = [' $ midiOn "^60" (ply 4)']
assert list(br.sites(lines, {92})) == []
i, cc, col, end, role = next(iter(br.sites(lines, {60})))
assert lines[i][col:end] == '"^60"'
assert (cc, role) == (60, br.GESTURE_ROLE)
def test_role_names_are_the_grid_role_names():
"""They index lcxl_grid.orbit_home() directly, so they must match it."""
import lcxl_grid
home = lcxl_grid.orbit_home(8)
assert home[br.GATE_ROLE] == 60
assert home[br.GESTURE_ROLE] == 92
# ----------------------------------------------------------------- top_level
def test_a_gate_word_nested_inside_a_gesture_does_not_win():
"""The real phunk d6 shape. `superimpose (struct ... . arpeggiate ...)` is a
held flourish that adds an arpeggiated layer; the `struct` builds that layer's
rhythm two levels down and says nothing about the button's role.
Before top_level(), this classified as a GATE and the repair tool moved it to
the latching row — the inversion, reintroduced one level down.
"""
body = ('(superimpose (\n struct "t . t(3,8)"\n . arpeggiate\n'
' . (|+ note 12)\n . (# cut 51)\n))')
assert br.classify(body) == br.GESTURE_ROLE
def test_a_gate_at_the_top_of_the_chain_still_wins():
assert br.classify('(mask "t(8,16,1)" . chop 16)') == br.GATE_ROLE
def test_parens_inside_a_string_do_not_break_depth_tracking():
"""`mask "t(4,8,1)"` must not read as two extra open parens — that would hide
the rest of the chain and silently return "unknown"."""
assert br.classify('(mask "t(4,8,1)")') == br.GATE_ROLE
assert br.top_level('(mask "t(4,8,1)" . chop 16)').count("(") == 0
assert "chop" in br.top_level('(mask "t(4,8,1)" . chop 16)')
def test_top_level_drops_nested_arguments_only():
chain = br.top_level('(ply 2 . (# pan "0.85 0.15") . (|+ note 12))')
assert "ply" in chain
assert "pan" not in chain and "note" not in chain
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