Merge pull request 'write_path stops reporting records it withheld itself as near misses' (#146) from dev into main

src/scribe/services/plugin_context.py

@@ -1008,7 +1008,34 @@ async def build_write_path_hint(
             int(note.id): float(score) for score, note in hits
             if int(note.id) in pulled
         }
-        hits = [(s, n) for s, n in hits if int(n.id) not in seen][:remaining]
+        shown = [(s, n) for s, n in hits if int(n.id) not in seen]
+        # WHAT THIS ARM WITHHELD AFTER THE SEARCH ANSWERED, and the reason
+        # `best_available_score` cannot always be reported here (#3739 again,
+        # from the side its fix did not reach).
+        #
+        # This arm is the one note arm that filters TWICE. `exclude_ids` takes
+        # `seen - pulled_seen` into the search, but the pulled-and-seen ids stay
+        # in the query deliberately — `resembles` above needs them — and are
+        # dropped in the line above instead. So the score the search reported is
+        # PRE that drop while the row's `result_count` is POST it, and a record
+        # the session had already been shown could be logged as something the
+        # BAR turned away. Live proof on the first read after #3739 shipped:
+        # write_path's near-miss max was 0.822 while the lowest score it ever
+        # RETURNED was 0.6857 — a "rejection" that beat every acceptance.
+        #
+        # The suppression column cannot rescue it the way it does for the rule
+        # arms: this arm's count would be PARTIAL, covering only the drops made
+        # here and not the ones `exclude_ids` made inside the search, and a
+        # partial number under a name that reads as complete is the substitution
+        # this whole milestone exists to stop.
+        #
+        # So the honest answer is null — "not measured on this call" — whenever
+        # this filter removed anything, because then the bar is not the only
+        # thing that turned something away and the reported score may belong to
+        # a record we withheld ourselves. Calls where nothing was dropped keep
+        # reporting it, which is most of them.
+        withheld_here = len(hits) - len(shown)
+        hits = shown[:remaining]
         record_retrieval(
             user_id=user_id, source="write_path", query=query,
             threshold=cfg["threshold"], limit=remaining,
@@ -1016,7 +1043,9 @@ async def build_write_path_hint(
             # recording it as a notes-only retrieval would misdescribe the
             # candidate set the threshold is being tuned against.
             project_id=scope_project, is_task=None, results=hits,
-            best_available=_rep_wp.get("best_available_score"),
+            best_available=(
+                None if withheld_here else _rep_wp.get("best_available_score")
+            ),
             duration_ms=(time.perf_counter() - t0) * 1000.0,
         )
         if hits:

tests/test_write_path_trigger.py

@@ -1526,3 +1526,84 @@ def test_hook_keeps_the_rule_channel_apart_from_the_other_three():
     assert "(.rule_ids // [])[]?" in src            # its own write-back
     # And it rides the same request as the rest, not a second round trip.
     assert "${rule_exclude_q}" in src
+
+
+# ── `best_available` describes the BAR, not this arm's own second filter ──────
+#
+# #3739 fixed the rule arms: a record the reader had already been shown was
+# being logged as something the ranker turned away, which made the near-miss
+# distribution report scores ABOVE the very threshold it is read against.
+#
+# The fix keyed on `suppressed_count`, and its NULL branch was justified by
+# "null means the caller passed its exclusions INTO the search, so the reported
+# score is already post-exclusion". That is true of auto_inject and reuse_slot.
+# It is NOT true here: this is the one note arm that filters twice. `exclude_ids`
+# takes `seen - pulled_seen` into the search, but the pulled-and-seen ids stay in
+# the query on purpose (the arm's query doubles as the resemblance test) and are
+# dropped afterwards in Python.
+#
+# Live proof, on the first read after that fix shipped: write_path's near-miss
+# max was 0.822 while the lowest score it ever RETURNED was 0.6857 — a
+# "rejection" that beat every acceptance.
+
+
+def _search_reporting(score, note):
+    """A stand-in search that fills `report` the way the real one does.
+
+    It ignores `exclude_ids`, which is exactly the condition being reproduced:
+    a record that is in `seen` comes back from the search anyway. In production
+    that happens because `pulled_seen` is deliberately left in the query; here
+    it needs no ledger, and the arm's handling is the same either way.
+    """
+    async def _search(uid, q, **kw):
+        report = kw.get("report")
+        if report is not None:
+            report["best_available_score"] = score
+        return [(score, note)]
+    return _search
+
+
+async def _write_path_row(rec, **kwargs):
+    from scribe.services import plugin_context as pc
+    with patch.object(pc, "get_writepath_config", AsyncMock(return_value=_cfg())), \
+         patch.object(pc.snippets_svc, "list_snippets", AsyncMock(return_value=([], 0))), \
+         patch.object(pc, "record_retrieval", rec), \
+         patch.object(pc, "owner_names_for", AsyncMock(return_value={})), \
+         patch.object(pc, "semantic_search_notes",
+                      _search_reporting(0.9, fake_note(
+                          id=7, title="scored", user_id=1, note_type="snippet"))):
+        await pc.build_write_path_hint(1, "src/x.py", code=REAL_CODE, **kwargs)
+    return next(c for c in rec.call_args_list
+                if c.kwargs["source"] == "write_path")
+
+
+@pytest.mark.asyncio
+async def test_a_record_this_arm_withheld_itself_is_not_a_near_miss():
+    """The defect: the row's count is POST this arm's filter and the score was
+    captured PRE it, so a withheld record is indistinguishable from one the bar
+    rejected — while scoring higher than anything the bar ever let through."""
+    rec = MagicMock()
+    row = await _write_path_row(rec, exclude_ids=[7])
+
+    assert row.kwargs["results"] == [], "the hit was withheld, so nothing shown"
+    assert row.kwargs["best_available"] is None, (
+        "a 0.9 record this arm withheld itself was reported as the best thing "
+        "the THRESHOLD turned away. It would read as a bar set far too high "
+        "when the bar never rejected it at all (#3739)"
+    )
+
+
+@pytest.mark.asyncio
+async def test_a_call_that_withheld_nothing_still_reports_what_the_bar_refused():
+    """The other half, and what stops the fix being 'never report it'.
+
+    Without this, setting `best_available=None` unconditionally passes the test
+    above while deleting the measurement #3670 was built for.
+    """
+    rec = MagicMock()
+    row = await _write_path_row(rec)
+
+    assert row.kwargs["best_available"] == 0.9, (
+        "nothing was withheld here, so the reported score describes the bar "
+        "and must survive"
+    )