Fix --fail-on-drift firing on a skipped/unmeasurable comparison (closes #472)

--fail-on-drift checked bool(result["flags"]), but flags also includes a
kind-mismatch dimension's "drift comparison skipped" notice - genuinely
informational, not evidence of drift (psi/drift_level are already zeroed
for exactly this case). A schema change between two snapshots (a column
reformatted, renamed, or dropped) falsely triggered "drift detected" in a
CI gate, with PSI 0.000 and drift_level explicitly "none" - no drift was
measured at all.

_build_flags() now also returns drift_detected: true iff a *real* drift
signal fired (score drop, missing-pct shift, drift_level != none, an
appeared/disappeared group, or an added/removed dimension) - everything
in flags except the kind-mismatch skip notices. compare() exposes it as a
new top-level field; the CLI now checks drift_detected instead of
bool(flags). Mirrored in assets/profiler-engine.js and documented in
faircode/SPEC.md section 8.

Verified with the exact repro from the issue - the banded-vs-raw age
comparison now exits 0 (while still printing the informational flag),
and a genuine representation-drift case still exits 1. Cross-checked
Python vs. JS engine parity directly. Full compare/JS-parity/CLI/report
test suite (110 tests) passes.

Author: yakew7

assets/profiler-engine.js

@@ -947,16 +947,25 @@
     var scoreDelta = (resultA.overall_score === null || resultB.overall_score === null)
       ? null : resultB.overall_score - resultA.overall_score;
 
-    var flags = [];
+    // flags is every human-readable notice, including a kind-mismatch
+    // dimension's "drift comparison skipped" message - informational, since
+    // the comparison genuinely could not be measured. driftDetected is the
+    // narrower, structural signal of whether any *real* drift was measured -
+    // matches faircode/compare.py's _build_flags() so a CLI-equivalent
+    // consumer wouldn't false-positive on a skipped/unmeasurable comparison
+    // the way checking flags.length alone would (#472).
+    var flags = [], driftDetected = false;
     if (scoreDelta !== null && scoreDelta <= -SCORE_DROP_FLAG) {
       flags.push('overall representation score dropped ' + Math.abs(scoreDelta) +
                  ' points (' + resultA.overall_score + ' → ' + resultB.overall_score + ')');
+      driftDetected = true;
     }
     dimensions.forEach(function (cd) {
       if (Math.abs(cd.missing_pct_delta) >= MISSING_DRIFT_FLAG) {
         flags.push(cd.name + ': missing-data share shifted ' +
                    (cd.missing_pct_a * 100).toFixed(1) + '% → ' +
                    (cd.missing_pct_b * 100).toFixed(1) + '%');
+        driftDetected = true;
       }
       if (cd.kind_mismatch) {
         if (cd.kind_a !== cd.kind_b) {
@@ -973,21 +982,24 @@
       if (cd.drift_level !== 'none') {
         flags.push(cd.name + ': ' + cd.drift_level +
                    ' representation drift (PSI ' + cd.psi.toFixed(2) + ')');
+        driftDetected = true;
       }
       cd.groups.forEach(function (g) {
         if (g.status === 'appeared') {
           flags.push(cd.name + ": '" + g.label + "' appeared (" +
                      (g.share_a * 100).toFixed(1) + '% → ' +
                      (g.share_b * 100).toFixed(1) + '%)');
+          driftDetected = true;
         } else if (g.status === 'disappeared') {
           flags.push(cd.name + ": '" + g.label + "' disappeared (" +
                      (g.share_a * 100).toFixed(1) + '% → ' +
                      (g.share_b * 100).toFixed(1) + '%)');
+          driftDetected = true;
         }
       });
     });
-    added.forEach(function (n) { flags.push("dimension '" + n + "' is present only in " + nameB); });
-    removed.forEach(function (n) { flags.push("dimension '" + n + "' is present only in " + nameA); });
+    added.forEach(function (n) { flags.push("dimension '" + n + "' is present only in " + nameB); driftDetected = true; });
+    removed.forEach(function (n) { flags.push("dimension '" + n + "' is present only in " + nameA); driftDetected = true; });
 
     return {
       a: { name: nameA, n_rows: resultA.n_rows,
@@ -997,7 +1009,8 @@
            overall_score: resultB.overall_score, grade: resultB.grade,
            dimensions_detected: resultB.dimensions_detected, note: resultB.note },
       score_delta: scoreDelta, dimensions: dimensions,
-      added_dimensions: added, removed_dimensions: removed, flags: flags
+      added_dimensions: added, removed_dimensions: removed, flags: flags,
+      drift_detected: driftDetected
     };
   }
 

faircode/SPEC.md

@@ -267,7 +267,16 @@ Top level: `score_delta = overall_score_b − overall_score_a` when both scores
 `null` otherwise. `flags` is assembled from: an
 overall-score drop of `≥ SCORE_DROP_FLAG` points, every dimension whose `|missing_pct_delta| ≥
 MISSING_DRIFT_FLAG`, every dimension whose `drift_level ≠ none`, every `appeared`/`disappeared`
-group, and every added/removed dimension.
+group, every added/removed dimension, **and** every `kind_mismatch` dimension's own
+"drift comparison skipped" notice - that last case is informational only (the comparison genuinely
+couldn't be measured, not evidence of drift), so it's the one category of `flags` entry
+**excluded** from `drift_detected`.
+
+`drift_detected` is a boolean - `true` iff at least one *real* drift signal fired (any of the first
+five categories above), `false` if `flags` is empty or contains only kind-mismatch notices. The CLI's
+`compare --fail-on-drift` checks `drift_detected`, not `bool(flags)`, so a schema change between two
+snapshots that makes a dimension unmeasurable (e.g. one side's ages banded, the other left raw)
+doesn't false-positive as "drift detected" in a CI gate (see issue #472).
 
 ### Result shape
 
@@ -290,7 +299,8 @@ group, and every added/removed dimension.
   ],
   "added_dimensions": ["income_bracket"],
   "removed_dimensions": [],
-  "flags": [ "race: significant representation drift (PSI 0.34)", "race: 'Asian' disappeared (10.0% → 0.0%)" ]
+  "flags": [ "race: significant representation drift (PSI 0.34)", "race: 'Asian' disappeared (10.0% → 0.0%)" ],
+  "drift_detected": true
 }
 ```
 

faircode/cli.py

@@ -364,7 +364,7 @@ def main(argv: list[str] | None = None) -> int:
             print(to_json(result, provenance=provenance))
         else:
             print(compare_to_terminal(result))
-        if args.fail_on_drift and result["flags"]:
+        if args.fail_on_drift and result["drift_detected"]:
             print(
                 f"error: representation drift detected ({len(result['flags'])} flag(s)) "
                 f"with --fail-on-drift set",

faircode/compare.py

@@ -149,19 +149,32 @@ def _compare_dimension(dim_a: dict, dim_b: dict) -> dict:
 
 def _build_flags(result_a: dict, result_b: dict, score_delta: int | None,
                  dimensions: list, added: list, removed: list,
-                 name_a: str, name_b: str) -> list:
+                 name_a: str, name_b: str) -> tuple[list, bool]:
+    """Returns (flags, drift_detected). `flags` is every human-readable
+    notice, including a kind-mismatch dimension's "drift comparison
+    skipped" message - informational, since the underlying comparison
+    genuinely could not be measured (psi/drift_level are already zeroed
+    for exactly this dimension in _compare_dimension()). `drift_detected`
+    is the narrower, structural signal of whether any *real* representation
+    drift was measured - CLI's --fail-on-drift checks this instead of
+    `bool(flags)`, so a schema change that made a comparison unmeasurable
+    (e.g. one side's ages banded, the other raw) doesn't false-positive as
+    "drift detected" the way any flags existing at all would (#472)."""
     flags: list[str] = []
+    drift_detected = False
     if score_delta is not None and score_delta <= -SCORE_DROP_FLAG:
         flags.append(
             f"overall representation score dropped {abs(score_delta)} points "
             f"({result_a['overall_score']} → {result_b['overall_score']})"
         )
+        drift_detected = True
     for cd in dimensions:
         if abs(cd["missing_pct_delta"]) >= MISSING_DRIFT_FLAG:
             flags.append(
                 f"{cd['name']}: missing-data share shifted "
                 f"{cd['missing_pct_a'] * 100:.1f}% → {cd['missing_pct_b'] * 100:.1f}%"
             )
+            drift_detected = True
         if cd["kind_mismatch"]:
             if cd["kind_a"] != cd["kind_b"]:
                 flags.append(
@@ -181,22 +194,27 @@ def _build_flags(result_a: dict, result_b: dict, score_delta: int | None,
                 f"{cd['name']}: {cd['drift_level']} representation drift "
                 f"(PSI {cd['psi']:.2f})"
             )
+            drift_detected = True
         for g in cd["groups"]:
             if g["status"] == "appeared":
                 flags.append(
                     f"{cd['name']}: '{g['label']}' appeared "
                     f"({g['share_a'] * 100:.1f}% → {g['share_b'] * 100:.1f}%)"
                 )
+                drift_detected = True
             elif g["status"] == "disappeared":
                 flags.append(
                     f"{cd['name']}: '{g['label']}' disappeared "
                     f"({g['share_a'] * 100:.1f}% → {g['share_b'] * 100:.1f}%)"
                 )
+                drift_detected = True
     for n in added:
         flags.append(f"dimension '{n}' is present only in {name_b}")
+        drift_detected = True
     for n in removed:
         flags.append(f"dimension '{n}' is present only in {name_a}")
-    return flags
+        drift_detected = True
+    return flags, drift_detected
 
 
 def compare(result_a: dict, result_b: dict, name_a="A", name_b="B") -> dict:
@@ -211,8 +229,8 @@ def compare(result_a: dict, result_b: dict, name_a="A", name_b="B") -> dict:
     dimensions = [_compare_dimension(dims_a[n], dims_b[n]) for n in shared]
     scores = (result_a["overall_score"], result_b["overall_score"])
     score_delta = scores[1] - scores[0] if None not in scores else None
-    flags = _build_flags(result_a, result_b, score_delta, dimensions,
-                         added, removed, name_a, name_b)
+    flags, drift_detected = _build_flags(result_a, result_b, score_delta, dimensions,
+                                        added, removed, name_a, name_b)
 
     return {
         "a": {"name": name_a, "n_rows": result_a["n_rows"],
@@ -228,4 +246,5 @@ def compare(result_a: dict, result_b: dict, name_a="A", name_b="B") -> dict:
         "added_dimensions": added,
         "removed_dimensions": removed,
         "flags": flags,
+        "drift_detected": drift_detected,
     }