port: FORMAT.md declared the port's own export invalid; assert audio.json's stems
Audits the open format spec against the validator by breaking each documented requirement. Four of five caught. The fifth is the doc's error: FORMAT.md said check refuses any peak >= 0 dBFS, where the implementation is kind-dependent -- a bgm is a sum we produced and is refused at full scale, an se/voice is a disc wave whose lossy decode overshoots and is allowed to +1.0. The doc was wrong about our own export: confirm ships at +0.18 and the ADV voice at +0.31. A consumer implementing a validator from FORMAT.md would have rejected a valid tree -- the file that exists so someone can check our work without trusting us. Corrected, with the +1.0 marked as a judgement. Also closes the last unread authored value: audio.json's was carried as stems_why only, so serde ignored the value. Now deserialised and asserted in the exporter (only sum is implemented); the assertion is proved to fire. Files, not fixes: a failed export leaves a tree with no manifest, and every tool then says 'is that an export tree?' -- which nearly made me conclude the validator was checking nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01N7FiFFFwbvG2uxdcEh8HyF
This commit is contained in:
@@ -79,6 +79,20 @@ pub struct BgmSpec {
|
||||
pub why: String,
|
||||
#[serde(default)]
|
||||
pub loop_why: Option<String>,
|
||||
/// How a bank's two sub-waves become one file. **Only `"sum"` is
|
||||
/// implemented**, and this field exists to say so when it is not.
|
||||
///
|
||||
/// 🔴 It was `stems_why` alone until 2026-08-30 — the *reason* was
|
||||
/// deserialised and the *value* was not, so `stems` sat in
|
||||
/// `authored/audio.json` being ignored by serde. Changing it to anything at
|
||||
/// all did nothing and warned nobody, which is the seventh instance in this
|
||||
/// port of an authored value with no reader.
|
||||
///
|
||||
/// It is ASSERTED rather than implemented: a weighted mix is not written,
|
||||
/// and inventing one would be a level decision nobody measured (HANDOFF Q10
|
||||
/// settles that the two waves are summed, not what wave 1 *is*).
|
||||
#[serde(default)]
|
||||
pub stems: Option<String>,
|
||||
#[serde(default)]
|
||||
pub stems_why: Option<String>,
|
||||
}
|
||||
@@ -370,6 +384,18 @@ pub fn export_bgm<S: DiscSource + ?Sized>(
|
||||
role: &str,
|
||||
spec: &BgmSpec,
|
||||
) -> Result<Option<Exported>> {
|
||||
// Assert the authored value this function was built for, rather than
|
||||
// silently doing something else. Only `sum` is implemented.
|
||||
if let Some(mode) = spec.stems.as_deref() {
|
||||
if mode != "sum" {
|
||||
bail!(
|
||||
"authored/audio.json bgm.{role}.stems is {mode:?}; \
|
||||
export_bgm implements only \"sum\" (HANDOFF Q10 settles that a \
|
||||
bank's two waves are summed; a weighting would be an unmeasured \
|
||||
level decision)"
|
||||
);
|
||||
}
|
||||
}
|
||||
let riffs = match media::sound_bank_riffs(source, &spec.bank) {
|
||||
Ok(r) if !r.is_empty() => r,
|
||||
Ok(_) => return Ok(None),
|
||||
|
||||
@@ -575,3 +575,30 @@ is a second, independent phase measurement and neither is wrong.
|
||||
⚠️ The port cannot separate a phase error from a systematic error in how it draws
|
||||
the sweeps — a geometry mistake could be absorbed by shifting the phase. The
|
||||
sharpness of the basin argues against that, but one capture cannot settle it.
|
||||
|
||||
---
|
||||
|
||||
## A failed export leaves a partial tree that reads as "not an export tree"
|
||||
|
||||
*Derived from HANDOFF `9ca1eb5`. Raised 2026-08-30 by the port. **Not blocked on
|
||||
anybody** — filed because the fix is a judgement about failure semantics, not a
|
||||
bug.*
|
||||
|
||||
When `sylpheed-export export` fails part-way — for instance on one of the new
|
||||
authored-value assertions — it leaves `export/` **without a `manifest.json`**.
|
||||
Every tool then reports *"has no manifest.json — is that an export tree?"*, which
|
||||
reads as a broken harness rather than as the aftermath of a deliberate abort.
|
||||
|
||||
It cost a wrong reading within minutes of being introduced: an audit of the
|
||||
validator reported every case as "no manifest" and nearly concluded the validator
|
||||
was checking nothing.
|
||||
|
||||
Writing the manifest **last** is correct — a manifest is a claim about a finished
|
||||
tree. So the candidate fixes are about the *message*, not the order:
|
||||
|
||||
* leave a marker on failure that the next tool can name (`export/.failed`), or
|
||||
* have `check` say *"no manifest — the last export did not finish"* when the tree
|
||||
has screens but no manifest, which is exactly the distinguishable case.
|
||||
|
||||
⚠️ Deliberately not chosen here: both change failure semantics across every tool,
|
||||
and neither is measured against anything. It goes to whoever owns that call.
|
||||
|
||||
@@ -6284,3 +6284,57 @@ instances of exactly that.
|
||||
Verified: clean boot with no invariant errors and unchanged timings; setting
|
||||
`left_right: "move"` produces the error; all five MODDING rules still pass; the
|
||||
oracle rows are unmoved.
|
||||
|
||||
## `FORMAT.md` declared the port's own export invalid, and a failed export is not atomic
|
||||
|
||||
Continuing the audit that has now found seven unexercised rules: `FORMAT.md` is
|
||||
the **open format spec** — written for a stranger reading the tree with no access
|
||||
to the disc or this exporter. So the question is whether what it promises is what
|
||||
`sylpheed-export check` enforces.
|
||||
|
||||
Five documented requirements, each broken in a copy of the tree:
|
||||
|
||||
| broken | caught |
|
||||
|---|---|
|
||||
| `unresolved` removed from a screen | ✅ |
|
||||
| `peak_dbfs` removed | ✅ |
|
||||
| `peak_dbfs` = −120 (silence) | ✅ |
|
||||
| `duration_s` removed | ✅ |
|
||||
| `peak_dbfs` = 0.0 on an `se` | **passes** |
|
||||
|
||||
### The last row is the doc's error, not the code's
|
||||
|
||||
`FORMAT.md` said flatly that check *"refuses a tree whose peak is ≤ −90 dBFS or
|
||||
**≥ 0 dBFS**"*. The implementation is kind-dependent and deliberate: a `bgm` is a
|
||||
sum **we** produced, so a peak at or above full scale is our arithmetic and is
|
||||
refused outright; an `se` or `voice` is a single wave off the disc, mastered near
|
||||
full scale, whose lossy decode overshoots by a fraction of a dB, and those are
|
||||
allowed to +1.0.
|
||||
|
||||
🔴 **And the doc was wrong about the port's own export.** It ships `confirm` at
|
||||
**+0.18 dBFS** and the `ADV` voice at **+0.31** — both above 0. A consumer
|
||||
implementing a validator from `FORMAT.md` would have rejected a valid tree, and
|
||||
the file that exists to let someone check our work without trusting us would have
|
||||
been the thing that misled them. Corrected, with the +1.0 marked as the judgement
|
||||
it is.
|
||||
|
||||
✅ Verified both directions: a `bgm` forced to 0.0 is refused with *"a SUM we
|
||||
produced clips"*; an `se` at 0.0 passes.
|
||||
|
||||
### 🔴 A failed export leaves a tree that is not an export tree
|
||||
|
||||
Found by accident, and worth more than the way it was found. Testing the new
|
||||
`stems` assertion, the exporter `bail!`ed part-way — and left `export/` **with no
|
||||
`manifest.json` at all**. Every subsequent tool then reported *"has no
|
||||
manifest.json — is that an export tree?"*, which reads as a broken harness rather
|
||||
than as the aftermath of a deliberate failure.
|
||||
|
||||
⚠️ It cost me a wrong reading immediately: the first run of the requirement audit
|
||||
above reported every case as "no manifest", and I nearly recorded that the
|
||||
validator was checking nothing. It was checking a tree that had been half-written.
|
||||
|
||||
The exporter writes the manifest last, which is the right order — a manifest is a
|
||||
claim about a tree, and a manifest for a tree that was never finished would be
|
||||
worse. So this is **filed rather than fixed**: the behaviour is defensible and the
|
||||
message is not, since "is that an export tree?" describes the symptom and hides
|
||||
the cause. What a stranger needs to be told is *the last export failed; re-run it*.
|
||||
|
||||
@@ -356,7 +356,7 @@ not done its job.
|
||||
|---|---|
|
||||
| `kind` | `se`, `bgm` or `voice`. The runtime dispatches on it, so it is a field rather than a prefix on `name` that a consumer would have to parse |
|
||||
| `name` | the **role**, not the disc asset: `move`, `confirm`, `back`, `main_menu`. Which bank plays a role is authored and expected to change; a rename on the disc side must not be a change to the Godot project. ⚠️ **`voice` is the exception and keys by MOVIE NAME** (`ADV`, `S00A`), because there is no role to name: the binding of recording to picture came off the disc's own movie manifest, so unlike a music bed nothing about it was chosen |
|
||||
| `peak_dbfs` | measured off the finished file. **Required.** Silence is the audio failure that looks like success — right duration, right channel count, right size, full of zeroes — and clipping is the other one, which the BGM can produce because it is a sum of two stems at unity gain. `sylpheed-export check` refuses a tree whose peak is ≤ −90 dBFS or ≥ 0 dBFS |
|
||||
| `peak_dbfs` | measured off the finished file. **Required.** Silence is the audio failure that looks like success — right duration, right channel count, right size, full of zeroes — and clipping is the other one, which the BGM can produce because it is a sum of two stems at unity gain. `sylpheed-export check` refuses a tree whose peak is ≤ −90 dBFS, and applies a **kind-dependent** upper bound. 🔴 This paragraph used to state a flat *≥ 0 dBFS* and was wrong about the port's own export: `confirm` ships at **+0.18** and the `ADV` voice at **+0.31**, so a consumer implementing a validator from this file would have rejected a valid tree. The rule is: a **`bgm`** is a sum *we* produced, so a peak at or above full scale is our arithmetic and is refused outright; an **`se`** or **`voice`** is a single wave off the disc, mastered near full scale, and a lossy decode of it overshoots by a fraction of a dB — those are allowed to **+1.0 dB**. ⚠️ The +1.0 is a judgement, not a measurement: a few tenths is reconstruction overshoot and a whole dB is not, and if a cue ever trips it the right response is to measure the overshoot distribution, not to loosen the bound |
|
||||
| `duration_s` | measured off the finished file, so that a claim about a cue's length can be checked against the finding that produced it |
|
||||
| `name_match` | the game's own cue identifier **guessed by name**. Absent means nobody claimed one — never that the binding is unknown. The binding is the measured part; the name is not |
|
||||
| `loop_mode` | what the runtime does at the end of the file, where that was authored. Absent on a cue: a cue ends |
|
||||
|
||||
Reference in New Issue
Block a user