diff --git a/src/scribe/mcp/tools/search.py b/src/scribe/mcp/tools/search.py index 20cd999..bccd208 100644 --- a/src/scribe/mcp/tools/search.py +++ b/src/scribe/mcp/tools/search.py @@ -169,10 +169,12 @@ async def retrieval_telemetry(days: int = 30) -> dict: `avg_result_count` and `p90_duration_ms`. THE NUMBER TO READ FIRST IS `near_misses.p90`, AGAINST THE THRESHOLD IN - FORCE FOR THAT SURFACE. It is measured only on the calls that returned - NOTHING, on the best score the ranker reached before the bar rejected it — - so it is the one figure here that says something the bar cannot make true - by construction. A bar at 0.72 turning away a stream of 0.71s is set too + FORCE FOR THAT SURFACE. It is measured on the calls the BAR turned away — + zero-result calls, minus the ones whose zero was a repeat the reader had + already been shown — using the best score the ranker reached before the bar + rejected it. So it is the one figure here that says something the bar + cannot make true by construction, and `max` is always below the threshold: + an above-bar candidate nobody excluded would have been returned. A bar at 0.72 turning away a stream of 0.71s is set too high by a hair and the surface is losing hits it should have had. The same bar turning away 0.30s is working, and the corpus simply had nothing. Both render as a zero-result call, and nothing else in this readout tells them diff --git a/src/scribe/services/retrieval_telemetry.py b/src/scribe/services/retrieval_telemetry.py index 18e94df..577aac8 100644 --- a/src/scribe/services/retrieval_telemetry.py +++ b/src/scribe/services/retrieval_telemetry.py @@ -360,15 +360,45 @@ async def retrieval_summary(user_id: int | None, *, days: int = 30) -> dict: } zero = case((RetrievalLog.result_count == 0, 1), else_=0) - # THE NEAR-MISS POPULATION: calls that returned nothing AND recorded what - # the bar turned away. Both conditions matter. Restricting to zero-result - # calls is what makes the number say something the bar cannot fix by - # construction — on a call that returned something, `best_available_score` - # equals `top_score` and adds nothing. Requiring the column to be non-null - # keeps rows written before #3670 out of the sample rather than letting - # them read as scoreless declines. - declined = (RetrievalLog.result_count == 0) & ( - RetrievalLog.best_available_score.isnot(None) + # THE NEAR-MISS POPULATION: calls that returned nothing BECAUSE THE BAR + # TURNED SOMETHING AWAY, and recorded what it was. Three conditions, and + # the third was missing for one deploy (#3739). + # + # Zero-result only: on a call that returned something, + # `best_available_score` equals `top_score` and adds nothing. + # + # Non-null only: rows written before #3670 genuinely do not know, and must + # not read as scoreless declines. + # + # AND NOT A REPEAT. A zero-result call is two unrelated events — the ranker + # found nothing above the bar, or it found only what this session had + # already been shown — and just the first says anything about the bar. That + # is the whole of #3497, and #3670 reintroduced the conflation one level up: + # the rule arms filter exclusions in PYTHON, after the search, so a rule + # that cleared the bar and was dropped as a repeat still reported a high + # `best_available_score` on a zero-result row. Live proof, first read after + # deploy: pre_tool_rule's near-miss max was 0.7457 while the lowest score it + # ever RETURNED was 0.7204 — a "rejection" that outscored acceptances. + # + # The NULL arm is principled, not permissive: `suppressed_count IS NULL` + # means the caller passed its exclusions INTO the search, which is exactly + # the case where the reported score is already post-exclusion and cannot be + # contaminated. Note arms stay measured; rule arms get cleaned. + # + # Deliberately conservative: a call carrying both a repeat and a lower + # genuine miss is dropped whole, losing that point. It undercounts; it + # cannot corrupt — the right way round for a number read against a bar. + # + # This also makes `near_misses.max < threshold` true BY CONSTRUCTION. An + # above-bar candidate that was not excluded would have been returned, so + # its call is not in this population at all. + declined = ( + (RetrievalLog.result_count == 0) + & (RetrievalLog.best_available_score.isnot(None)) + & ( + RetrievalLog.suppressed_count.is_(None) + | (RetrievalLog.suppressed_count == 0) + ) ) miss = case((declined, 1), else_=0) # `best_available_score` only for those rows; NULL elsewhere, and diff --git a/tests/test_services_retrieval_telemetry.py b/tests/test_services_retrieval_telemetry.py index ff5b8fd..243744c 100644 --- a/tests/test_services_retrieval_telemetry.py +++ b/tests/test_services_retrieval_telemetry.py @@ -994,6 +994,17 @@ async def test_the_near_miss_distribution_is_a_query_postgres_accepts(_dispose_e user_id=UID, source="pre_tool_rule", query="ls", threshold=0.72, limit=1, project_id=None, is_task=None, results=[], duration_ms=4.0, )) + # THE CASE WHOSE ABSENCE LET THIS GUARD PASS OVER BROKEN CODE (#3739). + # A zero-result call whose zero was a REPEAT, not a rejection: the ranker + # cleared the bar at 0.9 and the session had already been shown that rule, + # so the arm dropped it in Python after the search. Without the suppression + # arm of the predicate this row lands in the near-miss population and drags + # `max` to 0.9 — above the very threshold the field is read against. + await _insert_retrieval_log(_build_payload( + user_id=UID, source="pre_tool_rule", query="git commit", threshold=0.72, + limit=1, project_id=None, is_task=None, results=[], duration_ms=4.0, + best_available=0.9, suppressed=1, + )) try: out = await retrieval_summary(UID, days=30) @@ -1002,22 +1013,28 @@ async def test_the_near_miss_distribution_is_a_query_postgres_accepts(_dispose_e "zeros everywhere, which is #2663 exactly" ) src = out["sources"]["pre_tool_rule"] - assert src["calls"] == 5 - assert src["zero_result_calls"] == 4 + assert src["calls"] == 6 + assert src["zero_result_calls"] == 5 nm = src["near_misses"] assert nm is not None, "the near-miss block did not survive the query" assert nm["measured_calls"] == 3, ( - "the population is declines that RECORDED a score: three measured, " - "one unmeasured (excluded, not counted as a scoreless decline), and " - "one call that showed something (excluded — its best-available is " - "just its top score and says nothing about the bar)" + "the population is declines the BAR caused, that recorded a score. " + "Three qualify. Excluded: the unmeasured row (predates the column, " + "not a scoreless decline), the call that showed something (its " + "best-available is just its top score), and the REPEAT — a zero " + "the reader caused, not the bar (#3739)" ) assert nm["max"] == pytest.approx(0.7189, abs=1e-4), ( "the closest thing the bar turned away — 0.7189 against a 0.72 " "threshold, which is the reading the whole field exists to give" ) - assert nm["max"] < 0.72, "a near miss that cleared the bar is not a miss" + assert nm["max"] < 0.72, ( + "a rejection that outscores the bar is not a rejection. This is " + "structural once the suppression arm is in the predicate: an " + "above-bar candidate that was not excluded would have been " + "RETURNED, so its call cannot be in this population (#3739)" + ) assert 0.70 <= nm["p50"] <= 0.7189 finally: async with async_session() as s: