feat(ar-024): enforce the invariant statically, and delete the fallback it caught
AR-024's register row gives its verification tier as "Static check -- no bare cosine outside a tagged EXCEPTION". No such check existed, so the invariant was enforced by reading, and reading had missed a live violation. scripts/ci/check_raw_cosine.py is that check, wired into the traceability workflow as a blocking step. It is honest about its reach: it catches direct cosine_similarity() uses not routed through a calibration, and it cannot follow a cosine through a variable across statements. That limit is documented in the script rather than left for someone to discover after trusting a pass. What it caught, and what this commit removes with it: The identity matcher's no-calibration fallback thresholded raw cosine distance (match_threshold) plus a ratio test (match_ratio, match_ratio_ceil). Worse than the invariant breach: it fed max(0, cosine) into 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, and did. And it disagreed with the rest of the pipeline about what "the fit failed" means -- same_person_probability answers that with the untuned default sigmoid and a loud warning, so association stayed in probability space while matching alone left it. One run, two policies, no announcement. Now one rule: cal_.probability() always, with a warning when the fit is not real. A worse answer than a fitted calibration, a better one than a number whose units nothing else shares. TrackGallery::set_calibration is mandatory for the same reason. Its default was max(0, cosine), which made expand_band_lo = 0.90 mean "cosine > 0.9" in a test and "P(same person) > 0.9" in production. FaceTrackerFunc already threw without one; the expansion store now matches. One exception is recorded, in the calibration's own dedup. It is not a close call: at 1 - 1e-7 it asks whether two vectors are the same vector, and it runs on the fit's input, so a calibrated comparison there would have to be calibrated by the fit it is feeding. Also drops seven dead keys from the optimizer's CFG_KEYS. Config keys are read with a contains() check, so each one had been silently inert since the field behind it was deleted -- a sweep varying one of them measured nothing and reported an ordinary-looking F1. TRACES: AR-024, AR-023 | SR-002
This commit is contained in:
@@ -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 <catch2/catch_test_macros.hpp>
|
||||
#include <catch2/matchers/catch_matchers_floating_point.hpp>
|
||||
|
||||
@@ -19,6 +24,7 @@
|
||||
#include "types.hpp"
|
||||
|
||||
#include <algorithm>
|
||||
#include <stdexcept>
|
||||
#include <array>
|
||||
#include <cmath>
|
||||
#include <vector>
|
||||
@@ -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);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user