write_path stops reporting records it withheld itself as near misses #146

Merged
bvandeusen merged 1 commits from dev into main 2026-09-09 00:44:44 -04:00
2 changed files with 112 additions and 2 deletions
+31 -2
View File
@@ -1008,7 +1008,34 @@ async def build_write_path_hint(
int(note.id): float(score) for score, note in hits int(note.id): float(score) for score, note in hits
if int(note.id) in pulled 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( record_retrieval(
user_id=user_id, source="write_path", query=query, user_id=user_id, source="write_path", query=query,
threshold=cfg["threshold"], limit=remaining, 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 # recording it as a notes-only retrieval would misdescribe the
# candidate set the threshold is being tuned against. # candidate set the threshold is being tuned against.
project_id=scope_project, is_task=None, results=hits, 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, duration_ms=(time.perf_counter() - t0) * 1000.0,
) )
if hits: if hits:
+81
View File
@@ -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 assert "(.rule_ids // [])[]?" in src # its own write-back
# And it rides the same request as the rest, not a second round trip. # And it rides the same request as the rest, not a second round trip.
assert "${rule_exclude_q}" in src 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"
)