diff --git a/changes/554.fixed.md b/changes/554.fixed.md new file mode 100644 index 00000000..fbbadcc7 --- /dev/null +++ b/changes/554.fixed.md @@ -0,0 +1 @@ +**Chat now catches a false "I saved it" about a name with a title, like "Dr. Mehta".** Before, the check cut the sentence at "Dr.", so a different saved dentist counted as the same memory and the reply got no note. Now it reads the whole sentence, and when nothing was saved it adds the short note. \ No newline at end of file diff --git a/sentient/agent/outcomes.py b/sentient/agent/outcomes.py index d5b992c7..e9b28ed0 100644 --- a/sentient/agent/outcomes.py +++ b/sentient/agent/outcomes.py @@ -27,6 +27,8 @@ import re from typing import Any +from sentient.memory import origins + log = logging.getLogger(__name__) VERIFY_TIMEOUT_S = 15.0 @@ -532,14 +534,38 @@ def summary(checks: list[dict]) -> str: ) +# A period that does not end the sentence: after a title and its initials ("Dr. M. Mehta"), after "a.m." or "p.m." +# before a lowercase word ("7 a.m. on weekdays") and between "No." and a number ("apartment No. 12"). "St." and "No." +# before a capital can end one ("Main St. I've saved that...", "No. I've saved that..."), so they never glue the next +# sentence on. +_CLOCK = re.compile(r"\b[ap]\.m\.$", re.I) +_NUMBER_SIGN = re.compile(r"\w\s+No\.$") +_TITLE = re.compile(r"\b(?:Dr|Mr|Mrs|Ms|Mx|Prof)\.(?:\s+[A-Z]\.)*$|\b(?:e\.g|i\.e)\.$") +# A name after a title is no keyword: "Dr. May" is not a hedge and "Dr. Page" is not a page. Only the first word keeps +# the place check, so "Dr. May Notes" is still about notes. +_TITLE_NAME = re.compile(r"\b(?:Dr|Mr|Mrs|Ms|Mx|Prof)\.(?:\s+[A-Z]\w*\.?)+") +_TITLE_FIRST = re.compile(r"\b(?:Dr|Mr|Mrs|Ms|Mx|Prof)\.(?:\s+[A-Z]\.)*\s+[A-Z]\w*") + + +def _continues(previous: str, following: str) -> bool: + return bool( + _TITLE.search(previous) + or (_CLOCK.search(previous) and following[:1].islower()) + or (_NUMBER_SIGN.search(previous) and following[:1].isdigit()) + ) + + def memory_claim(text: str) -> str: """The first sentence of ``text`` in which Sentient says it kept something in its memory, or "". Folds curly apostrophes, and skips questions, offers, negations, earlier saves and saves to anything other than memory.""" - for sentence in re.split(r"(?<=[.!?])\s+|\n+", _straight_apostrophes(text)): - s = sentence.strip() - if not s or s.endswith("?") or _MEMORY_HEDGE.search(s) or _MEMORY_EARLIER.search(s): - continue - if _MEMORY_OTHER_PLACE.search(s): + for s in origins.sentences(_straight_apostrophes(text), _continues): + bare = _TITLE_NAME.sub("", s) + if ( + s.endswith("?") + or _MEMORY_HEDGE.search(bare) + or _MEMORY_EARLIER.search(bare) + or _MEMORY_OTHER_PLACE.search(_TITLE_FIRST.sub("", s)) + ): continue cleaned = re.sub(r"\bno (?:problem|worries)\b", "", s, flags=re.I) # "No problem, I've saved that..." for pattern in _MEMORY_CLAIMS: diff --git a/sentient/memory/origins.py b/sentient/memory/origins.py index 2758797c..b3de1ac6 100644 --- a/sentient/memory/origins.py +++ b/sentient/memory/origins.py @@ -17,6 +17,7 @@ import json import os import re +from collections.abc import Callable from typing import Any CONVERSATION = "conversation" @@ -167,14 +168,25 @@ def sources_for(fact: str, first: str, read: list[Read], *, held: bool, user_nam _ABBREVIATION = re.compile(r"(?:\b(?:Dr|Mr|Mrs|Ms|Prof|St|Jr|Sr|vs|etc|No|e\.g|i\.e)|\b[A-Z])\.$") -def quote(text: str, fact: str) -> str: - """The sentence of ``text`` that says most of ``fact`` (its start when none does), short.""" +def _after_abbreviation(previous: str, following: str) -> bool: + return bool(_ABBREVIATION.search(previous)) + + +def sentences(text: str, continues: Callable[[str, str], bool] = _after_abbreviation) -> list[str]: + """``text`` split into sentences, without cutting after an abbreviation: "Dr. Mehta" stays in one. ``continues`` + says whether the piece ``following`` a piece ``previous`` belongs to it.""" parts: list[str] = [] for p in (p.strip() for p in _SENTENCE.split(str(text or ""))): - if p and parts and _ABBREVIATION.search(parts[-1]): # "Dr. Mehta" is one sentence + if p and parts and continues(parts[-1], p): parts[-1] = f"{parts[-1]} {p}" elif p: parts.append(p) + return parts + + +def quote(text: str, fact: str) -> str: + """The sentence of ``text`` that says most of ``fact`` (its start when none does), short.""" + parts = sentences(text) if not parts: return "" want = _words(fact) diff --git a/tests/test_memory_claims.py b/tests/test_memory_claims.py index 681c9491..9c1e0a89 100644 --- a/tests/test_memory_claims.py +++ b/tests/test_memory_claims.py @@ -156,6 +156,34 @@ async def test_a_memory_about_something_else_does_not_back_the_claim(config, iso await s.stop() +@pytest.mark.parametrize(("stored", "claimed"), [("Dr. Kulkarni", "Dr. Mehta"), ("Dr. M. Kulkarni", "Dr. M. Mehta")]) +async def test_a_claim_with_a_title_is_compared_whole(config, isolated_home, stored, claimed): + """Cut at "Dr.", the claim lost the name, so a stored "Dr. Kulkarni" backed a save of "Dr. Mehta" (#554).""" + reply = f"I've saved that your dentist is {claimed}." + llm = FakeProvider(replies=[reply]) + s = await _app(config, isolated_home, llm, "dentist") + try: + await s.memory.remember(f"Maya's dentist is {stored}", use_llm=False) + sid = await s.store.create_session(channel="cli") + text, _ = await _turn(s, sid, f"remember that my dentist is {claimed}") + assert text == reply + NOTE + finally: + await s.stop() + + +async def test_a_claim_with_a_title_that_a_memory_backs_is_left_alone(config, isolated_home): + reply = "I've saved that your dentist is Dr. Mehta." + llm = FakeProvider(replies=[reply]) + s = await _app(config, isolated_home, llm, "dentist_saved") + try: + await s.memory.remember("Maya's dentist is Dr. Mehta", use_llm=False) + sid = await s.store.create_session(channel="cli") + text, _ = await _turn(s, sid, "remember that my dentist is Dr. Mehta") + assert text == reply + finally: + await s.stop() + + async def test_a_copied_correction_gets_no_second_one(config, isolated_home): reply = "I've saved that you wake up at 7 a.m." + NOTE llm = FakeProvider(replies=[reply]) @@ -254,4 +282,30 @@ def test_memory_claims_are_recognized_narrowly(): assert outcomes.memory_claim("Morning routines matter.\n\nI've saved that you wake up at 7.") == ( "I've saved that you wake up at 7." ) + # an abbreviation does not cut the claim short (#554), and a question after it is not part of it + assert outcomes.memory_claim("Sure. I've saved that your dentist is Dr. Mehta. Anything else?") == ( + "I've saved that your dentist is Dr. Mehta." + ) + for name in ("May", "Page", "Brian May"): # a surname that is also a hedge or a place word is still a name + assert outcomes.memory_claim(f"I've saved that your dentist is Dr. {name}.") == ( + f"I've saved that your dentist is Dr. {name}." + ) + assert not outcomes.memory_claim("I've saved that your dentist is Dr. Mehta?") + assert not outcomes.memory_claim("Should I save that your dentist is Dr. Mehta?") + # "St." and "No." can end a sentence, so they never glue the next one on + assert outcomes.memory_claim("I've saved that you live on Main St. Anything else?") == ( + "I've saved that you live on Main St." + ) + assert outcomes.memory_claim("You live on Main St. I've saved that you wake up at 7.") == ( + "I've saved that you wake up at 7." + ) + assert outcomes.memory_claim("No. I've saved that you like tea.") == "I've saved that you like tea." + # but a lowercase word, a number after "No." or the initials of a name continue the sentence + for claim in ( + "I've saved that your dentist is Dr. M. Mehta.", + "I've saved that you live at apartment No. 12.", + "I've saved that you wake up at 7 a.m. on weekdays.", + ): + assert outcomes.memory_claim(claim) == claim + assert not outcomes.memory_claim("I've saved your Dr. May Notes.") # still a save to notes, not to memory assert outcomes.unmade_memory_claim("I've saved that you like tea.", [{"ok": False}, {"ok": True}]) == ""