diff --git a/.gitea/workflows/traceability-check.yml b/.gitea/workflows/traceability-check.yml index b597b43..f71a8a6 100644 --- a/.gitea/workflows/traceability-check.yml +++ b/.gitea/workflows/traceability-check.yml @@ -65,6 +65,15 @@ jobs: - name: Traceability gate run: sh scripts/vendor/jray-project/scripts/traceability/traceability-gate.sh + # AR-024's register row names its verification tier as "Static check -- + # no bare cosine outside a tagged EXCEPTION". This is that check, and it + # belongs here rather than in unit-tests.yml because it is static + # analysis of source text, like everything else in this job, and needs + # no toolchain. It blocks: an untagged bare cosine is a defect by the + # invariant's own wording, not a warning. + - name: AR-024 — no bare cosine outside a recorded exception + run: python3 scripts/ci/check_raw_cosine.py + - name: Check modified files for traces if: github.event_name == 'pull_request' run: | diff --git a/scripts/ci/check_raw_cosine.py b/scripts/ci/check_raw_cosine.py new file mode 100755 index 0000000..0b5e00c --- /dev/null +++ b/scripts/ci/check_raw_cosine.py @@ -0,0 +1,220 @@ +#!/usr/bin/env python3 +"""Enforce the AR-024 invariant: never a raw cosine, always the calibration. + +TRACES: AR-024 | SR-002 + +docs/requirements.md gives AR-024's verification tier as "Static check -- no +bare cosine outside a tagged EXCEPTION | Grep-based; this is the invariant's +enforcement". This is that check. Until it existed the invariant was enforced +by reading, and reading missed a live violation: the identity matcher's +no-calibration fallback thresholded raw cosine distance and fed `max(0, cosine)` +into the Bayesian accumulation as though it were a posterior. + +WHAT IT CHECKS, precisely, because a static check that overclaims its reach is +worse than one with a stated scope: + + Every call to `cosine_similarity(...)` in C++ source must either + + (a) have its result consumed immediately by a calibration -- the call is + textually wrapped in `cal_(...)`, `calibrate_(...)`, `.probability(...)` + or similar; or + (b) sit under an `EXCEPTION: AR-024 ` comment within + EXCEPTION_SCOPE_LINES above it. + + Anything else is a defect, per CLAUDE.md: "treat any bare cosine comparison in + the code as a defect to be fixed". + +WHAT IT DOES NOT CHECK, and why you should not read a pass as more than it is: + + - It cannot follow a cosine through a variable across statements. A file that + stores `float s = cosine_similarity(a, b);` and compares `s` three lines + later is not caught. The codebase does not currently do this, and this check + exists partly to keep it that way, but it is a convention backed by review, + not by the tool. + - It says nothing about GEMM output. The similarity engine returns a whole + matrix of cosines and the matcher reads them directly; that path is correct + by inspection (every value goes through `cal_.probability`) and is not + verified here. + - A retired constant reintroduced under a new name is invisible to it. + +Exit status is 0 when clean, 1 when a violation is found, 2 on a usage error. +""" + +import argparse +import pathlib +import re +import sys + +# How far above a use an `EXCEPTION: AR-024` tag may sit and still cover it. +# Generous, because the house style puts a paragraph of reasoning between the +# tag and the code -- but bounded, so a tag cannot silently cover a whole file. +EXCEPTION_SCOPE_LINES = 25 + +CPP_SUFFIXES = {".h", ".hpp", ".hxx", ".cc", ".cpp", ".cxx", ".cu", ".cuh"} + +# src only, deliberately. The invariant governs what the PIPELINE decides -- +# CLAUDE.md's rule is "tag the unit that decides" -- whereas a test legitimately +# asserts properties of the metric space itself (that a vector's cosine with +# itself is 1, that the annex ended up holding the spoke it should have). Those +# are measurements of the code under test, not decisions shipped to a user, and +# sweeping them in would produce a wall of blanket EXCEPTION tags that would +# devalue the tag everywhere else. Pass --source-root tests to scan them anyway. +DEFAULT_ROOTS = ["src"] + +# Directories that are never this repo's code. +EXCLUDE_DIRS = { + "build", "build-ort", "external", "vendor", "__pycache__", + ".git", "node_modules", "models", +} + +COSINE_CALL = re.compile(r"\bcosine_similarity\s*\(") + +# The result is immediately handed to a calibration. Matches the house shapes: +# cal_(cosine_similarity(a, b)) +# calibrate_(cosine_similarity(a, b)) +# same_person(cosine_similarity(a, b)) +# cal_.probability(cosine_similarity(a, b)) +CALIBRATED = re.compile( + r"(?:\b(?:cal_|cal|calibrate_|calibrate|same_person|same_person_probability)" + r"\s*(?:\.\s*probability\s*)?\(\s*|\.\s*probability\s*\(\s*)" + r"cosine_similarity\s*\(" +) + +EXCEPTION_TAG = re.compile(r"EXCEPTION:\s*AR-024\b(.*)") + +# The function's own definition is not a use of it. +DEFINITION = re.compile(r"^\s*(?:inline\s+|static\s+|constexpr\s+)*float\s+" + r"cosine_similarity\s*\(") + +# The house style wraps long calls across lines: +# const float p = calibrate_( +# cosine_similarity(a, b)); +# so the calibration and the call it guards are not always on one line. Joining +# a small window before testing is what makes this check usable on real code +# rather than a generator of false positives that trains people to ignore it. +JOIN_LOOKBEHIND = 2 + + +def iter_sources(root: pathlib.Path, roots): + for rel in roots: + base = root / rel + if not base.exists(): + continue + for p in sorted(base.rglob("*")): + if p.suffix.lower() not in CPP_SUFFIXES: + continue + if any(part in EXCLUDE_DIRS for part in p.relative_to(root).parts): + continue + yield p + + +def covering_exception(lines, idx): + """Return the reason text of an EXCEPTION: AR-024 tag covering line `idx`.""" + lo = max(0, idx - EXCEPTION_SCOPE_LINES) + for j in range(idx, lo - 1, -1): + m = EXCEPTION_TAG.search(lines[j]) + if m: + return m.group(1).strip(" -—*/") or "(no reason given)" + return None + + +def check_file(path: pathlib.Path, root: pathlib.Path): + violations, exceptions = [], [] + try: + lines = path.read_text(encoding="utf-8", errors="replace").splitlines() + except OSError as e: + print(f"error: cannot read {path}: {e}", file=sys.stderr) + return violations, exceptions + + rel = path.relative_to(root) + for i, line in enumerate(lines): + if not COSINE_CALL.search(line): + continue + # A comment mentioning the function is prose, not a use. + stripped = line.lstrip() + if stripped.startswith(("//", "///", "*", "/*")): + continue + if DEFINITION.match(line): + continue + # Join a small window so a call wrapped across lines is still seen as + # calibrated. Whitespace is collapsed so the join reads as one statement. + window = " ".join( + lines[max(0, i - JOIN_LOOKBEHIND):i + 1] + ) + window = re.sub(r"\s+", " ", window) + if CALIBRATED.search(window): + continue + reason = covering_exception(lines, i) + if reason: + exceptions.append((rel, i + 1, line.strip(), reason)) + else: + violations.append((rel, i + 1, line.strip())) + return violations, exceptions + + +def main(): + ap = argparse.ArgumentParser(description=__doc__, + formatter_class=argparse.RawDescriptionHelpFormatter) + ap.add_argument("--root", default=None, + help="repository root (default: the script's ../..)") + ap.add_argument("--source-root", action="append", default=None, + help="directory to scan; repeatable (default: src, tests)") + args = ap.parse_args() + + root = pathlib.Path(args.root) if args.root \ + else pathlib.Path(__file__).resolve().parents[2] + roots = args.source_root or DEFAULT_ROOTS + + if not root.is_dir(): + print(f"error: root {root} is not a directory", file=sys.stderr) + return 2 + + all_violations, all_exceptions, n_files = [], [], 0 + for p in iter_sources(root, roots): + n_files += 1 + v, e = check_file(p, root) + all_violations += v + all_exceptions += e + + if n_files == 0: + # A scan that found nothing to read is a misconfiguration reporting a + # pass, which is the failure mode the traceability gate also guards. + print(f"error: scanned 0 source files under {root} ({', '.join(roots)})", + file=sys.stderr) + return 2 + + print("AR-024 — always the calibrated probability, never a raw cosine") + print("=" * 72) + print(f"Repo root : {root}") + print(f"Files scanned : {n_files} ({', '.join(roots)})") + print(f"Recorded excs. : {len(all_exceptions)}") + print(f"Violations : {len(all_violations)}") + + if all_exceptions: + print("\nRecorded exceptions (allowed, and each one is a claim to re-read):") + for rel, ln, src, reason in all_exceptions: + print(f" {rel}:{ln} {reason}") + print(f" {src}") + + if all_violations: + print("\nVIOLATIONS — a bare cosine with no recorded exception:") + for rel, ln, src in all_violations: + print(f" {rel}:{ln}") + print(f" {src}") + print("\nEvery similarity is converted through the sigmoid calibration") + print("before it is used, compared, or thresholded. A raw cosine means") + print("something different for every model, gallery and face size, and") + print("it cannot be combined with anything else.") + print("\nEither route it through the calibration, or, if the use is") + print("genuinely about the metric space rather than about a decision,") + print("record it:") + print(" // EXCEPTION: AR-024 ") + print("and add a row to CLAUDE.md's agreed-exceptions table.") + return 1 + + print("\nOK: no bare cosine outside a recorded exception.") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/scripts/optimizer/replay.py b/scripts/optimizer/replay.py index 6415841..fcac538 100644 --- a/scripts/optimizer/replay.py +++ b/scripts/optimizer/replay.py @@ -243,9 +243,25 @@ def build_minimal(annotations, movie, fps, cfg) -> dict: "anneal_sec": anneal, "actors": actors} -CFG_KEYS = ["detector_conf", "prob_threshold", "match_prior", "match_threshold", "match_ratio", - "match_ratio_ceil", "track_alpha", "track_min_iou", "track_max_embed_dist", - "track_max_frames_missing", "cut_revive_sim", "cut_inactive_max_frames", +# TRACES: AR-024 | SR-002 +# Keys the C++ Config actually still has. Seven names were removed here, all of +# them accepted silently for months after the fields behind them were deleted: +# +# match_threshold, match_ratio, match_ratio_ceil — the raw-cosine accept +# fallback, retired with AR-024's enforcement. +# track_max_embed_dist, cut_revive_sim — raw cosines, retired +# earlier by AR-024 when association moved into probability space. +# track_max_frames_missing, cut_inactive_max_frames — frame counts whose +# meaning changed with sample_fps, retired by AR-008/AR-013 in favour of +# track_extinction_sec. +# +# A sweep that varied one of these was measuring nothing, and reported a +# perfectly ordinary-looking F1 for its trouble. kpn_bindings.cpp reads config +# keys with a contains() check, so an unknown key is not an error — which makes +# a stale entry here silently inert rather than loudly wrong. +CFG_KEYS = ["detector_conf", "prob_threshold", "match_prior", + "track_alpha", "track_min_iou", "track_assoc_min_prob", + "track_extinction_sec", "extinction_sec", "anneal_sec"] diff --git a/src/config.hpp b/src/config.hpp index 0a8a9fe..267cea8 100644 --- a/src/config.hpp +++ b/src/config.hpp @@ -78,9 +78,15 @@ struct Config { // earlier 9-film scene-union-metric tuning (0.76) — that metric is now known to // have hidden out-of-cast false positives (see docs/optimizer-experiments.md). float prob_threshold{0.754f}; // posterior P(match | sim, prior) threshold - float match_threshold{0.45f}; // cosine distance hard ceiling fallback (no calibration) - float match_ratio{0.80f}; // ratio test fallback: accept if best/second < ratio - float match_ratio_ceil{0.65f}; // ratio test only fires below this absolute distance + // TRACES: AR-024 | SR-002 + // match_threshold (0.45), match_ratio (0.80) and match_ratio_ceil (0.65) are + // RETIRED, joining track_max_embed_dist, cut_revive_sim, expand_novelty_sim + // and expand_track_spread_max. All were raw cosine distances, and they were + // the accept rule whenever the calibration fit failed — so the one situation + // in which the pipeline knew its probabilities were untrustworthy was the + // one in which it stopped using them. An unfitted sigmoid is now the + // fallback everywhere, which is at least the same wrong number in every + // stage. See identity_matcher_node.hpp. // ── Cut detection ──────────────────────────────────────────────────────── float cut_threshold{0.70f}; // grayscale histogram correlation below this → hard cut diff --git a/src/gallery/gallery_calibration.hpp b/src/gallery/gallery_calibration.hpp index f67f4f8..2557747 100644 --- a/src/gallery/gallery_calibration.hpp +++ b/src/gallery/gallery_calibration.hpp @@ -162,6 +162,19 @@ inline GalleryCalibration calibrate_gallery( for (const auto& e : by_actor[ai]) { bool dup = false; for (const auto& k : kept) { + // EXCEPTION: AR-024 this asks whether two vectors are THE SAME + // VECTOR, not whether two faces are the same person. + // + // Two independent reasons, either sufficient. First, at + // 1 - 1e-7 the threshold is a floating-point identity test: it + // catches one source image embedded twice, and no genuine pair + // of distinct photographs lands there. Nothing about it is a + // decision, so there is nothing for a probability to mean. + // + // Second, and structurally: this IS the calibration fit. The + // dedup runs on its input, before (a, b) exist. A calibrated + // comparison here would have to be calibrated by the fit it is + // feeding, which is not a thing that can be arranged. if (cosine_similarity(e, k) > kDedupSimThreshold) { dup = true; break; } } if (!dup) kept.push_back(e); diff --git a/src/gallery/track_gallery.hpp b/src/gallery/track_gallery.hpp index dfd82e0..ee28aec 100644 --- a/src/gallery/track_gallery.hpp +++ b/src/gallery/track_gallery.hpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -163,10 +164,20 @@ struct TrackGallery { } /// TRACES: AR-024 | SR-005 - /// Supply the calibration belonging to the active embedder. Without it the - /// band falls back to treating cosine as probability, which is wrong but - /// bounded — and the default is loud in the header rather than silent. - void set_calibration(std::function c) { calibrate_ = std::move(c); } + /// Supply the calibration belonging to the active embedder. + /// + /// Required, not optional. The default used to be `max(0, cosine)` — a raw + /// cosine worn as a probability, which made `expand_band_lo = 0.90` mean + /// "cosine above 0.9" in a test and "P(same person) above 0.9" in + /// production. Those are wildly different gates, and nothing announced the + /// switch. `FaceTrackerFunc` already refuses to construct without a + /// calibration for the same reason; this now matches it. + void set_calibration(std::function c) { + if (!c) throw std::invalid_argument( + "track_gallery: a calibration is required — the admission band is " + "expressed in probability space (AR-024)"); + calibrate_ = std::move(c); + } /// Embeddings the band refused. A store that admits nothing is as wrong as /// one that admits everything, and neither is visible without this. @@ -341,8 +352,9 @@ private: } /// cosine → P(same person). The one probability space the pipeline reasons - /// in; see gallery_calibration.hpp's same_person_probability. - std::function calibrate_{[](float c) { return std::max(0.f, c); }}; + /// in; see gallery_calibration.hpp's same_person_probability. Never default + /// constructed to an identity-ish stand-in — see set_calibration. + std::function calibrate_; std::size_t rejected_{0}; ///< admissions refused by the band bool enabled_; diff --git a/src/kpn_bindings.cpp b/src/kpn_bindings.cpp index fbc6bd6..b486faf 100644 --- a/src/kpn_bindings.cpp +++ b/src/kpn_bindings.cpp @@ -168,9 +168,6 @@ static Config config_from_dict(nb::dict d) { // identity matcher getf("match_prior", cfg.match_prior); getf("prob_threshold", cfg.prob_threshold); - getf("match_threshold", cfg.match_threshold); - getf("match_ratio", cfg.match_ratio); - getf("match_ratio_ceil", cfg.match_ratio_ceil); // face tracker getf("track_alpha", cfg.track_alpha); getf("track_min_iou", cfg.track_min_iou); diff --git a/src/main.cpp b/src/main.cpp index 04c3c58..5d47b1f 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -22,7 +22,7 @@ // --output output JSON (default: annotations.json) // --fps sample rate in frames/sec (default: 1.0) // --verbosity <0|1|2> 0=minimal, 1=standard, 2=jellyfin-xray (default: 0) -// --match-threshold cosine dist threshold (default: 0.45) +// --prob-threshold posterior P(match) to accept (default: 0.754) // --extinction actor extinction window in seconds (default: 5.0) // --detector override SCRFD detector model path // --arcface override ArcFace model path @@ -155,7 +155,6 @@ static Config parse_args(int argc, char** argv) { else if (arg("--verbosity")) { int v = std::stoi(next()); cfg.verbosity = v == 2 ? Verbosity::xray : v == 1 ? Verbosity::standard : Verbosity::minimal; } else if (arg("--prior")) cfg.match_prior = std::stof(next()); else if (arg("--prob-threshold")) cfg.prob_threshold = std::stof(next()); - else if (arg("--match-threshold")) cfg.match_threshold = std::stof(next()); else if (arg("--extinction")) cfg.extinction_sec = std::stod(next()); else if (arg("--detector")) cfg.detector_model = next(); else if (arg("--detector-engine")) cfg.detector_engine = next(); @@ -165,8 +164,6 @@ static Config parse_args(int argc, char** argv) { else if (arg("--conf")) cfg.detector_conf = std::stof(next()); else if (arg("--max-faces")) cfg.max_faces = std::stoi(next()); else if (arg("--min-face-px")) cfg.min_face_px = std::stof(next()); - else if (arg("--ratio")) cfg.match_ratio = std::stof(next()); - else if (arg("--ratio-ceil")) cfg.match_ratio_ceil = std::stof(next()); else if (arg("--track-alpha")) cfg.track_alpha = std::stof(next()); else if (arg("--track-min-iou")) cfg.track_min_iou = std::stof(next()); else if (arg("--track-min-prob")) cfg.track_assoc_min_prob = std::stof(next()); diff --git a/src/nodes/identity_matcher_node.hpp b/src/nodes/identity_matcher_node.hpp index a281688..08d531d 100644 --- a/src/nodes/identity_matcher_node.hpp +++ b/src/nodes/identity_matcher_node.hpp @@ -19,19 +19,36 @@ // KPN node: compares each embedding against every reference embedding in the // actor gallery using cosine similarity. // -// Matching strategy — two modes selected at construction time: +// Matching strategy — one mode, always. // -// Calibrated (preferred): gallery calibration fits a sigmoid -// P(match) = σ(a·similarity + b) from intra/inter-class pairs. -// A face is accepted if P(match | best_actor) > prob_threshold. +// Gallery calibration fits a sigmoid P(match) = σ(a·similarity + b) from +// intra/inter-class pairs. A face is accepted if P(match | best_actor) > +// prob_threshold. Per-actor best similarity is the closest reference +// embedding (best-of-N). // -// Fallback (no calibration): dual-criterion accept — -// (a) best cosine distance < match_threshold, OR -// (b) ratio test: best_dist/second_best_dist < match_ratio -// AND best_dist < match_ratio_ceil. +/// TRACES: AR-024 | SR-002 +// **There is no raw-cosine fallback.** There used to be: when the fit was +// invalid this node switched to a cosine-distance ceiling plus a ratio test +// (`match_threshold`, `match_ratio`, `match_ratio_ceil`). Three things were +// wrong with it, and the third is the one that mattered. // -// In both modes, per-actor best similarity is determined by scanning -// reference embeddings and taking the closest (best-of-N). +// 1. It violated AR-024 outright, untagged — a bare cosine threshold means +// something different for every model, gallery and face size. +// 2. It disagreed with the rest of the pipeline about what "calibration +// failed" means. `same_person_probability` answers that question by +// falling back to the untuned default sigmoid and saying so loudly, so +// tracking and evidence weighting stayed in probability space while +// matching alone left it. One run, two policies. +// 3. Its accepted faces were still fed to `TrackRegistry::observe`, whose +// contract reads "posterior is a calibrated probability, never a raw +// cosine (AR-024) ... so the accumulation cannot be fed an uncalibrated +// number by a careless caller". It could. `max(0, cosine)` went straight +// into the log-odds accumulation as though it were a probability. +// +// An invalid fit now behaves exactly as everywhere else: the default sigmoid, +// with a warning that says the probabilities are not meaningful. That is a +// worse answer than a fitted calibration and a better one than a number whose +// units nothing else in the pipeline shares. // // Gallery scan: the full reference set (tens of thousands of 512-dim // embeddings) is uploaded to the GPU once at construction time and stays @@ -57,9 +74,6 @@ struct IdentityMatcherFunc { : gallery_(gallery) , prob_threshold_(cfg.prob_threshold) , log_prior_odds_(std::log(cfg.match_prior / (1.f - cfg.match_prior))) - , threshold_(cfg.match_threshold) - , ratio_(cfg.match_ratio) - , ratio_ceil_(cfg.match_ratio_ceil) , track_gallery_(cfg) { std::cerr << "[identity_matcher] flattening gallery embeddings...\n"; @@ -92,16 +106,21 @@ struct IdentityMatcherFunc { << cfg.gallery_path << "\n"; } - if (cal_.valid) { - std::cerr << "[identity_matcher] calibrated Bayesian matching" - << " prior=" << cfg.match_prior - << " P_threshold=" << prob_threshold_ - << " effective_sim_boundary=" - << cal_.boundary_at(prob_threshold_, log_prior_odds_) << "\n"; - } else { - std::cerr << "[identity_matcher] threshold matching (calibration skipped)" - << " threshold=" << threshold_ - << " ratio=" << ratio_ << " ratio_ceil=" << ratio_ceil_ << "\n"; + /// TRACES: AR-024 | SR-002 + // Same sentence either way, because it is the same decision rule; only + // the provenance of (a, b) differs. An unfitted sigmoid still returns + // plausible-looking probabilities, so the warning has to be the thing + // that distinguishes them — nothing downstream can. + std::cerr << "[identity_matcher] calibrated Bayesian matching" + << " prior=" << cfg.match_prior + << " P_threshold=" << prob_threshold_ + << " effective_sim_boundary=" + << cal_.boundary_at(prob_threshold_, log_prior_odds_) << "\n"; + if (!cal_.valid) { + std::cerr << "[identity_matcher] WARNING: the calibration is NOT fitted " + "(a=" << cal_.a << ", b=" << cal_.b << ") — matching runs " + "on the untuned default sigmoid, so prob_threshold is not " + "comparable to a tuned run's.\n"; } std::cerr << "[identity_matcher] gallery: " << gallery_.actors.size() << " actors, " @@ -214,39 +233,27 @@ struct IdentityMatcherFunc { if (sim > best_sim[ai]) best_sim[ai] = sim; } - int best_actor = -1; - int second_actor = -1; - float best_s = -std::numeric_limits::max(); - float second_s = -std::numeric_limits::max(); + // Only the best matters now. The runner-up was tracked solely for + // the retired ratio test, which asked whether the best cosine stood + // out from the second — a question the calibrated posterior does + // not need, since it already says how likely the best match is to + // be right rather than how much it beat its neighbour by. + int best_actor = -1; + float best_s = -std::numeric_limits::max(); for (int ai = 0; ai < static_cast(best_sim.size()); ++ai) { if (best_sim[ai] > best_s) { - second_s = best_s; - second_actor = best_actor; - best_s = best_sim[ai]; - best_actor = ai; - } else if (best_sim[ai] > second_s) { - second_s = best_sim[ai]; - second_actor = ai; + best_s = best_sim[ai]; + best_actor = ai; } } - (void)second_actor; - bool accept = false; - if (best_actor >= 0) { - if (cal_.valid) { - accept = cal_.probability(best_s, log_prior_odds_) > prob_threshold_; - } else { - float best_d = 1.f - best_s; - float second_d = (second_s > -std::numeric_limits::max()) - ? 1.f - second_s - : std::numeric_limits::max(); - bool absolute = best_d < threshold_; - bool ratio = (best_d < ratio_ceil_) && - (second_d == std::numeric_limits::max() || - best_d / second_d < ratio_); - accept = absolute || ratio; - } - } + /// TRACES: AR-024 | SR-002 + // One rule, whatever the fit's provenance. The cosine reaches a + // comparison only through cal_.probability(). + const float best_p = best_actor >= 0 + ? cal_.probability(best_s, log_prior_odds_) + : 0.f; + const bool accept = best_actor >= 0 && best_p > prob_threshold_; IdentifiedActor ia; // Map bbox back to original video resolution when dense_scale @@ -267,9 +274,7 @@ struct IdentityMatcherFunc { ia.imdb_id = gallery_.actors[best_actor].imdb_id; ia.tmdb_id = gallery_.actors[best_actor].tmdb_id; ia.jellyfin_id = gallery_.actors[best_actor].jellyfin_id; - ia.similarity = cal_.valid - ? cal_.probability(best_s, log_prior_odds_) - : best_s; + ia.similarity = best_p; } // Feed this face into per-film gallery expansion. best_actor/best_s @@ -283,12 +288,9 @@ struct IdentityMatcherFunc { // would make ownership depend on a per-frame threshold the redesign // exists to stop relying on. The registry discounts for correlation // and decides ownership from the accumulated posterior (AR-025). - if (registry_ && best_actor >= 0 && tf.track_ids[fi] >= 0) { - const float p = cal_.valid - ? cal_.probability(best_s, log_prior_odds_) - : std::max(0.f, best_s); - registry_->observe(tf.track_ids[fi], best_actor, p, tf.embeddings[fi]); - } + if (registry_ && best_actor >= 0 && tf.track_ids[fi] >= 0) + registry_->observe(tf.track_ids[fi], best_actor, best_p, + tf.embeddings[fi]); // TRACES: AR-019 | SR-005 // Ownership is the registry's, computed once. TrackGallery used to @@ -339,9 +341,6 @@ private: GalleryCalibration cal_; float prob_threshold_; float log_prior_odds_; - float threshold_; - float ratio_; - float ratio_ceil_; /// flat_emb_ is the BAKED reference set only — it is the calibration fit's /// input (AR-023) and is not touched again after construction. flat_actor_, /// by contrast, is the actor mapping parallel to the *engine's* rows, so it diff --git a/src/scene_preview.cpp b/src/scene_preview.cpp index b5e78be..81f64eb 100644 --- a/src/scene_preview.cpp +++ b/src/scene_preview.cpp @@ -71,7 +71,6 @@ static Config parse_args(int argc, char** argv) { else if (arg("--verbosity")) { int v = std::stoi(next()); cfg.verbosity = v == 2 ? Verbosity::xray : v == 1 ? Verbosity::standard : Verbosity::minimal; } else if (arg("--prior")) cfg.match_prior = std::stof(next()); else if (arg("--prob-threshold")) cfg.prob_threshold = std::stof(next()); - else if (arg("--match-threshold")) cfg.match_threshold = std::stof(next()); else if (arg("--extinction")) cfg.extinction_sec = std::stod(next()); else if (arg("--detector")) cfg.detector_model = next(); else if (arg("--detector-engine")) cfg.detector_engine = next(); @@ -81,8 +80,6 @@ static Config parse_args(int argc, char** argv) { else if (arg("--conf")) cfg.detector_conf = std::stof(next()); else if (arg("--max-faces")) cfg.max_faces = std::stoi(next()); else if (arg("--min-face-px")) cfg.min_face_px = std::stof(next()); - else if (arg("--ratio")) cfg.match_ratio = std::stof(next()); - else if (arg("--ratio-ceil")) cfg.match_ratio_ceil = std::stof(next()); else if (arg("--track-alpha")) cfg.track_alpha = std::stof(next()); else if (arg("--track-min-iou")) cfg.track_min_iou = std::stof(next()); else if (arg("--track-max-embed")) cfg.track_max_embed_dist = std::stof(next()); diff --git a/tests/test_track_gallery.cpp b/tests/test_track_gallery.cpp index 7c47ae1..309ffc9 100644 --- a/tests/test_track_gallery.cpp +++ b/tests/test_track_gallery.cpp @@ -7,10 +7,15 @@ // idempotent promotion, all through the public interface. // // The band is defined in PROBABILITY space (AR-024), so every case below states -// its own cosine → probability map instead of inheriting the header's fallback. -// A test that never names the mapping is not testing the band, it is testing a -// coincidence: with the fallback the two spaces happen to coincide, and a gate -// that silently reverted to raw cosine would still pass. +// its own cosine → probability map. It has to: set_calibration is now mandatory +// and there is no header default to inherit. +// +// There used to be one — `max(0, cosine)` — and it was the reason this comment +// was originally needed. Under it the two spaces coincided, so a test that +// forgot to name the mapping still passed, and a gate that silently reverted to +// raw cosine passed with it. The default is gone rather than merely discouraged, +// which is why identity_cal below is now an explicit choice a case makes and not +// a restatement of what would have happened anyway. #include #include @@ -19,6 +24,7 @@ #include "types.hpp" #include +#include #include #include #include @@ -381,3 +387,17 @@ TEST_CASE("promotions drain exactly once, in matrix order", CHECK(actor.size() == 6); CHECK(actor[5] == 4); } + +// ── AR-024: the calibration is not optional ───────────────────────────────── + +/// TRACES: UT-005 | AR-024 | SR-005 +TEST_CASE("a null calibration is refused, not silently replaced", "[track_gallery][AR-024]") { + // The class used to default calibrate_ to max(0, cosine). That made + // expand_band_lo = 0.90 mean "cosine above 0.9" here and "P(same person) + // above 0.9" in production — two very different gates, with nothing + // announcing which one was in force. FaceTrackerFunc already refused to + // construct without a calibration for exactly this reason; the expansion + // store now matches it. + TrackGallery tg(expand_cfg()); + CHECK_THROWS_AS(tg.set_calibration(nullptr), std::invalid_argument); +}