fix(report): choose the cascade threshold on one half and score it on the other

CHANGELOG.md

@@ -83,6 +83,13 @@ different event from one that moved because it was wrong.
 
 ### Fixed
 
+- The cascade's threshold was chosen and scored on the same rows, so the
+  coverage and cost it printed were its best case rather than what it would
+  do; with no temperature recommended it used every row and still called them
+  held-out (#30). It is now chosen on the fit half and reported on the
+  held-out half, and the 200-row minimum counts the held-out half, so a run
+  needs at least 400 scored rows for a threshold. **This changes which runs
+  print a threshold, and the cost and coverage printed with it.**
 - The cascade never considered escalating every case, because the highest
   observed score always kept the rows that reached it covered; with every
   case wrong and errors dear it chose a threshold costing $201 where

docs/PLAN.md

@@ -98,8 +98,11 @@ Settled during the build. Reopen one only with a reason, not from scratch.
   second, so a repeated name gets a suffix rather than replacing user records.
 - A refused recalibration prints no number: the verdict, the split sizes, and
   nothing that could be lifted into production code.
-- The cascade threshold is chosen on held-out rows, on the recalibrated scale
-  when a temperature was recommended, because a threshold set against a raw
+- The cascade threshold is chosen on the fit half of the rows and its cost and
+  coverage are reported on the held-out half, which neither the threshold nor
+  any temperature has seen; a threshold chosen and scored on the same rows
+  would report its best case. Both halves are on the recalibrated scale when a
+  temperature was recommended, because a threshold set against a raw
   overconfident probability sits in the wrong place.
 - Escalation cost and error cost are supplied by the caller and never defaulted.
   No benchmark can know them, and a made-up default would decide the threshold.

src/plumbline/report/markdown.py

@@ -348,92 +348,118 @@ def _cascade_section(
             "states where to set the threshold and what it buys.",
         ]
 
-    scored, scale = _cascade_rows(fit, successes, outcomes, semantics)
+    scored, scale = _cascade_rows(fit, successes, outcomes, semantics, options)
     if scored is None:
         return [*lines, "- Not reported. This arm reports no probability to threshold on."]
 
-    series, chosen_outcomes = scored
-    rows = len(chosen_outcomes)
+    (fit_series, fit_outcomes), (held_series, held_outcomes) = scored
+    chosen_on, rows = len(fit_outcomes), len(held_outcomes)
     if rows < options.min_threshold_rows:
         return [
             *lines,
-            f"- Not reported. A threshold picked on {rows} rows is fitted to noise; "
+            f"- Not reported. A threshold is chosen on one part of the rows and judged on "
+            f"rows it has not seen, and this arm leaves {rows} held-out rows; "
             f"{options.min_threshold_rows} held-out rows are the minimum before one is "
             "worth acting on. No threshold is given.",
         ]
 
     escalation = options.cost_escalation_usd
     error = options.cost_error_usd
     assert escalation is not None and error is not None
-    best = cascade.optimal_threshold(series, chosen_outcomes, escalation, error)
+    # Chosen on the fit rows, then scored on the held-out rows: a threshold
+    # picked and reported on the same rows is fitted to them, and its cost
+    # there is the best case rather than what it will do.
+    chosen = cascade.optimal_threshold(fit_series, fit_outcomes, escalation, error)
+    best = cascade.cascade_sweep(
+        held_series, held_outcomes, escalation, error, thresholds=[chosen.threshold]
+    )[0]
     all_escalated = rows * escalation
+    where = f"chosen on {chosen_on} rows and scored on {rows} held-out rows"
     caveat = (
         "- Escalated traffic is assumed to answer correctly, so this is the optimistic "
         "bound: whatever you escalate to has its own error rate and this number does not "
         "know it. The cheap arm's own per-case cost is excluded, because it is paid at "
         "every threshold and cannot move the optimum."
     )
-    if best.covered == 0:
+    if chosen.covered == 0:
         return [
             *lines,
-            f"- Escalating every case is cheapest: at these costs, keeping any of the {rows} "
-            f"rows on the cheap arm costs more in errors than escalating it. Expected cost "
-            f"${best.total_cost_usd:.2f} over {rows} rows "
+            f"- Escalating every case is cheapest ({where}): at these costs, keeping rows "
+            f"on the cheap arm costs more in errors than escalating them. Expected cost "
+            f"${best.total_cost_usd:.2f} over the {rows} held-out rows "
             f"(${best.cost_per_case_usd:.4f} per case).",
             caveat,
         ]
 
     return [
         *lines,
-        f"- At a threshold of {best.threshold:.3f} on the {scale} probability, "
+        f"- At a threshold of {chosen.threshold:.3f} on the {scale} probability ({where}), "
         f"{best.coverage * 100:.0f} percent of traffic stays on the cheap arm and the "
-        f"expected cost is ${best.total_cost_usd:.2f} over {rows} rows "
+        f"expected cost is ${best.total_cost_usd:.2f} over those {rows} rows "
         f"(${best.cost_per_case_usd:.4f} per case), versus ${all_escalated:.2f} if every "
         "case went to the expensive arm.",
-        f"- {best.escalated} of {rows} rows escalate. Of the {best.covered} covered rows, "
-        f"{best.errors_covered} are wrong, so covered accuracy is "
+        f"- {best.escalated} of {rows} held-out rows escalate. Of the {best.covered} covered "
+        f"rows, {best.errors_covered} are wrong, so covered accuracy is "
         f"{(best.accuracy_covered or 0.0):.3f}.",
         caveat,
     ]
 
 
+_Rows = tuple[ProbabilitySeries, list[bool]]
+
+
 def _cascade_rows(
     fit: _Fit,
     successes: Sequence[CaseRecord],
     outcomes: Sequence[bool],
     semantics: str,
-) -> tuple[tuple[ProbabilitySeries, list[bool]] | None, str]:
-    """The rows a threshold is chosen on, and what scale they are on.
-
-    A threshold set against a raw overconfident probability sits in the wrong
-    place, because 0.9 from an overconfident model is not 0.9. So when a
-    temperature was recommended, the threshold is chosen on the held-out rows
-    with that temperature applied: the same rows the temperature was judged
-    on, never the rows it was fitted on.
+    options: ReportOptions,
+) -> tuple[tuple[_Rows, _Rows] | None, str]:
+    """The rows a threshold is chosen on, the rows it is scored on, and their scale.
+
+    Always two disjoint halves: the recalibration split when there is one, or a
+    split made the same way when recalibration produced nothing. A threshold
+    set against a raw overconfident probability sits in the wrong place,
+    because 0.9 from an overconfident model is not 0.9, so when a temperature
+    was recommended both halves are on the recalibrated scale. The held-out
+    half is then unseen by the temperature and the threshold alike.
     """
     predictions = [record.prediction for record in successes if record.prediction]
     result = fit.result
+    split = (
+        result.split
+        if result is not None
+        else recalibration.make_split(len(predictions), options.fit_fraction, options.seed)
+    )
 
     if result is not None and result.temperature_to_use is not None:
         temperature = result.temperature_to_use
-        indices = result.split.eval_indices
-        values = tuple(_scaled(predictions[index], temperature) for index in indices)
-        chosen = [bool(outcomes[index]) for index in indices]
+
+        def value(index: int) -> float:
+            return _scaled(predictions[index], temperature)
+
+        scale = "recalibrated"
+    else:
+        if any(prediction.prob_selected is None for prediction in predictions):
+            return None, "raw"
+
+        def value(index: int) -> float:
+            probability = predictions[index].prob_selected
+            assert probability is not None
+            return probability
+
+        scale = "raw"
+
+    def half(indices: Sequence[int]) -> _Rows:
         return (
-            ProbabilitySeries(values=values, semantics=semantics),  # type: ignore[arg-type]
-            chosen,
-        ), "recalibrated"
-
-    raw = [prediction.prob_selected for prediction in predictions]
-    if any(value is None for value in raw):
-        return None, "raw"
-    return (
-        ProbabilitySeries(
-            values=tuple(value for value in raw if value is not None),
-            semantics=semantics,  # type: ignore[arg-type]
-        ),
-        [bool(outcome) for outcome in outcomes],
-    ), "raw"
+            ProbabilitySeries(
+                values=tuple(value(index) for index in indices),
+                semantics=semantics,  # type: ignore[arg-type]
+            ),
+            [bool(outcomes[index]) for index in indices],
+        )
+
+    return (half(split.fit_indices), half(split.eval_indices)), scale
 
 
 def _scaled(prediction: Prediction, temperature: float) -> float:

tests/test_report.py

@@ -382,6 +382,28 @@ def test_the_cascade_needs_the_two_numbers_no_benchmark_can_know() -> None:
     assert "stays on the cheap arm" not in body
 
 
+def test_without_a_temperature_the_threshold_is_still_scored_on_held_out_rows() -> None:
+    """A threshold chosen and scored on the same rows is in-sample, whatever it says (#30).
+
+    A calibrated arm gets no temperature, so there is no recalibrated scale; the
+    threshold must still be picked on one half and reported on the other, and
+    the 200-row minimum counts the held-out half. 300 rows leave 150 held out.
+    """
+    body = section(markdown.render([a_run(n_cases=300)], options=with_costs()), "Cascade")
+
+    assert "not reported" in body.lower()
+    assert "held-out" in body
+    assert "stays on the cheap arm" not in body
+
+
+def test_the_cascade_says_where_its_threshold_was_chosen_and_where_it_was_scored() -> None:
+    body = section(markdown.render([a_run(n_cases=600)], options=with_costs()), "Cascade")
+
+    assert "stays on the cheap arm" in body
+    assert "chosen on 300 rows" in body
+    assert "300 held-out rows" in body
+
+
 def test_a_threshold_is_refused_on_an_evaluation_set_too_small_to_support_one() -> None:
     body = section(markdown.render([a_run(n_cases=40)], options=with_costs()), "Cascade")