diff --git a/src/grimoire/services/plugin_context.py b/src/grimoire/services/plugin_context.py index ea077979..b111f7e3 100644 --- a/src/grimoire/services/plugin_context.py +++ b/src/grimoire/services/plugin_context.py @@ -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( diff --git a/tests/test_autoinject_context.py b/tests/test_autoinject_context.py index c8eb006c..a11a4091 100644 --- a/tests/test_autoinject_context.py +++ b/tests/test_autoinject_context.py @@ -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: diff --git a/tests/test_named_records.py b/tests/test_named_records.py index 71fb9d2e..2e2e8830 100644 --- a/tests/test_named_records.py +++ b/tests/test_named_records.py @@ -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") diff --git a/tests/test_retrieval_scopes.py b/tests/test_retrieval_scopes.py index a0accc7c..a827d202 100644 --- a/tests/test_retrieval_scopes.py +++ b/tests/test_retrieval_scopes.py @@ -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)}