spec: per-stem origin (separated / authored / user) — FEP #54 - #55
Open
topkoa wants to merge 5 commits into
Open
spec: per-stem origin (separated / authored / user) — FEP #54#55topkoa wants to merge 5 commits into
topkoa wants to merge 5 commits into
Conversation
stem_separation describes ONE separation of the WHOLE pack, so a pack whose stems have different
origins cannot describe itself. Those packs are ordinary, not exotic: a stem re-split later with a
better model; a stem hand-edited in an editor among machine-separated ones; a real recorded stem
delivered alongside separated ones.
Today a writer in that position must choose between lying in the manifest (stamping an engine over
stems it never produced, including ones a person made by hand) and destroying provenance (dropping
the key, discarding the truth about every stem nobody touched). Neither is acceptable, and the
spec's own cache-key promise is wrong in both directions once origins are mixed.
A stem MAY now describe itself, using the vocabulary-plus-provenance shape the format ALREADY uses
for lyrics (7.1/7.1.1) rather than a second idiom:
source separated | authored | user (absent => inherit stem_separation)
separation {engine, model, version} (this stem's, overriding the pack-level value)
edit {tool, version} (the HUMAN counterpart: what made a hand-made stem)
derived_from a stem id (lineage: the edited bass began as the separated bass)
`edit` is the key the first draft of the FEP lacked. It proposed `separation: null` for hand-edited
stems - a negative, which records only what a stem ISN'T and leaves a stem a person made with
nowhere to say what produced it. A hand-made stem deserves to say what made it.
`derived_from` is what lets a tool warn "your hand-edited bass came from the old split; re-splitting
would replace the audio your edit was built on" instead of silently overwriting someone's work.
MINOR: all four keys are OPTIONAL on an existing object. An older reader ignores them and falls back
to stem_separation exactly as today. A pack from a single whole-pack split does not change shape.
Spec, schema, example and changelog land together, per CONTRIBUTING. The extended example now
carries a re-split stem and a hand-edited one, and still validates.
Signed-off-by: topkoa <topkoa@gmail.com>
Collaborator
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe manifest schema, Feedpak v1 specification, extended example, and changelog document optional per-stem provenance, separation, editing, and lineage metadata with pack-level fallback, precedence rules, and validation constraints. ChangesPer-stem origin metadata
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Reader
participant StemEntry
participant PackManifest
Reader->>StemEntry: Read source and separation metadata
StemEntry->>PackManifest: Use stem_separation fallback when applicable
PackManifest-->>Reader: Resolve stem provenance
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
This PR updates the feedpak v1 specification and manifest schema to support per-stem provenance/origin (separated/authored/user), enabling packs whose stems come from mixed sources (e.g., partial re-splits and hand edits) to describe themselves accurately.
Changes:
- Extends the spec (§5.3 / §5.3.1) with per-stem
source,separation,edit, andderived_from. - Updates
schemas/manifest.schema.jsonto validate the new optional per-stem fields (including aneditobject definition). - Updates the extended example manifest and documents the change in
CHANGELOG.md.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| spec/feedpak-v1.md | Adds per-stem origin/provenance keys and a new explanatory subsection under stem_separation. |
| schemas/manifest.schema.json | Adds per-stem source/separation/edit/derived_from to stemEntry and defines $defs/edit. |
| examples/extended.feedpak/manifest.yaml | Exercises the new per-stem metadata (hand-edited stem + re-split stem) in the extended example. |
| CHANGELOG.md | Records the new per-stem origin feature under Unreleased. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@schemas/manifest.schema.json`:
- Around line 287-305: Define deterministic dependency semantics for the
optional stem provenance fields: update schemas/manifest.schema.json around the
stem properties and the provenance/edit definitions to either enforce compatible
explicit source values or encode the chosen inference rules; document the
corresponding reader resolution for manifests without source in
spec/feedpak-v1.md:499; and record the finalized dependency behavior in
CHANGELOG.md:18-22.
In `@spec/feedpak-v1.md`:
- Line 500: The pack-level stem_separation inheritance rule must explicitly
exclude the reserved full mixdown (id: full). Update spec/feedpak-v1.md at lines
500-500, schemas/manifest.schema.json at lines 293-293,
examples/extended.feedpak/manifest.yaml at lines 74-74, and CHANGELOG.md at
lines 16-17 to document, apply, demonstrate, and announce that full does not
inherit Demucs stem_separation provenance.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 79ebd57d-7553-4e86-a7e6-dd75eed08628
📒 Files selected for processing (4)
CHANGELOG.mdexamples/extended.feedpak/manifest.yamlschemas/manifest.schema.jsonspec/feedpak-v1.md
…diction in the schema The resolution order was underspecified — it said "the stem's own source (with separation/edit), if present", which left two real cases unanswered: a stem carrying `separation` but no `source`, and a stem saying `source: separated` with no `separation` of its own. A reader could reasonably resolve either as "unknown", which would lose provenance the manifest is plainly carrying. They are independent keys and now say so: `source` answers WHAT KIND of thing produced the stem, `separation` answers WHICH ENGINE. `separation` alone implies `separated`; `source: separated` alone inherits the pack-level engine. A truth table covers all five combinations. That surfaced a rule the first draft never stated: a stem whose source is `user` or `authored` MUST NOT also carry a `separation` object. It would assert that an engine produced audio a person made — the exact lie this FEP exists to prevent, re-entering through a combination I hadn't considered. The schema now REJECTS it, because a normative rule nothing validates is a suggestion. Lineage for an edited stem that began as a separated one goes in `derived_from`, where it belongs. Also: `edit.tool` is REQUIRED (the schema said so, the prose didn't), `source` carries an explicit `type: string` like every other enum in the schema, and the example's comment no longer claims that every stem declares its origin when `guitar` deliberately inherits. Verified: all six rows of the new table validate, and both contradictions are rejected. Found by Copilot on #55. Signed-off-by: topkoa <topkoa@gmail.com>
Collaborator
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
`full` is the complete mixdown — the audio separation was run ON, not a product of it. Under the inheritance rule I wrote, a `full` stem that declared nothing would silently inherit the pack-level `stem_separation` and have the pack claim demucs produced the original recording. That is false for every pack that has ever been split, and it is exactly the class of lie this FEP exists to kill — re-entering through the one stem the format reserves. The exception is now stated in every layer, because a rule that lives in only one of them is a rule that drifts: - spec: a Reader MUST NOT apply stem_separation to `id: full`; a Writer MUST NOT give it `source: separated` or a `separation` object. It MAY say `source: authored` positively. - schema: both are REJECTED for `id: full`, and the `source` description says why. - example: the retained `full` stem now carries `source: authored` and says what it is. - changelog: describes the exception. Verified: `full` + `separation` and `full` + `source: separated` are both rejected; `full` + `source: authored` and a bare `full` still validate; every other stem is unaffected. Found by CodeRabbit on #55. Signed-off-by: topkoa <topkoa@gmail.com>
Collaborator
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
…uired The resolution said a stem declaring `source: separated` with no `separation` of its own resolves "by the pack-level stem_separation". But the pack may not declare one, and that combination is perfectly honest: a pack converted from another format, or a stem whose producer never recorded the model, knows the audio came OUT OF a separator without knowing which. As written, a reader could treat that as invalid, or worse, invent a pack-level provenance that isn't there. Kind and engine are now separable everywhere they are described: source -> WHAT KIND of thing produced the stem (separated | authored | user) separation -> WHICH ENGINE (this stem's, else the pack's, else UNKNOWN) A Reader MUST NOT treat "known kind, unknown engine" as invalid, and MUST NOT invent provenance the pack does not carry. Spec (§5.3 field table, §5.3.1 algorithm + truth table), schema description and changelog now all say the same thing. Found by Copilot on #55. Signed-off-by: topkoa <topkoa@gmail.com>
Collaborator
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
…bigger one I wrote "never applies to the RESERVED `full` stem" in the §5.3 table and the schema description, while §5.3.1, the schema's own logic, and the example all allow `full` to carry `source: authored`. Three layers said one thing and two sentences said another — and the two sentences are the ones a writer reads first, so they would have looked at a valid manifest and believed it was invalid. The actual rule is narrower, and is now stated as it is: on `full`, `separated` is FORBIDDEN and inheritance never applies (it is the mixdown separation was run ON, not a product of it) — but `source` MAY still state a non-separated origin such as `authored`. Which is the whole point of letting `full` speak at all: it is the one stem that can say, positively, "a person made this, no engine did". Verified the prose against the schema rather than against itself: full+authored and full+user are accepted, full+separated and full+separation are rejected, a bare full still validates. Found by Copilot on #55. Signed-off-by: topkoa <topkoa@gmail.com>
Collaborator
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
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.
Implements FEP #54. Spec + schema + example + CHANGELOG land together, per CONTRIBUTING.
The gap
stem_separationdescribes one separation of the whole pack. A pack whose stems have different origins therefore cannot describe itself — and those packs are ordinary:A writer in that position must choose between lying in the manifest — stamping an engine over stems it never produced, including audio a person made by hand — and destroying provenance: dropping the key, which discards the truth about every stem nobody touched. The spec's own cache-key promise ("any change among them is a cache miss") is wrong in both directions once origins are mixed: a miss for stems that are current, a hit for stems no engine will ever reproduce.
The shape
A stem MAY now describe itself, using the vocabulary-plus-provenance idiom the format already uses for lyrics (§7.1 / §7.1.1) rather than inventing a second one:
sourceseparated|authored|user— absent ⇒ inherit the pack-levelstem_separationseparation{engine, model, version}for this stem, overriding the pack-level valueedit{tool, version}— the human counterpart: what produced a stem a person madederived_fromeditis the key the FEP's first draft lacked. It proposedseparation: nullfor hand-edited stems — a negative, which records only what a stem isn't and leaves a stem a person produced with nowhere to say what made it: no tool, no version, no lineage. A hand-made stem deserves to say what made it, exactly as hand-edited lyrics already do withlyrics_source: user.derived_fromis what lets a tool say "your hand-editedbasscame from the old split; re-splitting would replace the audio your edit was built on" instead of silently overwriting someone's work — the failure this whole FEP exists to prevent.Compatibility — MINOR
stem_separationexactly as today. Coarser view of provenance; nothing breaks.Verification
tools/validate.py— both examples PASS. The extended example now carries a re-split stem and a hand-edited one, so the change is exercised, not just described.separated+separation/user+edit+derived_from/authored+edit; rejects a bogussource, aseparationmissingmodel, a non-semverversion, and aneditwith notool— each for the right reason (I first probed with an incomplete manifest and every case was rejected for a missingduration, which would have been false confidence).Downstream
feedBack's re-split (plugin#11) is waiting on this: with it, a partial re-split records the new engine on the stems it replaced and leaves everything else — including hand-edited stems — honestly described.
Summary by CodeRabbit
New Features
source,separation,edit,derived_from) with clear inheritance/override rules against pack-level defaults.fullstem to ensure it never behaves like a machine-separated output.Documentation
tones.definitionsexample without changing the passthrough contract.