feat(prior-art): edit-time record-sync nudge — the sync class (#2708)
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 11s
CI & Build / TypeScript typecheck (push) Successful in 24s
CI & Build / integration (push) Successful in 26s
CI & Build / Python tests (push) Successful in 56s
CI & Build / Build & push image (push) Successful in 43s
CI & Build / Python lint (push) Successful in 3s
CI & Build / Plugin hooks (push) Successful in 11s
CI & Build / TypeScript typecheck (push) Successful in 24s
CI & Build / integration (push) Successful in 26s
CI & Build / Python tests (push) Successful in 56s
CI & Build / Build & push image (push) Successful in 43s
A snippet recorded AT the exact file being edited is not a reuse suggestion — it IS the record of the file being changed. The write-path hint now renders those as their own SYNC class: 'snippet #N records this file — updating the record is part of the edit (update_snippet / verify_snippet)'. Nearby and semantic hits stay the reuse menu. The two classes dedup on separate per-session channels (exclude_ids vs exclude_sync_ids, .ids vs .sync.ids in the hook), so a reuse hint shown early in a session can no longer silence the record-sync nudge when the recorded file itself is edited later. Sync surfacing is measured under its own note_usage source (write_path_sync) — its pull-through rate is the scoreboard for whether edit-time sync actually happens, per decision #2707 (no forge connection; records stay current in the session that has the context). Plugin 0.1.31. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -101,12 +101,14 @@ async def test_nothing_recorded_is_silent():
|
||||
assert out["context"] == "" and out["note_ids"] == []
|
||||
|
||||
|
||||
# --- arm 1: by place ---------------------------------------------------------
|
||||
# --- the sync class, and arm 1 by place --------------------------------------
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_snippet_at_this_path_is_surfaced_without_a_score():
|
||||
"""A snippet recorded HERE is prior art by definition, not by resemblance —
|
||||
it must not be subject to the similarity threshold."""
|
||||
async def test_snippet_at_this_path_becomes_the_sync_nudge():
|
||||
"""A snippet recorded AT the exact file is not a reuse suggestion — it IS
|
||||
the record of the file being edited (#2708). It surfaces without a score,
|
||||
framed as "updating the record is part of this edit", and is reported in
|
||||
sync_note_ids so the hook feeds the sync dedup channel."""
|
||||
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",
|
||||
@@ -116,11 +118,59 @@ async def test_snippet_at_this_path_is_surfaced_without_a_score():
|
||||
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"] == [12]
|
||||
assert '#12 [here] "debounce — rate-limit a callback"' in out["context"]
|
||||
assert "get_snippet(id)" in out["context"]
|
||||
assert out["sync_note_ids"] == [12]
|
||||
assert '#12 [records this file] "debounce — rate-limit a callback"' in out["context"]
|
||||
# The load-bearing sentence: the record's freshness belongs to this edit.
|
||||
assert "update_snippet" in out["context"]
|
||||
assert "verify_snippet" in out["context"]
|
||||
assert "src/x.py" in out["context"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_reuse_dedup_never_suppresses_the_sync_nudge():
|
||||
"""THE bug #2708 names: a title surfaced as a reuse hint twenty turns ago
|
||||
landed in exclude_ids — and then silenced "you are editing the recorded
|
||||
file right now", the one claim that must fire at the moment of change. The
|
||||
sync class dedups only against its own channel."""
|
||||
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, "seen as reuse already")], 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=REAL_CODE, exclude_ids=[12],
|
||||
)
|
||||
assert out["sync_note_ids"] == [12]
|
||||
assert "[records this file]" in out["context"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_sync_dedup_silences_a_sync_nudge_already_shown():
|
||||
"""Once the session has been told "snippet #12 records this file", every
|
||||
further edit of the same file stays quiet about it — on every arm: the
|
||||
directory query and the semantic arm would both re-surface it otherwise."""
|
||||
from scribe.services import plugin_context as pc
|
||||
|
||||
async def _listing(uid, **kw):
|
||||
# Recorded at the exact file, so it matches the file AND dir queries.
|
||||
return ([_snippet_item(12, "already nudged")], 1)
|
||||
|
||||
search = AsyncMock(return_value=[])
|
||||
with patch.object(pc, "get_writepath_config", AsyncMock(return_value=_cfg())), \
|
||||
patch.object(pc.snippets_svc, "list_snippets", _listing), \
|
||||
patch.object(pc, "semantic_search_notes", search), \
|
||||
patch.object(pc, "record_retrieval", MagicMock()):
|
||||
out = await pc.build_write_path_hint(
|
||||
1, "src/x.py", code=REAL_CODE, exclude_sync_ids=[12],
|
||||
)
|
||||
assert out["note_ids"] == [] and out["sync_note_ids"] == []
|
||||
assert "#12" not in out["context"]
|
||||
assert 12 in search.await_args.kwargs["exclude_ids"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_directory_is_only_consulted_when_the_file_leaves_room():
|
||||
from scribe.services import plugin_context as pc
|
||||
@@ -267,16 +317,24 @@ async def test_semantic_arm_only_asks_for_the_budget_the_place_arm_left():
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_session_dedup_excludes_ids_from_both_arms():
|
||||
async def test_session_dedup_excludes_ids_from_the_reuse_arms():
|
||||
"""exclude_ids governs the REUSE classes — nearby and semantic. (The sync
|
||||
class has its own channel; see the tests above.)"""
|
||||
from scribe.services import plugin_context as pc
|
||||
|
||||
async def _listing(uid, **kw):
|
||||
# Recorded at a SIBLING file, so it reaches only the directory query.
|
||||
if kw["path"] == "src":
|
||||
return ([_snippet_item(12, "already shown")], 1)
|
||||
return ([], 0)
|
||||
|
||||
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, "already shown")], 1))), \
|
||||
patch.object(pc.snippets_svc, "list_snippets", _listing), \
|
||||
patch.object(pc, "semantic_search_notes", search), \
|
||||
patch.object(pc, "record_retrieval", MagicMock()):
|
||||
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 nearby hit was already surfaced this session → dropped, not repeated.
|
||||
assert out["note_ids"] == []
|
||||
assert 12 in search.await_args.kwargs["exclude_ids"]
|
||||
|
||||
@@ -315,6 +373,33 @@ async def test_telemetry_uses_its_own_source():
|
||||
assert rec.call_args.kwargs["project_id"] == 4
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_sync_surfacing_is_measured_under_its_own_usage_source():
|
||||
"""The sync class's pull-through rate is #2708's scoreboard — whether
|
||||
sessions actually update the record when told they're editing it. Folding
|
||||
it into write_path_place would make that unmeasurable."""
|
||||
from scribe.services import plugin_context as pc
|
||||
|
||||
async def _listing(uid, **kw):
|
||||
if kw["path"] == "src/x.py":
|
||||
return ([_snippet_item(12, "records me")], 1)
|
||||
return ([_snippet_item(9, "sibling")], 1)
|
||||
|
||||
surfaced = MagicMock()
|
||||
with patch.object(pc, "get_writepath_config", AsyncMock(return_value=_cfg())), \
|
||||
patch.object(pc.snippets_svc, "list_snippets", _listing), \
|
||||
patch.object(pc, "semantic_search_notes", AsyncMock(return_value=[])), \
|
||||
patch.object(pc, "record_retrieval", MagicMock()), \
|
||||
patch.object(pc, "record_surfaced", surfaced), \
|
||||
patch.object(pc, "owner_names_for", AsyncMock(return_value={})):
|
||||
await pc.build_write_path_hint(1, "src/x.py", code=REAL_CODE)
|
||||
by_source = {
|
||||
c.kwargs["source"]: c.kwargs["note_ids"] for c in surfaced.call_args_list
|
||||
}
|
||||
assert by_source["write_path_sync"] == [12]
|
||||
assert by_source["write_path_place"] == [9]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_config_has_its_own_switch_and_threshold_but_shares_top_k():
|
||||
from scribe.services import plugin_context as pc
|
||||
@@ -693,11 +778,11 @@ def test_route_reads_every_arg_the_hook_sends():
|
||||
|
||||
from scribe.routes import plugin as routes
|
||||
src = inspect.getsource(routes.write_path_prior_art)
|
||||
for arg in ("path", "code", "repo", "project_id", "exclude_ids"):
|
||||
for arg in ("path", "code", "repo", "project_id", "exclude_ids", "exclude_sync_ids"):
|
||||
assert f'request.args.get("{arg}"' in src, f"route ignores {arg}"
|
||||
|
||||
hook = HOOK.read_text()
|
||||
for arg in ("path=", "code=", "repo=", "exclude_ids="):
|
||||
for arg in ("path=", "code=", "repo=", "exclude_ids=", "exclude_sync_ids="):
|
||||
assert arg in hook, f"hook never sends {arg}"
|
||||
|
||||
|
||||
@@ -788,7 +873,21 @@ def test_plugin_version_bumped_with_the_hook():
|
||||
ship."""
|
||||
manifest = json.loads((PLUGIN / ".claude-plugin" / "plugin.json").read_text())
|
||||
version = tuple(int(p) for p in manifest["version"].split("."))
|
||||
assert version >= (0, 1, 18)
|
||||
assert version >= (0, 1, 31)
|
||||
|
||||
|
||||
def test_hook_keeps_sync_and_reuse_dedup_apart():
|
||||
"""#2708's dedup audit, pinned: the hook holds TWO per-session id files and
|
||||
feeds each its own class — sync ids (snippets recording the edited file) to
|
||||
the sync file, the rest to the reuse file. One shared file is exactly the
|
||||
bug this replaced: a reuse hint early in the session silencing the record-
|
||||
sync nudge when the recorded file is edited later."""
|
||||
src = HOOK.read_text()
|
||||
assert ".sync.ids" in src # its own state file
|
||||
assert "exclude_sync_ids=" in src # its own query channel
|
||||
# The reuse file must NOT swallow sync ids — the write-back subtracts them.
|
||||
assert "(.note_ids // []) - (.sync_note_ids // [])" in src
|
||||
assert "(.sync_note_ids // [])[]?" in src
|
||||
|
||||
|
||||
def _hook_runtime_env():
|
||||
|
||||
Reference in New Issue
Block a user