dev → main: rule-arm review, moment usage by rule, no-subject auto-inject, copy check, S3 test wording #215
@@ -402,11 +402,13 @@ _CONCEPT_MIN_CHARS = 16
|
||||
_AUTOINJECT_MAX_TOP_K = MAX_BUDGET
|
||||
|
||||
# THE CONVERSATION BESIDE THE PROMPT. The operator's message alone is a weak
|
||||
# query mid-session: it is mostly a follow-up — "yes do that", "now fix the
|
||||
# filter" — that names nothing a record could match. The hook sends the tail of the last assistant reply as `context`, and it is
|
||||
# appended to a SHORT prompt only: a prompt that already says what it is about
|
||||
# is the better query on its own, and diluting it is the one way this can make
|
||||
# retrieval worse.
|
||||
# query mid-session: it is mostly a follow-up — "now fix the filter" — that
|
||||
# names too little for a record to match. The hook sends the tail of the last
|
||||
# assistant reply as `context`, and it is appended to a SHORT prompt only: a
|
||||
# prompt that already says what it is about is the better query on its own,
|
||||
# and diluting it is the one way this can make retrieval worse. A prompt that
|
||||
# names nothing at all ("yes do that") gets no notes menu (`_has_subject`);
|
||||
# the prompt rule arm still searches it with the tail.
|
||||
#
|
||||
# Neither number costs a model token. The query is embedding input; what the
|
||||
# session pays for is the menu, and the budget bounds that unchanged.
|
||||
@@ -418,6 +420,42 @@ _AUTOINJECT_CONTEXT_PROMPT_MAX = 280
|
||||
_AUTOINJECT_CONTEXT_MAX = 600
|
||||
|
||||
|
||||
# WORDS THAT NAME NO SUBJECT. A prompt made only of these ("keep going", "yes
|
||||
# please do", "1. yes 2. yes", "is it deployed?", "what is next?") is a turn of
|
||||
# the conversation, not a question about anything. Its query would be the
|
||||
# previous reply's tail alone, and the menu would hand back the records that
|
||||
# reply was built from, which the session already has in context. One word
|
||||
# outside this list ("now fix the filter") is a subject, and the prompt is
|
||||
# searched as usual.
|
||||
_NO_SUBJECT_WORDS = frozenset("""
|
||||
a about above after again agreed ahead all already also am an and any
|
||||
anything approved are as asked at awesome back be been before both build
|
||||
but by can carry ci commit continue continuing could cool correct
|
||||
deploy deployed described did do does done each else every everything
|
||||
fine finish first fix for from get go going good got great green had has
|
||||
have he here hold how i if im in instead into is it its just keep know
|
||||
last left let lets like looks loose lgtm make may maybe me mentioned merge
|
||||
milestone more move my next nice no not now of off ok okay on one open or
|
||||
order other our out over pass passes passing perfect please previous
|
||||
proceed project proposed push ready really recommended redeploy rest right
|
||||
said same see she ship should so some sounds stack stacking start step
|
||||
steps still stop suggested sure task tasks thank thanks that the them then
|
||||
there these they thing things this those through to try up us waiting
|
||||
want was we well were what whats when where which while who why will with
|
||||
work would yeah yep yes yet you your
|
||||
""".split())
|
||||
|
||||
|
||||
def _has_subject(prompt: str) -> bool:
|
||||
"""Does the operator's message name anything a record could be about?
|
||||
|
||||
Numbers are left out: a record named by number is looked up by id, not
|
||||
matched, so it never needs the ranked menu to be found.
|
||||
"""
|
||||
words = re.findall(r"[a-z]+", prompt.lower().replace("'", ""))
|
||||
return any(w not in _NO_SUBJECT_WORDS for w in words)
|
||||
|
||||
|
||||
def _autoinject_query(prompt: str, context: str) -> str:
|
||||
"""The prompt, with recent conversation appended when the prompt is thin.
|
||||
|
||||
@@ -612,10 +650,6 @@ async def build_autoinject_hint(
|
||||
# left to a ranking that can only match the words around them.
|
||||
named = await _named_records(user_id, record_refs_svc.named_record_ids(q))
|
||||
named_ids = [int(n.id) for n in named]
|
||||
# Everything below searches, logs and fills its slots on the ENRICHED
|
||||
# query, so the telemetry row records what was actually asked.
|
||||
q = _autoinject_query(q, context)
|
||||
|
||||
# THE LEDGER IS A RENDERING FACT, NOT A RETRIEVAL ONE. Excluding seen ids
|
||||
# from the search would make the second time a note is the best answer
|
||||
# SILENCE, indistinguishable from "nothing matched", and would measure
|
||||
@@ -629,15 +663,23 @@ async def build_autoinject_hint(
|
||||
# lesson slots in that order, and the surfacing rows are written there.
|
||||
# What is decided HERE is what makes this moment this moment — the query
|
||||
# the operator's words became, and the records they named by number.
|
||||
result = await rp.run_note_arm(
|
||||
rp.AUTO_INJECT,
|
||||
rp.NoteMoment(
|
||||
user_id=user_id, query=q, project_id=project_id,
|
||||
seen=frozenset(already), named=frozenset(named_ids),
|
||||
),
|
||||
floor=cfg["threshold"], budget=cfg["top_k"], io=_note_io(),
|
||||
)
|
||||
kept = result.menu
|
||||
if not _has_subject(q):
|
||||
# Nothing to search for: the menu would answer the previous reply,
|
||||
# not the operator. Records named by number are still shown.
|
||||
kept, chunks = [], {}
|
||||
else:
|
||||
# Everything below searches, logs and fills its slots on the ENRICHED
|
||||
# query, so the telemetry row records what was actually asked.
|
||||
q = _autoinject_query(q, context)
|
||||
result = await rp.run_note_arm(
|
||||
rp.AUTO_INJECT,
|
||||
rp.NoteMoment(
|
||||
user_id=user_id, query=q, project_id=project_id,
|
||||
seen=frozenset(already), named=frozenset(named_ids),
|
||||
),
|
||||
floor=cfg["threshold"], budget=cfg["top_k"], io=_note_io(),
|
||||
)
|
||||
kept, chunks = result.menu, result.chunks
|
||||
if not kept and not named:
|
||||
return empty
|
||||
|
||||
@@ -739,7 +781,7 @@ async def build_autoinject_hint(
|
||||
# a reason, and a reader cannot tell the two apart once indented alike.
|
||||
passage = "" if nid in already else _menu_passage(
|
||||
document_title(note.title, note.note_type, note.data, note.body),
|
||||
(result.chunks.get(nid) or {}).get("text"), name,
|
||||
(chunks.get(nid) or {}).get("text"), name,
|
||||
)
|
||||
who = _shared_by(note)
|
||||
lines.extend(menu_entry(
|
||||
|
||||
@@ -20,6 +20,7 @@ from grimoire.services.plugin_context import (
|
||||
_AUTOINJECT_CONTEXT_MAX,
|
||||
_AUTOINJECT_CONTEXT_PROMPT_MAX,
|
||||
_autoinject_query,
|
||||
_has_subject,
|
||||
_menu_name,
|
||||
_menu_passage,
|
||||
)
|
||||
@@ -56,6 +57,50 @@ def test_context_is_cut_from_the_end_of_the_reply():
|
||||
assert tail.endswith("TAIL") and "HEAD" not in tail
|
||||
|
||||
|
||||
# --- a prompt that names nothing ----------------------------------------------
|
||||
|
||||
@pytest.mark.parametrize("prompt", [
|
||||
"keep going", "yes please do", "1. yes 2. yes 3. yes", "start step 6",
|
||||
"is it deployed?", "awesome, what's next?", "go ahead",
|
||||
"is there anything else open in this project?",
|
||||
"please commit and let me know when it passes CI",
|
||||
])
|
||||
def test_a_turn_of_the_conversation_names_no_subject(prompt):
|
||||
assert not _has_subject(prompt)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("prompt", [
|
||||
"now fix the filter", "keep steward, it's our own app",
|
||||
"continue into DRY 14", "I've added a maintenance plan",
|
||||
"fix the Windows gate first, then step 2",
|
||||
])
|
||||
def test_one_word_of_its_own_is_a_subject(prompt):
|
||||
assert _has_subject(prompt)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_prompt_with_no_subject_gets_no_menu_and_no_search():
|
||||
"""Its query would be the previous reply's tail alone, and the menu would
|
||||
hand back what that reply was built from."""
|
||||
from grimoire.services import plugin_context as pc
|
||||
|
||||
search = AsyncMock(return_value=[(0.9, fake_note(id=11, title="the reply's own record", user_id=1))])
|
||||
retrieval = MagicMock()
|
||||
with patch.object(pc, "get_autoinject_config",
|
||||
AsyncMock(return_value={"enabled": True, "threshold": 0.55, "top_k": 3})), \
|
||||
patch.object(pc, "semantic_search_notes", search), \
|
||||
patch.object(pc, "record_retrieval", retrieval), \
|
||||
patch.object(pc, "record_surfaced", MagicMock()):
|
||||
out = await pc.build_autoinject_hint(
|
||||
1, "yes please do", project_id=2,
|
||||
context="I'll move the library filters into the side column.",
|
||||
)
|
||||
|
||||
assert out["context"] == "" and out["note_ids"] == []
|
||||
search.assert_not_called()
|
||||
retrieval.assert_not_called()
|
||||
|
||||
|
||||
# --- what the hook extracts --------------------------------------------------
|
||||
|
||||
def _rec(role: str, blocks: list[dict], sidechain: bool = False) -> str:
|
||||
|
||||
@@ -139,7 +139,7 @@ async def test_a_named_record_comes_first_and_is_not_repeated_as_a_match():
|
||||
other = fake_note(id=11, title="something that matched the words", user_id=1)
|
||||
hits = [(0.80, named), (0.78, other)]
|
||||
out, surfaced, retrieval = await _hint(
|
||||
"go ahead with 4448", hits=hits, records={4448: named},
|
||||
"go ahead with the filing fix in 4448", hits=hits, records={4448: named},
|
||||
)
|
||||
|
||||
ctx = out["context"]
|
||||
@@ -156,6 +156,23 @@ async def test_a_named_record_comes_first_and_is_not_repeated_as_a_match():
|
||||
assert row["suppressed"] == 1
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_prompt_with_no_subject_still_shows_what_it_names():
|
||||
""""go ahead with 4448" names a record and nothing else: the record is
|
||||
looked up and shown, and no ranked menu is searched for."""
|
||||
rec = _task(4448, "the named one")
|
||||
other = fake_note(id=11, title="what the reply was about", user_id=1)
|
||||
out, surfaced, retrieval = await _hint(
|
||||
"go ahead with 4448", hits=[(0.8, other)], records={4448: rec},
|
||||
context="I'll start on the importer next.",
|
||||
)
|
||||
|
||||
assert out["note_ids"] == [4448]
|
||||
assert "Possibly relevant" not in out["context"]
|
||||
assert "auto_inject" not in surfaced
|
||||
retrieval.assert_not_called()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_named_record_already_shown_is_a_pointer():
|
||||
rec = _task(4448, "the named one", "a long body that is already in context")
|
||||
|
||||
@@ -127,7 +127,7 @@ async def test_injected_menu_attributes_another_users_note():
|
||||
patch.object(plugin_context, "record_retrieval", MagicMock()), \
|
||||
patch.object(plugin_context, "owner_names_for",
|
||||
AsyncMock(return_value={9: "alex"})):
|
||||
out = await plugin_context.build_autoinject_hint(7, "anything")
|
||||
out = await plugin_context.build_autoinject_hint(7, "debounce helper")
|
||||
|
||||
lines = out["context"].splitlines()
|
||||
theirs_line = next(ln for ln in lines if "#2" in ln)
|
||||
@@ -160,7 +160,7 @@ async def test_injected_menu_labels_the_record_kind():
|
||||
AsyncMock(return_value={"enabled": True, "threshold": 0.5,
|
||||
"top_k": 5})), \
|
||||
patch.object(plugin_context, "record_retrieval", MagicMock()):
|
||||
out = await plugin_context.build_autoinject_hint(7, "anything")
|
||||
out = await plugin_context.build_autoinject_hint(7, "debounce helper")
|
||||
|
||||
lines = out["context"].splitlines()
|
||||
by_id = {n: next(ln for ln in lines if f"#{n} " in ln) for n in (1, 2, 3, 4, 5)}
|
||||
|
||||
Reference in New Issue
Block a user