Cross-language prior-art labelling + close the surfaced→pulled loop on get_task (#87)
CI & Build / Python lint (push) Successful in 2s
CI & Build / Plugin hooks (push) Successful in 7s
CI & Build / integration (push) Successful in 16s
CI & Build / TypeScript typecheck (push) Successful in 23s
CI & Build / Python tests (push) Successful in 41s
CI & Build / Build & push image (push) Successful in 19s
CI & Build / Python lint (push) Successful in 2s
CI & Build / Plugin hooks (push) Successful in 7s
CI & Build / integration (push) Successful in 16s
CI & Build / TypeScript typecheck (push) Successful in 23s
CI & Build / Python tests (push) Successful in 41s
CI & Build / Build & push image (push) Successful in 19s
Closes #2244 and #2245. Prior-art hits in a different language than the file being written are now labelled and explained rather than surfaced bare, and get_task records a pull so auto-inject's pull-through stops reading near-zero for the kind it mostly surfaces. No migration, no plugin manifest bump — server-side only.
This commit was merged in pull request #87.
This commit is contained in:
@@ -88,6 +88,11 @@ export interface SnippetListItem {
|
||||
* them, not one of your own. Absent means it's yours. */
|
||||
shared?: boolean;
|
||||
owner?: string | null;
|
||||
/** The recorded language, when one was given. Projected from the `data`
|
||||
* mirror so lists can show it without parsing the body — and so a prior-art
|
||||
* hit can be flagged as being in a DIFFERENT language than the file being
|
||||
* written, which is a shape to adapt rather than code to paste. */
|
||||
language?: string;
|
||||
/** Always present from the backend, zero-filled for records with no events. */
|
||||
usage?: SnippetUsage;
|
||||
/** Present on the detail record; the list feed carries it when a check has
|
||||
|
||||
@@ -27,6 +27,7 @@ from scribe.services import rulebooks as rulebooks_svc
|
||||
from scribe.services import systems as systems_svc
|
||||
from scribe.services import task_logs as task_logs_svc
|
||||
from scribe.services import trash as trash_svc
|
||||
from scribe.services.note_usage import record_pulled
|
||||
|
||||
|
||||
async def list_tasks(
|
||||
@@ -99,6 +100,14 @@ async def get_task(task_id: int) -> dict:
|
||||
data["suppressed_rules"] = applicable.get("suppressed_rules", [])
|
||||
data["suppressed_topics"] = applicable.get("suppressed_topics", [])
|
||||
data.update(await access_svc.describe_provenance(uid, note))
|
||||
# Same reasoning as get_note's record_pulled, and this is the tool where it
|
||||
# matters MOST: auto-inject ranks kind-blind over a corpus that is
|
||||
# overwhelmingly tasks and issues, so tasks dominate what it surfaces. Without
|
||||
# this the surfaced→pulled loop was open exactly where the volume is — every
|
||||
# surfaced task counted as never-pulled because the tool that opens one didn't
|
||||
# say so, driving auto-inject's measured pull-through toward zero for its own
|
||||
# dominant kind. #1038 and #2085 are explicitly gated on that number (#2245).
|
||||
record_pulled(user_id=uid, note_id=int(note.id), source="mcp_get_task")
|
||||
return data
|
||||
|
||||
|
||||
|
||||
@@ -211,6 +211,15 @@ def _note_to_item(note: Note) -> dict:
|
||||
# body parsing and stays a plain projection of the column. Omitted entirely
|
||||
# when unchecked, so "no key" and "never verified" don't become two states
|
||||
# the client has to tell apart.
|
||||
# Snippet language, same reasoning as the verdict below: a plain projection of
|
||||
# the `data` mirror, no body parsing. Needed because a prior-art hit in a
|
||||
# DIFFERENT language than the file being written is useful as the shape of a
|
||||
# solution but must not be mistaken for code to paste (#2244) — and the caller
|
||||
# can only say "different" if the language is on the item.
|
||||
language = (note.data or {}).get("language") if note.data else None
|
||||
if language:
|
||||
item["language"] = language
|
||||
|
||||
verdict = (note.data or {}).get("verification") if note.data else None
|
||||
if verdict and verdict.get("status"):
|
||||
item["verification"] = {
|
||||
|
||||
@@ -391,15 +391,98 @@ async def build_autoinject_hint(
|
||||
# this exact file" is a stronger claim than "this resembles something".
|
||||
|
||||
|
||||
def _prior_art_line(item: dict, marker: str, owner: str | None) -> str:
|
||||
"""One menu line: `- #12 [here] "title"`, attributed when it isn't yours."""
|
||||
def _prior_art_line(item: dict, marker: str, owner: str | None, foreign_lang: str = "") -> str:
|
||||
"""One menu line: `- #12 [here] "title"`, attributed when it isn't yours.
|
||||
|
||||
A foreign language is folded into the marker (`[similar 0.72 · python]`)
|
||||
rather than appended after the title, so the reader sees it while still
|
||||
reading the score — the two together are the judgement being offered.
|
||||
"""
|
||||
title = (item.get("title") or "(untitled)").replace("\n", " ").strip()
|
||||
line = f"> - #{item['id']} [{marker}] \"{title}\""
|
||||
mark = f"{marker} · {foreign_lang}" if foreign_lang else marker
|
||||
line = f"> - #{item['id']} [{mark}] \"{title}\""
|
||||
if owner:
|
||||
line += f" — shared by {owner}, treat as a suggestion"
|
||||
return line
|
||||
|
||||
|
||||
# --- cross-language prior art (#2244) ----------------------------------------
|
||||
# Retrieval is concept-shaped now, and concepts are language-agnostic: a query
|
||||
# about a TypeScript union-find matches a PYTHON snippet at 0.72-0.73, comfortably
|
||||
# over the bar. That is a feature — the operator's framing is "borrow the shape of
|
||||
# the solution even when the code isn't directly reusable" — but only if the line
|
||||
# SAYS so. An unlabelled Python hit offered while writing TypeScript either gets
|
||||
# dismissed as irrelevant or, worse, pasted into a .ts file. Measured note: this
|
||||
# cross-language matching predates concept queries; it was always happening, just
|
||||
# never disclosed.
|
||||
#
|
||||
# Deliberately NOT gated behind a stricter threshold for foreign-language hits: a
|
||||
# higher bar would suppress exactly the shape-borrowing this is for. Label, don't
|
||||
# filter.
|
||||
_LANG_BY_EXT = {
|
||||
"py": "python", "pyi": "python",
|
||||
"ts": "typescript", "tsx": "typescript", "mts": "typescript", "cts": "typescript",
|
||||
"js": "javascript", "jsx": "javascript", "mjs": "javascript", "cjs": "javascript",
|
||||
"vue": "vue", "svelte": "svelte",
|
||||
"go": "go", "rs": "rust", "rb": "ruby", "php": "php",
|
||||
"java": "java", "kt": "kotlin", "kts": "kotlin", "scala": "scala",
|
||||
"c": "c", "h": "c", "cc": "cpp", "cpp": "cpp", "cxx": "cpp", "hpp": "cpp",
|
||||
"cs": "csharp", "swift": "swift", "m": "objectivec", "mm": "objectivec",
|
||||
"sh": "shell", "bash": "shell", "zsh": "shell", "fish": "shell",
|
||||
"sql": "sql", "css": "css", "scss": "scss", "less": "less",
|
||||
"html": "html", "htm": "html", "yml": "yaml", "yaml": "yaml",
|
||||
"toml": "toml", "ini": "ini", "dockerfile": "dockerfile",
|
||||
"ex": "elixir", "exs": "elixir", "erl": "erlang", "hs": "haskell",
|
||||
"lua": "lua", "pl": "perl", "r": "r", "dart": "dart", "zig": "zig",
|
||||
}
|
||||
|
||||
# `language` on a snippet is operator-typed free text, so fold the spellings that
|
||||
# mean the same thing before comparing. Anything unrecognised passes through
|
||||
# lowercased — an unknown-but-equal pair still compares equal, which is the only
|
||||
# thing this needs to get right.
|
||||
_LANG_ALIASES = {
|
||||
"py": "python", "python3": "python",
|
||||
"ts": "typescript", "tsx": "typescript",
|
||||
"js": "javascript", "jsx": "javascript", "node": "javascript",
|
||||
"sh": "shell", "bash": "shell", "zsh": "shell", "shell-script": "shell",
|
||||
"c++": "cpp", "cplusplus": "cpp", "c#": "csharp", "objective-c": "objectivec",
|
||||
"golang": "go", "rs": "rust", "rb": "ruby", "yml": "yaml",
|
||||
"postgres": "sql", "postgresql": "sql", "psql": "sql",
|
||||
"vuejs": "vue", "vue3": "vue",
|
||||
}
|
||||
|
||||
|
||||
def _canonical_language(name: str) -> str:
|
||||
"""Fold a free-text language name to a comparable token ("" if absent)."""
|
||||
token = (name or "").strip().lower()
|
||||
return _LANG_ALIASES.get(token, token)
|
||||
|
||||
|
||||
def _language_for_path(path: str) -> str:
|
||||
"""The language implied by a file path's extension ("" when unknown)."""
|
||||
tail = (path or "").rsplit("/", 1)[-1].lower()
|
||||
if tail.startswith("dockerfile"):
|
||||
return "dockerfile"
|
||||
if "." not in tail:
|
||||
return ""
|
||||
return _LANG_BY_EXT.get(tail.rsplit(".", 1)[-1], "")
|
||||
|
||||
|
||||
def _foreign_language(item: dict, target: str) -> str:
|
||||
"""The item's language when it DIFFERS from the target file's, else "".
|
||||
|
||||
Returns "" whenever either side is unknown: we can only claim a mismatch we
|
||||
can actually establish, and a wrong "· python" tag is worse than no tag.
|
||||
Same-language hits stay unlabelled so the common case keeps a clean line.
|
||||
"""
|
||||
if not target:
|
||||
return ""
|
||||
theirs = _canonical_language(item.get("language") or "")
|
||||
if not theirs or theirs == target:
|
||||
return ""
|
||||
return theirs
|
||||
|
||||
|
||||
def _concept_doc(code: str) -> str:
|
||||
"""The first doc-ish prose in `code`: docstring, else JSDoc, else leading comments."""
|
||||
m = _CONCEPT_PY_DOC.search(code)
|
||||
@@ -619,7 +702,14 @@ async def build_write_path_hint(
|
||||
continue
|
||||
scored.append((
|
||||
f"similar {score:.2f}",
|
||||
{"id": int(note.id), "title": note.title, "user_id": note.user_id},
|
||||
{
|
||||
"id": int(note.id), "title": note.title, "user_id": note.user_id,
|
||||
# Carried so the line can disclose a cross-language hit
|
||||
# (#2244). The semantic arm is where these actually arise —
|
||||
# a snippet recorded at the path you're editing is almost
|
||||
# never in another language, but a concept match easily is.
|
||||
"language": (note.data or {}).get("language") if note.data else None,
|
||||
},
|
||||
))
|
||||
|
||||
menu = (placed + scored)[:top_k]
|
||||
@@ -631,19 +721,36 @@ async def build_write_path_hint(
|
||||
if it.get("user_id") is not None and int(it["user_id"]) != user_id
|
||||
})
|
||||
|
||||
target_lang = _language_for_path(path)
|
||||
rendered: list[tuple[dict, str, str | None, str]] = []
|
||||
for marker, item in menu:
|
||||
owner_id = item.get("user_id")
|
||||
owner = None
|
||||
if owner_id is not None and int(owner_id) != user_id:
|
||||
owner = owners.get(int(owner_id)) or "another user"
|
||||
rendered.append((item, marker, owner, _foreign_language(item, target_lang)))
|
||||
|
||||
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 "
|
||||
"(titles only; shown once per session):",
|
||||
]
|
||||
# Say what a language tag MEANS, and only when one is actually on the menu.
|
||||
# Without this the reader has to infer why "· python" is attached to a hit on
|
||||
# a .ts file, and the two ways of guessing wrong are both bad: dismiss it as
|
||||
# irrelevant, or paste Python into TypeScript. Retrieval matches on concept,
|
||||
# so these are genuinely useful — as the SHAPE of a solution, not as code.
|
||||
if any(lang for _i, _m, _o, lang in rendered):
|
||||
lines.append(
|
||||
"> A tagged language means that snippet is in a DIFFERENT language "
|
||||
"than this file — it matched on what it does, so treat it as the "
|
||||
"shape of a solution to adapt, not code to copy."
|
||||
)
|
||||
|
||||
note_ids: list[int] = []
|
||||
for marker, item in menu:
|
||||
for item, marker, owner, foreign_lang in rendered:
|
||||
note_ids.append(int(item["id"]))
|
||||
owner_id = item.get("user_id")
|
||||
owner = None
|
||||
if owner_id is not None and int(owner_id) != user_id:
|
||||
owner = owners.get(int(owner_id)) or "another user"
|
||||
lines.append(_prior_art_line(item, marker, owner))
|
||||
lines.append(_prior_art_line(item, marker, owner, foreign_lang))
|
||||
|
||||
# Split by arm, which is the whole reason this table exists. The place arm
|
||||
# carries no score and so has no home in retrieval_logs; before #2085 a
|
||||
|
||||
@@ -22,6 +22,9 @@ def _note(nid, title, user_id=1):
|
||||
n = MagicMock()
|
||||
n.id, n.title, n.user_id = nid, title, user_id
|
||||
n.note_type = "snippet"
|
||||
# See the same note in test_write_path_trigger: an auto-created mock on
|
||||
# `.data` is truthy and would leak into a rendered menu line (#2244).
|
||||
n.data = None
|
||||
return n
|
||||
|
||||
|
||||
@@ -183,3 +186,37 @@ async def test_auto_inject_records_what_survived_the_margin_gate():
|
||||
|
||||
assert surfaced.call_args.kwargs["note_ids"] == [1]
|
||||
assert surfaced.call_args.kwargs["source"] == "auto_inject"
|
||||
|
||||
|
||||
def test_every_getter_that_can_be_surfaced_also_records_a_pull():
|
||||
"""Rule #33 contract check — and the one that would have caught #2245.
|
||||
|
||||
`surfaced` and `pulled` only mean something as a PAIR: the rate between them
|
||||
is what #1038 and #2085 gate on. That pair is only closed if the tool which
|
||||
OPENS a record reports it. `get_note` did, `get_snippet` did, `get_task` did
|
||||
NOT — and auto-inject ranks kind-blind over a corpus that is overwhelmingly
|
||||
tasks and issues, so the gap sat exactly where the volume is: every surfaced
|
||||
task counted as never-pulled, dragging measured pull-through toward zero for
|
||||
the menu's own dominant kind.
|
||||
|
||||
Asserted by source inspection rather than by calling the tools, because the
|
||||
failure is a MISSING call — which no behavioural test of the tool's return
|
||||
value can see.
|
||||
"""
|
||||
import inspect
|
||||
|
||||
from scribe.mcp.tools import notes as notes_tools
|
||||
from scribe.mcp.tools import snippets as snippet_tools
|
||||
from scribe.mcp.tools import tasks as task_tools
|
||||
|
||||
getters = (
|
||||
(notes_tools, "get_note"),
|
||||
(task_tools, "get_task"),
|
||||
(snippet_tools, "get_snippet"),
|
||||
)
|
||||
for module, name in getters:
|
||||
src = inspect.getsource(getattr(module, name))
|
||||
assert "record_pulled(" in src, (
|
||||
f"{name} can be surfaced in an auto-inject menu but records no pull — "
|
||||
"its pull-through rate will read as zero regardless of real usage"
|
||||
)
|
||||
|
||||
@@ -26,6 +26,10 @@ def _snippet_item(nid, title, user_id=1):
|
||||
def _note(nid, title, user_id=1):
|
||||
n = MagicMock()
|
||||
n.id, n.title, n.user_id = nid, title, user_id
|
||||
# Explicitly None, not left to MagicMock's auto-attribute: the semantic arm
|
||||
# reads `note.data` for the snippet's language (#2244), and an auto-created
|
||||
# mock there is truthy, so it would render its repr into the menu line.
|
||||
n.data = None
|
||||
return n
|
||||
|
||||
|
||||
@@ -585,6 +589,94 @@ async def test_the_floor_judges_the_raw_payload_not_the_concept():
|
||||
search2.assert_not_called()
|
||||
|
||||
|
||||
# --- cross-language prior art is disclosed, not filtered (#2244) --------------
|
||||
|
||||
def test_language_for_path_and_canonicalisation():
|
||||
from scribe.services import plugin_context as pc
|
||||
assert pc._language_for_path("src/a/b.ts") == "typescript"
|
||||
assert pc._language_for_path("src/a/b.py") == "python"
|
||||
assert pc._language_for_path("ops/Dockerfile") == "dockerfile"
|
||||
assert pc._language_for_path("README") == ""
|
||||
assert pc._language_for_path("weird/thing.qqq") == ""
|
||||
# Operator-typed spellings that mean the same thing must compare equal.
|
||||
assert pc._canonical_language("Python3") == pc._canonical_language("py")
|
||||
assert pc._canonical_language("TSX") == pc._canonical_language("typescript")
|
||||
assert pc._canonical_language("C++") == "cpp"
|
||||
# Unknown-but-equal still compares equal, which is all this must get right.
|
||||
assert pc._canonical_language("Brainfuck") == pc._canonical_language("brainfuck")
|
||||
|
||||
|
||||
def test_foreign_language_only_claims_a_mismatch_it_can_establish():
|
||||
from scribe.services import plugin_context as pc
|
||||
assert pc._foreign_language({"language": "python"}, "typescript") == "python"
|
||||
assert pc._foreign_language({"language": "py"}, "python") == "" # same, aliased
|
||||
assert pc._foreign_language({"language": ""}, "typescript") == "" # theirs unknown
|
||||
assert pc._foreign_language({"language": "python"}, "") == "" # target unknown
|
||||
assert pc._foreign_language({}, "typescript") == "" # no field at all
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_cross_language_hit_is_labelled_and_explained():
|
||||
"""The fail state this closes: an unlabelled Python hit offered while writing
|
||||
TypeScript is either dismissed as irrelevant or pasted into the .ts file."""
|
||||
from scribe.services import plugin_context as pc
|
||||
note = _note(7, "group_pairs — collapse related pairs into groups")
|
||||
note.data = {"language": "python"}
|
||||
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", AsyncMock(return_value=[(0.72, note)])), \
|
||||
patch.object(pc, "record_retrieval", MagicMock()), \
|
||||
patch.object(pc, "owner_names_for", AsyncMock(return_value={})):
|
||||
out = await pc.build_write_path_hint(1, "frontend/src/lib/cluster.ts", code=REAL_CODE)
|
||||
assert "[similar 0.72 · python]" in out["context"]
|
||||
assert "shape of a solution to adapt, not code to copy" in out["context"]
|
||||
# Disclosed, NOT filtered — a stricter bar for foreign hits would suppress
|
||||
# exactly the shape-borrowing this exists for.
|
||||
assert out["note_ids"] == [7]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_same_language_hit_is_not_labelled_and_gets_no_preamble():
|
||||
"""The common case keeps a clean line; the explanation only appears when
|
||||
there is something on the menu it explains."""
|
||||
from scribe.services import plugin_context as pc
|
||||
note = _note(7, "debounce — rate-limit a callback")
|
||||
note.data = {"language": "python"}
|
||||
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", AsyncMock(return_value=[(0.72, note)])), \
|
||||
patch.object(pc, "record_retrieval", MagicMock()), \
|
||||
patch.object(pc, "owner_names_for", AsyncMock(return_value={})):
|
||||
out = await pc.build_write_path_hint(1, "src/scribe/services/x.py", code=REAL_CODE)
|
||||
assert "[similar 0.72]" in out["context"]
|
||||
assert "·" not in out["context"]
|
||||
assert "shape of a solution" not in out["context"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_an_unknown_target_extension_never_invents_a_mismatch():
|
||||
"""A wrong "· python" tag is worse than no tag at all."""
|
||||
from scribe.services import plugin_context as pc
|
||||
note = _note(7, "helper — does a thing")
|
||||
note.data = {"language": "python"}
|
||||
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", AsyncMock(return_value=[(0.72, note)])), \
|
||||
patch.object(pc, "record_retrieval", MagicMock()), \
|
||||
patch.object(pc, "owner_names_for", AsyncMock(return_value={})):
|
||||
out = await pc.build_write_path_hint(1, "scripts/thing.qqq", code=REAL_CODE)
|
||||
assert "· python" not in out["context"]
|
||||
|
||||
|
||||
def test_prior_art_line_keeps_language_and_attribution_together():
|
||||
from scribe.services.plugin_context import _prior_art_line
|
||||
item = {"id": 12, "title": "group_pairs — collapse pairs"}
|
||||
assert _prior_art_line(item, "similar 0.72", None, "python") == \
|
||||
'> - #12 [similar 0.72 · python] "group_pairs — collapse pairs"'
|
||||
both = _prior_art_line(item, "similar 0.72", "alex", "python")
|
||||
assert "· python" in both and "shared by alex" in both
|
||||
|
||||
|
||||
# --- the route between them --------------------------------------------------
|
||||
|
||||
def test_route_reads_every_arg_the_hook_sends():
|
||||
|
||||
Reference in New Issue
Block a user