From 7952ecd12f4e3ffab579ef58b58346027cd89d2e Mon Sep 17 00:00:00 2001 From: che cheng Date: Sat, 1 Aug 2026 15:16:53 +0800 Subject: [PATCH 1/2] feat: decode_deterministic becomes a three-value enum + extract the honest gate (#118, #120) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #118 settled the representation: the field records WHAT WE KNOW, not what the backend is. Bool? overloaded nil to mean both 'legacy row' and 'this backend ignores the flag'. DecodeDeterminism separates them — deterministic-enforced flag consumed, on fallback-enabled flag consumed, off flag-not-consumed backend ignores the flag, and this makes NO claim about whether its decode is actually reproducible Legacy rows stay on field-absence, which is no longer overloaded. An unknown wire value throws DecodingError.dataCorrupted (loud), while absent and explicit null both decode to nil. #120 item 1: the honest gate moves out of the benchmark loop into DecodeDeterminism.forBackend(_:flagRequested:), a pure non-optional function next to flagConsumingBackends. It lives on the enum because the rule IS the meaning of the cases — adding a flag-consuming backend is now one edit in the file that documents the invariant it can violate. The 'never lie' property is finally unit-testable without running a real backend. #120 item 2: --run-kind is a RunKind ExpressibleByArgument (mirroring the existing HallucinationFilterMode precedent), so a typo fails at the CLI boundary with exit 64 instead of surviving to bench CI. MeasurementRow.runKind stays String? — the type constrains the entry point, not the stored row. Both value sets carry a keep-in-sync note pointing at bench's validate_measurements.py; there is no mechanical link (the #120 Residue). bestASR-bench side landed as c93a70e. Refs #118 Refs #120 --- CHANGELOG.md | 17 +- Sources/BestASRKit/CommandCore.swift | 12 +- Sources/BestASRKit/Contribution/Sharing.swift | 7 +- Sources/BestASRKit/Models/DataModels.swift | 54 +++++++ Sources/BestASRKit/Store/BenchmarkStore.swift | 4 +- Sources/BestASRKit/Store/StoreTables.swift | 12 +- Sources/bestasr/BestASRCommand.swift | 19 ++- .../RunKindDecodeDeterministicTests.swift | 149 ++++++++++++++++-- 8 files changed, 236 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 945b19b..3fba275 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,13 +28,16 @@ All notable changes to bestASR are documented here. The format follows `run_kind` (`release-sweep` | `adhoc`) marks how a row was produced, so a per-version snapshot's candidate census can be checked mechanically instead of assumed; the new `bestasr benchmark --run-kind` flag carries it, and - `scripts/release-sweep.sh` stamps `release-sweep`. `decode_deterministic` is - **tri-state on purpose**: `true`/`false` only for backends that actually - consume `--decode-deterministic` (WhisperKit, whisper.cpp), and `null` for - backends where the flag is a silent no-op (mlx-audio) — a row never claims a - decode condition its backend ignored. The bench side validates both only when - present (`PsychQuant/bestASR-bench@e728f1a`). Whether the determinism axis - should become a per-backend enum is tracked in #118. + `scripts/release-sweep.sh` stamps `release-sweep`. `decode_deterministic` + records the decode condition as a **three-value enum** (#118): + `deterministic-enforced` / `fallback-enabled` for backends that actually + consume `--decode-deterministic` (WhisperKit, whisper.cpp), and + `flag-not-consumed` for backends that ignore it (mlx-audio's is a silent + no-op; the Fluid backends have no such knob) — which makes no claim about + whether those decodes are reproducible, rather than lying in either + direction. An **absent** field still means "legacy row, predates the field"; + `null` is never written. The bench side validates both only when present + (`PsychQuant/bestASR-bench@e728f1a`). ### Fixed diff --git a/Sources/BestASRKit/CommandCore.swift b/Sources/BestASRKit/CommandCore.swift index 812cb0a..6d11a4e 100644 --- a/Sources/BestASRKit/CommandCore.swift +++ b/Sources/BestASRKit/CommandCore.swift @@ -587,12 +587,12 @@ public struct CommandCore: Sendable { contextErrorRate: measured.contextErrorRate, hfRevision: seededRow?.hfRevision, runKind: runKind, - // decode_deterministic is meaningful only for whisper-family backends that - // actually consume the flag; mlx-audio ignores it (silent no-op, #111/#118), - // so record nil there rather than lie. - decodeDeterministic: - [ModelGrid.backendWhisperKit, ModelGrid.backendWhisperCpp].contains(record.backend) - ? decodeDeterministic : nil)) + // The honest gate lives in DecodeDeterminism.forBackend (#120 + // item 1) so the "never lie" invariant is unit-testable without + // running a real backend. Non-optional by construction: a live + // measurement always knows something; nil is for legacy rows. + decodeDeterministic: .forBackend( + record.backend, flagRequested: decodeDeterministic))) } } diff --git a/Sources/BestASRKit/Contribution/Sharing.swift b/Sources/BestASRKit/Contribution/Sharing.swift index 8fbf541..5a6450e 100644 --- a/Sources/BestASRKit/Contribution/Sharing.swift +++ b/Sources/BestASRKit/Contribution/Sharing.swift @@ -35,9 +35,10 @@ public struct SubmissionRow: Codable, Sendable, Equatable { /// Provenance of the underlying measurement (#111) — carried through so the /// published row is self-contained. nil for legacy rows. public let runKind: String? - /// Deterministic-decode flag as recorded on the measurement (#111); nil for - /// backends that ignore it (mlx-audio) and for legacy rows. - public let decodeDeterministic: Bool? + /// Decode-determinism condition as recorded on the measurement (#111/#118); + /// nil ONLY for legacy rows that predate the field — a backend that ignores + /// `--decode-deterministic` publishes `.flagNotConsumed`, not nil. + public let decodeDeterministic: DecodeDeterminism? public let contributor: String public let chip: String public let unifiedMemoryGB: Double diff --git a/Sources/BestASRKit/Models/DataModels.swift b/Sources/BestASRKit/Models/DataModels.swift index 95d5875..ed87f18 100644 --- a/Sources/BestASRKit/Models/DataModels.swift +++ b/Sources/BestASRKit/Models/DataModels.swift @@ -194,6 +194,60 @@ public enum MetricKind: String, Codable, Sendable { case wer } +/// What a measurement row may HONESTLY say about the decode that produced it +/// (#118). Records what we KNOW, not what the backend is — the pre-#118 `Bool?` +/// overloaded `nil` to mean both "legacy row" and "this backend ignores the +/// flag". A row's field stays `Optional`: an ABSENT field (nil) means the row +/// predates the field entirely and makes no claim at all; `nil` is deliberately +/// NOT one of the cases below. +/// +/// Wire values are hyphenated snake-case and are a cross-repo contract with the +/// bench validator (bestASR-bench `tools/validate_measurements.py`). +public enum DecodeDeterminism: String, Codable, Sendable { + /// The backend consumed `--decode-deterministic` and it was ON: temperature + /// fallback re-decoding was disabled, so the same audio yields the same text. + case deterministicEnforced = "deterministic-enforced" + /// The backend consumed `--decode-deterministic` and it was OFF: temperature + /// fallback re-decoding is live, so a re-run may differ. + case fallbackEnabled = "fallback-enabled" + /// The backend ignores `--decode-deterministic` entirely (mlx-audio treats it + /// as a silent no-op; the Fluid backends have no such knob). This makes NO + /// claim about whether that backend's decode is actually reproducible — we + /// don't know, and saying either "deterministic" or "fallback" would be a + /// lie (#118). + case flagNotConsumed = "flag-not-consumed" + + /// Backends that actually read `--decode-deterministic`. Adding a backend + /// here is the ONE edit that lets its rows start making a determinism claim + /// — until then it honestly reports `.flagNotConsumed`. + public static let flagConsumingBackends: Set = [ + ModelGrid.backendWhisperKit, ModelGrid.backendWhisperCpp, + ] + + /// The honest gate (#120 item 1): which condition a row measured on + /// `backend` may claim, given the flag the user asked for. Non-optional — + /// every real measurement can say *something*; the row's optionality is + /// reserved for legacy rows that predate the field. + public static func forBackend(_ backend: String, flagRequested: Bool) -> DecodeDeterminism { + guard flagConsumingBackends.contains(backend) else { return .flagNotConsumed } + return flagRequested ? .deterministicEnforced : .fallbackEnabled + } +} + +/// How a measurement was produced (#111) — the `run_kind` value domain, and the +/// Swift side's single source of truth for it (#120 item 2). +/// +/// KEEP IN SYNC with the bench validator's `run_kind` enum (bestASR-bench +/// `tools/validate_measurements.py`). There is no mechanical link between the +/// two definitions: a value added here and not there fails only in the bench +/// repo's CI, and vice versa (#120 Residue, deliberately unfixed). +public enum RunKind: String, Codable, Sendable, CaseIterable { + /// A sweep driven by `scripts/release-sweep.sh`. + case releaseSweep = "release-sweep" + /// A one-off local benchmark. + case adhoc +} + /// One (backend × model × quantization) configuration to measure. public struct BenchmarkCandidate: Sendable, Equatable, Hashable { public let backend: BackendID diff --git a/Sources/BestASRKit/Store/BenchmarkStore.swift b/Sources/BestASRKit/Store/BenchmarkStore.swift index 37832aa..cb07d97 100644 --- a/Sources/BestASRKit/Store/BenchmarkStore.swift +++ b/Sources/BestASRKit/Store/BenchmarkStore.swift @@ -160,7 +160,9 @@ public struct BenchmarkStore: Sendable { quantization: record.quantization) // Legacy flat-cache rows predate the #111 provenance fields — there // is no run-kind and no decode-deterministic flag to recover here, so - // both stay nil (legacy-safe) rather than being fabricated. + // both stay nil (legacy-safe) rather than being fabricated. Under + // #118 that nil is unambiguous: "the field did not exist when this + // was measured", NOT "this backend ignores the flag". let measurement = MeasurementRow( modelId: modelId, corpusId: corpus.corpusId, machineId: machine.machineId, measuredAt: record.measuredAt, metricKind: record.metricKind, diff --git a/Sources/BestASRKit/Store/StoreTables.swift b/Sources/BestASRKit/Store/StoreTables.swift index ee21f7f..3738da7 100644 --- a/Sources/BestASRKit/Store/StoreTables.swift +++ b/Sources/BestASRKit/Store/StoreTables.swift @@ -160,11 +160,11 @@ public struct MeasurementRow: Codable, Sendable, Equatable { /// How this measurement was produced: "release-sweep" (scripts/release-sweep.sh) /// vs "adhoc" (a one-off local benchmark). nil for legacy rows (#111). public let runKind: String? - /// Whether temperature-fallback re-decoding was disabled for reproducibility. - /// Meaningful only for whisper-family backends that consume the flag; nil for - /// backends that ignore it (mlx-audio is a silent no-op, #111/#118) and for - /// legacy rows measured before the flag existed. - public let decodeDeterministic: Bool? + /// What is known about this row's decode: enforced determinism, live + /// temperature fallback, or a backend that ignores the flag (#118 — see + /// `DecodeDeterminism`). nil ONLY for legacy rows measured before the field + /// existed; "this backend ignores the flag" is `.flagNotConsumed`, not nil. + public let decodeDeterministic: DecodeDeterminism? enum CodingKeys: String, CodingKey { case modelId = "model_id" @@ -189,7 +189,7 @@ public struct MeasurementRow: Codable, Sendable, Equatable { metricKind: MetricKind, errorRate: Double, rtf: Double, peakMemoryGB: Double, warmupSeconds: Double, appVersion: String, macosVersion: String, contextErrorRate: Double? = nil, hfRevision: String? = nil, - runKind: String? = nil, decodeDeterministic: Bool? = nil + runKind: String? = nil, decodeDeterministic: DecodeDeterminism? = nil ) { self.modelId = modelId self.corpusId = corpusId diff --git a/Sources/bestasr/BestASRCommand.swift b/Sources/bestasr/BestASRCommand.swift index 7bf9e7f..0d201dc 100644 --- a/Sources/bestasr/BestASRCommand.swift +++ b/Sources/bestasr/BestASRCommand.swift @@ -32,6 +32,14 @@ struct BestASR: AsyncParsableCommand { // (Same package as the enum, so no @retroactive.) extension HallucinationFilterMode: ExpressibleByArgument {} +// #120 item 2: --run-kind used to take an arbitrary String, so a typo travelled +// verbatim into the store and only failed in the bench repo's CI — fail-loud at +// the wrong end. Typing the option rejects unknown values at the CLI boundary +// (ArgumentParser's usage error, exactly like --hallucination-filter) and lists +// the domain in --help. The value set itself lives on BestASRKit.RunKind, which +// carries the "keep in sync with the bench validator" note. +extension RunKind: ExpressibleByArgument {} + // Command handlers delegate to BestASRKit.CommandCore (design D1: the executable // stays a thin argument-parsing shell; behavior lives in the library where the // test target can reach it). @@ -188,12 +196,15 @@ struct Benchmark: AsyncParsableCommand { """) var decodeDeterministic = false + // Value domain enforced at parse time (#120 item 2); unset stays nil, which + // is a valid state ("no provenance tag"). The domain must stay in sync with + // the bench validator's run_kind enum — see BestASRKit.RunKind. @Option( help: """ - Provenance tag stamped onto each measurement's run_kind field \ - (e.g. release-sweep for scripts/release-sweep.sh; omit for ad-hoc runs) + Provenance tag stamped onto each measurement's run_kind field: \ + release-sweep (scripts/release-sweep.sh) | adhoc; omit to leave it unset """) - var runKind: String? + var runKind: RunKind? @Flag(help: "Emit machine-readable JSON instead of the table") var json = false @@ -212,7 +223,7 @@ struct Benchmark: AsyncParsableCommand { contextDir: contextDir, allGrid: allGrid, decodeDeterministic: decodeDeterministic, - runKind: runKind + runKind: runKind?.rawValue ) ) } diff --git a/Tests/BestASRKitTests/RunKindDecodeDeterministicTests.swift b/Tests/BestASRKitTests/RunKindDecodeDeterministicTests.swift index ccd1757..b380394 100644 --- a/Tests/BestASRKitTests/RunKindDecodeDeterministicTests.swift +++ b/Tests/BestASRKitTests/RunKindDecodeDeterministicTests.swift @@ -2,12 +2,22 @@ import Foundation import Testing @testable import BestASRKit +@testable import bestasr /// #111 provenance schema: two optional fields (`run_kind`, /// `decode_deterministic`) added to the measurement + submission rows, mirroring /// the `hf_revision` precedent — optional, nil-defaulted, snake_case, and /// backward-compatible via synthesized Codable (nil optionals encode as absent /// keys, absent keys decode as nil). +/// +/// #118 narrowed `decode_deterministic` from `Bool?` to `DecodeDeterminism?`: +/// `nil` used to mean BOTH "legacy row" and "this backend ignores the flag". +/// Now the three enum cases record what we KNOW about the decode, and `nil` +/// keeps exactly one meaning — the field was absent (legacy row). +/// +/// #120 item 1 locks the "never lie" invariant of the honest gate as a pure, +/// unit-testable helper; item 2 moves `--run-kind` value-domain validation to +/// the CLI boundary. struct RunKindDecodeDeterministicTests { private func iso8601Encoder() -> JSONEncoder { let encoder = JSONEncoder() @@ -22,6 +32,16 @@ struct RunKindDecodeDeterministicTests { return decoder } + /// A minimal row whose only interesting field is the determinism condition. + private func row(_ determinism: DecodeDeterminism?) -> MeasurementRow { + MeasurementRow( + modelId: "whisperkit|whisper|tiny|default", corpusId: "c", machineId: "m", + measuredAt: Date(timeIntervalSince1970: 1_800_000_000), metricKind: .wer, + errorRate: 0.1, rtf: 0.2, peakMemoryGB: 1, warmupSeconds: 1, + appVersion: "0.4.0", macosVersion: "27.0", + runKind: "release-sweep", decodeDeterministic: determinism) + } + // MARK: MeasurementRow @Test func `Legacy measurement JSON without the new keys decodes to nil`() throws { @@ -31,16 +51,13 @@ struct RunKindDecodeDeterministicTests { """ let row = try iso8601Decoder().decode(MeasurementRow.self, from: Data(legacy.utf8)) #expect(row.runKind == nil) + // #118: nil means "the field was absent", NOT an enum case. A legacy row + // makes no claim at all about how it was decoded. #expect(row.decodeDeterministic == nil) } @Test func `Measurement row round-trips the new fields preserving values`() throws { - let row = MeasurementRow( - modelId: "whisperkit|whisper|tiny|default", corpusId: "c", machineId: "m", - measuredAt: Date(timeIntervalSince1970: 1_800_000_000), metricKind: .wer, - errorRate: 0.1, rtf: 0.2, peakMemoryGB: 1, warmupSeconds: 1, - appVersion: "0.4.0", macosVersion: "27.0", - runKind: "release-sweep", decodeDeterministic: true) + let row = row(.deterministicEnforced) let json = String(decoding: try iso8601Encoder().encode(row), as: UTF8.self) // snake_case keys on disk (spec: BCNF store field naming). #expect(json.contains("\"run_kind\"")) @@ -48,12 +65,15 @@ struct RunKindDecodeDeterministicTests { let decoded = try iso8601Decoder().decode(MeasurementRow.self, from: Data(json.utf8)) #expect(decoded == row) #expect(decoded.runKind == "release-sweep") - #expect(decoded.decodeDeterministic == true) + #expect(decoded.decodeDeterministic == .deterministicEnforced) } @Test func `Nil new fields encode as absent keys, mirroring hf_revision`() throws { // Default init leaves runKind / decodeDeterministic (and hfRevision) nil; // synthesized Codable omits nil optionals, so the keys never appear. + // CROSS-REPO CONTRACT: the bench validator (bestASR-bench + // tools/validate_measurements.py) accepts an ABSENT key as "legacy row"; + // an explicit null would be a schema violation. let row = MeasurementRow( modelId: "whisperkit|whisper|tiny|default", corpusId: "c", machineId: "m", measuredAt: Date(timeIntervalSince1970: 1_800_000_000), metricKind: .wer, @@ -62,9 +82,87 @@ struct RunKindDecodeDeterministicTests { let json = String(decoding: try iso8601Encoder().encode(row), as: UTF8.self) #expect(!json.contains("run_kind")) #expect(!json.contains("decode_deterministic")) + #expect(!json.contains("null")) #expect(!json.contains("hf_revision")) // same omit-nil behavior we mirror } + // MARK: DecodeDeterminism wire format (#118) + + @Test func `Each determinism case round-trips through its hyphenated wire string`() throws { + let expected: [DecodeDeterminism: String] = [ + .deterministicEnforced: "deterministic-enforced", + .fallbackEnabled: "fallback-enabled", + .flagNotConsumed: "flag-not-consumed", + ] + for (value, wire) in expected { + #expect(value.rawValue == wire) + let json = String(decoding: try iso8601Encoder().encode(row(value)), as: UTF8.self) + #expect(json.contains("\"decode_deterministic\":\"\(wire)\"")) + let decoded = try iso8601Decoder().decode(MeasurementRow.self, from: Data(json.utf8)) + #expect(decoded.decodeDeterministic == value) + } + } + + @Test func `Unknown decode_deterministic wire value fails the decode loudly`() throws { + // Fail loud, not silently nil: an unrecognized condition must never be + // read back as "legacy row" (that would launder an unknown claim into + // "no claim"). Synthesized Codable throws DecodingError.dataCorrupted. + let bogus = """ + {"app_version":"0.3.0","corpus_id":"c","decode_deterministic":"bogus","error_rate":0.2,"machine_id":"m","macos_version":"27.0","measured_at":"2026-01-01T00:00:00Z","metric_kind":"wer","model_id":"whisperkit|whisper|tiny|default","peak_memory_gb":0.4,"rtf":0.1,"warmup_seconds":2} + """ + let error = #expect(throws: DecodingError.self) { + try iso8601Decoder().decode(MeasurementRow.self, from: Data(bogus.utf8)) + } + guard case .dataCorrupted? = error else { + Issue.record("expected DecodingError.dataCorrupted, got \(String(describing: error))") + return + } + + // Contrast: an explicit null is NOT an unknown value — decodeIfPresent + // reads it as absent, so a null-emitting encoder still round-trips as + // "legacy row" rather than exploding (the read side is tolerant; our + // WRITE side never emits null — see the absent-keys test above). + let explicitNull = """ + {"app_version":"0.3.0","corpus_id":"c","decode_deterministic":null,"error_rate":0.2,"machine_id":"m","macos_version":"27.0","measured_at":"2026-01-01T00:00:00Z","metric_kind":"wer","model_id":"whisperkit|whisper|tiny|default","peak_memory_gb":0.4,"rtf":0.1,"warmup_seconds":2} + """ + let row = try iso8601Decoder().decode(MeasurementRow.self, from: Data(explicitNull.utf8)) + #expect(row.decodeDeterministic == nil) + } + + // MARK: The honest gate (#120 item 1) + + @Test func `Flag-consuming backends record what the flag actually did`() { + #expect( + DecodeDeterminism.forBackend(ModelGrid.backendWhisperKit, flagRequested: true) + == .deterministicEnforced) + #expect( + DecodeDeterminism.forBackend(ModelGrid.backendWhisperKit, flagRequested: false) + == .fallbackEnabled) + #expect( + DecodeDeterminism.forBackend(ModelGrid.backendWhisperCpp, flagRequested: true) + == .deterministicEnforced) + #expect( + DecodeDeterminism.forBackend(ModelGrid.backendWhisperCpp, flagRequested: false) + == .fallbackEnabled) + } + + @Test func `Backends that ignore the flag never claim determinism`() { + // NEVER LIE: mlx-audio's --decode-deterministic is a silent no-op and + // the Fluid backends have no such knob, so a requested flag must NOT + // become a determinism claim (#111/#118). flag-not-consumed makes no + // claim about whether those decodes are in fact reproducible. + for backend in [ + ModelGrid.backendMLXAudio, ModelGrid.backendFluidParakeet, + ModelGrid.backendFluidParaformer, ModelGrid.backendFluidSenseVoice, + "some-future-backend", + ] { + #expect( + DecodeDeterminism.forBackend(backend, flagRequested: true) == .flagNotConsumed) + #expect( + DecodeDeterminism.forBackend(backend, flagRequested: false) == .flagNotConsumed) + } + } + // MARK: SubmissionRow (denormalized publish row carries the same provenance) @Test func `Submission packaging threads the new provenance through`() throws { @@ -74,22 +172,22 @@ struct RunKindDecodeDeterministicTests { measuredAt: Date(timeIntervalSince1970: 1_752_800_000), metricKind: .cer, errorRate: 0.12, rtf: 0.14, peakMemoryGB: 3.1, warmupSeconds: 8.0, appVersion: "0.14.0", macosVersion: "26.0", - runKind: "release-sweep", decodeDeterministic: true) + runKind: "release-sweep", decodeDeterministic: .deterministicEnforced) let submissions = SubmissionPackager.package( local: [row], machines: [MachineRow(chip: "Apple M5 Max", unifiedMemoryGB: 128)], canonicalCorpusIds: ["abc123abc123"], publishedKeys: [], contributor: "che") #expect(submissions.count == 1) #expect(submissions[0].runKind == "release-sweep") - #expect(submissions[0].decodeDeterministic == true) + #expect(submissions[0].decodeDeterministic == .deterministicEnforced) // JSONL round-trips the snake_case keys. let jsonl = try SubmissionPackager.encodeJSONL(submissions) #expect(jsonl.contains("\"run_kind\"")) - #expect(jsonl.contains("\"decode_deterministic\"")) + #expect(jsonl.contains("\"decode_deterministic\":\"deterministic-enforced\"")) let decoded = try iso8601Decoder().decode( SubmissionRow.self, from: Data(jsonl.trimmingCharacters(in: .whitespacesAndNewlines).utf8)) #expect(decoded.runKind == "release-sweep") - #expect(decoded.decodeDeterministic == true) + #expect(decoded.decodeDeterministic == .deterministicEnforced) } @Test func `Legacy submission JSON without the new keys decodes to nil`() throws { @@ -100,4 +198,33 @@ struct RunKindDecodeDeterministicTests { #expect(row.runKind == nil) #expect(row.decodeDeterministic == nil) } + + // MARK: --run-kind value domain at the CLI boundary (#120 item 2) + + @Test func `Run-kind vocabulary is the single Swift-side source of truth`() { + #expect(Set(RunKind.allCases.map(\.rawValue)) == ["release-sweep", "adhoc"]) + #expect(RunKind(rawValue: "bogus") == nil) + } + + @Test func `Known run kinds parse and reach the core as their wire strings`() throws { + let sweep = try Benchmark.parse(["a.wav", "--reference", "r.txt", "--run-kind", "release-sweep"]) + #expect(sweep.runKind == .releaseSweep) + #expect(sweep.runKind?.rawValue == "release-sweep") + let adhoc = try Benchmark.parse(["a.wav", "--reference", "r.txt", "--run-kind", "adhoc"]) + #expect(adhoc.runKind == .adhoc) + // An omitted flag stays nil — "no provenance tag" is a valid state. + let bare = try Benchmark.parse(["a.wav", "--reference", "r.txt"]) + #expect(bare.runKind == nil) + } + + @Test func `Unknown run kind is rejected at the CLI boundary, not in bench CI`() { + // Pre-#120 a typo travelled verbatim into the store and only failed in + // the bench repo's CI — fail-loud at the wrong end. + #expect(throws: (any Error).self) { + try Benchmark.parse(["a.wav", "--reference", "r.txt", "--run-kind", "release_sweep"]) + } + #expect(throws: (any Error).self) { + try Benchmark.parse(["a.wav", "--reference", "r.txt", "--run-kind", "bogus"]) + } + } } From a9692d3be46775e2aa134a5b81030e5f2b00568e Mon Sep 17 00:00:00 2001 From: che cheng Date: Sat, 1 Aug 2026 15:43:21 +0800 Subject: [PATCH 2/2] =?UTF-8?q?fix:=20verify=20round-1=20=E2=80=94=20hones?= =?UTF-8?q?t=20docs,=20correct=20citation,=20lock=20the=20old-bool=20shape?= =?UTF-8?q?=20(#130)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Seven convergent verify findings, all in-scope by the DA's boundary (this PR may fix text it wrote and text it made false, plus tests in the file it created — not what it merely made visible): - CHANGELOG cited bestASR-bench@e728f1a as the authority for the enum. That commit's validator enforces isinstance(..., bool) — it REJECTS every value the paragraph introduces. Now c93a70e. Two legs caught it independently; shipped text that contradicts itself is the worst kind. - The compat sentence covered only an ABSENT field. It now also states what happens to the boolean this same unreleased entry used to describe: rejected, not coerced, with the evidence that nothing ever wrote one. - #120 was missing from the CHANGELOG entirely, though --run-kind bogus failing at parse is user-visible. Added, including WHY the stored field stays a string. - The #109 paragraph said --decode-deterministic is 'forwarded elsewhere', which now reads as contradicting the #118 paragraph beside it. It is a silent no-op elsewhere — that is the whole reason flag-not-consumed exists. - deterministicEnforced's doc claimed 'the same audio yields the same text'. An over-claim inside a PR about not over-claiming — disabling temperature fallback does not establish byte-identical output across runtime, hardware or model revision. It now says what was observed: the setting was enforced. - 'hyphenated snake-case' is kebab-case. - RunKind's doc claimed to be the Swift side's single source of truth. It is not: the row types stay String?, so a library caller can still store anything and bench CI stays the backstop. The comment now states that scope AND the reason it differs from decode_deterministic — that field's domain is derived from this repo's backend roster, while run_kind is human-typed provenance whose vocabulary is already incomplete (the regression gate benchmarks with no --run-kind at all). Closing it at the row type would trade a loud CI failure for a silently dropped row. Plus two tests: the pre-#118 boolean shape is now locked as REJECTED (a census expires, a test does not — and main still contains the bool-writer today), and SubmissionRow's absent-not-null contract is asserted on the type that actually crosses to the bench repo, not only via MeasurementRow. Forward-compat is now documented on the enum: adding a case is breaking for older readers AND fails invisibly, because both consumers turn the throw into a quiet drop. Mechanism-level fix tracked in #131. Refs #118 Refs #120 --- CHANGELOG.md | 18 ++++++-- Sources/BestASRKit/Models/DataModels.swift | 43 +++++++++++++----- .../RunKindDecodeDeterministicTests.swift | 44 +++++++++++++++++++ 3 files changed, 92 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3fba275..1fa6750 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,8 @@ All notable changes to bestASR are documented here. The format follows deterministic measurement snapshot. Runs the priority-1 runnable candidates (default ceiling; `--all-grid` widens to the whole grid) over the **canonical community corpora** (`bestasr bench pull`) with `--decode-deterministic` - (binding for Whisper-family backends; forwarded elsewhere), so the local + (binding for Whisper-family backends; a silent no-op elsewhere — see the + `decode_deterministic` entry below), so the local store gains a "performance of each model under this pinned version" record (measurements carry `app_version` / `macos_version` / machine id). Canonical corpora are what `bestasr bench submit` can publish — the bench leaderboard @@ -36,8 +37,19 @@ All notable changes to bestASR are documented here. The format follows no-op; the Fluid backends have no such knob) — which makes no claim about whether those decodes are reproducible, rather than lying in either direction. An **absent** field still means "legacy row, predates the field"; - `null` is never written. The bench side validates both only when present - (`PsychQuant/bestASR-bench@e728f1a`). + `null` is never written, though a reader accepts it as equivalent to absent. + A value the reader does not recognize — including a `decode_deterministic` + boolean from the earlier shape of this same unreleased entry — is **rejected**, + not coerced; no released version ever wrote one, and no row in either repo or + any local store carries one. The bench side validates both only when present + (`PsychQuant/bestASR-bench@c93a70e`). + + `--run-kind` also gained a value domain at the CLI (#120): a typo now fails at + parse (`exit 64`) instead of surviving into a submission and failing in the + bench repo's CI. The stored field stays a plain string on purpose — that + vocabulary is human-typed provenance and already incomplete (the regression + gate benchmarks with no `--run-kind` at all), so closing it at the row type + would trade a loud CI failure for a silently dropped row. ### Fixed diff --git a/Sources/BestASRKit/Models/DataModels.swift b/Sources/BestASRKit/Models/DataModels.swift index ed87f18..3dce40a 100644 --- a/Sources/BestASRKit/Models/DataModels.swift +++ b/Sources/BestASRKit/Models/DataModels.swift @@ -201,11 +201,23 @@ public enum MetricKind: String, Codable, Sendable { /// predates the field entirely and makes no claim at all; `nil` is deliberately /// NOT one of the cases below. /// -/// Wire values are hyphenated snake-case and are a cross-repo contract with the -/// bench validator (bestASR-bench `tools/validate_measurements.py`). +/// Wire values are kebab-case and are a cross-repo contract with the bench +/// validator (bestASR-bench `tools/validate_measurements.py`). +/// +/// FORWARD COMPAT: decoding is strict — an unrecognized wire value throws, and +/// both consumers turn that throw into a quiet drop (`BenchmarkStore.load` +/// records a warning nothing reads; `SubmissionPackager.publishedKeys` uses +/// `try?`). So adding a case here is a BREAKING change for any older client +/// still reading the same store, and it fails invisibly rather than loudly. +/// Adding a fourth case therefore needs a migration story, not just an +/// `case` line (#131). public enum DecodeDeterminism: String, Codable, Sendable { - /// The backend consumed `--decode-deterministic` and it was ON: temperature - /// fallback re-decoding was disabled, so the same audio yields the same text. + /// The backend consumed `--decode-deterministic` and it was ON — i.e. the + /// deterministic *setting was enforced*: temperature-fallback re-decoding + /// was disabled for this run. That is what we observed; it is deliberately + /// NOT a promise that re-running yields byte-identical text, which also + /// depends on runtime, hardware and model revision (#118: never claim more + /// than the evidence). case deterministicEnforced = "deterministic-enforced" /// The backend consumed `--decode-deterministic` and it was OFF: temperature /// fallback re-decoding is live, so a re-run may differ. @@ -234,13 +246,24 @@ public enum DecodeDeterminism: String, Codable, Sendable { } } -/// How a measurement was produced (#111) — the `run_kind` value domain, and the -/// Swift side's single source of truth for it (#120 item 2). +/// How a measurement was produced (#111) — the `run_kind` value domain as the +/// CLI accepts it (#120 item 2). +/// +/// SCOPE, deliberately: this constrains the `--run-kind` OPTION, not the stored +/// field. `MeasurementRow.runKind` / `SubmissionRow.runKind` stay `String?`, so +/// a library caller can still store a value outside this domain, and bench CI +/// stays the backstop for that path. That is the opposite choice from +/// `DecodeDeterminism`, on purpose: that field's domain is *derived* from this +/// repo's own backend roster (a total function over it), whereas run_kind is +/// human-typed provenance whose vocabulary is already demonstrably incomplete — +/// `scripts/regression-gate.sh` benchmarks with no `--run-kind` at all. Closing +/// this domain at the row type would trade a loud CI failure for a silent +/// stale-data read (an undecodable row is dropped, promoting a superseded one). /// -/// KEEP IN SYNC with the bench validator's `run_kind` enum (bestASR-bench -/// `tools/validate_measurements.py`). There is no mechanical link between the -/// two definitions: a value added here and not there fails only in the bench -/// repo's CI, and vice versa (#120 Residue, deliberately unfixed). +/// KEEP IN SYNC with the bench validator's `run_kind` set (bestASR-bench +/// `tools/validate_measurements.py`). There is no mechanical link: a value added +/// here and not there fails only in the bench repo's CI, and vice versa +/// (#120 Residue, deliberately unfixed). public enum RunKind: String, Codable, Sendable, CaseIterable { /// A sweep driven by `scripts/release-sweep.sh`. case releaseSweep = "release-sweep" diff --git a/Tests/BestASRKitTests/RunKindDecodeDeterministicTests.swift b/Tests/BestASRKitTests/RunKindDecodeDeterministicTests.swift index b380394..5b11039 100644 --- a/Tests/BestASRKitTests/RunKindDecodeDeterministicTests.swift +++ b/Tests/BestASRKitTests/RunKindDecodeDeterministicTests.swift @@ -227,4 +227,48 @@ struct RunKindDecodeDeterministicTests { try Benchmark.parse(["a.wav", "--reference", "r.txt", "--run-kind", "bogus"]) } } + + // MARK: - the pre-#118 shape (#130 verify) + + @Test func `A pre-118 boolean decode_deterministic is rejected, not coerced`() throws { + // #111 briefly wrote `decode_deterministic` as a JSON boolean before #118 + // settled on the enum. Nothing released ever emitted one and no row in + // either repo carries one — but a census expires, and this does not. + // Locking the behaviour states the choice: an old boolean is REJECTED + // rather than migrated (true -> deterministic-enforced), because the + // mapping would have to re-derive the backend to stay honest. + for legacy in ["true", "false"] { + let json = """ + {"model_id":"whisperkit|whisper|large-v3|default","corpus_id":"c1", + "machine_id":"m1","measured_at":"2026-07-30T00:00:00Z","metric_kind":"cer", + "error_rate":0.1,"rtf":0.5,"peak_memory_gb":1.0,"warmup_seconds":0.0, + "app_version":"0.16.0","macos_version":"27.0", + "decode_deterministic":\(legacy)} + """ + #expect(throws: DecodingError.self) { + _ = try self.iso8601Decoder().decode(MeasurementRow.self, from: Data(json.utf8)) + } + } + } + + @Test func `A submission row with no provenance omits the keys on the wire`() throws { + // SubmissionRow is what actually crosses to the bench repo, so the + // absent-not-null contract has to hold on THIS type, not only on + // MeasurementRow — the bench validator treats null as absent now, but + // the producer is still expected never to write one. + let bare = MeasurementRow( + modelId: "whisperkit|whisper|large-v3|default", corpusId: "abc123abc123", + machineId: MachineRow.id(chip: "Apple M5 Max", unifiedMemoryGB: 128), + measuredAt: Date(timeIntervalSince1970: 1_752_800_000), metricKind: .cer, + errorRate: 0.12, rtf: 0.14, peakMemoryGB: 3.1, warmupSeconds: 8.0, + appVersion: "0.14.0", macosVersion: "26.0") // both provenance fields omitted + let rows = SubmissionPackager.package( + local: [bare], machines: [MachineRow(chip: "Apple M5 Max", unifiedMemoryGB: 128)], + canonicalCorpusIds: ["abc123abc123"], publishedKeys: [], contributor: "tester") + #expect(rows.count == 1) + let jsonl = try SubmissionPackager.encodeJSONL(rows) + #expect(!jsonl.contains("decode_deterministic")) + #expect(!jsonl.contains("run_kind")) + #expect(!jsonl.contains("null")) + } }