Design-system delivery, the contrast fix, recall scoping, and backup coverage #93
@@ -14,7 +14,9 @@ import logging
|
||||
import math
|
||||
import os
|
||||
|
||||
from sqlalchemy import delete, select
|
||||
from collections.abc import Sequence
|
||||
|
||||
from sqlalchemy import delete, or_, select
|
||||
|
||||
from scribe.models import async_session
|
||||
from scribe.models.embedding import NoteEmbedding
|
||||
@@ -114,7 +116,8 @@ async def semantic_search_notes(
|
||||
threshold: float = _SIMILARITY_THRESHOLD,
|
||||
project_id: int | None = None,
|
||||
is_task: bool | None = None,
|
||||
note_type: str | None = None,
|
||||
note_type: str | Sequence[str] | None = None,
|
||||
task_kind: str | Sequence[str] | None = None,
|
||||
orphan_only: bool = False,
|
||||
scope: str = "own",
|
||||
) -> list[tuple[float, Note]]:
|
||||
@@ -123,8 +126,15 @@ async def semantic_search_notes(
|
||||
Scores are cosine similarities in [-1, 1]; only notes at or above
|
||||
*threshold* are returned, sorted highest-first.
|
||||
|
||||
`note_type` narrows to a single record kind (e.g. "snippet"), for callers
|
||||
that want prior art rather than everything embedded.
|
||||
`note_type` narrows to a record kind, or several (e.g. "snippet", or
|
||||
("snippet", "note")), for callers that want prior art rather than everything
|
||||
embedded.
|
||||
|
||||
`task_kind` restricts TASKS to the given kinds while leaving non-task notes
|
||||
untouched. That asymmetry is the point: "recorded experience" is issues plus
|
||||
dev-logs, and those differ on `is_task`, so neither `note_type` nor `is_task`
|
||||
alone can express it. With `note_type="note", task_kind="issue"` a caller
|
||||
gets fixed problems and durable notes without the open to-do list.
|
||||
|
||||
`scope` ("own" | "browse" | "read", see access.notes_visibility_clause)
|
||||
decides how far this may see. It exists because this one function serves
|
||||
@@ -179,11 +189,22 @@ async def semantic_search_notes(
|
||||
stmt = stmt.where(Note.status.isnot(None))
|
||||
elif is_task is False:
|
||||
stmt = stmt.where(Note.status.is_(None))
|
||||
# Narrow to one kind of record. Composes with is_task rather than
|
||||
# replacing it — 'snippet' is a non-task note_type, so a caller asking
|
||||
# for prior art gets snippets and not the dev-log that mentions them.
|
||||
# Narrow to one kind of record, or several. Composes with is_task
|
||||
# rather than replacing it — 'snippet' is a non-task note_type, so a
|
||||
# caller asking for prior art gets snippets and not the dev-log that
|
||||
# mentions them.
|
||||
if note_type:
|
||||
stmt = stmt.where(Note.note_type == note_type)
|
||||
kinds = [note_type] if isinstance(note_type, str) else list(note_type)
|
||||
stmt = stmt.where(Note.note_type.in_(kinds))
|
||||
# Restrict TASKS to certain kinds while leaving notes alone. A note
|
||||
# has no task_kind that means anything, so a plain `.in_()` would
|
||||
# drop every dev-log — which is exactly the record a caller asking
|
||||
# for prior experience wants most.
|
||||
if task_kind:
|
||||
tkinds = [task_kind] if isinstance(task_kind, str) else list(task_kind)
|
||||
stmt = stmt.where(
|
||||
or_(Note.status.is_(None), Note.task_kind.in_(tkinds))
|
||||
)
|
||||
if exclude_ids:
|
||||
stmt = stmt.where(NoteEmbedding.note_id.notin_(exclude_ids))
|
||||
stmt = stmt.where(distance <= max_distance).order_by(distance.asc()).limit(limit)
|
||||
|
||||
@@ -290,6 +290,65 @@ def _record_kind(note) -> str:
|
||||
return note.note_type or "note"
|
||||
|
||||
|
||||
_REUSE_KINDS = ("snippet", "process")
|
||||
|
||||
|
||||
async def _reserve_slot_for_reuse(
|
||||
user_id: int,
|
||||
query: str,
|
||||
kept: list,
|
||||
cfg: dict,
|
||||
*,
|
||||
project_id: int | None,
|
||||
exclude_ids: set[int],
|
||||
) -> list:
|
||||
"""Guarantee the reuse-shaped kinds one slot, if one clears threshold (#2246).
|
||||
|
||||
Ranking by raw cosine is blind to what KIND of record answers what kind of
|
||||
ask, and the corpus makes that fatal rather than merely imperfect: Scribe's
|
||||
project records are *about software work*, so a task titled "surface snippets
|
||||
before the agent writes code" is a near-perfect lexical match for "write a
|
||||
function…" while being useless as an answer to it. Measured live, a prompt
|
||||
asking for a helper returned three records about BUILDING the retrieval
|
||||
system and zero snippets.
|
||||
|
||||
The bias is structural and gets WORSE as the project record grows — which is
|
||||
the direction Scribe is supposed to grow. Snippets are ~0.5% of the corpus
|
||||
here; no threshold tuning fixes a 200:1 ratio.
|
||||
|
||||
So the reserved hit is deliberately NOT held to the margin band. The band
|
||||
measures distance from the top overall score, and that top score is the very
|
||||
thing snippets lose to. It still has to clear the configured threshold, so a
|
||||
weak snippet cannot buy the slot — silence stays the default.
|
||||
"""
|
||||
if any(_record_kind(n) in _REUSE_KINDS for _s, n in kept):
|
||||
return kept # reuse already represented; nothing to do
|
||||
|
||||
top_k = cfg["top_k"]
|
||||
reuse = await semantic_search_notes(
|
||||
user_id, query,
|
||||
limit=1,
|
||||
threshold=cfg["threshold"],
|
||||
project_id=project_id,
|
||||
exclude_ids=exclude_ids | {int(n.id) for _s, n in kept},
|
||||
note_type=_REUSE_KINDS,
|
||||
scope="browse",
|
||||
)
|
||||
# Belt and braces on top of exclude_ids: a record already on the menu must
|
||||
# never appear twice, and one slot is all this is entitled to.
|
||||
kept_ids = {int(n.id) for _s, n in kept}
|
||||
fresh = [(s, n) for s, n in reuse if int(n.id) not in kept_ids][:1]
|
||||
if not fresh:
|
||||
return kept
|
||||
|
||||
# Take the LAST slot, never the first: the strongest overall hit is still the
|
||||
# best answer to the prompt, and displacing it would trade one blindness for
|
||||
# another.
|
||||
if len(kept) >= top_k:
|
||||
return kept[:top_k - 1] + fresh
|
||||
return (kept + fresh)[:top_k]
|
||||
|
||||
|
||||
async def build_autoinject_hint(
|
||||
user_id: int,
|
||||
query: str,
|
||||
@@ -341,6 +400,10 @@ async def build_autoinject_hint(
|
||||
# Margin gate: keep only hits close to the strongest one.
|
||||
top_score = hits[0][0]
|
||||
kept = [(s, n) for s, n in hits if s >= top_score - _AUTOINJECT_BAND]
|
||||
kept = await _reserve_slot_for_reuse(
|
||||
user_id, q, kept, cfg, project_id=(project_id or None),
|
||||
exclude_ids=set(exclude_ids or []),
|
||||
)
|
||||
|
||||
# A collaborator's note can reach this menu via a shared project, and the
|
||||
# operator never asked for it — so say whose it is. Unattributed, it reads as
|
||||
@@ -685,7 +748,20 @@ async def build_write_path_hint(
|
||||
threshold=cfg["threshold"],
|
||||
project_id=scope_project,
|
||||
exclude_ids=seen,
|
||||
note_type="snippet",
|
||||
# Snippets AND recorded experience (#2246). This arm was
|
||||
# snippets-only, which is auto-inject's mistake inverted: an issue
|
||||
# saying "we tried this and it deadlocked", or a dev-log recording
|
||||
# how a problem was solved, is prior art for the code about to be
|
||||
# written — arguably better prior art than a resembling helper,
|
||||
# because it says what NOT to do.
|
||||
#
|
||||
# `task_kind="issue"` keeps the open to-do list out. A task titled
|
||||
# "add debouncing to the search box" resembles the code being
|
||||
# written and answers nothing; an ISSUE is corrective work with a
|
||||
# root cause in it, and a non-task note is durable knowledge. Both
|
||||
# earned their place; a todo did not.
|
||||
note_type=("snippet", "note"),
|
||||
task_kind="issue",
|
||||
# Same reasoning as auto-inject: nobody asked for this, so it takes
|
||||
# the browse scope and never surfaces a one-to-one direct share.
|
||||
scope="browse",
|
||||
@@ -693,7 +769,10 @@ async def build_write_path_hint(
|
||||
record_retrieval(
|
||||
user_id=user_id, source="write_path", query=query,
|
||||
threshold=cfg["threshold"], limit=remaining,
|
||||
project_id=scope_project, is_task=False, results=hits,
|
||||
# is_task is None, not False: this arm now returns issues too, and
|
||||
# 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,
|
||||
duration_ms=(time.perf_counter() - t0) * 1000.0,
|
||||
)
|
||||
if hits:
|
||||
@@ -701,8 +780,15 @@ async def build_write_path_hint(
|
||||
for score, note in hits:
|
||||
if score < top_score - _AUTOINJECT_BAND:
|
||||
continue
|
||||
# Name the kind unless it's a snippet — the menu's default and
|
||||
# the header's default reading. An issue or a dev-log offered
|
||||
# here is a different KIND of claim ("this was already tried")
|
||||
# and an unlabelled line would be read as "here is code to
|
||||
# reuse", which is the opposite of what it says.
|
||||
kind = _record_kind(note)
|
||||
scored.append((
|
||||
f"similar {score:.2f}",
|
||||
f"similar {score:.2f}" if kind == "snippet"
|
||||
else f"similar {score:.2f} · {kind}",
|
||||
{
|
||||
"id": int(note.id), "title": note.title, "user_id": note.user_id,
|
||||
# Carried so the line can disclose a cross-language hit
|
||||
@@ -733,7 +819,9 @@ async def build_write_path_hint(
|
||||
|
||||
lines = [
|
||||
f"> Prior art already recorded in Scribe for `{path}` — open one with "
|
||||
"`get_snippet(id)` and reuse it rather than writing a fresh one-off "
|
||||
"`get_snippet(id)` for a snippet, `get_task(id)` for an issue, "
|
||||
"`get_note(id)` otherwise. Reuse a snippet rather than writing a fresh "
|
||||
"one-off; read an issue before repeating what it records "
|
||||
"(titles only; shown once per session):",
|
||||
]
|
||||
# Say what a language tag MEANS, and only when one is actually on the menu.
|
||||
|
||||
@@ -295,3 +295,174 @@ async def test_build_session_context_caps_length():
|
||||
|
||||
assert len(out["context"]) <= 9000 + 60 # cap + truncation note
|
||||
assert "truncated" in out["context"]
|
||||
|
||||
|
||||
# --- the reuse slot (#2246) --------------------------------------------------
|
||||
|
||||
_CFG = {"enabled": True, "threshold": 0.55, "top_k": 3}
|
||||
|
||||
|
||||
async def _autoinject(main_hits, reuse_hits, cfg=None):
|
||||
"""Run build_autoinject_hint with the two semantic queries stubbed in order:
|
||||
the unscoped pool first, then the reserved reuse query."""
|
||||
calls: list[dict] = []
|
||||
|
||||
async def fake_search(*_a, **kw):
|
||||
calls.append(kw)
|
||||
return reuse_hits if kw.get("note_type") else main_hits
|
||||
|
||||
with patch("scribe.services.plugin_context.get_autoinject_config",
|
||||
AsyncMock(return_value=dict(cfg or _CFG))), \
|
||||
patch("scribe.services.plugin_context.semantic_search_notes",
|
||||
AsyncMock(side_effect=fake_search)), \
|
||||
patch("scribe.services.plugin_context.record_retrieval", MagicMock()), \
|
||||
patch("scribe.services.plugin_context.record_surfaced", MagicMock()), \
|
||||
patch("scribe.services.plugin_context.owner_names_for",
|
||||
AsyncMock(return_value={})):
|
||||
from scribe.services.plugin_context import build_autoinject_hint
|
||||
out = await build_autoinject_hint(1, "write a debounce helper")
|
||||
return out, calls
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_snippet_takes_the_last_slot_when_none_won_on_score():
|
||||
"""The measured failure: a prompt asking for a helper returned three project
|
||||
records ABOUT building the retrieval system, and zero snippets. Scribe's own
|
||||
records are about software work, so they share vocabulary with any coding
|
||||
prompt while answering none of them."""
|
||||
main = [(0.66, _note(1, "Step 3: title-first auto-inject")),
|
||||
(0.65, _note(2, "Task-reminder dedup query crashes", is_task=True)),
|
||||
(0.64, _note(3, "Drafter hardening · write-path trigger", is_task=True))]
|
||||
reuse = [(0.58, _note(9, "debounce — collapse rapid calls", note_type="snippet"))]
|
||||
|
||||
out, calls = await _autoinject(main, reuse)
|
||||
|
||||
assert out["note_ids"] == [1, 2, 9] # last slot displaced, not the first
|
||||
assert "#9" in out["context"] and "[snippet]" in out["context"]
|
||||
# The reserved query is scoped to the reuse kinds and asks for exactly one.
|
||||
assert calls[1]["note_type"] == ("snippet", "process")
|
||||
assert calls[1]["limit"] == 1
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_the_reserved_query_is_skipped_when_a_snippet_already_won():
|
||||
"""No second query, and no slot spent twice, when ranking already did the
|
||||
right thing — the fix must be invisible in the case it isn't needed."""
|
||||
main = [(0.81, _note(9, "debounce helper", note_type="snippet")),
|
||||
(0.80, _note(1, "some task", is_task=True))]
|
||||
|
||||
out, calls = await _autoinject(main, [])
|
||||
|
||||
assert len(calls) == 1 # reserved query never ran
|
||||
assert out["note_ids"] == [9, 1]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_weak_snippet_does_not_buy_the_slot():
|
||||
"""The reserved hit skips the MARGIN band — that band is what snippets lose
|
||||
to — but never the threshold. Silence stays the default; a slot spent on an
|
||||
irrelevant snippet is how a menu teaches people to ignore it."""
|
||||
main = [(0.66, _note(1, "a task", is_task=True))]
|
||||
|
||||
out, calls = await _autoinject(main, []) # threshold returned nothing
|
||||
|
||||
assert out["note_ids"] == [1]
|
||||
assert len(calls) == 2 # it asked, and got nothing
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_the_reserved_hit_is_not_held_to_the_margin_band():
|
||||
"""0.58 is 0.08 below the top hit. Under the band it would survive; the point
|
||||
is that it must survive even when it wouldn't — a snippet losing to a
|
||||
same-vocabulary project record by a wide margin is the whole bug."""
|
||||
main = [(0.90, _note(1, "a task", is_task=True))]
|
||||
reuse = [(0.58, _note(9, "debounce", note_type="snippet"))]
|
||||
|
||||
out, _calls = await _autoinject(main, reuse)
|
||||
|
||||
assert 9 in out["note_ids"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_process_counts_as_reuse_too():
|
||||
"""A stored process answers 'how do we do X here' the same way a snippet
|
||||
answers 'what do we already have' — both lose to the same project records."""
|
||||
main = [(0.70, _note(1, "a task", is_task=True))]
|
||||
reuse = [(0.60, _note(8, "DRY pass process", note_type="process"))]
|
||||
|
||||
out, _ = await _autoinject(main, reuse)
|
||||
assert 8 in out["note_ids"]
|
||||
|
||||
# …and one already on the menu suppresses the reserved query.
|
||||
out2, calls2 = await _autoinject(
|
||||
[(0.70, _note(8, "DRY pass process", note_type="process"))], [])
|
||||
assert len(calls2) == 1
|
||||
|
||||
|
||||
# --- write-path widened beyond snippets (#2246, the mirror half) -------------
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_write_path_semantic_arm_asks_for_experience_not_just_snippets():
|
||||
"""The inverse of auto-inject's mistake. This arm was snippets-only, so an
|
||||
issue saying "we tried this and it deadlocked" could never reach the moment
|
||||
that code was about to be written — arguably the better prior art, because
|
||||
it says what NOT to do."""
|
||||
from scribe.services import plugin_context as pc
|
||||
|
||||
hits = [(0.72, _note(9, "debounce helper", note_type="snippet")),
|
||||
(0.70, _note(7, "Debounce dropped the trailing call", is_task=True,
|
||||
task_kind="issue"))]
|
||||
search = AsyncMock(return_value=hits)
|
||||
rec = MagicMock()
|
||||
with patch.object(pc, "get_writepath_config",
|
||||
AsyncMock(return_value={"enabled": True, "threshold": 0.6,
|
||||
"top_k": 3})), \
|
||||
patch.object(pc.snippets_svc, "list_snippets",
|
||||
AsyncMock(return_value=([], 0))), \
|
||||
patch.object(pc, "semantic_search_notes", search), \
|
||||
patch.object(pc, "record_retrieval", rec), \
|
||||
patch.object(pc, "record_surfaced", MagicMock()), \
|
||||
patch.object(pc, "owner_names_for", AsyncMock(return_value={})), \
|
||||
patch.object(pc, "concept_query", MagicMock(return_value="debounce a callback")):
|
||||
out = await pc.build_write_path_hint(
|
||||
1, "src/utils/debounce.ts", code="x" * 400,
|
||||
)
|
||||
|
||||
kw = search.await_args.kwargs
|
||||
assert kw["note_type"] == ("snippet", "note")
|
||||
# An open to-do resembling the code answers nothing; an ISSUE carries a root
|
||||
# cause and a NOTE carries durable knowledge. Only the todo is excluded.
|
||||
assert kw["task_kind"] == "issue"
|
||||
# Telemetry must not claim this was a notes-only retrieval any more.
|
||||
assert rec.call_args.kwargs["is_task"] is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_write_path_labels_a_non_snippet_hit_with_its_kind():
|
||||
"""An unlabelled issue on this menu reads as "here is code to reuse", which
|
||||
is the opposite of what it says. A snippet stays unlabelled — it is the
|
||||
menu's default and the header's default reading."""
|
||||
from scribe.services import plugin_context as pc
|
||||
|
||||
hits = [(0.72, _note(9, "debounce helper", note_type="snippet")),
|
||||
(0.71, _note(7, "Debounce dropped the trailing call", is_task=True,
|
||||
task_kind="issue"))]
|
||||
with patch.object(pc, "get_writepath_config",
|
||||
AsyncMock(return_value={"enabled": True, "threshold": 0.6,
|
||||
"top_k": 3})), \
|
||||
patch.object(pc.snippets_svc, "list_snippets",
|
||||
AsyncMock(return_value=([], 0))), \
|
||||
patch.object(pc, "semantic_search_notes", AsyncMock(return_value=hits)), \
|
||||
patch.object(pc, "record_retrieval", MagicMock()), \
|
||||
patch.object(pc, "record_surfaced", MagicMock()), \
|
||||
patch.object(pc, "owner_names_for", AsyncMock(return_value={})), \
|
||||
patch.object(pc, "concept_query", MagicMock(return_value="debounce a callback")):
|
||||
out = await pc.build_write_path_hint(
|
||||
1, "src/utils/debounce.ts", code="x" * 400,
|
||||
)
|
||||
|
||||
ctx = out["context"]
|
||||
assert "· issue]" in ctx # the issue says what it is
|
||||
assert '[similar 0.72] "debounce helper"' in ctx # the snippet does not
|
||||
# The header now names the right opener for each kind.
|
||||
assert "get_task(id)" in ctx and "get_snippet(id)" in ctx
|
||||
|
||||
Reference in New Issue
Block a user