Write-path prior-art precision: own threshold + payload floor (#85)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 9s
CI & Build / integration (push) Successful in 17s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Successful in 42s
CI & Build / Build & push image (push) Successful in 21s
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 9s
CI & Build / integration (push) Successful in 17s
CI & Build / TypeScript typecheck (push) Successful in 33s
CI & Build / Python tests (push) Successful in 42s
CI & Build / Build & push image (push) Successful in 21s
Closes #2223. Gives the write-path semantic arm its own similarity threshold (kb_writepath_threshold, default 0.68) and a 48-non-whitespace-char payload floor, so unrelated code no longer reads as prior art. Auto-inject keeps 0.55. No migration, no plugin manifest bump — server-side only.
This commit was merged in pull request #85.
This commit is contained in:
@@ -23,6 +23,10 @@ const kbInjectEnabled = ref(true);
|
|||||||
const kbInjectThreshold = ref("0.55");
|
const kbInjectThreshold = ref("0.55");
|
||||||
const kbInjectTopK = ref("3");
|
const kbInjectTopK = ref("3");
|
||||||
const kbWritePathEnabled = ref(true);
|
const kbWritePathEnabled = ref(true);
|
||||||
|
// The write-path arm's OWN threshold, stricter than auto-inject's 0.55 above:
|
||||||
|
// code embeddings sit on a much higher similarity floor than prose, so 0.55 let
|
||||||
|
// unrelated code through (#2223). Shares top-k, not the threshold.
|
||||||
|
const kbWritePathThreshold = ref("0.68");
|
||||||
// Near-duplicate report floor. Deliberately looser than the 0.90 write-time
|
// Near-duplicate report floor. Deliberately looser than the 0.90 write-time
|
||||||
// gate: that one BLOCKS a create and must be unforgiving of noise, this one only
|
// gate: that one BLOCKS a create and must be unforgiving of noise, this one only
|
||||||
// suggests a merge the operator reviews (services/dedup.py).
|
// suggests a merge the operator reviews (services/dedup.py).
|
||||||
@@ -76,9 +80,13 @@ async function saveKbInject() {
|
|||||||
// default, not to 0 — a 0 floor would report every snippet as a duplicate of
|
// default, not to 0 — a 0 floor would report every snippet as a duplicate of
|
||||||
// every other one.
|
// every other one.
|
||||||
const dupT = Math.min(1, Math.max(0, Number(kbDuplicateThreshold.value) || 0.82));
|
const dupT = Math.min(1, Math.max(0, Number(kbDuplicateThreshold.value) || 0.82));
|
||||||
|
// Same `|| default` reasoning as dupT: falling back to 0 would surface every
|
||||||
|
// snippet in the corpus on every edit, which is the failure this knob fixes.
|
||||||
|
const wpT = Math.min(1, Math.max(0, Number(kbWritePathThreshold.value) || 0.68));
|
||||||
kbInjectThreshold.value = String(t);
|
kbInjectThreshold.value = String(t);
|
||||||
kbInjectTopK.value = String(k);
|
kbInjectTopK.value = String(k);
|
||||||
kbDuplicateThreshold.value = String(dupT);
|
kbDuplicateThreshold.value = String(dupT);
|
||||||
|
kbWritePathThreshold.value = String(wpT);
|
||||||
savingKbInject.value = true;
|
savingKbInject.value = true;
|
||||||
kbInjectSaved.value = false;
|
kbInjectSaved.value = false;
|
||||||
try {
|
try {
|
||||||
@@ -86,9 +94,11 @@ async function saveKbInject() {
|
|||||||
kb_autoinject_enabled: kbInjectEnabled.value ? 'true' : 'false',
|
kb_autoinject_enabled: kbInjectEnabled.value ? 'true' : 'false',
|
||||||
kb_autoinject_threshold: String(t),
|
kb_autoinject_threshold: String(t),
|
||||||
kb_autoinject_top_k: String(k),
|
kb_autoinject_top_k: String(k),
|
||||||
// Its own switch, but deliberately the same threshold/ceiling — see
|
// Its own switch AND its own threshold (shares only the ceiling) — see
|
||||||
// WRITEPATH_ENABLED_KEY in services/plugin_context.py.
|
// WRITEPATH_DEFAULT_THRESHOLD in services/plugin_context.py for the
|
||||||
|
// measurements that split them.
|
||||||
kb_writepath_enabled: kbWritePathEnabled.value ? 'true' : 'false',
|
kb_writepath_enabled: kbWritePathEnabled.value ? 'true' : 'false',
|
||||||
|
kb_writepath_threshold: String(wpT),
|
||||||
kb_duplicate_threshold: String(dupT),
|
kb_duplicate_threshold: String(dupT),
|
||||||
});
|
});
|
||||||
kbInjectSaved.value = true;
|
kbInjectSaved.value = true;
|
||||||
@@ -474,6 +484,9 @@ onMounted(async () => {
|
|||||||
kbInjectTopK.value = allSettings.kb_autoinject_top_k;
|
kbInjectTopK.value = allSettings.kb_autoinject_top_k;
|
||||||
}
|
}
|
||||||
kbWritePathEnabled.value = allSettings.kb_writepath_enabled !== "false";
|
kbWritePathEnabled.value = allSettings.kb_writepath_enabled !== "false";
|
||||||
|
if (allSettings.kb_writepath_threshold !== undefined) {
|
||||||
|
kbWritePathThreshold.value = allSettings.kb_writepath_threshold;
|
||||||
|
}
|
||||||
if (allSettings.kb_duplicate_threshold !== undefined) {
|
if (allSettings.kb_duplicate_threshold !== undefined) {
|
||||||
kbDuplicateThreshold.value = allSettings.kb_duplicate_threshold;
|
kbDuplicateThreshold.value = allSettings.kb_duplicate_threshold;
|
||||||
}
|
}
|
||||||
@@ -1220,8 +1233,31 @@ function formatUserDate(iso: string): string {
|
|||||||
Checks the file Claude is about to write or edit against your recorded
|
Checks the file Claude is about to write or edit against your recorded
|
||||||
snippets — what's already kept at that path, and what resembles the code
|
snippets — what's already kept at that path, and what resembles the code
|
||||||
being written — so a helper you already have is offered before it's
|
being written — so a helper you already have is offered before it's
|
||||||
rewritten. Uses the same threshold and ceiling above, and never blocks the
|
rewritten. Uses the ceiling above with its own threshold below, and never
|
||||||
edit. Off = prior art surfaces only on your own prompts.
|
blocks the edit. Off = prior art surfaces only on your own prompts.
|
||||||
|
</p>
|
||||||
|
</div>
|
||||||
|
<div class="field">
|
||||||
|
<label for="kb-writepath-threshold">Prior-art confidence threshold (0–1)</label>
|
||||||
|
<input
|
||||||
|
id="kb-writepath-threshold"
|
||||||
|
v-model="kbWritePathThreshold"
|
||||||
|
type="number"
|
||||||
|
min="0"
|
||||||
|
max="1"
|
||||||
|
step="0.01"
|
||||||
|
class="input"
|
||||||
|
style="max-width: 8rem"
|
||||||
|
/>
|
||||||
|
<p class="field-hint">
|
||||||
|
Stricter than the prompt threshold above on purpose. Any two pieces of
|
||||||
|
code look somewhat alike — shared keywords, indentation, structure — so
|
||||||
|
resemblance scores start higher for code than for prose, and a bar tuned
|
||||||
|
for prompts flags unrelated code as prior art. Lower this if genuine
|
||||||
|
duplicates go unnoticed; raise it if you're being offered snippets that
|
||||||
|
have nothing to do with what's being written. Snippets recorded at the
|
||||||
|
exact file are always shown regardless — those are prior art by
|
||||||
|
location, not by resemblance.
|
||||||
</p>
|
</p>
|
||||||
</div>
|
</div>
|
||||||
<div class="field">
|
<div class="field">
|
||||||
|
|||||||
@@ -120,6 +120,12 @@ def _location_clause(parts: dict[str, str]):
|
|||||||
# the verdict's: jsonpath compares the two fields within the row, so this stays
|
# the verdict's: jsonpath compares the two fields within the row, so this stays
|
||||||
# one index-served predicate rather than a post-filter that would make the
|
# one index-served predicate rather than a post-filter that would make the
|
||||||
# pagination total a lie.
|
# pagination total a lie.
|
||||||
|
#
|
||||||
|
# Same two-dialect discipline as the location filter above, and the same hazard:
|
||||||
|
# THE TWO MUST CHANGE TOGETHER. tests/test_snippet_drift_check.py is the guard —
|
||||||
|
# it walks both dialects over the same cases, including the one that motivated
|
||||||
|
# `attention` existing (an ok verdict whose code_sha has gone stale, which is
|
||||||
|
# neither `drifted` nor `unverified` yet plainly needs looking at).
|
||||||
|
|
||||||
def verification_matches(data: dict | None, value: str) -> bool:
|
def verification_matches(data: dict | None, value: str) -> bool:
|
||||||
"""Python dialect of the verification predicate.
|
"""Python dialect of the verification predicate.
|
||||||
|
|||||||
@@ -56,16 +56,47 @@ AUTOINJECT_DEFAULT_ENABLED = True
|
|||||||
AUTOINJECT_DEFAULT_THRESHOLD = 0.55
|
AUTOINJECT_DEFAULT_THRESHOLD = 0.55
|
||||||
AUTOINJECT_DEFAULT_TOP_K = 3
|
AUTOINJECT_DEFAULT_TOP_K = 3
|
||||||
|
|
||||||
# The write-path trigger (#2082) gets its own on/off switch but SHARES the
|
# The write-path trigger (#2082) gets its own on/off switch, its own threshold,
|
||||||
# threshold and top-k above. One knob for "how loud may Scribe be" is easier to
|
# and shares only top-k. It originally shared the threshold too, on the argument
|
||||||
# reason about than two that drift; the switch is separate because wanting prior
|
# that one "how loud may Scribe be" knob beats two that drift — and reserved the
|
||||||
# art on writes while keeping prompts quiet (or the reverse) is a real
|
# split for when telemetry showed the two surfaces wanted different values.
|
||||||
# preference, and it's the only part of the gate an operator can judge without
|
#
|
||||||
# data. The two surfaces log under different `source` values, so if the
|
# #2223 is that evidence. Measured against the live instance, the semantic arm's
|
||||||
# telemetry ever shows they want different thresholds, splitting them is a
|
# scores for CODE sit far above what the same threshold means for PROSE:
|
||||||
# data-backed change rather than a guess made up front.
|
# near-duplicate of a recorded helper 0.73-0.74 (true positive)
|
||||||
|
# unrelated colour math / Vue SFC / CSS 0.55-0.63 (false positive)
|
||||||
|
# `x = 1` 0.58 (false positive)
|
||||||
|
# Any two Python-shaped payloads share keywords, indentation and structure, so
|
||||||
|
# the floor for "some code" is ~0.55-0.63 — auto-inject's 0.55 lands INSIDE that
|
||||||
|
# noise band, and 6 of 8 probe payloads produced a nudge (4 of them noise). The
|
||||||
|
# margin gate can't rescue it either: _AUTOINJECT_BAND is relative to the top
|
||||||
|
# hit, so with a single hit it never engages.
|
||||||
|
#
|
||||||
|
# 0.68 clears every measured false positive with margin and still sits 0.05
|
||||||
|
# below both true positives. Auto-inject keeps 0.55 — it was tuned on prose and
|
||||||
|
# is not implicated. Tune from retrieval_logs (source='write_path') + note_usage
|
||||||
|
# pull-through (#2085) once a real corpus accrues; a cross-encoder rerank
|
||||||
|
# (#1038) would subsume this bump.
|
||||||
WRITEPATH_ENABLED_KEY = "kb_writepath_enabled"
|
WRITEPATH_ENABLED_KEY = "kb_writepath_enabled"
|
||||||
|
WRITEPATH_THRESHOLD_KEY = "kb_writepath_threshold"
|
||||||
WRITEPATH_DEFAULT_ENABLED = True
|
WRITEPATH_DEFAULT_ENABLED = True
|
||||||
|
WRITEPATH_DEFAULT_THRESHOLD = 0.68
|
||||||
|
|
||||||
|
# Minimum SUBSTANCE (non-whitespace chars) a payload must carry before the
|
||||||
|
# semantic arm will run at all — the cheap half of the operator's #89 idea
|
||||||
|
# ("a sliding scale between number of characters and semantic threshold").
|
||||||
|
#
|
||||||
|
# Deliberately NOT a settings knob and deliberately conservative. Its job is
|
||||||
|
# only to drop payloads too small to carry meaning, where an embedding is noise
|
||||||
|
# rather than signal: `x = 1`, a renamed variable, a changed string literal —
|
||||||
|
# which is what most single-line Edits look like, and the majority of Edits are
|
||||||
|
# single-line. 48 sits below the smallest plausible reusable helper (a one-line
|
||||||
|
# `def` with a body runs ~60), so it errs toward keeping recall and leaves
|
||||||
|
# precision to the threshold above, which is where the measured separation is.
|
||||||
|
# The full length↔threshold CURVE is still open in #89 — the operator flagged it
|
||||||
|
# as wanting a brainstorm, so this stays a flat floor rather than an invented
|
||||||
|
# scale. It also saves a pointless embedding round-trip on trivial edits.
|
||||||
|
WRITEPATH_MIN_CODE_CHARS = 48
|
||||||
|
|
||||||
# Margin gate: drop any hit more than this far below the top hit's score, so a
|
# Margin gate: drop any hit more than this far below the top hit's score, so a
|
||||||
# single strong match doesn't drag in a wall of barely-passing neighbours.
|
# single strong match doesn't drag in a wall of barely-passing neighbours.
|
||||||
@@ -307,15 +338,31 @@ def _prior_art_line(item: dict, marker: str, owner: str | None) -> str:
|
|||||||
|
|
||||||
|
|
||||||
async def get_writepath_config(user_id: int) -> dict:
|
async def get_writepath_config(user_id: int) -> dict:
|
||||||
"""Write-path trigger settings: its own `enabled`, auto-inject's gates."""
|
"""Write-path trigger settings: its own `enabled` and `threshold`, auto-inject's top_k.
|
||||||
|
|
||||||
|
The threshold OVERRIDES the inherited auto-inject value — code embeddings
|
||||||
|
have a much higher similarity floor than prose, so the two surfaces need
|
||||||
|
different bars. See WRITEPATH_DEFAULT_THRESHOLD for the measurements (#2223).
|
||||||
|
top_k is still shared: "how many titles at once" means the same thing on
|
||||||
|
both surfaces, and nothing suggests they want different ceilings.
|
||||||
|
"""
|
||||||
cfg = await get_autoinject_config(user_id)
|
cfg = await get_autoinject_config(user_id)
|
||||||
enabled_raw = await get_setting(
|
enabled_raw = await get_setting(
|
||||||
user_id, WRITEPATH_ENABLED_KEY,
|
user_id, WRITEPATH_ENABLED_KEY,
|
||||||
"true" if WRITEPATH_DEFAULT_ENABLED else "false",
|
"true" if WRITEPATH_DEFAULT_ENABLED else "false",
|
||||||
)
|
)
|
||||||
|
|
||||||
|
try:
|
||||||
|
threshold = float(await get_setting(
|
||||||
|
user_id, WRITEPATH_THRESHOLD_KEY, str(WRITEPATH_DEFAULT_THRESHOLD)))
|
||||||
|
except (TypeError, ValueError):
|
||||||
|
threshold = WRITEPATH_DEFAULT_THRESHOLD
|
||||||
|
threshold = min(1.0, max(0.0, threshold))
|
||||||
|
|
||||||
return {
|
return {
|
||||||
**cfg,
|
**cfg,
|
||||||
"enabled": enabled_raw.strip().lower() in ("true", "1", "yes", "on"),
|
"enabled": enabled_raw.strip().lower() in ("true", "1", "yes", "on"),
|
||||||
|
"threshold": threshold,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
@@ -332,11 +379,14 @@ async def build_write_path_hint(
|
|||||||
convention snippet locations are recorded in. `code` is what's about to be
|
convention snippet locations are recorded in. `code` is what's about to be
|
||||||
written, used only as the semantic query.
|
written, used only as the semantic query.
|
||||||
|
|
||||||
Same four anti-bloat gates as auto-inject (threshold, margin, session dedup
|
Carries auto-inject's anti-bloat gates (margin, session dedup via
|
||||||
via `exclude_ids`, titles-never-bodies) plus the shared top-k cap across BOTH
|
`exclude_ids`, titles-never-bodies) plus the shared top-k cap across BOTH
|
||||||
arms — so a file with a lot of recorded history can't turn one edit into a
|
arms — so a file with a lot of recorded history can't turn one edit into a
|
||||||
wall of text. Returns empty context when disabled, when there's no path, or
|
wall of text. Two gates are its OWN, because code is not prose: a stricter
|
||||||
when nothing is recorded — which is the common case, and the point.
|
similarity threshold, and a minimum-substance floor on `code` below which the
|
||||||
|
semantic arm doesn't run at all (#2223 — see WRITEPATH_DEFAULT_THRESHOLD and
|
||||||
|
WRITEPATH_MIN_CODE_CHARS). Returns empty context when disabled, when there's
|
||||||
|
no path, or when nothing is recorded — which is the common case, and the point.
|
||||||
|
|
||||||
Note the repo↔project mapping is deliberately one-way: the hook sends a git
|
Note the repo↔project mapping is deliberately one-way: the hook sends a git
|
||||||
remote, which the ROUTE resolves to `project_id` through the repo bindings.
|
remote, which the ROUTE resolves to `project_id` through the repo bindings.
|
||||||
@@ -394,6 +444,13 @@ async def build_write_path_hint(
|
|||||||
scored: list[tuple[str, dict]] = []
|
scored: list[tuple[str, dict]] = []
|
||||||
remaining = top_k - len(placed)
|
remaining = top_k - len(placed)
|
||||||
query = (code or "").strip()
|
query = (code or "").strip()
|
||||||
|
# Drop payloads too small to carry meaning before spending an embedding on
|
||||||
|
# them — a one-line Edit is not a helper being rewritten, and its embedding
|
||||||
|
# scores off the corpus floor rather than off any real resemblance (#2223).
|
||||||
|
# Whitespace doesn't count: code is indentation-heavy, so raw length would
|
||||||
|
# let a deeply-nested one-liner through on padding alone.
|
||||||
|
if len("".join(query.split())) < WRITEPATH_MIN_CODE_CHARS:
|
||||||
|
query = ""
|
||||||
if remaining > 0 and query:
|
if remaining > 0 and query:
|
||||||
t0 = time.perf_counter()
|
t0 = time.perf_counter()
|
||||||
hits = await semantic_search_notes(
|
hits = await semantic_search_notes(
|
||||||
|
|||||||
@@ -30,11 +30,31 @@ def _note(nid, title, user_id=1):
|
|||||||
|
|
||||||
|
|
||||||
def _cfg(**over):
|
def _cfg(**over):
|
||||||
base = {"enabled": True, "threshold": 0.55, "top_k": 3}
|
base = {"enabled": True, "threshold": 0.68, "top_k": 3}
|
||||||
base.update(over)
|
base.update(over)
|
||||||
return base
|
return base
|
||||||
|
|
||||||
|
|
||||||
|
# The semantic arm ignores payloads carrying less than WRITEPATH_MIN_CODE_CHARS
|
||||||
|
# of non-whitespace substance (#2223), so any test that expects it to RUN has to
|
||||||
|
# pass something a real Edit would plausibly contain. `code="x"` used to be the
|
||||||
|
# fixture here, which meant the gate tests were passing payloads that in
|
||||||
|
# production are exactly the noise the floor now drops.
|
||||||
|
REAL_CODE = '''def debounce(fn, wait=0.25):
|
||||||
|
"""Rate-limit a callback so it fires once after the last call."""
|
||||||
|
timer = None
|
||||||
|
|
||||||
|
def wrapped(*a, **kw):
|
||||||
|
nonlocal timer
|
||||||
|
if timer:
|
||||||
|
timer.cancel()
|
||||||
|
timer = threading.Timer(wait, fn, a, kw)
|
||||||
|
timer.start()
|
||||||
|
|
||||||
|
return wrapped
|
||||||
|
'''
|
||||||
|
|
||||||
|
|
||||||
# --- silence: the common case ------------------------------------------------
|
# --- silence: the common case ------------------------------------------------
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
@@ -44,7 +64,7 @@ async def test_disabled_returns_empty_and_touches_nothing():
|
|||||||
with patch.object(pc, "get_writepath_config", AsyncMock(return_value=_cfg(enabled=False))), \
|
with patch.object(pc, "get_writepath_config", AsyncMock(return_value=_cfg(enabled=False))), \
|
||||||
patch.object(pc.snippets_svc, "list_snippets", listing), \
|
patch.object(pc.snippets_svc, "list_snippets", listing), \
|
||||||
patch.object(pc, "semantic_search_notes", search):
|
patch.object(pc, "semantic_search_notes", search):
|
||||||
out = await pc.build_write_path_hint(1, "src/x.py", code="def f(): ...")
|
out = await pc.build_write_path_hint(1, "src/x.py", code=REAL_CODE)
|
||||||
assert out["context"] == "" and out["note_ids"] == []
|
assert out["context"] == "" and out["note_ids"] == []
|
||||||
listing.assert_not_called()
|
listing.assert_not_called()
|
||||||
search.assert_not_called()
|
search.assert_not_called()
|
||||||
@@ -57,7 +77,7 @@ async def test_no_path_returns_empty():
|
|||||||
search = AsyncMock()
|
search = AsyncMock()
|
||||||
with patch.object(pc, "get_writepath_config", AsyncMock(return_value=_cfg())), \
|
with patch.object(pc, "get_writepath_config", AsyncMock(return_value=_cfg())), \
|
||||||
patch.object(pc, "semantic_search_notes", search):
|
patch.object(pc, "semantic_search_notes", search):
|
||||||
out = await pc.build_write_path_hint(1, " ", code="def f(): ...")
|
out = await pc.build_write_path_hint(1, " ", code=REAL_CODE)
|
||||||
assert out["context"] == ""
|
assert out["context"] == ""
|
||||||
search.assert_not_called()
|
search.assert_not_called()
|
||||||
|
|
||||||
@@ -69,7 +89,7 @@ async def test_nothing_recorded_is_silent():
|
|||||||
patch.object(pc.snippets_svc, "list_snippets", AsyncMock(return_value=([], 0))), \
|
patch.object(pc.snippets_svc, "list_snippets", AsyncMock(return_value=([], 0))), \
|
||||||
patch.object(pc, "semantic_search_notes", AsyncMock(return_value=[])), \
|
patch.object(pc, "semantic_search_notes", AsyncMock(return_value=[])), \
|
||||||
patch.object(pc, "record_retrieval", MagicMock()):
|
patch.object(pc, "record_retrieval", MagicMock()):
|
||||||
out = await pc.build_write_path_hint(1, "src/x.py", code="def f(): ...")
|
out = await pc.build_write_path_hint(1, "src/x.py", code=REAL_CODE)
|
||||||
assert out["context"] == "" and out["note_ids"] == []
|
assert out["context"] == "" and out["note_ids"] == []
|
||||||
|
|
||||||
|
|
||||||
@@ -86,7 +106,7 @@ async def test_snippet_at_this_path_is_surfaced_without_a_score():
|
|||||||
patch.object(pc, "semantic_search_notes", AsyncMock(return_value=[])), \
|
patch.object(pc, "semantic_search_notes", AsyncMock(return_value=[])), \
|
||||||
patch.object(pc, "record_retrieval", MagicMock()), \
|
patch.object(pc, "record_retrieval", MagicMock()), \
|
||||||
patch.object(pc, "owner_names_for", AsyncMock(return_value={})):
|
patch.object(pc, "owner_names_for", AsyncMock(return_value={})):
|
||||||
out = await pc.build_write_path_hint(1, "src/x.py", code="def f(): ...")
|
out = await pc.build_write_path_hint(1, "src/x.py", code=REAL_CODE)
|
||||||
assert out["note_ids"] == [12]
|
assert out["note_ids"] == [12]
|
||||||
assert '#12 [here] "debounce — rate-limit a callback"' in out["context"]
|
assert '#12 [here] "debounce — rate-limit a callback"' in out["context"]
|
||||||
assert "get_snippet(id)" in out["context"]
|
assert "get_snippet(id)" in out["context"]
|
||||||
@@ -109,7 +129,7 @@ async def test_directory_is_only_consulted_when_the_file_leaves_room():
|
|||||||
patch.object(pc, "semantic_search_notes", AsyncMock(return_value=[])), \
|
patch.object(pc, "semantic_search_notes", AsyncMock(return_value=[])), \
|
||||||
patch.object(pc, "record_retrieval", MagicMock()), \
|
patch.object(pc, "record_retrieval", MagicMock()), \
|
||||||
patch.object(pc, "owner_names_for", AsyncMock(return_value={})):
|
patch.object(pc, "owner_names_for", AsyncMock(return_value={})):
|
||||||
out = await pc.build_write_path_hint(1, "src/lib/x.py", code="x")
|
out = await pc.build_write_path_hint(1, "src/lib/x.py", code=REAL_CODE)
|
||||||
# The file alone filled the budget, so the directory was never queried.
|
# The file alone filled the budget, so the directory was never queried.
|
||||||
assert calls == ["src/lib/x.py"]
|
assert calls == ["src/lib/x.py"]
|
||||||
assert out["note_ids"] == [1, 2, 3]
|
assert out["note_ids"] == [1, 2, 3]
|
||||||
@@ -129,7 +149,7 @@ async def test_directory_hits_are_marked_nearby_not_here():
|
|||||||
patch.object(pc, "semantic_search_notes", AsyncMock(return_value=[])), \
|
patch.object(pc, "semantic_search_notes", AsyncMock(return_value=[])), \
|
||||||
patch.object(pc, "record_retrieval", MagicMock()), \
|
patch.object(pc, "record_retrieval", MagicMock()), \
|
||||||
patch.object(pc, "owner_names_for", AsyncMock(return_value={})):
|
patch.object(pc, "owner_names_for", AsyncMock(return_value={})):
|
||||||
out = await pc.build_write_path_hint(1, "src/lib/x.py", code="x")
|
out = await pc.build_write_path_hint(1, "src/lib/x.py", code=REAL_CODE)
|
||||||
assert '#9 [nearby] "sibling helper"' in out["context"]
|
assert '#9 [nearby] "sibling helper"' in out["context"]
|
||||||
|
|
||||||
|
|
||||||
@@ -143,7 +163,7 @@ async def test_a_failing_location_lookup_does_not_sink_the_hint():
|
|||||||
AsyncMock(return_value=[(0.80, _note(5, "throttle — …"))])), \
|
AsyncMock(return_value=[(0.80, _note(5, "throttle — …"))])), \
|
||||||
patch.object(pc, "record_retrieval", MagicMock()), \
|
patch.object(pc, "record_retrieval", MagicMock()), \
|
||||||
patch.object(pc, "owner_names_for", AsyncMock(return_value={})):
|
patch.object(pc, "owner_names_for", AsyncMock(return_value={})):
|
||||||
out = await pc.build_write_path_hint(1, "src/x.py", code="def f(): ...")
|
out = await pc.build_write_path_hint(1, "src/x.py", code=REAL_CODE)
|
||||||
assert out["note_ids"] == [5]
|
assert out["note_ids"] == [5]
|
||||||
|
|
||||||
|
|
||||||
@@ -157,7 +177,7 @@ async def test_semantic_arm_is_snippet_only_and_browse_scoped():
|
|||||||
patch.object(pc.snippets_svc, "list_snippets", AsyncMock(return_value=([], 0))), \
|
patch.object(pc.snippets_svc, "list_snippets", AsyncMock(return_value=([], 0))), \
|
||||||
patch.object(pc, "semantic_search_notes", search), \
|
patch.object(pc, "semantic_search_notes", search), \
|
||||||
patch.object(pc, "record_retrieval", MagicMock()):
|
patch.object(pc, "record_retrieval", MagicMock()):
|
||||||
await pc.build_write_path_hint(1, "src/x.py", code="def f(): ...")
|
await pc.build_write_path_hint(1, "src/x.py", code=REAL_CODE)
|
||||||
kwargs = search.await_args.kwargs
|
kwargs = search.await_args.kwargs
|
||||||
# Prior art means snippets — not the dev-log that happens to mention one.
|
# Prior art means snippets — not the dev-log that happens to mention one.
|
||||||
assert kwargs["note_type"] == "snippet"
|
assert kwargs["note_type"] == "snippet"
|
||||||
@@ -174,7 +194,7 @@ async def test_margin_gate_applies_to_the_semantic_arm():
|
|||||||
patch.object(pc, "semantic_search_notes", AsyncMock(return_value=hits)), \
|
patch.object(pc, "semantic_search_notes", AsyncMock(return_value=hits)), \
|
||||||
patch.object(pc, "record_retrieval", MagicMock()), \
|
patch.object(pc, "record_retrieval", MagicMock()), \
|
||||||
patch.object(pc, "owner_names_for", AsyncMock(return_value={})):
|
patch.object(pc, "owner_names_for", AsyncMock(return_value={})):
|
||||||
out = await pc.build_write_path_hint(1, "src/x.py", code="def f(): ...")
|
out = await pc.build_write_path_hint(1, "src/x.py", code=REAL_CODE)
|
||||||
assert out["note_ids"] == [11, 22]
|
assert out["note_ids"] == [11, 22]
|
||||||
assert "#33" not in out["context"]
|
assert "#33" not in out["context"]
|
||||||
|
|
||||||
@@ -210,7 +230,7 @@ async def test_place_beats_meaning_and_the_cap_covers_both_arms():
|
|||||||
AsyncMock(return_value=[(0.9, _note(7, "scored"))])), \
|
AsyncMock(return_value=[(0.9, _note(7, "scored"))])), \
|
||||||
patch.object(pc, "record_retrieval", MagicMock()), \
|
patch.object(pc, "record_retrieval", MagicMock()), \
|
||||||
patch.object(pc, "owner_names_for", AsyncMock(return_value={})):
|
patch.object(pc, "owner_names_for", AsyncMock(return_value={})):
|
||||||
out = await pc.build_write_path_hint(1, "src/x.py", code="x")
|
out = await pc.build_write_path_hint(1, "src/x.py", code=REAL_CODE)
|
||||||
# Location hit first; the whole menu stays within top_k.
|
# Location hit first; the whole menu stays within top_k.
|
||||||
assert out["note_ids"] == [1, 7]
|
assert out["note_ids"] == [1, 7]
|
||||||
assert len(out["note_ids"]) <= 2
|
assert len(out["note_ids"]) <= 2
|
||||||
@@ -231,7 +251,7 @@ async def test_semantic_arm_only_asks_for_the_budget_the_place_arm_left():
|
|||||||
patch.object(pc.snippets_svc, "list_snippets", _listing), \
|
patch.object(pc.snippets_svc, "list_snippets", _listing), \
|
||||||
patch.object(pc, "semantic_search_notes", search), \
|
patch.object(pc, "semantic_search_notes", search), \
|
||||||
patch.object(pc, "record_retrieval", MagicMock()):
|
patch.object(pc, "record_retrieval", MagicMock()):
|
||||||
await pc.build_write_path_hint(1, "src/x.py", code="x")
|
await pc.build_write_path_hint(1, "src/x.py", code=REAL_CODE)
|
||||||
assert search.await_args.kwargs["limit"] == 2
|
assert search.await_args.kwargs["limit"] == 2
|
||||||
|
|
||||||
|
|
||||||
@@ -244,7 +264,7 @@ async def test_session_dedup_excludes_ids_from_both_arms():
|
|||||||
AsyncMock(return_value=([_snippet_item(12, "already shown")], 1))), \
|
AsyncMock(return_value=([_snippet_item(12, "already shown")], 1))), \
|
||||||
patch.object(pc, "semantic_search_notes", search), \
|
patch.object(pc, "semantic_search_notes", search), \
|
||||||
patch.object(pc, "record_retrieval", MagicMock()):
|
patch.object(pc, "record_retrieval", MagicMock()):
|
||||||
out = await pc.build_write_path_hint(1, "src/x.py", code="x", exclude_ids=[12])
|
out = await pc.build_write_path_hint(1, "src/x.py", code=REAL_CODE, exclude_ids=[12])
|
||||||
# The location hit was already surfaced this session → dropped, not repeated.
|
# The location hit was already surfaced this session → dropped, not repeated.
|
||||||
assert out["note_ids"] == []
|
assert out["note_ids"] == []
|
||||||
assert 12 in search.await_args.kwargs["exclude_ids"]
|
assert 12 in search.await_args.kwargs["exclude_ids"]
|
||||||
@@ -262,7 +282,7 @@ async def test_another_users_snippet_is_attributed():
|
|||||||
patch.object(pc, "semantic_search_notes", AsyncMock(return_value=[])), \
|
patch.object(pc, "semantic_search_notes", AsyncMock(return_value=[])), \
|
||||||
patch.object(pc, "record_retrieval", MagicMock()), \
|
patch.object(pc, "record_retrieval", MagicMock()), \
|
||||||
patch.object(pc, "owner_names_for", AsyncMock(return_value={42: "alex"})):
|
patch.object(pc, "owner_names_for", AsyncMock(return_value={42: "alex"})):
|
||||||
out = await pc.build_write_path_hint(1, "src/x.py", code="x")
|
out = await pc.build_write_path_hint(1, "src/x.py", code=REAL_CODE)
|
||||||
assert "shared by alex, treat as a suggestion" in out["context"]
|
assert "shared by alex, treat as a suggestion" in out["context"]
|
||||||
|
|
||||||
|
|
||||||
@@ -277,7 +297,7 @@ async def test_telemetry_uses_its_own_source():
|
|||||||
AsyncMock(return_value=[(0.9, _note(7, "scored"))])), \
|
AsyncMock(return_value=[(0.9, _note(7, "scored"))])), \
|
||||||
patch.object(pc, "record_retrieval", rec), \
|
patch.object(pc, "record_retrieval", rec), \
|
||||||
patch.object(pc, "owner_names_for", AsyncMock(return_value={})):
|
patch.object(pc, "owner_names_for", AsyncMock(return_value={})):
|
||||||
await pc.build_write_path_hint(1, "src/x.py", code="x", project_id=4)
|
await pc.build_write_path_hint(1, "src/x.py", code=REAL_CODE, project_id=4)
|
||||||
rec.assert_called_once()
|
rec.assert_called_once()
|
||||||
assert rec.call_args.kwargs["source"] == "write_path"
|
assert rec.call_args.kwargs["source"] == "write_path"
|
||||||
assert rec.call_args.kwargs["source"] != "auto_inject"
|
assert rec.call_args.kwargs["source"] != "auto_inject"
|
||||||
@@ -285,7 +305,7 @@ async def test_telemetry_uses_its_own_source():
|
|||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_config_has_its_own_switch_but_shares_the_gates():
|
async def test_config_has_its_own_switch_and_threshold_but_shares_top_k():
|
||||||
from scribe.services import plugin_context as pc
|
from scribe.services import plugin_context as pc
|
||||||
stored = {pc.WRITEPATH_ENABLED_KEY: "false"}
|
stored = {pc.WRITEPATH_ENABLED_KEY: "false"}
|
||||||
with patch.object(pc, "get_setting",
|
with patch.object(pc, "get_setting",
|
||||||
@@ -293,11 +313,116 @@ async def test_config_has_its_own_switch_but_shares_the_gates():
|
|||||||
cfg = await pc.get_writepath_config(1)
|
cfg = await pc.get_writepath_config(1)
|
||||||
# Its own switch is off while auto-inject stays on...
|
# Its own switch is off while auto-inject stays on...
|
||||||
assert cfg["enabled"] is False
|
assert cfg["enabled"] is False
|
||||||
# ...and the gates are auto-inject's, not a second set that could drift.
|
# ...its threshold is its OWN and stricter than auto-inject's, because code
|
||||||
assert cfg["threshold"] == pc.AUTOINJECT_DEFAULT_THRESHOLD
|
# embeddings have a higher similarity floor than prose (#2223). Sharing 0.55
|
||||||
|
# made unrelated code — including `x = 1` at 0.58 — clear the bar.
|
||||||
|
assert cfg["threshold"] == pc.WRITEPATH_DEFAULT_THRESHOLD
|
||||||
|
assert cfg["threshold"] > pc.AUTOINJECT_DEFAULT_THRESHOLD
|
||||||
|
# ...and top_k is still shared: "how many titles at once" means the same
|
||||||
|
# thing on both surfaces.
|
||||||
assert cfg["top_k"] == pc.AUTOINJECT_DEFAULT_TOP_K
|
assert cfg["top_k"] == pc.AUTOINJECT_DEFAULT_TOP_K
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_writepath_threshold_is_operator_tunable_and_clamped():
|
||||||
|
"""Rule #25 — it's a product knob, so it has to be settable, and a typo in
|
||||||
|
the settings box must not be able to open the gate to everything."""
|
||||||
|
from scribe.services import plugin_context as pc
|
||||||
|
|
||||||
|
async def _cfg_with(raw):
|
||||||
|
stored = {pc.WRITEPATH_THRESHOLD_KEY: raw}
|
||||||
|
with patch.object(pc, "get_setting",
|
||||||
|
AsyncMock(side_effect=lambda uid, k, d: stored.get(k, d))):
|
||||||
|
return await pc.get_writepath_config(1)
|
||||||
|
|
||||||
|
assert (await _cfg_with("0.9"))["threshold"] == 0.9
|
||||||
|
assert (await _cfg_with("5"))["threshold"] == 1.0 # clamped up
|
||||||
|
assert (await _cfg_with("-3"))["threshold"] == 0.0 # clamped down
|
||||||
|
# Garbage falls back to the default rather than to 0.0, which would surface
|
||||||
|
# every snippet in the corpus on every edit.
|
||||||
|
assert (await _cfg_with("banana"))["threshold"] == pc.WRITEPATH_DEFAULT_THRESHOLD
|
||||||
|
|
||||||
|
|
||||||
|
# --- the minimum-substance floor on the semantic arm (#2223) ------------------
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_trivial_payload_skips_the_semantic_arm_entirely():
|
||||||
|
"""`x = 1` scored 0.58 against an unrelated helper on the live instance — the
|
||||||
|
embedder's floor for "some code", not a resemblance. A payload too small to
|
||||||
|
carry meaning must not reach the embedder at all, and must not cost a
|
||||||
|
retrieval_logs row either."""
|
||||||
|
from scribe.services import plugin_context as pc
|
||||||
|
search, rec = AsyncMock(return_value=[]), MagicMock()
|
||||||
|
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, "semantic_search_notes", search), \
|
||||||
|
patch.object(pc, "record_retrieval", rec):
|
||||||
|
out = await pc.build_write_path_hint(1, "src/x.py", code="x = 1")
|
||||||
|
assert out["context"] == "" and out["note_ids"] == []
|
||||||
|
search.assert_not_called()
|
||||||
|
rec.assert_not_called()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_the_floor_counts_substance_not_indentation():
|
||||||
|
"""Code is indentation-heavy, so raw length would let a deeply-nested
|
||||||
|
one-liner through on padding alone."""
|
||||||
|
from scribe.services import plugin_context as pc
|
||||||
|
padded = " " * 400 + "return None" # long, but almost no substance
|
||||||
|
search = AsyncMock(return_value=[])
|
||||||
|
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, "semantic_search_notes", search), \
|
||||||
|
patch.object(pc, "record_retrieval", MagicMock()):
|
||||||
|
await pc.build_write_path_hint(1, "src/x.py", code=padded)
|
||||||
|
assert len(padded) > pc.WRITEPATH_MIN_CODE_CHARS # would pass a raw-length check
|
||||||
|
search.assert_not_called()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_the_floor_does_not_block_the_place_arm():
|
||||||
|
"""The floor is a statement about the semantic query, not about the edit. A
|
||||||
|
snippet recorded at this exact path is prior art however small the change."""
|
||||||
|
from scribe.services import plugin_context as pc
|
||||||
|
search = AsyncMock(return_value=[])
|
||||||
|
with patch.object(pc, "get_writepath_config", AsyncMock(return_value=_cfg())), \
|
||||||
|
patch.object(pc.snippets_svc, "list_snippets",
|
||||||
|
AsyncMock(return_value=([_snippet_item(12, "here helper")], 1))), \
|
||||||
|
patch.object(pc, "semantic_search_notes", search), \
|
||||||
|
patch.object(pc, "record_retrieval", MagicMock()), \
|
||||||
|
patch.object(pc, "owner_names_for", AsyncMock(return_value={})):
|
||||||
|
out = await pc.build_write_path_hint(1, "src/x.py", code="x = 1")
|
||||||
|
assert out["note_ids"] == [12]
|
||||||
|
search.assert_not_called()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_a_real_helper_clears_the_floor():
|
||||||
|
"""The floor errs toward keeping recall — anything that plausibly IS a
|
||||||
|
reusable helper has to get through, or the feature stops working."""
|
||||||
|
from scribe.services import plugin_context as pc
|
||||||
|
search = AsyncMock(return_value=[(0.80, _note(5, "debounce — …"))])
|
||||||
|
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, "semantic_search_notes", search), \
|
||||||
|
patch.object(pc, "record_retrieval", MagicMock()), \
|
||||||
|
patch.object(pc, "owner_names_for", AsyncMock(return_value={})):
|
||||||
|
out = await pc.build_write_path_hint(1, "src/x.py", code=REAL_CODE)
|
||||||
|
assert out["note_ids"] == [5]
|
||||||
|
search.assert_called_once()
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_floor_stays_below_the_smallest_plausible_helper():
|
||||||
|
"""A guard on the constant itself: raising it far enough to reject a real
|
||||||
|
one-line helper would silently gut the arm rather than sharpen it."""
|
||||||
|
from scribe.services import plugin_context as pc
|
||||||
|
smallest_real_helper = 'def slug(t): return re.sub(r"[^a-z0-9]+", "-", t.lower()).strip("-")'
|
||||||
|
substance = len("".join(smallest_real_helper.split()))
|
||||||
|
assert pc.WRITEPATH_MIN_CODE_CHARS < substance
|
||||||
|
# And still strictly above a degenerate one-liner.
|
||||||
|
assert pc.WRITEPATH_MIN_CODE_CHARS > len("".join("x = 1".split()))
|
||||||
|
|
||||||
|
|
||||||
# --- the route between them --------------------------------------------------
|
# --- the route between them --------------------------------------------------
|
||||||
|
|
||||||
def test_route_reads_every_arg_the_hook_sends():
|
def test_route_reads_every_arg_the_hook_sends():
|
||||||
|
|||||||
Reference in New Issue
Block a user