Move the shared code out, and the bots with it - #3
Closed
chalkwalk wants to merge 141 commits into
Closed
Conversation
Practice mode is about to stop being a separate code path. NinjamClient keys remote players purely off the wire, so a room served on 127.0.0.1 lights up the whole connected UI -- phase bar, remote strips, routing, chat, sync, recording -- with no special-casing at all. FakeNinjamServer could not be promoted to do this: it accepts exactly one client and fakes remote audio by echoing your own uploads back. A room needs genuine N-way relay, so PracticeServer is new. The fixture stays for the fault injection the tests need. The server carries no clock. Ninjam's interval grid is entirely client-side, so a server authenticates, tracks who is in the room, and relays. Two things worth knowing: Subscription is per channel, not per player. An unsubscribed client sends a usermask of zero rather than omitting the entry, so presence in the map is not consent -- and UPLOAD_INTERVAL_WRITE carries no channel index, only a GUID, so the relay remembers which channel each upload belongs to in order to filter the writes that follow. Getting this wrong sent audio to a client that had asked for none, which is the mechanism that keeps a room of bots cheap. Writing to a departed peer killed the process. juce::StreamingSocket::write calls ::send with no flags and JUCE only suppresses SIGPIPE for named pipes, so on Linux the default disposition terminates the host -- a DAW. It is a narrow race that survives casual testing; a server writing to several peers that come and go hits it immediately. SocketWrite suppresses it per write rather than process-wide, because a plugin does not get to change its host's signal disposition. NinjamClient has the same exposure against a real server. The server-side parsers (0x80, 0x81, 0x82) and builders (0x00, 0x02, 0x03) join the rest of the wire format in NinjamProtocol, so they get the same bounds checking and the same truncation sweep. Tests drive real NinjamClients rather than hand-built frames: the property under test is that this room is indistinguishable from one on the network, and a test that spoke the protocol itself could pass while the client saw nothing. ctest: 3/3. ASan/UBSan: 165012 passes, 0 failures, only the four known libvorbis lines. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A bot is a Ninjam client. Not a server-side fake and not a special case
inside NinjamClient: it opens a socket and joins a room like any other
player.
Two things follow, and both are the point. It can join any server -- join()
takes a host and a port and has no idea which it is -- and it exercises the
same code you do, so a practice room tests the real encoder, relay, decoder,
interval delay and mixer rather than a parallel path built to look like them.
That was the whole reason for doing this.
The spike said bots could drive NinjamClient::processCapturedAudio directly:
one call sends one complete interval, it already runs off the audio thread
via callAsync, and writeFull documents being called from several threads
under a lock. So the transmit path needed no changes at all.
Bots must be trivially easy to get rid of, because they can be pointed at a
real server and the failure mode to design against is a bot nobody can evict.
Four rules, all in PracticeBot so they hold wherever it is pointed:
- No reconnection, ever. Server exits, network drops, kicked by an admin:
one path, and terminal. The absence of retry logic is the feature, so it
is commented as such -- otherwise someone will helpfully add it.
- Bots leave when the player who brought them leaves. On a real server this
is the rule that matters: walking away is enough, with nothing to
remember.
- A private message saying part, leave, exit or stop works from ANYONE in
the room, not just the owner. Making people find a bot's owner before they
can remove it is exactly the annoyance being avoided.
- A bot answers help by saying how to remove it.
Each has a test, and the owner-departure rule was checked by breaking it and
watching the test go red.
Bots are deaf by default, via a new NinjamClient::setDefaultRecvEnabled. An
unsubscribed channel never causes the server to send an interval, so it never
causes one to be allocated: a room of bots costs one client's worth of
interval buffers rather than one per bot. Turning recv off after connecting
would leave a window where audio arrives anyway.
One conductor thread drives the whole band rather than one per bot. Bots that
share a clock stay tight with each other for free, which is what a band is.
Its phase is free-running and deliberately not chased to the player's, per
PRINCIPLES 9.
The server now sends real JOIN and PART rather than a MSG saying so, because
NinjamClient only maintains room membership from those, and the
owner-departure rule depends on it being right.
The band is silent so far. Voices are next; the loop was worth proving first.
ctest 3/3. ASan/UBSan 165012 passes, only the four known libvorbis lines.
TSan 165076 passes, zero warnings.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two foundations, no audio yet. Euclidean rhythms, lifted with cosmetic changes from a sibling project (chalkwalk/seq_play src/core/Euclidean.h) where it had already arrived at the shape this codebase wants: header-only, JUCE-free, no allocation in the hot path. One integer buys a pattern that is idiomatic rather than mechanical -- E(3,8) is the tresillo, E(5,8) the cinquillo -- so the drums need no pattern data shipped or maintained. The tests came across too, plus the property that makes these musical (no two gaps differ by more than a step) and an exhaustive check that hit() and pattern() cannot disagree, since the render loop uses one and bar-level reasoning the other. Harmony models a chord as an ABSOLUTE root plus an explicit list of tones, not as a scale degree. That is the whole reason the file exists. A degree can only name a chord diatonic to the current mode, and the interesting harmony is not: a tritone substitution's root is a tritone from the degree it replaces, an altered dominant has tones in no mode of the key, and a borrowed chord is by definition from elsewhere. Root-plus-tones makes all of those expressible now, so adding them later is one function rather than a new model everywhere. realise() is the seam, and says so. Triads are stacked out of the scale rather than looked up per mode, so all seven modes are right for free: Lydian's II is major where Ionian's ii is minor, Dorian's IV major where Aeolian's iv is minor. Both are tested. Defaults are mode-aware. I-V-vi-IV over a minor tonic yields a minor v, which is weak and not what anyone means by "the four chords", so minorish modes get i-VI-III-VII instead -- and minorish is decided by asking the scale for its third rather than by listing modes at the call site. A progression fills exactly one interval, so every interval is a complete loop. Each client plays a received interval from its own downbeat, so this is what keeps the band from drifting against a listener whose phase is its own, and it means a dropped interval costs a bar rather than shifting the harmony from then on. Chord changes are placed by the same Euclidean generator as the drums: N chords over BPI beats is E(N, BPI). At rotation 0 that is exactly an even division -- the Bresenham form and integer division agree, which is a pleasant accident -- so the default is the obvious one and rotation is there to displace changes off the beat when a seed asks. MusicalKey grows scaleSteps and degreeToMidi: it could name notes but not make them. ctest 3/3. Euclidean 536 passes, Harmony 387, both 0 failures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Kit, Bass and Keys join the practice room and play. Drums are Euclidean patterns with Euclidean accents; the bass locks to the kick rather than rolling its own figure, because two unrelated patterns fight where a shared density and a displacement locks; and the keys hold one sustained chord per slot, since a pad that stabs is not a pad. Following the room lives in PracticeBot, not PracticeRoom, and that is the point: tempo and BPI from SERVER_CONFIG_CHANGE, the key from a [key: ...] chat line, the chords from | Am | F | C | G |. Everything a band member needs arrives over the wire, so a bot follows wherever it is pointed, with nothing orchestrating it. The chord parser refuses to guess. Jamtaba's reads "I AM TIRED OF THIS" as a progression because it treats I and l as separators -- a real case in their test suite -- so one unrecognised token rejects the whole line, and prose in chat leaves the harmony alone. Tested. Shake, new or again in room chat rerolls every bot; the same words privately reroll one, so you can keep a drum pattern and change the bassline. The new seed is a hash of the old rather than an increment, so the next figure is unrelated rather than adjacent. The drums clip without help. Three voices overlap and the kick rings for 0.32 s, which at 16 BPI is several hits deep; the measured worst case was 1.41. Nothing between the render and the encoder catches a peak, and Vorbis turns a clipped signal into real distortion, so the kit carries a headroom trim. The clipping was found by a test sweep, not by ear. TSan then found a genuine race in NinjamClient, not in the new code: run() closed the socket without holding writeMutex while writeFull was inside ::send on the message thread. That is a race on the file descriptor itself -- the number can be reused by the next open, so the write lands somewhere else entirely. It has always been there; nothing wrote from another thread at the moment of teardown often enough to catch it until a room had several clients coming and going in one process. Closing under writeMutex fixes it, and the bot's redundant channel-info resend on connect (NinjamClient already sends it on auth) is gone, which was the trigger. Set ANTIPHON_BAND_WAV to hear it. Every other assertion here is statistical and statistics cannot tell you whether a groove is any good. ctest 3/3. ASan/UBSan 166197 passes, only the four known libvorbis lines. TSan 166197 passes, zero warnings -- one before this fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three faults, found by listening. The tests had passed throughout. The bass was in the wrong key. Its anchor was MIDI 28, which is E1 and not C1, and a chord root is a pitch class where 0 means C -- so every root came out a major third sharp while the keys, anchored correctly at 60, played the chord. Two voices disagreeing about the harmony. Both anchors are named constants now, with the reason they must be a C written down. It was also inaudible: 41-78 Hz is below what a laptop or a small monitor reproduces at all. C2 rather than C1 puts it at 65-123 Hz. And it was shaped like a kick -- a sine with a fast exponential decay, in the same octave as one. Pitch cannot separate those two because a bass note and a kick share a register by design; shape and timbre have to. The bass now holds (attack, body, release) for as long as the gap to the next note, and is weighted towards its harmonics rather than its fundamental, which is also what makes it audible on a speaker that cannot reproduce the fundamental. The kick gained a beater click at 1.4 kHz for the same reason: its body lands at 50 Hz, where most speakers do nothing. A chord change now always gets a bass note, whether or not the figure has an onset there. Before, the rotation could mean the change was announced by nobody and the first thing heard over a new chord was its fifth. A bass player lands on the change. Two lessons about the tests, both worth more than the fix: The test that should have caught the wrong key compared the bass against the keys and asserted only that one was below the other. Two things that move together prove nothing (PRINCIPLES 5). It now asserts the absolute pitch class at every chord change. And the first version of that test read three semitones flat, consistently enough to look like a transposition bug in the synthesis. It was the instrument: TestSignal::dominantFrequency counts threshold crossings, and the new bass has a strong second harmonic, so the waveform is asymmetric and its negative lobe does not always reach the threshold. Autocorrelation finds the period instead, where harmonics reinforce the answer rather than confuse it. That is the third measurement error in this project's history and it followed the same shape as the others. Known and not addressed here: the bass has the same pulse count as the kick, so it doubles rather than plays against it. Real bass parts are denser. Next pass. ctest 3/3. BotBand 138 passes, 0 failures. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…sed. The band is now a rhythm section and a lead, which is the point rather than one more instrument: any single part can be muted or sent home and played by a person instead, so the room supports a drummer or a rhythm guitarist as readily as someone soloing over the chords. The melodic writing ports MelodyGen's spine from chalkwalk/seq_play rather than the file, which depends on a 735-line Scale.h that MusicalKey already covers. What was worth having is the coupling of metric strength to note strength: the interval downbeat outranks a bar head, which outranks a half bar, which outranks a beat, which outranks an off-beat eighth -- and strong beats may only take chord tones while weak ones pass through the scale. That is what makes a generated line sound intended rather than sprinkled. Contour and per-interval reroll come from the same place, so the line develops across a phrase instead of repeating. The notes are exact, so they are asserted exactly: in key, chord tones on strong beats, inside the lead register. The bass is saturated now. tanh flattens the peaks and fills in the harmonics, so it reads louder while its peak goes DOWN -- which matters in a mix with headroom to respect, and the added harmonics are what a small speaker actually reproduces. Turning the gain up would have done neither. Two real bugs, both found only because a fourth bot made the timing worse. PracticeRoom::stop ignored what stopThread told it. Four bots rendering an interval is four Vorbis encodes of several seconds each, which under a sanitiser overruns two seconds easily -- so the wait timed out, the return went unchecked, and the bots were destroyed underneath a conductor still using them. FakeNinjamServer carries a comment about this exact mistake costing a day of CI. The return is checked now, the budget is bigger, and run() checks for the exit BETWEEN bots so it can leave mid-render rather than finishing all four first. And the owner-departure rule was hooked to the wrong event. A leaving player produces a USER_INFO_CHANGE marking their channels inactive and THEN a PART, and only the PART removes them from roomMembers -- so checking on user-info alone looked while the owner was still listed, found them present, and never looked again. Release timing hid it; ASan reproduced it every run. The check now runs from both callbacks. PracticeServer no longer locks recursively. juce::CriticalSection permits it and the code worked, but a lock whose depth depends on the call path is one nobody can reason about, and TSan does not model the recursion so it reported every acquisition. The relay helpers now say Locked in their names and the public entry points take the lock. ctest 3/3. ASan/UBSan 166414 passes twice over, no findings outside the four known libvorbis lines. TSan 166414 passes, zero warnings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The melody worked in major and not in minor because only half of MelodyGen's coupling had been ported. It has TWO strength axes -- beat strength and note strength -- and pairs them. The beat axis was here; the note axis had been flattened to "chord tones on strong beats, any scale tone otherwise", which in Aeolian let the flat sixth land on beat three and, since notes are held until the next one, sit on it rather than pass through. The tiers are derived from the chord rather than listed per mode, by the avoid-note rule: a scale tone a semitone above a chord tone is the one that clashes. That gives the flat sixth in Aeolian over i, the fourth in Ionian over I, the flat second in Phrygian -- and correctly leaves Lydian's sharp fourth alone, since it is a whole tone above the third and is the point of the mode rather than a note to handle carefully. Seven modes, one rule. Strong beats now take chord tones only, ordinary beats comfortable scale tones, and off-beats may touch a colour note -- capped to an eighth, so it passes rather than sits. Both halves of the minor problem. The bass is twice as dense at twice the resolution, and lands on every kick. Matching the kick one for one made it sound like a second kick drum. I claimed in a comment that doubling gives the containment for free, because E(2p,2s) contains E(p,s). It does not: at step 2j the test reduces to (2jp) mod s < p, not the kick's (jp) mod s < p. The test I wrote to check the claim disagreed with it, which is the entire reason for writing tests that assert properties rather than restate the implementation. renderBass takes the union of the kick's onsets and the doubled figure's instead, so the property holds by construction, and the test now checks it in the rendered audio rather than in the figure. The pitch instrument needed fixing twice more. Preferring the shortest lag scoring within 90% of the best rejected the subharmonic but overcorrected, landing between semitones and reporting C sharp for a C. Only integer divisions of the best lag are considered now. This is the fourth measurement error in this project's history and the second in this file. ctest 3/3. ASan/UBSan 166875 passes, no findings outside the four known libvorbis lines. TSan 166875 passes, zero warnings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…in it. Doubling the kick's pulses gives a count that shares a factor with the doubled step count, and a Euclidean figure whose pulses and steps share a factor repeats inside the bar: E(8,32) has period four -- `x...` eight times, which is a metronome, not a bass part -- and E(16,32) has period two. So the bass was escaping "sounds like a second kick drum" only to arrive at "sounds like a machine", on every other seed. The count is now nudged to the nearest one coprime with the steps. Nearest, not larger: the union with the kick's onsets already guarantees the bass is never sparser than the kick, so the count is free to move either way and the seed picks the direction. Coprime rather than merely odd, because odd is only sufficient when the step count is a power of two. At BPI 12 and 24 -- both ordinary Ninjam values -- 9, 15 and 21 share a factor of three and repeat anyway. The test sweeps every BPI for exactly this reason. Euclidean gains patternPeriod and nearestCoprimePulses, and patternPeriod is checked against the period the pattern actually has rather than against its own arithmetic. A common factor is a CHOICE, not a fault, and the code says so: a short period is what makes a kick a pulse you can rely on. The nudge is applied to the bass and deliberately not to the drums, and there is a test asserting the kick still gets repeating figures -- it would otherwise be easy to "fix" the drums into losing the thing that makes them drums. ctest 3/3. Euclidean 13211 passes. ASan/UBSan 179556 passes, no findings outside the four known libvorbis lines. TSan 179556 passes, zero warnings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The kick was the quietest thing in the kit and a decaying sine is why: it is the least loud waveform there is for a given peak, so the drum spent all its headroom on a 50 Hz fundamental that a laptop speaker does not reproduce at all. Shaping it with tanh fills in harmonics where small speakers work; measured crest factor falls from 3.58 to 2.42 and the kit gains 3.2 dB with the peak moving 0.80 to 0.84. Then the same function across the whole kit, gently. Three drums summed are three drums; shaping the sum is what makes them one thing, because the nonlinearity sees the total and the hats duck a little under each kick. That intermodulation only exists in the sum, which is why no amount of per-voice shaping produces it. Another 2.4 dB, ending at rms -22.8 dBFS and a worst-case peak of 0.909 across the seed sweep. The bus stage reads the whole buffer, so the drum voice now needs its own cleared buffer rather than merely adding into whatever it is given, and the contract in BotBand.h says so. Each of the four new tests was checked by reinstating the bug: kick drive zero reddens two, bus drive zero reddens one, and saturate as identity reddens four. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two problems with one cause: the chart was being read as a list when it
is a document with timing in it.
"| Dm7 | C# Csus |" says the second bar holds two chords, so Dm7 lasts
twice as long as either. Parsed flat it became three chords evenly
spread -- 3+3+2 beats of an eight-beat interval, where the notation
plainly says 4+2+2. Bars now survive the parse, and a Chart is laid onto
the interval by applying the Euclidean generator twice: bars over the
interval, then each bar's chords over its own beats. At one chord per bar
that is arithmetically what happened before, which is asserted across
bpi 1-16 and one to eight chords rather than assumed -- it is the test
that says every existing recording of the band still sounds the same.
The grid is eighths, so a bar one beat long can still hold two chords,
and the four places that each re-derived chord timing -- the bass change
detector, the keys span scan and the lead's two lookups -- now read one
table. The lead gets a chord change inside a beat for free, which it
could not see before.
Second, "Csus" did not parse at all. The suffix table matched exactly and
knew fourteen spellings, so sus, slash basses, ninths and parenthesised
alterations were all refused -- including three of the Jamtaba vectors
this suite has always claimed to accept. It now reads what players write
and can write it back out again, deriving the name from the tones so an
altered chord names itself without an enum entry for it. Five tones is
still the voicing ceiling: a thirteenth keeps its name, its seventh and
its thirteenth, and loses the rungs between.
That closes a real hole. isChordProgression validated only each token's
first letter, so a line could be coloured green in the chat pane and then
silently rejected by the band. Both now ask the same tokeniser.
Two bugs the new tests caught before the fix: an added ninth with no
seventh under it was named "C9", which is a chord with one more note in
it; and an unbalanced bracket was silently dropped, so "C(" was a C major
triad. Each layout test was checked by reinstating the bug.
ASan on the parser: 566 passes, no findings.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
renderKeys voiced every chord in root position from C4, so C to Am moved all three voices when two of them are the same note. That is the sound of a machine reading a list: no common tone is ever held, and the pad lurches by a sixth where a player would move a tone. Harmony::voiceLead picks an inversion and an octave per chord to minimise total movement, and does it around the CYCLE rather than along the line. The chart repeats every interval, so the last chord's move back to the first is a real move -- it is the seam heard every time round -- and it is costed like any other. Dropping that one term from the objective changes the answer: C Am F G ends on B-D-G with it and on D-G-B without, 12 semitones of movement against 18. There is a test that says exactly that. Measured honestly, the outer loop over every starting voicing has never changed a result: 244 progressions -- every diatonic seventh loop of twelve tonics in four modes, plus chromatic ones -- gave identical voicings with the start fixed. It is kept for a few hundred integer operations because it is the difference between "optimal" and "optimal given where we happened to begin", and the comment says so rather than implying it earns its keep. The tests pin exact voicings. Every looser assertion tried first -- common tones held, the turnaround being a small share of the total, beating root position -- passed under a deliberately broken implementation, which is the whole argument for exactness in an integer layer. Three mutations were checked: one inversion only, no wrap cost, and a free added voice. A slash chord now reaches the bass player, which is whose note it is. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
If someone announces "| Dm7 | G7 | Cmaj7 |" the key is not in doubt, but nothing was asking: the header stayed blank and the band kept playing whatever it started in. Harmony::inferKey scores every candidate by how its chord tones sit against the scale, weighted by how much each tone DISCRIMINATES rather than by how important it sounds. A perfect fifth is in the scale for six of the seven degrees, so it almost never rules a key out; the third is what separates major from minor and Dorian from Aeolian. Roots still count for most. Content alone cannot tell a key from its relative -- they are the same seven notes -- so three things a progression DOES break the tie: opening on the tonic, resolving onto it, and putting a major chord on the fifth degree. That last one is why "Am Dm E7 Am" reads as A minor rather than C major, since E7's G# is in neither scale but E is the fifth of A and nothing at all in C. The confidence threshold is calibrated against the table in the tests rather than picked, and the table is the specification. Half its entries assert that the answer is NOT confident: "Am F C G" is genuinely ambiguous between C major and its relative, and saying so is the correct output, not a failure. A suggestion that is wrong half the time is worse than no suggestion. Three mutations checked: with the third weighted at zero, "Am Dm E7 Am" comes back A major; without the resolution bonus, "G F C G" comes back C major and confident; with a flat prior across modes, four entries lose their confidence. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A chord chart is a document, and a document should be legible in the notation its reader thinks in. Harmony can now go both ways: romanName turns a chord into "ii7", "V7", "bVI" or "#ivo" against a key, and parseDegreeChart turns "| I | vi IV |" or "| 1 | 4 | b6 |" back into chords. Roman numerals are chromatic and mechanical. A chord whose root is not in the scale is named by where it sits -- III7, bVI -- rather than by guessing what it is doing; V7/vi is a claim about intent and two readings are usually defensible, where a root's position is not a matter of opinion. The one convention worth honouring is the tritone, which is #IV to everybody and bV to nobody. Roman case carries the quality, so the symbol must not say it twice: Dm7 in C is ii7, not iim7. An arabic degree takes whatever chord the key already has there, so "1 4 5" is major in a major key and minor in a minor one, while an altered degree stays major because "b6" nearly always means the borrowed major chord. Degrees will never travel on the wire -- the client resolves them against the session key and sends absolute chords -- so a bot, a Jamtaba user and anything else in the room see chords they already understand, and there is one place the resolution can be wrong rather than five. Bar lines are required for a degree chart exactly as they are for chord names, so "2 5 1" in conversation stays conversation. A zero is accepted where a diminished sign is meant. The degree sign is not ASCII and not on a keyboard, and half the people who write viio write vii0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The room could announce a chart and there was nowhere to see it. Now the chart is drawn as a row of chord names above the phase bar, each at the point in the interval where it actually starts, so the teal fill sweeps through them and the next change is something you can see coming rather than read about. Position is the information: it is why this is not a line of text at the end of the key row. The roman numerals go there instead, at the far end of row 2, because what a numeral carries is the shape of the progression rather than its timing. The header grows from 80 to 96 only while a chart is showing, so an idle window is unchanged. Only ever a chart somebody announced. Drawing a progression the room did not agree to would be a lie -- the other players are not playing it -- and the practice room's band is the one case where a default chart is the truth, which waits for the room to be wired into the processor at all. Chords are evidence about the key, so an announced chart is also where inferKey gets asked. A confident guess appears on the existing chip, below a live vote and above the DAW tempo proposal, and clicking it sends exactly the tagged key /key sends. Nothing new goes on the wire and no client decides anything the room did not. /chords takes either notation and resolves degrees locally: "/chords ii V I" leaves as "| Dm7 | G7 | Cmaj7 |", so the bots and any Jamtaba user in the room see chords they already understand. The key and the chart are in the spoken status now, because they are in the drawn one -- the two disagreed about the key before this. The chord SOUNDING is deliberately absent: it changes several times a bar, and reading state that moves on a timer is what PRINCIPLES 11 refuses. The audit gets a chart state, and its "this state was never reached" guard now compares keyboard reach as well as component count -- a chip appearing is the same tree with two more stops in it, and counting only nodes would have called the new state unreached when that is the whole point of it. Seven states, no findings. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
DESIGN.md gains section 6.3 for the harmony layer -- the chord model, why a chart keeps its bars, the layout table every voice reads, voice leading around the loop, and why key inference offers rather than decides. It is appended next to the practice material rather than inserted, because section numbers are cited from src/ and a renumber means sweeping every reference. Section 10.4 gains the chord timeline, the chip precedence ladder, and the rule that the spoken status carries the key and the chart but never the chord sounding now. ROADMAP.md had no work area for the band, the bots or harmony at all -- the last ten commits built a band without one. "The band's harmony" now carries what shipped and what did not: chart repetition, harmony beyond diatonic, fuller voicings, and the practice room still not being wired into the processor, which is why the timeline's practice-room rule is written but unreachable. Two new areas beside it. A tutorial bot, because practice is the best introduction to Antiphon and nothing says so, and it is the natural home for the talking form of the key suggestion. And splitting the client out into its own repository -- written down because the thought recurs, with an explicit instruction to move it to NON-GOALS.md if the answer is no. The harmony readout is added to the existing "Level check gesture" area rather than getting its own: the argument is identical, since a chord changing several times a bar can no more be announced on a timer than a level can. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The synthesis is about to be replaced with physical models, and models are tuned by ear over dozens of small changes. There was no way to do that: the loop was edit a constant, rebuild, run the suite, write an audition WAV. AntiphonVoiceLab renders one voice with every parameter as a flag, so trying a value costs no rebuild, and renders the whole band through the real path so what you hear is what the room hears. It prints peak, rms, crest, fundamental and brightness, and those come from src/AudioMeasure.h -- which is now also what the unit tests assert against. Tuning by ear and setting a test threshold use one instrument rather than two that can quietly disagree. The pitch detector moved there out of a test file's anonymous namespace, and calibrating it against signals of known pitch immediately found two faults in it. It read a 440 Hz sine as 40 Hz. A quarter-second window is 11 periods of 440 to the sample, so lag 1200 correlates exactly as well as lag 109 and floating point decided which won. Nothing in the band had shown it, because the bass sits at 60-140 Hz where the longest lag considered is under three periods and the tie cannot arise -- it would have appeared the moment anything was measured higher up. Fixing that by taking the shortest lag scoring within 2% of the best then read everything 3% sharp, because the correlation is broad around each peak and the first lag over the threshold sits several samples before the true period. It has to be the shortest local MAXIMUM, which is what it now does. Both faults were in code that had been in use for weeks and passing. That is the argument for the whole step: a detector nothing can check is a detector nobody knows is wrong. Also new: brightnessHz, an energy-weighted mean frequency taken from the signal's own slope rather than a spectrum -- exact for a pure tone, one pass, no FFT. It exists to be a second opinion on the measure that compares the bass against the keys, which today is a zero-crossing count and was once fooled by an asymmetric waveform into reading three semitones flat. Baseline for the work to come, from the lab: bass brightness 230 Hz against the keys' 356 Hz, kit 1817 Hz, band mix peak 0.409. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A proposal, not a description: nothing in docs/BOT-CHAT.md is built. It exists to be argued with, and the open questions at the end are the parts genuinely undecided rather than merely unwritten. The premise it argues for is that "more conversational" is the wrong target. A bot that holds a conversation with pattern-matched replies is charming for three exchanges and irritating forever, and a bot with an opinion about everything makes the chat pane useless for the humans. What is worth building is a player who is concentrating: present, quick and precise when asked, and otherwise silent. Presence comes from the readiness, not the chatter. Having no language model is an advantage rather than a compromise. A bot in somebody's jam room must never say anything strange, must behave identically every run so it can be tested, must need no network beyond the Ninjam socket, and must answer instantly. A cue table gives all four by construction. Two things shaped the design. Bots are deaf by construction -- an unsubscribed client is never sent interval data at all, which is what keeps a four-bot room costing one client's worth of buffers -- so a bot cannot know whether you are playing, and must never sound as though it could. And the interactions are chat only: musical interaction is separate future work, so every idea beginning "when the player..." is absent by decision rather than by oversight. The document is deliberately concrete about restraint: a token budget that caps four bots at about eight unprompted lines in five minutes, one flat honest fallback for anything unmatched rather than a plausible guess, "quiet" as important a word as "part", and unprompted speech off outside the practice room. Every one of those is written as an assertion a test can make, because the test that keeps it from becoming annoying is the one worth writing first. Also a section of worked transcripts, including one where twenty minutes of playing produces no lines at all, since if those read as annoying the design is wrong and that is the cheapest way to find out. ROADMAP's tutorial bot area becomes "Bots that talk" and points at it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Teaching moves to a fifth, instrument-less tutor bot. The four players are playing the changes; a drummer who interrupts to explain the interval model is not a drummer. Three properties fall out and each is worth more than it costs: a room started by someone experienced simply has four bots, the tutor parts of its own accord when the thread is done, and because it is not a player its budget can be generous where theirs is mean. The flat fallback is replaced, because it was the design's weakest point and it is what makes rule-based bots feel like talking to a wall. Exact command words work only when you type the magic phrase, which teaches you the thing is a vending machine. So section 5 is now a pipeline rather than a keyword table: pull the slots out of the raw text first, because MusicalKey and Harmony already parse the domain's nouns and need the capitals; normalise away vocatives, contractions and politeness, which is half of what "indirect" means; stem with Porter, which generalises to words nobody listed; repair typos with Damerau-Levenshtein; map 150 surface words onto twenty concepts; read four flags off the sentence's shape -- question, imperative, negation, second person -- which is what a part-of-speech tagger would have been used for without needing one; then score the intents. Deliberately the same shape as Harmony::inferKey: score the candidates, require a margin over the runner-up, and when the margin is not there, say so rather than guess. One idea, two places. Three outcomes rather than two, and the middle one is the point. Confident answers. Ambiguous asks a narrow question naming both candidates, which is nearly free since the scorer already knows what it was torn between. Lost reports what it did recognise instead of shrugging. And courtesy gets silence -- the fallback is for something that looks like a request, and a bot that answers "thanks" with a menu is the wall. One turn of memory, so "and the chords?" resolves. Two fields. The claim that indirect phrasing works is worth nothing unasserted, so it becomes a corpus and a number: a few hundred phrasings with their intents, a second corpus that must resolve to nothing, and a fallback rate to quote and drive down. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
test/fixtures/bot-phrases.txt is 519 lines of what people actually type at a bot in a jam room, written before the parser that has to read them. Nine intents, plus fifteen phrasings that are genuinely ambiguous and must be clarified rather than guessed, plus ninety-two that must not be answered at all -- greetings, courtesy, humans talking to each other, somebody asking after Dave. That last section is the largest on purpose, because it is the one that keeps the bots civil. It is plain text so extending it needs no C++, and the rule is written at the top: when a real phrasing misses, add it, watch the test go red, then widen the lexicon -- and if widening would take more than a word or two, that is the design over-reaching rather than the corpus being short. It also makes "how big should the lexicon be" answerable by measurement instead of by argument. The tutor gets ears, for the owner alone and for exactly one question: does this look like an instrument somebody could hear? Not whether it is any good -- it has no business having an opinion -- but silence, a faint signal, clicks, or clipping, told apart from playing by pitched content OR transients on a grid OR sustained energy with a plausible duty cycle, so that a guitar, a kit and a pad all pass by different routes. Every signal it needs is already in src/AudioMeasure.h. Three rules keep that from becoming a nag, and they matter more than the thresholds: it gates which encouraging line is said and never a criticism, uncertainty says the neutral line, and a sparse part or a quiet warm-up must never be told it is not playing. The presence-only subscription is dropped rather than deferred -- the check needs real decoded audio, so presence was never the thing needed. And section 14 captures something worth not losing: a bot can be more responsive than a human, because it receives a whole interval at once and composes a whole interval at once. At the start of N+1 it holds your complete phrase, ending and all, while a human listener has heard only its opening -- and both are heard at N+2. It can answer your ending in the interval where a person is still hearing it. Not a latency trick; a consequence of the form. With the architecture that keeps it honest: analysis biases the existing generator, and with no analysis the band plays exactly as it does today. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Section 5 was called "Being addressed" and said nothing about being addressed, which mattered because four bots answering one "what are you playing" is the exact annoyance the feature exists to avoid. The rule: at most one bot ever answers, and cold silence is the default. First contact has to be explicit -- a private message, a name, a near-miss on a name, or the instrument noun -- because an unaddressed question in a room of eight is not a question for a bot. After that, follow-ups work without repeating the name for a few turns, which is the difference between a conversation and a series of commands. A message beginning with any other participant's name is for them, and that test comes first, so "dave, what does the kit sound like" is answered by nobody. One answer without coordination, because every bot sees the same chat and the same user list and can therefore compute every other bot's score and answer only if it wins. That is the third use of this trick -- one bot acknowledges a key change, one bot answers a question, and identical inputs through an identical function give agreement for free. test/fixtures/bot-addressing.txt is the corpus, deliberately separate from the phrase corpus because it tests a different axis: not what a message means but whose it is. 102 cases, and the largest single class is the 40 that nobody may answer. Also a new roadmap area for the other half of feeling alive. The parts are generated fresh every interval and never return, so a long session meanders -- the lead literally rerolls its contour from the interval index, which is a rule that says never repeat. The cheap fix is the same trick again: every bot knows the interval index, so a form table, a shared intensity curve and per-voice rest thresholds give recurrence, tension, release and staggered drop-outs with nobody listening to anybody. With the interlock that will bite if it is missed: BotBandTests asserts two consecutive drum intervals are not bit-identical, and real repetition is exactly what breaks that. The answer is not to weaken the test. A phrase should repeat in its figure and never in its performance, which is what the swing and per-hit jitter of the synthesis work provide -- played identically twice it is a loop, played fractionally differently it is a band. The two want doing in that order. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
src/BotDsp.h holds the primitives the physical models need: a state-variable filter, a delay line, a plucked string, a modal bank, band-limited oscillators, a cabinet and a room. No voice uses them yet. JUCE-free, allocation-free and free of Antiphon's own types, so the whole file can be lifted into another project as a copy rather than a port -- which is the point, since seq_play has none of this either. Testing the claims rather than the code found four things. The polyBLEP ported from seq_play makes aliasing WORSE than no correction. The polynomial carries a downward step, and adding it to a saw that already steps downward doubles the discontinuity instead of cancelling it. Measured below the fundamental of a 5 kHz saw at 48 kHz: naive 0.071, seq_play's signs 0.129, corrected 0.033. So upstream's oscillators alias harder than if the feature were switched off, and the same inversion is in its pulse. Worth fixing there. The string's tuning compensation was wrong, and my own first version of it was wrong in a different way. A one-pole loop filter delays by a/(1-a) samples -- 0.3 for a bright pluck, 1.0 for a dull one -- not the half sample I hardcoded. Correcting it took the worst tuning error from 0.34% to 0.031%, and the test is now tight enough that the hardcoded version fails it. That error was invisible until the pitch detector got sharper. It could only report sampleRate over an integer lag, which at 660 Hz and 48 kHz is a resolution of 1.4% -- coarse enough to hide the whole defect. It now fits a parabola through the correlation either side of the peak. And two tests that passed under a deliberately broken implementation, so both were replaced. "The highs go before the fundamental" holds even with no damping at all, because the interpolation in the loop lowpasses by itself; it is now two strings plucked identically and damped differently, which isolates the bridge. The pitch test tolerated 2%, which is six times the error it was meant to catch. Four mutations are now caught: no damping, no delay compensation, the old hardcoded compensation, and seq_play's polyBLEP sign. The flush threshold is -180 dBFS rather than denormal range, so tails reach zero within a second of becoming inaudible and "silent" is an equality a test can assert rather than a small number to argue about. ASan clean over 891 assertions. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
All three drums were the same idea with different filters -- an envelope on a sine, or an envelope on noise. That is why the kit read as chip tune, and why the snare and hat measured crest factors of 17 and 21: noise bursts with a shape on them rather than instruments. The kick is a struck membrane now. Four modes at the Bessel ratios of a circular head -- 1, 1.593, 2.136, 2.653 -- which are INHARMONIC, and that is what stops a drum sounding like a pitched note. The upper modes die in a twentieth of the time the fundamental takes, so the strike darkens within its first tenth of a second. The head's tension falls as it relaxes from the blow, which is the pitch drop the old exponential sweep was imitating without any modes underneath it. And the beater is separate from the head: a couple of milliseconds of bandpassed contact noise that does not ring, which is what makes a kick sound hit rather than played. The snare is two instruments in one shell, and treating them as one is why it sounded like a filtered click. The head is a damped membrane; the wires underneath have their OWN envelope, longer than the head's, so they keep rattling after the shell has stopped. That independence is most of what makes a snare a snare. The hat is metal, and metal is inharmonic: six squares at seq_play's Cymbal ratios through a highpass, plus a shorter noise burst for the two cymbals meeting. Filtered noise alone has no pitch structure at all and the ear hears it as a gate rather than a cymbal. Two constants were re-derived together, and the pair is more interesting than either. Resonators overshoot in a way additive voices cannot, so at the old headroom of 0.55 the sweep clipped at 1.0264. Raising the bus drive turns out to buy loudness and almost no peak control -- 1.1 to 1.8 gained 2.4 dB and moved the worst peak by 0.15 dB -- while headroom does the opposite. So drive now sets the level and the trim sets the ceiling: 0.44 and 1.8 put the kit at rms 0.078, exactly where the additive kit sat, with a worst peak of 0.9599 across 96 combinations rather than 0.909 across 36. Both threshold comments were re-measured rather than left stale. The kick is 3.52 unshaped against 2.50 shaped, so the crest limit of 3.0 still has teeth. The kit is 0.078 with both saturation stages, 0.049 without the kick's own and 0.043 without the bus. One finding worth keeping: brightness would have been the wrong instrument for the kick. Saturating it RAISES its low-order harmonics, which pulls the energy-weighted mean frequency down from 165 Hz to 150 -- so the shaped kick reads as the duller one. Crest is the measure that tracks what is actually happening. ModalBank is peak-normalised rather than energy-normalised, so lengthening a drum's tail no longer quietens it and every gain in the bank does not have to be found again. ASan clean. Before and after WAVs are in ~/antiphon-before and ~/antiphon-after at the same seed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The drums were three sounds in a vacuum, panned nowhere. Now they are a kit heard through overheads: early reflections at prime-ish millisecond taps that differ left from right, into a short diffuse tail, twelve per cent wet. The tap pattern is what tells you how big a room is, which is why this is taps with a little smear behind them rather than a reverb -- a smooth tail with no early pattern reads as an effect rather than as a place. That difference between the sides IS the stereo image. Nothing else here knows about stereo at all: the bass, the keys and the lead are close-miked instruments standing in one spot and stay mono, so the listener's pan control still decides where they sit. BotBand::renderInterval takes a right channel, null for callers that do not want one, and isStereo says which voices fill it. PracticeBot skips its mirror copy for those. This costs no bandwidth whatsoever: the encoder has always run two channels for every bot, so the stereo was already being paid for and was carrying the same samples twice. The room runs BEFORE the bus saturation, for two reasons that agree. The console hears the room rather than the other way round; and the room ADDS its reflections to the dry signal, so the soft clip has to be downstream or the sum leaves the headroom the trim was measured for. The voice lab renders stereo now, and gained a "kit" voice that goes through the real BotBand path rather than driving a bare BotVoice function -- so the room can be heard at all, which it could not be before. Its WAV writer takes a second channel. Three mutations checked: declaring the kit mono, giving both sides the same taps, and dropping the bus stage after the room. ASan clean over 1222 assertions plus the practice room. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The four voices had never been levelled against each other, and the pad
sat 10 dB ABOVE the drums. That is why the new kit was clearer on its own
than in the mix: it could improve as much as it liked and stay buried
under the chords.
Targets, as rms over an interval:
Kit -16 dBFS the anchor
Bass -14 dBFS two up, because the bass carries a jam
Keys -19 dBFS three down, because chords are the floor
Lead -16 dBFS level with the kit
Two deviations from what was asked for, both deliberate.
The kit was asked to sit at -15 rms with a -8 peak, which is a crest
factor of 7 dB -- a smashed bus. Raising the level by saturating harder
costs almost exactly what it gains: pushing the bus drive from 1.8 to 5.0
bought 6 dB of level and spent 5 dB of crest, which is the punch the
modal drums exist for. So level is set by GAIN and made safe by a
ceiling, not by driving the saturator. BotDsp::softClip is lifted from
seq_play: exactly the identity below its knee, asymptotic above it, so
the body of the signal is untouched and only peaks that would have
clipped the encoder are caught. The kit now spends 1.6% of its samples
above the knee, which is peak limiting rather than a brick wall, and
there is a test that says so -- because a ceiling makes "nothing clips"
true by construction and would otherwise let a trim be cranked to ten
and still pass.
The lead was asked for at -12, three above the bass and the loudest thing
in the band. It is level with the kit instead: it is the voice you mute
to play that part yourself, and a melody three dB above the bass owns the
mix rather than joining it. One constant if that reads wrong.
Absolute level is nearly a free parameter here and the targets are less
precious than they look -- every remote channel arrives multiplied by
kDefaultRemoteChannelVolume with a fader of its own. Crest factor is not
free, which is why the anchor sits where the kit needs only gentle
limiting.
The kit's level floor was re-derived, because the trim lifted every
figure and the old 0.068 had stopped discriminating: 0.152 as it stands,
0.096 without the kick's own shaping, 0.087 without the bus stage, so the
floor is 0.12.
The new balance test asserts an ordering and a spread rather than four
numbers, since the exact levels move with the seed and the ordering is
the part that matters. Three mutations checked: the old flat trims fail
it, a cranked trim fails it, and removing the ceiling clips at 1.45.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
RMS is not loudness. A kick and a hi-hat at the same rms are nowhere near
the same loudness, because the ear is far less sensitive at 50 Hz than at
8 kHz -- so balancing a band by rms flatters whatever is lowest, and the
drums were the thing being balanced.
AudioMeasure gains integrated loudness to ITU-R BS.1770: K-weighting as a
pair of biquads derived from the analogue prototype rather than tabulated
for one rate, then the absolute and relative gates, so a sparse part is
measured by how loud it is when it plays rather than by how much silence
surrounds it.
Validated against ffmpeg's ebur128 rather than against itself, which is
the only way a loudness figure means anything. Five signals, agreement
inside 0.05 LU:
1 kHz -20 dBFS stereo -19.99 against -20.0
1 kHz -20 dBFS mono -23.00 against -23.0
1 kHz -6 dBFS stereo -6.01 against -6.0
60 Hz -20 dBFS stereo -23.59 against -23.6
8 kHz -20 dBFS stereo -16.65 against -16.7
The last two are the point of the whole exercise: identical rms, 6.9 LU
apart.
The balance was then checked in loudness and left alone. Measured over
five seeds as the stereo pair each bot actually transmits, it comes to
Bass -11.7, Lead -12.7, Kit -13.4, Keys -16.7 LUFS -- the intended shape,
within 0.7 dB on every voice. The two units agreed to about half a LU
because all four voices carry real midrange; the bass is not a sub, so
K-weighting had little to separate.
Not adjusted, deliberately, and the reason is worth more than the
correction would have been: the KIT'S OWN loudness varies by 3.7 LU from
seed to seed, purely because a busy Euclidean figure has more hits than a
sparse one. Tuning a trim by half a dB against material that moves by
four is false precision. So `shake` currently changes how loud the band
is as well as what it plays, which is now a roadmap item with the
instrument to fix it already in hand.
The voice lab reports LUFS per voice and takes --lufs to normalise a
render onto a target, so an A/B is about timbre and not about which one
is louder. It warns when that would clip: matching a hi-hat to -18 LUFS
wants +11 dB and sends its peaks to 1.5, and a comparison of clipped
files is a comparison of distortion. The warning names the target that
would have fitted.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The band's integrated loudness was already being reported, but the comparison that matters is between whole-band renders -- and the interesting ones come from builds that no longer exist. A render from three commits ago is a file and nothing else, so the only fair way to A/B it against today's is to measure both and bring them to one loudness. `file` takes paths rather than a voice: it reports channels, rate, duration, peak and integrated loudness, and with --lufs writes a matched copy. A mono file is measured duplicated, which is what a bot does to it on the way out. What it says about the work so far, which is worth knowing before listening again: the three band renders are already within 0.7 LU of each other. The original and the rebuilt kit are identical at -19.2 LUFS, and the balance pass came out 0.7 LU quieter rather than louder -- it redistributed rather than added. So the differences heard between them were the sound and not the level, which is what one would hope but not what one should assume. Matched copies at -20 LUFS are in ~/antiphon-after/matched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Karplus-Strong: a delay line the length of the period, a bridge that loses a little every round trip and loses the top first, an excitation injected at one point along the string. What that buys over four summed sines is the thing no additive voice has -- the timbre changes AS the note decays, bright for a tenth of a second and dark for the rest of its life, which is most of what makes a note sound played rather than switched on. It reopens f82d9ce, which made this voice sustained because the plucked version it replaced was a sine with a fast decay, in the same octave as a kick. That commit's argument was that shape and timbre must separate them since pitch cannot; a string that rings with a full set of harmonics satisfies it where a decaying sine never did. On articulation, which is what was asked for. Velocity does NOT switch technique -- a threshold anywhere in the range would make two notes either side of it sound like different instruments. It changes the excitation continuously: harder is louder, brighter, and more percussive at the contact, which is what a real instrument does. Technique is the other axis, chosen once from the seed and held: fingered, picked, muted, each a RANGE that velocity moves along rather than a point. A test sweeps velocity in nine steps and fails if any single step jumps more than 45% of the total range -- deliberately breaking it into a switch at 0.6 makes it fail by 192 Hz. Four things the measurements changed, none of which I would have found by listening. The string's excitation cutoff was absolute, so the bass came out with a spectral centroid of 1.7 kHz -- energy centred around its twelfth harmonic, which is a guitar. Both instruments agreed, so it was not an artefact. It now scales with the note: brightness is about WHICH HARMONIC a string reaches, not which frequency, and the same code then gives a dark bass and a bright guitar. 1700 Hz to 543. Cabinet was one pole pair, which is not a cabinet. A speaker in a box is fourth-order or steeper, and at 12 dB per octave a bass amp still passes enough two-kilohertz content to sound like a very low guitar. The pluck's spectrum was nearly flat to its corner where a real one falls away fast above the first few harmonics; two poles rather than one took the centroid to 543 Hz. And the dynamics were too subtle to measure, which means too subtle to hear: a passing note sat 2.2 dB under an accent and the test could not tell the part from one with no dynamics at all -- 1.34 against 1.28. Widened to 1.50 against 1.27. A muted bass decays in a fifth of the time a fingered one does, so it delivers far less energy and dropped 6 LU out of the band whenever the seed chose it. A player compensates by digging in and so does this -- but gain raises peak and energy together, and at 0.45 s the compensation needed sent single notes to 1.19 and would have parked the voice in the ceiling. 0.7 s is still unmistakably muted and needs half of it. Spread 7.1 LU to 2.3. The balance test now averages across six seeds rather than asserting per seed, and that is forced rather than chosen: a voice's level depends on how busy its figure is, so at an unlucky seed the bass lands a quarter of a decibel under the kit and no trim fixes that without making the others wrong. Making a seed stop changing the volume is on the roadmap. fundamentalHz now analyses at most six periods of its lowest candidate. It is O(samples x lags) and the string tests were measuring half a second at 96 kHz -- 43 seconds in that suite alone, past the ctest timeout -- for precision a handful of periods already gives. 43 seconds to 16, with the 0.1% tuning assertions untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The pad was two detuned sines with a linear envelope, which is the least
realistic thing left in the band. It is now a subtractive voice of the kind
every stage polysynth of the period was: two tunable oscillators, a four-pole
lowpass with an envelope on it, a little noise in the mixer, saturation at
each gain stage, and a stereo chorus on the output.
Deliberately bread and butter. No sync, no ring modulator, no modulation
matrix, and resonance capped well short of self-oscillation -- none of those
is what somebody comping behind a jam is doing.
Three patches, chosen by the seed and then jittered inside ranges chosen by
listening to both ends of each one. The seed does not turn knobs; it picks a
patch and moves inside it, which is what makes a seed-chosen timbre a sweet
spot rather than a lottery. A test asserts the outer bounds across 400 seeds.
BotDsp gains Chorus. The two sides read one delay line a quarter cycle apart
rather than in antiphase: nearly as wide, and it folds down to mono without a
comb filter in it, which matters in a room full of people on one speaker.
Four things the measurements caught that listening would not have:
- The output stage was clamping, not shaping. Every seed peaked at exactly
1.198, which is 1/tanh(1.2) and therefore the ceiling of the shaper. A
note loud enough on its own drives four of them into a limiter.
- The sub-octave square made the pad's own fundamental read an octave low.
Under a four-part voicing that is a second chord sitting on the bass
player. It is gone; the second oscillator is tunable instead, which is
what two oscillators means.
- Patch choice was worth 6.4 LU, so the seed changed how loud the band was.
A measured per-character output level takes it to 2.4 LU.
- The keys are brighter than the bass again (1090 Hz against 559), which
restores the ordering the plucked string inverted and left on the roadmap.
Keys at -16.6 LUFS, unchanged from the old pad, so the A/B is level-matched.
Renders in 0.14 s per four-second interval.
Six mutations proven to fail: chorus bypassed, chorus depth zeroed, no
per-character level, envelope bypassed, detune pulling both oscillators the
same way, resonance opened to self-oscillation, and nothing moving at all.
The pitch test is calibrated against the detector's measured floor of 0.16%
on this signal rather than against a number that looked tight.
ctest 100%, 189631 assertions. ASan clean.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Six changes, all of them from listening rather than from a measurement, and in each case the number that was wrong turned out to be a different number than the obvious one. The brass patch did not sound brassy because its filter envelope had no attack at all -- widest on the first sample and closing from there, which is the shape of something plucked. It swells now: shut at one to two harmonics, a third of a second to open, eight to fourteen times up. That needed the envelope restructured rather than retuned, because it used to decay all the way back to the closed cutoff, so "closed" had to double as a usable sustained tone and there was nowhere to sweep from. It has a sustain now, which is what the third knob on a real one is for. The pad's release had to finish inside its own chord slot, which is a player lifting both hands cleanly between every chord. Notes ring past note-off and overlap the chord that replaces them. Attacks up, releases down. The bass had a tone control fitted, and it is the only filter in that voice that follows the note. The others are right to be fixed -- a body resonance is an air cavity, a cabinet is a speaker in a box -- but that made the instrument's brightness depend on which note it played: 2.2 kHz is the fifth harmonic of a high note and the fiftieth of a low one. 559 -> 417 Hz. The snare read high, and the body was never the reason: 185 Hz is about right for a fourteen-inch drum. What the ear takes for a snare's pitch is the wires and the stick, and those sat at 4.2 kHz and 1.6 kHz, which is a rim. All three moved down, the balance moved off the wires and onto the body, and the tail is longer. Centroid 5968 -> 3460 Hz, and autocorrelation now finds a body where it previously found none at all. And two effects that were switched on but not audible. Measured as mid against side: the kit's room sat 22 dB down at a correlation of 0.988, which is a mono kit with a hint of something behind it. The dry is common to both channels and only the wet differs, so the mix number IS the image. 0.12 -> 0.32 gives -13.6 dB; 0.45 starts sounding like a reverb rather than a room. The keyboard's chorus went 0.55 -> 0.75, from -11.6 dB to -9.6 dB. Levels re-fitted after every change, and the three patches now sit within 0.09 LU of each other. The keys are deliberately 5 dB under the kit rather than the 3.4 an equal loudness would give: loudness says how loud a thing is, not how much room it takes up, and a pair of filtered saws masks far more than two sines at the same reading. That one is a judgement overruling a meter, and it is written down as one. Four new tests -- the brass swell, the release running past note-off, the snare's body, and the bass's tone following the note. One had to be rethought rather than retuned: the longer snare decay made autocorrelation report 81 Hz, a slow beat between two deliberately inharmonic modes. A drum does not have a pitch, so the test now asks the question it can actually answer. ctest 100%. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The lead was three sine harmonics with an envelope. It is now an electric
piano, a guitar or a lead synth, chosen by the seed and pinnable by anyone in
the room.
Three rather than one because of what the lead bot is FOR: it plays the part
you are most likely to want to take over, so you mute it and play that part
yourself. Which instrument is in your way depends entirely on what you are
holding, and a guitarist does not want to practise against a guitar.
- The electric piano is a struck tine: a metal bar clamped at one end, whose
modes are nowhere near harmonic -- the first overtone is around six times
the fundamental, not twice -- with a tonebar alongside it and tremolo,
through an amp. Velocity does something here it does not do elsewhere in
the band: played gently it is nearly a sine and played hard the upper mode
barks, because that is a mode the hammer was too soft to reach rather than
a filter opening.
- The guitar is the BASS'S STRING at a different length, which is the
argument for having built a physical model at all. What separates them is
pick position, bridge damping, how long the note may ring and what box it
is heard through -- every one a property of the instrument rather than of
the synthesis. One model, two instruments.
- The synth is one pulse oscillator into a four-pole filter with a short
sweep on it. Deliberately narrower than the pad: no detune, no chorus. A
line does not need to be wide, it needs to cut, and the things that make a
sound sit in a mix are the wrong things for a sound that must sit on top.
The vibrato is kept from the voice this replaces -- it was the one thing about
the old lead that already sounded played.
Notes ring past the slot they were played in, as the keys now do. A line whose
every note stops dead at the next one is a sequencer.
Asking is by PRIVATE MESSAGE only, and the split from room chat is the point.
The key and the chords are things the whole band must agree about, so they are
shouted; what the soloist is holding is nobody else's business, and "guitar" is
a word that turns up in ordinary conversation -- a room where saying it
silently reconfigures a bot has a poltergeist in it. A bot that does not play
the lead says so rather than accepting a setting it will never read, and any
bot will answer "sound" with what it is playing, which is otherwise
unknowable since the seed picks it.
The override survives a shake. Somebody who asked for a guitar because they
came to practise keyboards has not changed their mind by asking for a
different tune.
Levels fitted as the pad's were: the three instruments were 11.7 LU apart and
are now within 1.0, with the guitar the one left slightly under -- a sparse
plucked voice hits a peak ceiling before it reaches a loudness target, which
is the same limit the muted bass ran into.
ctest 100%.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
NinjamProtocol is the one file of the six whose API changed on the way out: a JUCE-free library cannot have juce::MemoryBlock and juce::String in its signatures, so payloads are ByteBuffer and every string is std::string. That is about a hundred conversions across NinjamClient, PracticeServer, FakeNinjamServer, LoopbackTests and ReferenceFixtureTests, done by hand and by site rather than by pattern -- an earlier attempt at this drove it with regex and produced a .toStdString() on a juce::uint8*, which is why the second attempt reads each one. The seam is deliberately visible rather than wrapped. juce::String constructs implicitly from std::string, so parsed fields flow into the UI untouched; the other direction costs an explicit .toStdString() that says plainly where the boundary is. Three types moved to std::string wholesale -- the usermask and upload-channel maps and the fixture loader -- because their keys and values only ever come off the wire and converting them back was pure ceremony. The extraction found a defect on its first run: the handshake aborted inside buildAuthReply, from three clamps transcribed rather than translated during the port out of this repository. Fixed and covered upstream (9e1e6be); the submodule here is that commit. 124,400 passes, 0 failures; 6/6 ctest. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The last unactioned verdict in ../ECOSYSTEM.md's dependency table. The rule is to take the dependency when the thing has a SPECIFICATION you could fail to meet, and BS.1770 is the clearest case in the ecosystem: 107 lines of K-weighting biquads and two gating passes, deleted in favour of libebur128 (MIT), vendored at modules/libebur128. Not a bug fix. The implementation being removed was right -- the point is that being right once is not the same as staying right, and a reimplementation gives you no way to tell which you have. What is coming is momentary and short-term loudness, loudness range and true peak, each a further piece of the same standard to track by hand. The target is declared in this project's CMakeLists rather than by add_subdirectory: upstream's declares cmake_minimum_required(VERSION 2.8.12), which CMake 4 refuses outright, and also builds a shared library, tests and pkg-config files a static consumer has no use for. The library is one C file. Its bundled queue/ is used on every platform rather than only where sys/queue.h is missing, because Windows has none at all and one behaviour on three platforms beats a conditional nobody wanted. The swap was measured before it was made, not after. Against the five ffmpeg goldens libebur128 lands within 0.048 LU worst case. But those are steady sines, where every block has equal energy and the gate never decides anything -- so they cannot tell two implementations apart, and passing them proves only that the K-weighting matched. Compared directly on material where the gate does decide -- sparse bursts, a passage below the relative threshold, one straddling it, dynamic noise 26 dB apart -- the old code and libebur128 agree to under 0.001 LU on every case. So the relative gate was correct and had no test. It has one now: a tail 14 dB down is discarded and one 6 dB down is not, which brackets the -10 LU threshold from both sides and fails if the gate stops gating. Confirmed to fail with EBUR128_MODE_I weakened to MODE_M, and the silence floor confirmed to fail if -HUGE_VAL is passed through instead of clamped. 124,402 passes, 0 failures; 6/6 ctest. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…writing it. The active-focus block is dated 2026-08-15 and says BotLanguage and BotAnswer have no caller. They do: BotChat wires both into PracticeBot, and this same roadmap still lists BotChat as unbuilt. Marked stale rather than re-derived -- the ordering is a decision to make deliberately, not a side effect of a documentation pass. Its last line, "explicitly not next: breaking the repository up", now reads as contradicted by events and is not. The argument was against restructuring around a feature no user can reach, and nothing was: what left this repository is four pieces of general-purpose code that happened to live here, and the NinjamClient stayed for precisely the reason that entry gives. Worth writing down, because the surrounding text otherwise looks overruled. libebur128's row in the dependency table now says adopted rather than adopt. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… touched. main gained holistic keyboard shortcuts, macOS modifier compatibility, a shortcuts dialog and a SelectionModel on 2026-08-14, while this branch was extracting libraries. Checked before merging, because the same divergence in Anvil turned out to have shipped a library built from stale sources: this one touches nothing that moved. It is editor, channel strips, Shortcuts and their tests -- no protocol, no DSP, no music theory. One conflict, and it is the two halves of the same line. main added ShortcutsDialog.cpp to the source list exactly where this branch removed Sha1.cpp and VorbisCodec.cpp, which now arrive from chalkwalk-ninjam. Kept the addition, kept the removals. Everything else merged clean, including PluginEditor.cpp, which both sides had edited. 124,434 passes (up from 124,402 -- the shortcuts tests came with it), 0 failures, 6/6 ctest, and the accessibility audit still passes, which is the one that would notice if the merge had dropped a control's name. Note for later: main committed a .DS_Store, which is now on this branch too. Left alone rather than deleted as a side effect of a merge. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
.DS_Store is macOS Finder's per-folder metadata -- icon positions, window geometry, view mode -- written into any directory the Finder opens. It is machine-local by definition: 10 KB describing how one folder looked on one Mac, meaningless on anybody else's disk and rewritten every time that Mac looks at the folder again. One arrived in 3a1193b. Removed from the tree, and .gitignore gains a proper OS-detritus section rather than one line -- the Windows and macOS siblings are listed now instead of each getting its own commit later. Removed here rather than by rewriting 3a1193b, which is where it entered: that commit is the tip of a PUBLIC repository with an active fork, and it is already a merge parent of this branch. Rewriting it would break somebody else's pull and still leave the file in this branch's tree, so it would have had to be deleted here regardless. History keeps one dead 10 KB blob; nothing reads it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Twenty-two references across docs, CMake, source headers and tests told the
reader to see ../ECOSYSTEM.md. That file is private -- it lives in its own LAN
repository -- so every one of them was a dead link for anybody who is not its
author, and one of them was inside a FATAL_ERROR message shown at configure
time to someone whose JUCE submodule is missing.
What each reference was actually carrying is kept, stated here instead of
delegated:
* The dependency rule ("take the dependency when the thing has a
SPECIFICATION you could fail to meet") is quoted where it is applied, in
CMakeLists and in AudioMeasure.h, rather than cited.
* The adopted-from-chalkwalk-ninjam headers keep their provenance line; only
the pointer goes.
* The roadmap and AGENTS entries now say an ecosystem plan exists and is kept
outside this repository -- which is true, is not a secret, and is all a
reader needs -- rather than linking it.
* SharedContractTests names its counterpart as "the same suite in Lockstep"
rather than by a path into a private tree.
Saying the plan exists is fine. Linking it, citing its structure, or naming
private on-disk paths is not.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…alysis. The four Chalkwalk applications are disjoint. This roadmap carried a long "Breaking the repository up" section comparing this project to a sibling at length -- which layers exist, which euclidean implementation is right, whose melody generator does what, and a list of improvements one project should take from the other. Thirty-two references to sibling projects, twenty-nine of them inside that one section. It was already marked superseded, and its decisions already live in the ecosystem plan, which covers all four projects rather than this one. So it is replaced by a short account of what actually happened: four pieces of general-purpose code left this repository and came back as submodules. Nothing actionable is lost, which was checked rather than assumed. The melody work this section proposed for Antiphon already has its own home under "Melodic shaping: the two terms held back", stated in this project's own terms. The note-strength model and the melody synthesis are both recorded in the ecosystem plan. Remaining references are anonymised: the FluidLite SF3 loop-point patch keeps the patch name and loses the repository, and the Windows generator finding keeps the fix and loses whose CI hit it first. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`MusicalKey` and `Harmony` are used by the bots AND by the plugin's chat UI -- announcing a key and reading a chord chart are room features that work with no band in the room -- so they belong to neither, and their destination is `chalkwalk-music`, which is strictly JUCE-free. `juce::String` was the only thing keeping them here. `Harmony.cpp` now compiles with `-Isrc -Ilibs/music/include` and no JUCE on the include path at all, which is the claim worth making rather than the absence of a grep hit. `src/TextUtil.h` is the six operations these files actually perform -- trim, case, starts-with, index-of, split, join -- and no more. It is not a string library; it travels with the files when they move, and merges there with `chalkwalk::music::detail`, which already has its own trim and split for Scala files. Guessing now at what that merge wants would be inventing an interface for a caller that does not exist. `RoomHarmony` goes with them: it is 68 lines of pure policy over the two and belongs on the same side of the line. A ctest enforces it. The failure this guards is silent and late -- one `juce::String` added in passing still builds, still passes, and is only found when somebody tries to move the file -- so it is checked the same way the standalone macro is, and it names the file and line. Confirmed to fail by reinstating one. Everything else is boundary conversion at callers that stay JUCE: `PluginEditor`, `PracticeBot`, `ChatFormat`, the labs and the tests. Some of those conversions are permanent, because the UI is a JUCE program; the ones in `BotAnswer` and `BotChat` are not, and go when those files follow. Pure refactor: no behaviour change, and the suites say so -- Harmony 3355, BotBand 8625, BotChat 471, MusicalKey 95, RoomHarmony 22, and the whole of ctest green. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
"Move it to chalkwalk-music" is the obvious reading of the previous
comment and it is wrong for two thirds of the file.
The SCALE half is already duplicated: `Key{tonic, Mode}` is the
diatonic, no-modifier special case of `chalkwalk::music::KeySig`, whose
brightness axis IS the mode -- `toKeySig` in BotBand.cpp maps the seven
one for one and says so. That half should collapse into KeySig: deleted,
not relocated.
The NOTATION half -- spelling a pitch class as Bb or A#, parsing "D
minor", naming a scale's notes -- has no counterpart in that library at
all, which has `modeName` and nothing that reads or spells. It is an
addition rather than a move.
The TAG half is Ninjam wire protocol and must not go there.
Harmony overlaps less than it looks, and the note says why: SoundingChord
is a projection for ranking note strength, whose pitch-class mask cannot
express an extension in its own register or a slash bass.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The bots are secondary to what Antiphon is for and a disproportionate
amount of its code, and they are going to `chalkwalk-jambot`. Moving
them straight out would be a migration whose only proof was that it
still compiled. So the separation happens here first, where the tests
that already cover them keep running against every step of it -- the
same way Anvil's resonator and exciter were separated in place before
they became chalkwalk-physical.
`src/jambot/` is what leaves: the recogniser, the answering, the join,
the names, the ensemble, the instruments and the play states. Sixteen
files, and every one already builds with no reference to a plugin
header.
The `jambot-boundary` ctest is what makes that a boundary rather than a
directory. It lists the outward includes and fails when the set CHANGES
-- in either direction, so a new dependency is caught and a resolved one
cannot rot in the list. Both confirmed by breaking them.
There are exactly three, and they are the extraction's blockers rather
than an oversight:
../Harmony.h and ../MusicalKey.h -- music theory, and already JUCE-free.
They go to chalkwalk-music, and jambot cannot leave before they do:
Antiphon's chat UI parses charts with no band in the room, so putting
them in the bot library would make the plugin depend on the band in
order to read `| Am | F |`.
../ChatFormat.h -- which splits. `isVotableBpm`/`isVotableBpi` are facts
about the server's vote range and belong with the protocol; the rest
is chat rendering and stays here.
PracticeServer and PracticeRoom stay for now, by decision: the server is
small and porting it off juce::StreamingSocket buys nothing yet.
PracticeBot stays with them because it owns a NinjamClient, and
inverting that into an interface the bots declare -- thirteen methods out
and six callbacks in, measured -- is its own step.
Pure move: no behaviour change, and all eight ctest suites green either
side of it. `tools/CMakeLists.txt` is a third source list that AGENTS.md
did not mention; it does now, since it is the one that broke.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`Harmony` and the notational key are music theory, needed by the plugin's chat UI and by the bots alike, so they belonged to neither and are now in `chalkwalk-music` (d1d328c there). This is the consuming half. `src/Harmony.h` is one line: an alias. Nothing of Antiphon's was left to add. `src/MusicalKey.h` is a re-export list plus the part that did NOT move -- the `[key: ...]` tag and the `/key` advice line. Naming the re-exports individually rather than aliasing the namespace is what lets the tag live beside them, and it makes the header an honest statement of what Antiphon takes from the library. Neither shape changes a call site: `Harmony::` and `MusicalKey::` still mean what they did, and the 1,860 lines behind them are somewhere else. The Harmony suite went with the code, as it should -- a library that cannot verify itself is a library you have to trust. `MusicalKeyTests` split along the same seam and what remains here is `KeyTagTests`: how a key travels over NINJAM chat, which is a protocol decision rather than a musical one. One thing this surfaced that was not visible before. `BotAnswer` uses `announcementAdvice` -- the `/key D minor` line a bot tells a player to type -- so the bots need the TAG, not just the key. Both of jambot's remaining outward includes are therefore the same kind of thing, which only became clear once Harmony left: NINJAM protocol text that a bot needs in order to say what to type. `MusicalKey.h` for the tag, `ChatFormat.h` for the vote ranges. Their home is chalkwalk-ninjam, and when they land there the boundary list is empty. Three down to two, and the two are one problem rather than two. The JUCE-free guard follows the same journey: it now watches what is still on its way out rather than what has arrived. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Changing `Harmony` meant committing and pushing chalkwalk-music before Antiphon's three thousand assertions could be run against it, which is a round trip through GitHub to answer a question the local machine already knows. CHALKWALK_MUSIC_DIR, CHALKWALK_DSP_DIR and CHALKWALK_NINJAM_DIR point at a checkout instead of the submodule, as cache or environment variables. One helper rather than three copies of the same block, since the three libraries differ only in their name. Configure prints OVERRIDE when one is set, because the submodule SHA no longer describes what was built -- that is the whole cost of it, and the comment says which things must therefore not use it: CI, and anything whose result is meant to be attributable, PARITY's measurements above all. A number that cannot name the commit that produced it is not a measurement. Same shape and much of the same wording as Anvil's CHALKWALK_PHYSICAL_DIR, deliberately: one pattern across the ecosystem is worth more than a better one used in one place. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The `[key: ...]` envelope and the `!vote` ranges are in
`chalkwalk::ninjam::conventions` now (7faa72c there). This is the
consuming half, and with it `src/jambot` reaches back into Antiphon for
nothing at all.
`src/MusicalKey.h` is three inline functions composing the envelope with
the notation, and `MusicalKey.cpp` is gone entirely. `ChatFormat`
re-exports the vote predicates under the names this project already
uses.
The composition is duplicated -- Antiphon has it and so does
`src/jambot/Music.h` -- and that is deliberate rather than overlooked.
What must not be duplicated is the CONVENTION, and it is not: the
brackets, the prefix and the ranges have one home. `parseName(extract(x))`
is glue, and a shared home for glue would be a fourth place for
something to live, which is the problem this whole exercise has been
about.
Two things that had to be got right rather than guessed:
- The obvious API -- `parseAnnouncement(line) -> Key` -- would have
made the protocol library depend on the music library. Text in, text
out avoids it, and is the better seam anyway: the envelope is not
music and a client carrying some other notation can reuse it.
- jambot may not DEFINE anything in `namespace MusicalKey`, because
Antiphon opens the same namespace to add the tag and two headers
defining one function is a collision rather than a boundary. It
contributes a using-directive and calls the convention directly at
the one site that needs it.
The labs now link chalkwalk::ninjam, which is honest: they render the
band, and the band reads the room's conventions.
Boundary check reads "clean -- nothing reaches back into Antiphon", and
still bites when given something to bite. All eight suites green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The layout map still described three blockers that no longer exist. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`PracticeBot` owned a `NinjamClient`. That is the last thing tying the band to this plugin, and it ties it hard: the bots cannot leave, and the client cannot be replaced by the smaller one a standalone jambot would want. `jambot/BotClient.h` is what a bot actually needs -- thirteen calls out and six back, measured against what `PracticeBot` used rather than designed from what a client can do. It is small because almost none of a client is a bot's business: a bot is deaf by construction, so it needs no mixer, no playback queue, no interval delay and no audio device. JUCE-free and `std::string`, deliberately: this interface IS the line the bots are extracted along, so it must not carry a type from either side of it. `NinjamBotClient` is the adapter, and it is nothing but conversions -- which is the JUCE boundary made into a class you can point at. The payoff arrived immediately and is worth more than the tidiness. `PracticeBot` has never had a test of its own, because every question about it needed a server, a thread and several seconds of waiting; `PracticeRoomTests` does that and takes three minutes. A fake client is thirty lines and answers synchronously, so the class now has six tests covering what it says, what it transmits, when it stops and what it does when told to go. Those tests found a real gap on their first run: a parted bot went on answering. The guard had always been the transport's -- disconnect stopped the messages, so the question never arose -- and the interface promises no such thing. Relying on a guarantee nobody stated is exactly what breaks when the thing underneath is swapped, which is the entire point of having an interface. The bot checks for itself now. One of my own expectations was wrong rather than the code: a chart announced in D minor and moved to D major does NOT transpose its Am, because A is the fifth degree and a minor v is an override rather than something the mode gave. The test says C major now, where the answer is obvious. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`src/jambot` and `PracticeBot` now contain no JUCE at all. That was the last thing standing between the bots and a command line: a library that drags a GUI framework in for its strings cannot run from one. Most of it was substitution -- `std::string`, `std::mutex`, `std::vector`, two float buffers instead of an `AudioBuffer`. One part was not, and it is the reason this needed thinking about rather than sed. TIMERS ARE A HOST CONCERN, like the socket. Three things a bot does are "wait, then check whether it is still worth doing": the arrival roster, the delay before speaking for the band, and the countdown after its owner leaves. All three ran on `juce::Timer`, which is the message thread -- the same thread the client delivers callbacks on. That is not incidental: the band-reply delay reads a flag `onChatMessage` writes, and they cannot overlap today because there is only one thread. A free-standing scheduler thread would have turned that into a race, silently, in code that already passes its tests. So `BotClient` grew a `createTimer`, the host says which thread it fires on, and the threading model is unchanged. `BotAnswer` and `BotChat` came too, since PracticeBot could not be JUCE-free while the thing it asks for words is not. One behaviour nearly went out with the syntax: stripping `.toLowerCase()` from a provenance line would have made a bot say "Dave said so" where the test expects "dave said so". Caught by the suite, but worth naming -- that is the failure mode of a mechanical pass, and the only defence is that the assertions were there first. The `jambot-boundary` check now covers both halves of being extractable: nothing reaches back into Antiphon, and nothing reaches for JUCE. Confirmed by breaking each. `PracticeBot` is still in `src/` for exactly one reason: `RoomHarmony.h`. It is 68 lines of real policy sitting on BOTH shared libraries -- it reads the key envelope and parses a chart -- so it fits in neither, and Antiphon's chat display needs it with no band in the room. That is the same argument that kept Harmony out of the bots, and it wants deciding rather than shuffling. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The layout map described only half the check, and still said JUCE was what kept PracticeBot in src/. It is one include. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`src/jambot` is now the whole of what leaves: the recogniser, the
answering, the ensemble, the instruments, the play states, the bot
itself and the loop that drives it. What stays is the hosting a practice
room needs and a command-line bot does not -- the loopback server, and
the room that puts a server and a band together.
`PracticeBot` was held here by one include. `RoomHarmony` sat on both
shared libraries -- it read the key envelope and parsed a chart -- so it
fitted in neither, and Antiphon's chord display needs it with no band in
the room.
The same seam that unstuck the envelope unsticks this: text in, and only
the text a music library can read. `Harmony::Session` in chalkwalk-music
now owns the RULE -- preserve what was written, re-derive what was
delegated -- taking a key NAME and a chart LINE, with no opinion on how
either arrived. `src/RoomHarmony.h` is seven lines of dispatch over it,
and `PracticeBot` has the same seven inline. Duplicating a dispatch is
cheaper than giving glue a home of its own, and what must not be
duplicated -- the rule and the convention -- is not.
The conductor came across as `jambot::Conductor`: one thread, a deadline
and a callback, free-running because Ninjam's absolute interval phase is
free and chasing a player's would buy nothing. `std::condition_variable`
rather than a sleep, because an interval is seconds long and a process
that waits one out before exiting reads as hung.
Two things I got right for the wrong reason and corrected:
- The predicated wait is not about the common case, where the check
after the wait already catches a stop. It closes the lost-wakeup
window -- a notify landing before the thread reaches the wait would
otherwise cost a whole interval. Measured: no difference in the
suite, which is why the comment now says what it is actually for.
- `stop()` joins rather than waiting to a deadline. The deadline
existed because the bots are destroyed after it; joining is the safe
half of that trade, and `renderOneInterval` checks between bots so
the longest it can block is one bot's encode.
Boundary still clean on both halves. All eight suites green.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`AudioMeasure` is now `chalkwalk::dsp::measure`, and `src/AudioMeasure.h` is an alias to it -- the same shape `Harmony.h` took, so not one call site changed. Nothing about peak, rms, crest, brightness, pitch or loudness was ever specific to a Ninjam client, and the ecosystem had noticed in the worst way: `peak` and `rms` existed three times over across these repositories and `fundamentalHz` twice. Two pitch detectors is two answers to one question. This project has three measurement errors on record and one of them was a private pitch detector that read 294.7 Hz for a 440 Hz tone, chased all the way through a fix before the instrument was suspected (`PRINCIPLES §5`). libebur128 goes with it, which is the part worth reading twice: it is no longer vendored here at all. chalkwalk-dsp keeps its primitives header-only and dependency-free and puts measurement in a SECOND target, so the plugin links `chalkwalk::dsp` and never sees a loudness meter -- `nm -D` on the VST3 finds zero ebur128 symbols. Only the test and tool targets ask for `measure`. The suite went with it too, unchanged, on chalkwalk-ninjam's harness shim: 72 assertions before the move and 72 after. Reinstating a 5% error in `crest` turns the ported suite red, so it is measuring the new header and not a stale one. Verified against the submodule rather than an override: 8/8 ctest, and the `--lufs` normalisation still matches -29.6 to -27.0 LUFS through the tool. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The bots' TESTS move with the bots, and until now nothing said so. The
boundary check globbed `src/jambot` and reported "clean" while the suites
covering that directory included `../src/AudioMeasure.h` sixty-three times.
A directory is not extractable if its tests are not.
So the check grows a second list, over the ten bot suites named individually
-- test/ holds Antiphon's own suites too, and only these travel. Both failure
modes are exercised: an unlisted outward include reports the file and the
header, and a blocker struck off without being removed from the list fails
rather than rotting there.
Outward includes are down to one. `BotDspTests` and `BotBandTests` now reach
for `<chalkwalk/dsp/Measure.h>` directly rather than through Antiphon's alias.
`../src/MusicalKey.h` is what is left and cannot move today: both headers
would open `namespace MusicalKey` in a single build, which is a collision
rather than a boundary. The resolution is written where the blocker is listed
-- five inline functions composing chalkwalk-ninjam's envelope with
chalkwalk-music's notation, glue rather than knowledge, so each side composes
them for itself once the two builds are separate.
`BandPatch` moves to `src/jambot` as well. It is the band's knobs and the
lab's patch format, already JUCE-free and already reaching only inward; it sat
beside the plugin for no reason but history.
Two scripts turn out to have been broken by the earlier staging move, and
surveying the extraction unit is what found them:
- `make_wordlist.py` wrote `src/BotDictionary.h`, a path that no longer
exists. Regenerating the dictionary would have appeared to work and
changed nothing. It now reproduces the committed header byte for byte.
- `lexicon_gaps.py` read `src/BotLanguage.cpp`, and its one-off `botstem`
compile carried no include paths for the shared libraries -- so it had
been failing since the theory moved to chalkwalk-music.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Their suites run inside this project's ctest, so their Windows failure is this project's too -- it just has not been seen here, because this repository's CI has not run since the bots were written. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This project set no standard at all, so it inherited JUCE's C++17 throughout. It is set at the top rather than per-target because `juce_add_plugin` compiles its format wrappers independently with a cxx_std_17 floor, and a PUBLIC `cxx_std_20` never reaches them. The reason it must be one standard and not merely a preference: JUCE has inlines gated on `__cpp_char8_t`, so a tree compiling some translation units at 17 and some at 20 is an ODR hazard. Anvil and the sequencer already carry these three lines and the same comment. One source change: `BotLanguage` held a `Concept concept;` member, and `concept` is a keyword in C++20. It reads `meaning` now, the same rename chalkwalk-jambot took. Submodules follow the three libraries to the same floor. 8/8 ctest. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nothing changes here: this tree still compiles as C++20 throughout, because CMAKE_CXX_STANDARD 20 at the top is the maximum and a library's cxx_std_17 is a minimum. The libraries are simply usable by projects that are not this one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`src/jambot/` is gone, and with it the ten suites covering it, both corpora,
both generator scripts and the boundary check. 38 files. What arrives is
`libs/jambot`, the repository they were extracted into, and the direction of
the dependency is now enforced by the build rather than described by a test:
Antiphon uses the bots, the bots know nothing about Antiphon, and their suite
runs standalone. A boundary a build enforces does not need a test to state it.
This was a fork until now, and it had already cost twice in one day -- the
`concept` rename and the narrowing fix both had to be applied in two places.
What stays is the HOSTING a practice room needs and a command-line bot does
not: `PracticeServer`, `PracticeRoom`, and `NinjamBotClient`, which is the
whole of what ties the band to this plugin's transport. The two labs stay too;
`AntiphonBandLab` is a JUCE GUI application and could never have travelled.
Two things the adoption found, both of which the staging had predicted:
- `chalkwalk_add_library` was not reentrant. Antiphon adds music, dsp and
ninjam, then adds jambot, which added them again -- a duplicate CMake
target name, not a version conflict, and a hard configure failure. It now
returns early when the target exists, the same rule chalkwalk-ninjam
applies to its vendored ogg and vorbis, so a nested library needs no
submodules of its own and the outer SHA describes the build.
- `MusicalKey` collided exactly where the boundary check said it would: both
headers defined the same five glue functions. Antiphon keeps its copy,
because a plugin reading a key out of chat must not need the band, and the
library's moved to `KeyTag`. That reads better than the collision did --
MusicalKey is what a key IS, KeyTag is how a room says it, and they come
from different libraries.
`docs/BOT-CHAT.md` goes too; it was living in both repositories and it is the
bots' design document, not this one's. Its ten cross-references now point at
`libs/jambot/docs/`.
8/8 ctest, with the bots' 25,705 assertions now running as a dependency's
suite rather than ours. `ninjam-unit-tests` drops from 182s to 105s.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CI found a real bug the first time it ran here since 14 August, and it was not in the port: the bots' delay-and-watch arbitration used `base + hash % span`, which has no minimum separation. Two of the four names a default room picks landed 32 ms apart, both timers fired in one scheduling wake on the macOS runner, and four tests failed on one cause -- the roster posted twice, the band answering as a chorus, a half-stopped band answering three-strong. Fixed in chalkwalk-jambot by ranking bots in the sorted list they already compute identically, times a 400 ms stagger. `PracticeRoomTests` builds that roster now too: it is the test that deliberately stops whichever bot would win a flat race, so it has to agree with the bots about who that is. The formatting gate had not run since 14 August either, and 13 files had drifted. Reformatted with the pinned clang-format 20.1.8 -- the system one here is 21.1.8 and disagrees, which is worth knowing before trusting a local `clang-format -i`. 8/8 ctest, format clean. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
Author
|
Closing unopened -- this project does not use pull requests. The branch fast-forwards onto main and goes there directly; it was opened only to get CI to run, which it did, and which found the arbitration race fixed in 61735d8. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
140 commits, and the branch is a clean fast-forward --
mainhas nothing thisbranch lacks. Draft, and opened for CI rather than for merge: this
repository has not been built on macOS or Windows since 14 August, and a good
deal has changed under it.
That gap is not theoretical.
chalkwalk-jambot's first CI run, on codeextracted from here, immediately found a narrowing conversion (
intinto auint8_tinside a braced initialiser) that gcc accepts and both clang andMSVC reject. It had been in the tree for months, invisible because nothing but
gcc had ever compiled it.
What changed
The bots left.
src/jambot/is nowchalkwalk-jambot, extracted
with
git filter-reposo its history came across intact --git log --followstill reaches the commit that first brought a band to the practice room. What
stays here is the hosting a practice room needs and a command-line bot does
not: the loopback server, the room, and
NinjamBotClient.The instruments left.
AudioMeasureischalkwalk::dsp::measure. It movedbecause the ecosystem had grown three copies of
peakandrmsand two offundamentalHz-- and an uncalibrated detector is how a measurement error getsmistaken for a bug.
libebur128went with it and is no longer vendored here;it is a second CMake target, so the plugin links the primitives and never a
loudness meter.
The whole tree is C++20. It was inheriting JUCE's C++17 by setting nothing
at all. It is set at the top rather than per-target because
juce_add_plugincompiles its format wrappers with their own
cxx_std_17floor -- and becauseJUCE has inlines gated on
__cpp_char8_t, a tree mixing standards is an ODRhazard rather than merely an untidy one.
The shared libraries stay at a C++17 floor and are compiled at 20 here,
which is what
target_compile_featuresmeans. Each of them now tests bothstandards on three platforms.
What to look at
src/MusicalKey.hkeeps its[key: ...]composition rather than taking thelibrary's. A plugin reading a key out of chat must not need the band; the
library's copy is
KeyTagfor the same reason.cmake/ChalkwalkLibrary.cmakeis now reentrant. These libraries nest, and asecond
add_subdirectoryis a duplicate target name rather than a versionconflict.
cmake/CheckJambotBoundary.cmakeis gone. It existed to keep the botsextractable; they are extracted, and a boundary the build enforces does not
need a test to describe it.
Verified
8/8 ctest locally on Linux, including the bots' 25,705 assertions running as a
dependency's suite. macOS and Windows are what this PR is for.