fix(bench): enforce tolerance bands on the regression side only (#1160)

CHANGELOG/v4.md

@@ -41,6 +41,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **Four adversarial promotion cases sat marked as known failures after the rule started handling them ([#1162](https://github.com/robotrocketscience/aelfrice/issues/1162)).** `tests/fixtures/promotion_adversarial.json` marks a case `known_failure` and the suite runs it as `xfail`. Commit 39745247 fixed the all-stopword promotion path — pass 2 is skipped when the lock text normalizes to no tokens, so the empty/empty Jaccard 1.0 convention no longer reaches the promotion decision — which made C6-01..04 start passing. They reported `XPASS`, and nothing noticed: `XPASS` is a non-failing outcome under `strict=False`, so no gate reads it. The markers therefore described a defect the rule no longer had while suppressing four assertions that had begun working. The cases are now `regression_cases` with a rationale naming what closed them, the marker is `strict=True`, and `xfail_strict` is set as the pytest default so a future marker added without an explicit `strict=` inherits the loud behaviour. No other `xfail` exists in the suite today, so nothing else changes outcome — this removes the way back in. Tests assert both the per-marker strictness and the default, and that the two fixture buckets stay disjoint (moving a closed case is a two-step edit, and doing only half of it is now a red build rather than a silent one).
 - **The supersession exclusion arm silently returned an empty pack when retired beliefs dominated a query ([#1205](https://github.com/robotrocketscience/aelfrice/issues/1205)).** [#1187](https://github.com/robotrocketscience/aelfrice/issues/1187) fixed the arm shrinking the pack by widening the candidate fetch and retrying, but bounded that at three rounds — which made the bound the *normal* termination rather than a backstop. At `l1_limit=50` the rounds reach 200, so a query whose 200 strongest matches were all retired returned **nothing** while a hundred current, matching beliefs sat below the widest fetch attempted: 200/300 retired returned 0, 250/300 returned 0. The starvation was moved out 4x, not removed. It also truncated with no signal, so a short pack was indistinguishable from a store that genuinely could not fill one — the silent-cap shape [#1160](https://github.com/robotrocketscience/aelfrice/issues/1160) exists to remove, and a floor that binds only on retired-heavy queries biases the ratified demote-vs-exclude bench on exactly the corpus slice where supersession is most active. Widening now continues until the search is genuinely exhausted, with a cap high enough that reaching it means something pathological, and reaching it traces to stderr naming the limit and the survivor count. The arm still costs more queries than demote — with 2 of 300 retired at `l1_limit=50` it already widens once — which is recorded so the bench does not compare arm latency naively.
 - **`edge_rerank` silently dropped `ScoredHop`'s trail and federation scope ([#1207](https://github.com/robotrocketscience/aelfrice/issues/1207), under [#1162](https://github.com/robotrocketscience/aelfrice/issues/1162)).** `apply_edge_type_rerank` rebuilt each hop field by field. `ScoredHop` gained `belief_id_trail` (#658) and `owning_scope` (#690) six days after that module was written (#421), and both carry defaults — so the enumerated constructor kept type-checking while erasing them. A federated hop came out relabelled as local, which routes subsequent reads to the wrong database, and the #658 compound-confidence derivation lost the trail it consumes. It now copies the hop and overrides only `score`, so the next field added upstream arrives for free, and the test derives its expectation from `dataclasses.fields(ScoredHop)` rather than a hand-written list that would reproduce the same failure. The multiplicative penalty is **not** the inversion that made `uri_baki.apply_supersession_demote` unusable on the L1 rerank: `ScoredHop.score` starts at 1.0 and is multiplied by `BFS_EDGE_WEIGHTS` values in `(0, 1]`, so the domain is positive and halving genuinely demotes — checked rather than assumed from the resemblance. **Still unreachable:** nothing in `src/` imports this module, while its producer (`aelf doctor` writing `POTENTIALLY_STALE` edges, #387) is live and `BFS_EDGE_WEIGHTS` pins that type at 0.0 on the grounds that this pass demotes it — so nothing demotes a potentially-stale belief in production today. Wiring it or deleting it is an operator call left on #1207; this change makes it correct rather than correct-and-inert.
+- **Two-sided tolerance bands classified genuine benchmark improvements as regressions ([#1160](https://github.com/robotrocketscience/aelfrice/issues/1160)).** `compute_band` is symmetric, so a metric could bust its band by getting better. The canonical cut makes this unavoidable rather than hypothetical: `mab.Accurate_Retrieval.exact_match` is 0.0 and the absolute floor is +/-2 points, so *every* possible improvement lands outside the band, and LoCoMo category 5 is pinned at 0.0 the same way — the nightly would have reported a band-busting FAIL on a real fix to either. Bands are now enforced on the regression side only, via a per-metric direction table (`METRIC_DIRECTIONS`) keyed on the leaf metric name and falling back to the leaf's parent, because bucketed metrics key their leaves by bucket id — LoCoMo's per-category F1 lands at `category_f1.1`..`.5`, where the leaf name is `"1"` and only the parent says what is measured. Leaf wins on a tie, so `count.correct` reads as a score while `temporal-reasoning.count` reads as a corpus invariant. **Unclassified metrics stay two-sided**: a metric given the wrong direction goes blind to regressions in its real direction, which is strictly worse than the false failure this fixes, so a new metric fails loudly until someone classifies it. Leaving the band on the improving side is a WARN, not a silent pass — a large unexplained gain here is as likely to be an artifact as a win, since token-F1 over a retrieval blob rises when the token budget falls. Verified against the real canonical file: the two improvements above now roll up WARN, while a `substring_exact_match` drop, an `avg_latency_ms` rise, and a `total_questions` corpus shrink all still FAIL. Latency and ingest-time metrics invert (the rise is the regression); corpus-size metrics stay two-sided since a shrinking corpus is not an improvement.
 
 ## [4.2.0] - 2026-07-21
 

benchmarks/tolerance.py

@@ -10,9 +10,12 @@
   known-noisy metrics; overrides take precedence over defaults.
 - Soft warning: drift inside the band but >50% of the band width
   emits a notice without failing.
+- Direction (#1160): bands are enforced on the regression side only,
+  per `METRIC_DIRECTIONS`. Leaving the band on the improving side
+  WARNs instead of failing. Unclassified metrics stay two-sided.
 
 Spec: docs/design/v2_reproducibility_harness.md.
-Issue: #437.
+Issue: #437, #1160.
 """
 from __future__ import annotations
 
@@ -37,6 +40,106 @@
 ABSOLUTE_FLOOR = 0.02
 
 
+class Direction(str, Enum):
+    """Which side of the band is a regression, per #1160.
+
+    Two-sided bands treat any movement as a failure, so a real ranking
+    win registers as FAIL — canonical `mab.Accurate_Retrieval.
+    exact_match = 0.0` with the ±0.02 absolute floor means *every*
+    improvement lands outside the band. One-sided bands enforce only
+    the regression side.
+    """
+
+    HIGHER_IS_BETTER = "higher_is_better"
+    LOWER_IS_BETTER = "lower_is_better"
+    TWO_SIDED = "two_sided"
+
+
+# Per-metric direction, keyed by the leaf metric name and falling back
+# to the leaf's parent key (see `direction_for`).
+#
+# Unlisted metrics are TWO_SIDED on purpose. A metric given the *wrong*
+# direction goes blind to regressions in its real direction, which is
+# strictly worse than the false failure one-sided bands exist to fix,
+# so the default has to be the conservative one: a new metric fails
+# loudly as out-of-band until someone classifies it here. Do not
+# replace this with substring matching on the metric name — an oddly
+# named metric would then silently inherit the wrong direction.
+METRIC_DIRECTIONS: dict[str, Direction] = {
+    # --- quality: a drop is the regression -------------------------
+    "accuracy": Direction.HIGHER_IS_BETTER,
+    "accuracy_pct": Direction.HIGHER_IS_BETTER,
+    "category_f1": Direction.HIGHER_IS_BETTER,  # parent of "1".."5"
+    "correct": Direction.HIGHER_IS_BETTER,
+    "exact_match": Direction.HIGHER_IS_BETTER,
+    "exact_match_pct": Direction.HIGHER_IS_BETTER,
+    "f1": Direction.HIGHER_IS_BETTER,
+    "f1_pct": Direction.HIGHER_IS_BETTER,
+    "items_with_any_nonzero_metric": Direction.HIGHER_IS_BETTER,
+    "overall_f1": Direction.HIGHER_IS_BETTER,
+    "perfect_cases": Direction.HIGHER_IS_BETTER,
+    "score_pct": Direction.HIGHER_IS_BETTER,
+    "substring_exact_match": Direction.HIGHER_IS_BETTER,
+    "substring_exact_match_pct": Direction.HIGHER_IS_BETTER,
+    "total_correct": Direction.HIGHER_IS_BETTER,
+    # --- cost: a rise is the regression ----------------------------
+    "avg_latency_ms": Direction.LOWER_IS_BETTER,
+    "total_ingest_time_s": Direction.LOWER_IS_BETTER,
+    # --- corpus invariants: any drift is a defect ------------------
+    # These size the run rather than score it. A change means the
+    # corpus or the dispatcher moved, which invalidates the comparison
+    # in either direction.
+    "count": Direction.TWO_SIDED,
+    "domain_counts": Direction.TWO_SIDED,  # parent of the domain keys
+    "n": Direction.TWO_SIDED,
+    "n_runs": Direction.TWO_SIDED,
+    "total": Direction.TWO_SIDED,
+    "total_cases": Direction.TWO_SIDED,
+    "total_episodes": Direction.TWO_SIDED,
+    "total_ingest_turns": Direction.TWO_SIDED,
+    "total_qa": Direction.TWO_SIDED,
+    "total_queries": Direction.TWO_SIDED,
+    "total_questions": Direction.TWO_SIDED,
+    "type_counts": Direction.TWO_SIDED,  # parent of the type keys
+    # --- deliberately ambiguous ------------------------------------
+    # Retrieval volume is neither good nor bad on its own, and per
+    # #1160 it is what inflates token-F1: halving the budget doubles
+    # reported F1 while retrieving strictly less. Movement in either
+    # direction is worth a look, so both sides stay enforced.
+    "avg_beliefs": Direction.TWO_SIDED,
+    "avg_beliefs_per_query": Direction.TWO_SIDED,
+    # Hop-class summaries: `sem` may be a standard error (lower
+    # better) or a mean score (higher better), and `split_delta_pp` is
+    # a signed gap. Unresolved, so conservative.
+    "multi_hop_mean_sem_pct": Direction.TWO_SIDED,
+    "single_hop_mean_sem_pct": Direction.TWO_SIDED,
+    "split_delta_pp": Direction.TWO_SIDED,
+}
+
+
+def direction_for(path: tuple[str, ...]) -> Direction:
+    """Return the band direction for a leaf metric path.
+
+    Looks up the leaf name first, then its parent. The fallback exists
+    because bucketed metrics key their leaves by bucket id, not by
+    metric name: LoCoMo's per-category F1 lands at
+    ``...category_f1.1`` through ``.5``, so the leaf name is ``"1"``
+    and only the parent says what is being measured. Leaf wins on a
+    tie, so ``...count.correct`` resolves as ``correct`` (higher is
+    better) while ``...temporal-reasoning.count`` resolves as the
+    invariant ``count``.
+
+    Anything unresolved is TWO_SIDED — see `METRIC_DIRECTIONS`.
+    """
+    if not path:
+        return Direction.TWO_SIDED
+    if path[-1] in METRIC_DIRECTIONS:
+        return METRIC_DIRECTIONS[path[-1]]
+    if len(path) > 1 and path[-2] in METRIC_DIRECTIONS:
+        return METRIC_DIRECTIONS[path[-2]]
+    return Direction.TWO_SIDED
+
+
 class Verdict(str, Enum):
     PASS = "pass"
     WARN = "warn"
@@ -67,6 +170,10 @@ class BandCheck:
     band_kind: str          # "relative" | "absolute" | "override"
     verdict: Verdict
     note: str = ""
+    # #1160. Which side of the band was enforced. Defaulted so the
+    # SKIP/missing BandChecks, which never consult a direction, stay
+    # constructible without one.
+    direction: Direction = Direction.TWO_SIDED
 
 
 def _relative_band_pct(metric_name: str, overrides: dict[str, float]) -> float:
@@ -103,13 +210,39 @@ def compute_band(
 
 def classify(
     canonical: float, observed: float, lower: float, upper: float,
+    *, direction: Direction = Direction.TWO_SIDED,
 ) -> tuple[Verdict, str]:
-    """Map (observed) into pass/warn/fail given the band."""
-    if observed < lower or observed > upper:
-        return Verdict.FAIL, (
+    """Map (observed) into pass/warn/fail given the band.
+
+    `direction` (#1160) decides which side of the band is enforced.
+    The band itself is unchanged — both bounds are still computed and
+    reported — but leaving the band on the *improving* side is a WARN
+    rather than a FAIL.
+
+    It is not a silent PASS. This repo's own canonical numbers are the
+    argument: token-F1 over a retrieval blob rises when the token
+    budget falls, so a large unexplained gain is as likely to be a
+    measurement artifact as a win. WARN keeps it visible without
+    failing the nightly on a genuine improvement.
+
+    Defaults to TWO_SIDED so existing positional callers are unchanged.
+    """
+    below, above = observed < lower, observed > upper
+    if below or above:
+        regressed = (
+            (direction is not Direction.LOWER_IS_BETTER) if below
+            else (direction is not Direction.HIGHER_IS_BETTER)
+        )
+        detail = (
             f"observed {observed:.4f} outside band "
             f"[{lower:.4f}, {upper:.4f}]"
         )
+        if regressed:
+            return Verdict.FAIL, detail
+        return Verdict.WARN, (
+            f"{detail} on the improving side ({direction.value}); "
+            f"not a regression — confirm it is real before recutting"
+        )
     half = (upper - lower) / 2.0
     if half == 0:
         return Verdict.PASS, "zero-width band; exact match required"
@@ -238,11 +371,14 @@ def check_report(
             metric_name, cano_val,
             overrides=metric_overrides, floor=floor,
         )
-        verdict, note = classify(cano_val, obs_val, lower, upper)
+        direction = direction_for(path)
+        verdict, note = classify(
+            cano_val, obs_val, lower, upper, direction=direction,
+        )
         checks.append(BandCheck(
             path=path, canonical=cano_val, observed=obs_val,
             lower=lower, upper=upper, band_kind=kind,
-            verdict=verdict, note=note,
+            verdict=verdict, note=note, direction=direction,
         ))
     return checks
 

docs/design/v2_reproducibility_harness.md

@@ -108,14 +108,15 @@ Each metric carries `lower` and `upper` bounds. Default policy:
 - **Relative band:** ±X% of the canonical value, where X is per-metric (defaults: F1 ±7%, exact-match ±10%, latency ±25%).
 - **Absolute floor:** the band never falls below ±2 percentage points (prevents tiny-value flapping; e.g. 0.5% → 0.55% is below numeric noise but +10% relative).
 - **Per-metric override:** the canonical JSON can declare wider bands for known-noisy metrics (LLM-judge runs, anything with a non-deterministic ranker tie-break).
+- **Direction (amended by [#1160](https://github.com/robotrocketscience/aelfrice/issues/1160)):** both bounds are still computed and reported, but only the *regression* side fails. Leaving the band on the improving side is a WARN. Symmetric bands made real wins into failures — canonical `mab.Accurate_Retrieval.exact_match = 0.0` with the ±2-point floor put every possible improvement outside its band, and LoCoMo category 5 was pinned at 0.0 the same way, so fixing it would have registered as a band-busting regression. WARN rather than silent PASS because a large unexplained gain in this harness is as likely to be an artifact as a win: token-F1 over a retrieval blob rises when the token budget falls. Direction is a per-metric table (`benchmarks/tolerance.py: METRIC_DIRECTIONS`), keyed on the leaf metric name and falling back to its parent for bucketed metrics; **anything unclassified stays two-sided**, because a metric given the wrong direction goes blind to regressions in its real direction — worse than the false failure this fixes.
 
 The bands are calibrated on the first canonical run by running it ≥3 times and taking the observed range × 1.5. This is the **calibration-pass** the issue body alludes to and is documented as a one-time operation in the harness README. Re-calibration is a deliberate operator action, not automatic on drift.
 
 ### CI shape — two-tier
 
 **Nightly cron (`replay-soak-gate.yml`-style new workflow):**
 - Runs `aelf bench all`, writes `benchmarks/results/v2.0.0-cron-<date>.json`.
-- Compares against `v2.0.0.json` per-metric; any value outside its `tolerance_band` is a CI **fail** (issue auto-opened).
+- Compares against `v2.0.0.json` per-metric; any value outside its `tolerance_band` **on the regression side** is a CI **fail** (issue auto-opened). Outside on the improving side is a WARN — see the direction bullet above.
 - Soft warnings (drift inside the band but >50% of the band width) emit a workflow notice but pass.
 - Commits the cron JSON to a `benchmark-results` branch on PR for diffability; never pushes to main.