write_path stops reporting records it withheld itself as near misses #146
@@ -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:
|
||||||
|
|||||||
@@ -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"
|
||||||
|
)
|
||||||
|
|||||||
Reference in New Issue
Block a user