Skip to content

spec: MIDI-voiced sound sources — soundfont engine, source role, GM floor, tones binding (1.18.0) — FEP #61 - #62

Merged
topkoa merged 3 commits into
mainfrom
fep/sound-source-realization
Jul 21, 2026
Merged

spec: MIDI-voiced sound sources — soundfont engine, source role, GM floor, tones binding (1.18.0) — FEP #61#62
topkoa merged 3 commits into
mainfrom
fep/sound-source-realization

Conversation

@gionnibgud

@gionnibgud gionnibgud commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Implements FEP #61 (closes #61): a chart can now declare what a MIDI part — piano, keys, drums — should sound like, with a guaranteed General-MIDI floor. It extends the existing rig model (#44) rather than adding a parallel format.

What lands

  • soundfont realization engine (source blocks only; sf2 reserved, format open), addressed as library ref + voice bank/program (both OPTIONAL, absent ⇒ 0).
  • source block role — a generator at the head of the chain; its input is the Reader-supplied MIDI event stream (pack note data during playback or live controller input alike). Source-plus-effects composition falls out of the existing graph model (input(MIDI) → source → amp → reverb → output).
  • intent.gm — a normative-when-present General-MIDI floor so a GM-only Reader still voices the part ({program} melodic; {percussion, kit} kits). Kit numbers are the 0-based wire/SF2 values (0 = Standard, 8 = Room, …), matching gm.program.
  • Binding (FEP option A): arrangement-entry tones (§5.2), available whether or not the arrangement has a file, with manifest-wins-wholesale precedence over in-JSON tones (Writers SHOULD NOT emit both); new top-level drum_tones anchors the drum part's binding next to drum_tab.
  • Conformance (§7.9): realization selection now gates on engine and format (a general clarification — already implicitly true for plugin); source blocks SHOULD fall back to intent.gm rather than skip; control events are honoured by whatever voices the part.
  • Prose: rigs are instrument-neutral signal chains (§5.1, §6.9, §7.9 framing).

Compatibility

Additive MINOR (1.17.0 → 1.18.0; retargeted after #63 took 1.17.0) per §4.2 — no required field removed, renamed, or repurposed. Unknown engine/role values are MUST-preserve (§7.9), and an older reader — never having heard of intent.gm — voices nothing for such a part, exactly the silence it produces today. Every schema stays additionalProperties: true, so a 1.18 pack validates under earlier schemas.

Composed with #63 (drums as arrangements, 1.17.0): a type: drums arrangement binds sound through its entry tones (precedence); the top-level drum_tones binds the song-level primary part and is the fallback for packs without drum arrangements; a Reader MUST NOT apply both to the same part — the same alias pattern #63 uses for drum_tab. Stated in §7.5 "Multiple drum parts".

Two small normative additions beyond the FEP's literal text

Flagging these so they're reviewed as choices, not missed:

  • soundfont is scoped "source blocks only" in the reserved-engine list (a generator engine on an effect block is meaningless).
  • bank/program are stated OPTIONAL, absent ⇒ 0 (the SF2 default), closing an otherwise-undefined case.
  • intent.gm.kit likewise absent ⇒ 0 (Standard) — from the review's underspecified-corners list.

Open items intentionally left for follow-ups

None block this PR. Future MINORs may add non-sf2 soundfont formats (sfz, ds) — format is left open for exactly that — as renderers appear.

Checks

tools/check_versions.py, tools/validate.py (both example packs, dir + zip form), the pytest suite (57), mkdocs build --strict (link/anchor check), the Draft 2020-12 schema self-check, and ruff all pass locally. The extended example pack exercises a source rig for both a keys grand piano and a drum kit, plus both binding keys.


Spec side of the instrument-pathways program, co-owned with the editor team (see the FEP #61 thread); discussion is on Discord.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • Added soundfont-based MIDI sound sources with GM floor support and bank/preset selection.
    • Introduced manifest drum_tones for the primary drum part and per-arrangement tones bindings with wholesale override precedence.
    • Added grand-piano and rock-kit rig examples.
    • Clarified realization conformance (requires matching engine/format) and tightened voiced-source skipping/control-event behavior.
  • Documentation

    • Updated the feedpak specification, README, and changelog to v1.18.0 and refreshed the manifest/rigs schemas accordingly.

@coderabbitai

coderabbitai Bot commented Jul 20, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Feedpak v1.18.0 adds MIDI-voiced source rigs with soundfont realizations and General MIDI metadata, introduces arrangement and drum tone bindings, updates schemas and examples, and synchronizes specification version references.

Changes

Feedpak v1.18.0 specification

Layer / File(s) Summary
MIDI source rig contracts
schemas/rigs.schema.json, spec/feedpak-v1.md
Defines the reserved source role, optional intent.gm, soundfont realizations, engine/format gating, fallback voicing, and control-event behavior.
Tone binding precedence
schemas/manifest.schema.json, spec/feedpak-v1.md
Adds arrangement tones override semantics and top-level drum_tones fallback behavior.
Examples and release alignment
examples/extended.feedpak/*, CHANGELOG.md, README.md, spec/feedpak-v1.md
Adds piano and drum soundfont fixtures and updates v1.18.0 version references and release documentation.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

  • Issue 44 — Extends the engine-agnostic rig and tone-binding model with MIDI sound sources and soundfont support.

Suggested reviewers: topkoa, chrisbewithyou

Sequence Diagram(s)

sequenceDiagram
  participant Reader
  participant Manifest
  participant RigsJSON
  participant Soundfont
  Reader->>Manifest: Read tones or drum_tones binding
  Manifest->>RigsJSON: Resolve base_rig
  RigsJSON->>Soundfont: Load selected sf2 realization
  Soundfont-->>Reader: Voice MIDI events
Loading
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed The PR implements the requested additive MIDI-voice extension: soundfont source blocks, source role, GM fallback, tones/drum_tones binding, and updated conformance prose.
Out of Scope Changes check ✅ Passed The changes stay within the spec, schema, examples, and versioning updates needed for FEP #61, with no unrelated features introduced.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main spec update: MIDI-voiced sound sources, soundfont engine, source role, GM floor, and tones binding for 1.18.0.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fep/sound-source-realization

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@ChrisBeWithYou

Copy link
Copy Markdown
Contributor

Editor side — compatibility confirmed ✅ (co-owner note from the feedback-plugin-editor drums-as-arrangement + instrument-type work).

Checked #62 against the open editor PRs — no editor changes needed, and the design choices reinforce each other:

Two forward-looking coordination notes (not blockers):

  1. When the editor grows multiple drum charts (its own parts[]/per-arrangement-drum_tab FEP), the single top-level drum_tones will want a per-drum-part story — worth reconciling when that FEP lands.
  2. Whoever implements spec: MIDI-voiced sound sources — soundfont engine, source role, GM floor, tones binding (1.18.0) — FEP #61 #62 in the editor backend should honor "SHOULD NOT emit both manifest + in-JSON tones" — a routes.py concern, untouched by the current frontend PRs.

Nice bonus: the drum GM floor (intent.gm = {percussion, kit}, 0-based) lines up directly with the editor's existing GM-percussion voicing — persisting a charter's drum-kit choice into drum_tones is a clean future consumer.

@topkoa topkoa left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed against latest main. Nothing blocking — the cross-references check out (the §7.9 control-events bullet's spd/sph/spu and hi-hat/choke citations are accurate), all five check_versions.py locations are updated, the example placeholders follow the existing nam/ir conventions, and the wholesale-precedence rule is cleanly stated from both ends. Three small inline comments below, plus one non-blocking observation:

Underspecified corners (arguably fine under the format's permissive style — noting them so leaving them open is a choice, and a future clarification FEP can pick them up):

  • intent.gm carrying both program and percussion/kit — undefined which wins.
  • percussion: true with kit absent — presumably kit 0 (Standard), by analogy with the bank/program absent-⇒-0 rule, but unstated.
  • A source block that is not at the head of the chain: the spec defines the head-position case and scopes out parallel audio input, but never says how a Reader should treat a nonconforming mid-chain source (skip it? treat the rig as invalid?).

Comment thread spec/feedpak-v1.md Outdated
Comment thread spec/feedpak-v1.md Outdated
Comment thread examples/extended.feedpak/rigs.json Outdated
@gionnibgud

Copy link
Copy Markdown
Contributor Author

Thanks — all three were right, all three fixed in 73b8da6.

  1. Header date. A genuine miss on my side. I checked c536d76 and it changed exactly the two things you'd expect — the header Date: and the §4.1 version-history ledger paragraph. I did the ledger and missed the date, and as you say check_versions.py doesn't cover that line, so CI was never going to catch it. Now 2026-07-20, matching the CHANGELOG.

  2. Realizations row vs. the conformance bullet. Sharpest catch of the three — this PR created the mismatch by amending the bullet and leaving the table row behind. Row now reads "…whose engine (and format, where the engine defines one) it supports…", your suggested wording verbatim. One extra: schemas/rigs.schema.json carries the same sentence in the realizations $comment, so I mirrored it there too and it would otherwise have drifted the same way.

  3. "Rock Kit" with kit: 0. Agreed, and the "examples get copied" argument is the right one — that snippet is exactly what people will paste. Now kit: 16 (Power) with the sf2 program moved to 16 to match, so the example is coherent end to end.

Full local CI mirror re-run after the changes: check_versions, validate (both packs), pytest, mkdocs build --strict, the Draft 2020-12 schema check, and ruff all pass.

@gionnibgud

Copy link
Copy Markdown
Contributor Author

Thanks for the compatibility pass — good to have it confirmed from the editor side rather than assumed.

Two things in there are especially useful to have on record:

  • The editor already emits arrangements[*].tones. That means this PR formalises something the editor is already doing, rather than asking it to change — the binding is aligned, not ahead of the client.
  • Drums-as-type:"drums" is a session representation that still persists as song-level drum_tab. That's exactly the distinction that made drum_tones a top-level key here. Worth noting: an earlier read on the [FEP] Sound-source realization — MIDI-driven instrument voices (soundfont engine, source role, GM floor) #61 thread had me conclude the opposite — that the editor's drums-as-arrangement work would retire the anchor — and the pathway charrette's §2.5 ("do not promote them to first-class manifest arrangements") is what corrected it. Your note confirms that correction from the implementation side.

On your two forward-looking notes, both recorded on our side:

  1. Per-drum-part binding when multiple drum charts land. Agreed this is the reconciliation point. It belongs with the deferred parts[] frame rather than here — when drums genuinely become multiple parts, drum_tones becomes the single-part case and binding moves per-part, exactly as an arrangement's tones works today. That's a shape this PR is deliberately forward-compatible with.
  2. Backend honoring "SHOULD NOT emit both". Noted as a routes.py concern, out of scope for the current frontend PRs. Whoever picks up the writer side should treat the manifest entry as authoritative and simply not write the in-JSON tones alongside it.

Nothing here needs a change to #62 — flagging both as follow-ups rather than folding them in.

topkoa
topkoa previously approved these changes Jul 21, 2026

@topkoa topkoa left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All three addressed exactly in 73b8da6 — header date now matches the CHANGELOG, the realizations row and the mirrored schema \ now agree with the §7.9 bullet (good catch extending it to rigs.schema.json), and the Power-kit example is coherent end to end. CI green on the new head. The underspecified corners from the review body are fine as recorded follow-ups. LGTM — approving.

@topkoa

topkoa commented Jul 21, 2026

Copy link
Copy Markdown
Collaborator

Heads-up: this PR went CONFLICTING after #63 (drums as arrangements) merged to main this morning. Flagging what the rebase involves so it's one pass rather than a discovery loop — the approval stands, and none of this changes the design.

Textual conflicts (4 files). Both PRs edit the same spots:

  • CHANGELOG.md — both add a ## [1.17.0] section at the top.
  • spec/feedpak-v1.md — version header, the §4.1 ledger paragraph, and the §5.2 arrangement-entry table (this PR adds tones right where spec: drums as arrangements — type:"drums" + per-arrangement drum_tab pointer (1.17.0) #63 added drum_tab, both after notation).
  • schemas/manifest.schema.json — both add a property to arrangementEntry.
  • examples/extended.feedpak/manifest.yamlfeedpak_version plus adjacent edits in the arrangements/drum region.

(README.md merges clean only because both sides made the identical version edit.)

Version collision — main is now 1.17.0. #63 claimed it first, so this PR needs a re-bump to 1.18.0 across all five check_versions.py locations, with its CHANGELOG section slotted above #63's.

One deliberate edit, not just conflict resolution: the §5.1 rationale for top-level drum_tones"carried at the top level because drum_tab is a song-level key with no arrangements[] entry" — is no longer true on main. With #63, a drum part can be a type: "drums" arrangement entry, which can carry the ordinary per-arrangement tones binding. This is exactly the per-drum-part reconciliation flagged in the thread above (it just arrived a day later than expected). Suggested resolution, a couple of sentences rather than a redesign:

  • Scope drum_tones to the song-level/primary drum_tab key specifically.
  • Note that a type: "drums" arrangement entry binds its sound via the entry-level tones this PR already defines — multi-drum packs fall out for free.
  • Precedence question worth closing while in there: for the primary drum part, if a pack carries both top-level drum_tones and a type: "drums" arrangement entry aliasing the song-level drum_tab with its own tones (the pattern in spec: drums as arrangements — type:"drums" + per-arrangement drum_tab pointer (1.17.0) #63's extended example), which wins? The manifest-wins-wholesale spirit suggests the entry's tones, with drum_tones as the fallback for packs without drum arrangements — but stating it beats implying it.

Happy to re-review promptly once it's rebased so this doesn't stall.

gionnibgud and others added 3 commits July 21, 2026 16:20
…loor, tones binding (1.17.0) — FEP #61

A chart can now declare what a MIDI part (piano, keys, drums) should sound
like, with a guaranteed General-MIDI floor:

- reserved realization engine 'soundfont' (source blocks only; sf2 reserved,
  format open), addressed as ref + bank/program
- reserved block role 'source' — generator at the head of the chain, input is
  the Reader-supplied MIDI event stream
- OPTIONAL intent.gm — normative-when-present GM floor; kit numbers are
  0-based wire/SF2 values
- binding: arrangement-entry tones (manifest, wholesale precedence over
  in-JSON tones) + top-level drum_tones for the drum part
- conformance: engine+format gate selection (general clarification); source
  blocks SHOULD voice from intent.gm rather than skip; control events honoured
- prose: rigs are instrument-neutral signal chains (5.1/6.9/7.9)

Spec, schemas, extended example, CHANGELOG, and version strings updated
together; check_versions, validate, and the test suite pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: gionnibgud <gionnibgud@gmail.com>
… row, GM Power kit in the example

- header Date: 2026-07-19 -> 2026-07-20, matching the CHANGELOG's 1.17.0 date
  (repo convention per c536d76; check_versions.py does not guard this line)
- the §7.9 realizations table row now mirrors the conformance bullet's
  engine-and-format gate; same wording applied to the matching $comment in
  rigs.schema.json
- the example 'Rock Kit' declares the GM Power kit (16, 0-based) rather than
  Standard (0), and its sf2 program matches

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: gionnibgud <gionnibgud@gmail.com>
…e the drum sound-binding precedence

Reconciles this PR with #63, which landed while in review and took 1.17.0:

- version retarget 1.17.0 -> 1.18.0 everywhere check_versions.py guards
- drum sound binding composed with drums-as-arrangements, stated not implied:
  a type:drums arrangement binds sound through its entry tones (precedence);
  top-level drum_tones binds the song-level primary part and is the fallback
  for packs without drum arrangements; a Reader MUST NOT apply both to the
  same part, and a Writer SHOULD keep drum_tones mirroring the primary —
  the exact alias pattern #63 established for drum_tab
- intent.gm kit absent => 0 (Standard), closing the asymmetry with the
  bank/program absent-=>-0 rule (review follow-up)
- example: the primary drums arrangement carries entry tones; drum_tones
  mirrors it as the alias

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Signed-off-by: gionnibgud <gionnibgud@gmail.com>
@gionnibgud

Copy link
Copy Markdown
Contributor Author

Rebased onto main after #63 landed — retargeted to 1.18.0, and the two features are now explicitly composed.

#63 (drums as arrangements) merged while this was in review and took 1.17.0, which made this branch conflict. Rather than only resolving text, the rebase reconciles the two designs — a4f7657 on top of the previously-approved content:

  • Version retarget 1.17.0 → 1.18.0 everywhere check_versions.py guards.
  • Drum sound binding composed with drums-as-arrangements, stated rather than implied: a type: drums arrangement binds its sound through its entry tones like any other arrangement (precedence); the top-level drum_tones binds the song-level primary part and is the fallback for packs without drum arrangements; a Reader MUST NOT apply both to the same part, and a Writer SHOULD keep drum_tones mirroring the primary — the exact alias pattern spec: drums as arrangements — type:"drums" + per-arrangement drum_tab pointer (1.17.0) #63 established for drum_tab. New bullet in §7.5 "Multiple drum parts", mirrored in the §5.1 drum_tones row.
  • The extended example now exercises both paths: the primary Drums arrangement carries entry tones, with drum_tones as its matching alias.

Also folded in from @topkoa's review-body corners (the cheap one): intent.gm.kit absent ⇒ 0 (Standard), closing the asymmetry with the bank/program absent-⇒-0 rule. The other two corners — gm carrying both melodic and percussion forms, and a non-head source block — are recorded on our side as follow-ups for a clarification FEP, per the approval note.

@topkoa: the force-push may have dismissed the approval — the delta since what you approved is exactly a4f7657 (the reconciliation commit); a look at just that commit should be enough to re-confirm.

Full local check suite passes on the new head: check_versions (1.18.0 consistent), validate (both packs), pytest (58), mkdocs build --strict, Draft 2020-12 schema check.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
spec/feedpak-v1.md (2)

1416-1417: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider adding a source rig to the example.

Since this release introduces source blocks and soundfont realizations (and the extended manifest.yaml example references grand-piano and rock-kit rigs), adding a simple source rig to this JSON block would clearly illustrate the new schema in action.

💡 Proposed addition to the example
       "graph": {                    // OPTIONAL; non-serial topology (parallel/stereo/wet-dry)
         "nodes": ["input", "amp", "output"],
         "edges": [["input", "amp"], ["amp", "output"]]
       }
-    }
+    },
+    {
+      "id": "grand-piano",
+      "name": "Concert Grand",
+      "instrument": "keys",
+      "blocks": [
+        {
+          "role": "source",
+          "intent": {
+            "kind": "instrument",
+            "gm": { "program": 0 }
+          },
+          "realizations": [
+            {
+              "engine": "soundfont",
+              "format": "sf2",
+              "ref": "sounds/piano.sf2",
+              "bank": 0,
+              "program": 0
+            }
+          ]
+        }
+      ]
+    }
   ]
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@spec/feedpak-v1.md` around lines 1416 - 1417, Extend the JSON example block
in spec/feedpak-v1.md with a simple source rig, demonstrating the newly
introduced source block and soundfont realization schema. Use the existing
grand-piano or rock-kit rig references from the manifest example so the example
remains consistent with the documented configuration.

1400-1400: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Align the inline comment with the updated engine-and-format rules.

The conformance text and realization table now explicitly state that a Reader gates on engine and format when matching realizations. Updating this inline comment reflects that alignment.

💡 Proposed fix
-          "realizations": [         // ordered; reader renders the FIRST engine it supports
+          "realizations": [         // ordered; reader renders the FIRST realization whose engine (and format) it supports
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@spec/feedpak-v1.md` at line 1400, Update the inline comment on the
realizations field to state that readers render the first realization whose
engine and format they support, aligning it with the conformance and
realization-table rules.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@spec/feedpak-v1.md`:
- Around line 1416-1417: Extend the JSON example block in spec/feedpak-v1.md
with a simple source rig, demonstrating the newly introduced source block and
soundfont realization schema. Use the existing grand-piano or rock-kit rig
references from the manifest example so the example remains consistent with the
documented configuration.
- Line 1400: Update the inline comment on the realizations field to state that
readers render the first realization whose engine and format they support,
aligning it with the conformance and realization-table rules.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 502e1d92-3240-4259-8e47-e2b4d5e65e72

📥 Commits

Reviewing files that changed from the base of the PR and between 73b8da6 and a4f7657.

📒 Files selected for processing (7)
  • CHANGELOG.md
  • README.md
  • examples/extended.feedpak/manifest.yaml
  • examples/extended.feedpak/rigs.json
  • schemas/manifest.schema.json
  • schemas/rigs.schema.json
  • spec/feedpak-v1.md
🚧 Files skipped from review as they are similar to previous changes (4)
  • README.md
  • examples/extended.feedpak/rigs.json
  • schemas/manifest.schema.json
  • schemas/rigs.schema.json

@gionnibgud gionnibgud changed the title spec: MIDI-voiced sound sources — soundfont engine, source role, GM floor, tones binding (1.17.0) — FEP #61 spec: MIDI-voiced sound sources — soundfont engine, source role, GM floor, tones binding (1.18.0) — FEP #61 Jul 21, 2026

@topkoa topkoa left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approving after the rebase. Verified the delta claim with a range-diff: the first two commits carry the previously-approved content faithfully onto the new main (only #63's own changes appear), and the reconciliation is exactly a4f7657. That commit covers the full checklist — 1.18.0 consistent across every check_versions.py location, the drum sound-binding precedence stated normatively in §7.5 and mirrored in the §5.1 drum_tones row (entry tones win, drum_tones is the primary-part fallback, MUST NOT apply both — the alias pattern matching #63's drum_tab rule), and the extended example exercising both paths. Closing the kit-absent⇒0 asymmetry while in there was a nice extra. CI green on the new head. LGTM.

@topkoa
topkoa merged commit a66000b into main Jul 21, 2026
8 checks passed
@topkoa
topkoa deleted the fep/sound-source-realization branch July 21, 2026 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEP] Sound-source realization — MIDI-driven instrument voices (soundfont engine, source role, GM floor)

3 participants