Give extract and analyze one label-parser tie-break, not two

mod.rs had two independent implementations of "pick the winning label
parser". select_result owns the rule — highest confidence wins, and on a tie
the earlier entry in PARSERS wins — and carries a regression test for a past
bug where analyze() picked the LAST equal-confidence parser instead of the
first. extract(), the path that actually ships, re-derived the same rule with
its own inline scan and had no test of its own.

The tie-break is load-bearing rather than incidental: array order encodes a
trust ordering, with the hand-vetted parsers registered ahead of the ones
that detect on any BD-J disc. A later parser winning a tie means the disc
gets labels from a less trusted source, silently.

extract now collects its candidates and calls select_result. Confirmed by
flipping the tie-break to prefer the later entry: the shared test fails,
where before the fix that mutation was invisible to the whole suite.

This is the seventh instance tonight of one policy implemented twice with
only one copy hardened, and the second in this file after the chapter
mark_type filter.
This commit is contained in:
Matthew Jackson
2026-08-01 12:15:48 -07:00
parent c4ad4184ec
commit 51d2b14d03
+18 -11
View File
@@ -525,7 +525,7 @@ fn generate_audio_label_inner(
} }
fn extract(reader: &mut dyn SectorSource, udf: &UdfFs) -> Vec<StreamLabel> { fn extract(reader: &mut dyn SectorSource, udf: &UdfFs) -> Vec<StreamLabel> {
let mut best: Option<(&'static str, ParseResult)> = None; let mut candidates: Vec<(&'static str, ParseResult)> = Vec::new();
for (name, detect, parse) in PARSERS { for (name, detect, parse) in PARSERS {
if !detect(reader, udf) { if !detect(reader, udf) {
continue; continue;
@@ -537,14 +537,17 @@ fn extract(reader: &mut dyn SectorSource, udf: &UdfFs) -> Vec<StreamLabel> {
if result.labels.is_empty() { if result.labels.is_empty() {
continue; continue;
} }
// Pick highest confidence. Equal confidence → first wins candidates.push((name, result));
// (array order tiebreaker).
match &best {
None => best = Some((name, result)),
Some((_, b)) if result.confidence > b.confidence => best = Some((name, result)),
_ => {}
}
} }
// One tie-break rule, one implementation. `select_result` owns it and
// carries a regression test for a past bug where the LAST equal-confidence
// parser won instead of the first. `extract` — the path that actually
// ships — used to re-derive the same rule inline with a hand-rolled `>`
// scan and had no test of its own, so that fixed bug could have silently
// recurred here. Array order encodes a trust ordering (the hand-vetted
// parsers are registered ahead of the ones that detect on any BD-J disc),
// so "first wins on a tie" is load-bearing, not incidental.
let best = select_result(&candidates).map(|(n, r)| (*n, r.clone()));
let (name, mut labels) = match best { let (name, mut labels) = match best {
Some((n, r)) => { Some((n, r)) => {
tracing::info!( tracing::info!(
@@ -1184,9 +1187,13 @@ mod registry_tests {
} }
/// `select_result` must pick the highest-confidence non-empty result /// `select_result` must pick the highest-confidence non-empty result
/// and, on a confidence tie, the FIRST in array order — matching /// and, on a confidence tie, the FIRST in array order (regression for the
/// `extract()`'s strict-`>` first-wins scan (regression for the old /// old `analyze()` `max_by(...then(Equal))` no-op that picked the LAST).
/// `analyze()` `max_by(...then(Equal))` no-op that picked the LAST). ///
/// `extract()` — the path that actually ships — used to re-derive this
/// same rule with its own inline `>` scan and had no test at all, so the
/// bug this test guards against could have recurred there unnoticed. It
/// now calls `select_result`, so this test covers both.
#[test] #[test]
fn select_result_first_wins_on_tie() { fn select_result_first_wins_on_tie() {
// Two parsers, equal (Medium) confidence: the first must win. // Two parsers, equal (Medium) confidence: the first must win.