Skip to content

Commit 80ef132

Browse files
committed
feat(memory): routing-guard advisory and index-based entry locator
Add a structural detector that nudges the agent toward editing the authoritative project file when a memory write looks like a rule or value-assignment that belongs in a file (a recurring failure mode: a correction rephrased as a "preference" and stored in memory while the governing file stays stale). The guard never blocks — it only appends a one-line advisory on add/replace — so false positives cost a sentence. Also let replace/remove identify an entry by 0-based `index` (from `list`) as a deterministic alternative to `old_text` substring matching; out-of-range indices report the inventory so the retry is guided. Slash completer now surfaces the matched alias as the menu label (`/res` -> `/resume`) while keeping name matches ranked above alias-only matches. memory.md guidance updated for authoritative-files-first and the new index-based locator.
1 parent b69177e commit 80ef132

9 files changed

Lines changed: 365 additions & 37 deletions

File tree

src/pythinker_code/project_memory.py

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -275,11 +275,28 @@ def _match_one(entries: list[str], old_text: str) -> int | MemoryOpResult:
275275
)
276276
return matches[0]
277277

278-
async def replace(self, target: Target, old_text: str, new_content: str) -> MemoryOpResult:
278+
def _locate(self, entries: list[str], old_text: str, index: int | None) -> int | MemoryOpResult:
279+
"""Resolve which entry to mutate. ``index`` (0-based, from `list`) is the
280+
deterministic path — preferred when substring matching is uncertain;
281+
``old_text`` is the substring fallback. Out-of-range indices report the
282+
inventory so the retry is guided, not a guess."""
283+
if index is not None:
284+
if not 0 <= index < len(entries):
285+
return MemoryOpResult(
286+
False,
287+
f"No entry at index {index} ({len(entries)} stored). "
288+
f"Current entries:\n{self._inventory(entries)}",
289+
)
290+
return index
291+
if old_text:
292+
return self._match_one(entries, old_text)
293+
return MemoryOpResult(False, "Provide old_text or index to identify the entry.")
294+
295+
async def replace(
296+
self, target: Target, old_text: str, new_content: str, *, index: int | None = None
297+
) -> MemoryOpResult:
279298
old_text = old_text.strip()
280299
new_content = new_content.strip()
281-
if not old_text:
282-
return MemoryOpResult(False, "old_text cannot be empty.")
283300
if not new_content:
284301
return MemoryOpResult(False, "new_content cannot be empty. Use 'remove' to delete.")
285302
blocked = scan_memory_content(new_content)
@@ -293,7 +310,7 @@ async def replace(self, target: Target, old_text: str, new_content: str) -> Memo
293310
return MemoryOpResult(
294311
False, f"Memory read failed ({exc}); aborting write to avoid data loss."
295312
)
296-
idx = self._match_one(entries, old_text)
313+
idx = self._locate(entries, old_text, index)
297314
if isinstance(idx, MemoryOpResult):
298315
return idx
299316
limit = self._char_limit(target)
@@ -312,10 +329,10 @@ async def replace(self, target: Target, old_text: str, new_content: str) -> Memo
312329
await self._write_entries(target, candidate)
313330
return MemoryOpResult(True, "Entry replaced.")
314331

315-
async def remove(self, target: Target, old_text: str) -> MemoryOpResult:
332+
async def remove(
333+
self, target: Target, old_text: str, *, index: int | None = None
334+
) -> MemoryOpResult:
316335
old_text = old_text.strip()
317-
if not old_text:
318-
return MemoryOpResult(False, "old_text cannot be empty.")
319336
path = await self._path_for(target)
320337
async with self._async_lock, self._file_lock(path):
321338
try:
@@ -324,7 +341,7 @@ async def remove(self, target: Target, old_text: str) -> MemoryOpResult:
324341
return MemoryOpResult(
325342
False, f"Memory read failed ({exc}); aborting write to avoid data loss."
326343
)
327-
idx = self._match_one(entries, old_text)
344+
idx = self._locate(entries, old_text, index)
328345
if isinstance(idx, MemoryOpResult):
329346
return idx
330347
entries.pop(idx)

src/pythinker_code/tools/memory/__init__.py

Lines changed: 44 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,20 @@
66

77
from pythinker_code.project_memory import ProjectMemoryStore
88
from pythinker_code.soul.agent import Runtime
9+
from pythinker_code.tools.memory.routing_guard import routing_signals
910
from pythinker_code.tools.utils import load_desc
11+
from pythinker_code.utils.logging import logger
12+
13+
# Appended (never substituted) to a successful add/replace whose content looks
14+
# like it governs behavior defined in a project file. The tool is blind to
15+
# conversation context, so it can only nudge — enforcement lives in memory.md
16+
# guidance and the model's own awareness of the correction it just received.
17+
_ROUTING_ADVISORY = (
18+
"\n\nNote: this looks like it may set a rule that lives in a project file "
19+
"(a protocol, spec, or config you read). If so, edit that file directly — "
20+
"memory does not change files you follow, so the change would be silently "
21+
"lost. Memory is only for durable facts with no authoritative home."
22+
)
1023

1124

1225
class Params(BaseModel):
@@ -16,6 +29,11 @@ class Params(BaseModel):
1629
old_text: str | None = Field(
1730
default=None, description="Unique substring identifying the entry to replace/remove."
1831
)
32+
index: int | None = Field(
33+
default=None,
34+
description="0-based index of the entry to replace/remove (from `list`). "
35+
"Deterministic alternative to old_text — prefer it when a substring match is uncertain.",
36+
)
1937

2038

2139
class Memory(CallableTool2[Params]):
@@ -38,9 +56,14 @@ async def __call__(self, params: Params) -> ToolReturnValue:
3856
return ToolError(
3957
message="content is required for add/replace.", brief="memory: no content"
4058
)
41-
if params.action in ("replace", "remove") and not (params.old_text or "").strip():
59+
if (
60+
params.action in ("replace", "remove")
61+
and not (params.old_text or "").strip()
62+
and params.index is None
63+
):
4264
return ToolError(
43-
message="old_text is required for replace/remove.", brief="memory: no old_text"
65+
message="old_text or index is required for replace/remove.",
66+
brief="memory: no locator",
4467
)
4568

4669
if params.action == "list":
@@ -51,10 +74,12 @@ async def __call__(self, params: Params) -> ToolReturnValue:
5174
result = await self._store.add(params.target, params.content or "")
5275
elif params.action == "replace":
5376
result = await self._store.replace(
54-
params.target, params.old_text or "", params.content or ""
77+
params.target, params.old_text or "", params.content or "", index=params.index
5578
)
5679
else:
57-
result = await self._store.remove(params.target, params.old_text or "")
80+
result = await self._store.remove(
81+
params.target, params.old_text or "", index=params.index
82+
)
5883

5984
if not result.ok:
6085
message = result.message
@@ -72,4 +97,18 @@ async def __call__(self, params: Params) -> ToolReturnValue:
7297
rearm = getattr(self._runtime, "rearm_injection", None)
7398
if rearm is not None:
7499
rearm("project_memory")
75-
return ToolOk(output=result.message, message=result.message, brief=params.action)
100+
message = result.message
101+
if params.action in ("add", "replace"):
102+
signals = routing_signals(params.content or "")
103+
if signals:
104+
logger.bind(
105+
event="memory_guard_advisory",
106+
action=params.action,
107+
target=params.target,
108+
signals=signals,
109+
).debug(
110+
"memory write tripped routing advisory: {preview}",
111+
preview=(params.content or "")[:60],
112+
)
113+
message += _ROUTING_ADVISORY
114+
return ToolOk(output=message, message=message, brief=params.action)

src/pythinker_code/tools/memory/memory.md

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,20 +5,33 @@ WRITE FACTS, NOT INSTRUCTIONS:
55
- "Project uses pytest with xdist" ✓ — "Run tests with pytest -n 4" ✗
66
- "User prefers concise answers" ✓ — "Always respond concisely" ✗
77

8+
AUTHORITATIVE FILES FIRST:
9+
If a fact would change how you already behave according to a file you read (AGENTS.md, a
10+
protocol/spec/config), EDIT that file — do not store it here. Memory does not change files
11+
you follow, so the change would be silently lost. Rephrasing a correction as a preference
12+
("set the limit to 100" → "user prefers a 100-word limit") does NOT turn a file edit into a
13+
memory fact. When the user corrects a rule, find and edit the file that governs it first.
14+
815
TWO TARGETS:
916
- `memory`: project facts — conventions, architecture notes, gotchas, key file locations.
1017
- `user`: how the user likes to work in this repo — preferences, style, do/don't.
1118

1219
DO NOT store: task progress, session outcomes, "fixed bug X / merged PR Y /
1320
Phase N done", PR/issue numbers, commit SHAs, file counts, or anything stale
1421
within a week. Those are transient and belong in the session journal, not memory.
15-
NEVER store secrets, tokens, or credentials.
22+
Store only non-obvious facts — deviations, gotchas, conventions — not baseline
23+
behavior any agent would assume. Store rules the user actually stated, never ones
24+
you inferred from conversation fragments. NEVER store secrets, tokens, or credentials.
1625

1726
ACTIONS:
1827
- `add`: append a new entry (requires `content`).
19-
- `replace`: update an entry (`old_text` = a unique substring of the target entry; `content` = new text).
20-
- `remove`: delete an entry (`old_text` = a unique substring of the target entry).
21-
- `list`: show current entries with their sizes and how much room is free (read-only).
28+
- `replace`: update an entry — identify it by `index` or `old_text`; `content` = new text.
29+
- `remove`: delete an entry — identify it by `index` or `old_text`.
30+
- `list`: show current entries, each with its `[index]`, size, and how much room is free (read-only).
31+
32+
IDENTIFYING AN ENTRY: prefer `index` (the `[N]` shown by `list`) — it is exact. Use `old_text`
33+
(a unique substring) only when you are sure of the literal stored text. If a remove/replace
34+
fails to match, run `list` and retry with the `index` rather than guessing the substring again.
2235

2336
WHEN A WRITE IS REJECTED FOR SPACE:
2437
The error reports the exact free budget and lists existing entries. Do NOT retry the
Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
"""Structural detector for memory writes that probably belong in a file.
2+
3+
A recurring failure mode: the user *corrects* a rule that lives in an
4+
authoritative project file (a protocol/spec/config the agent reads), and the
5+
agent — to satisfy the "write facts, not instructions" rule — rephrases the
6+
correction as a "preference" and stores it in memory. The file stays stale, the
7+
correction is silently lost, and memory bloats with directives.
8+
9+
The tool is blind to conversation context, so it cannot *know* whether a
10+
governing file exists. It can only spot the structural shape of a correction and
11+
*advise*. This module is that detector: pure, deterministic, no project
12+
knowledge, no I/O. The tool layer appends a one-line nudge when a signal trips;
13+
it never blocks, so false positives cost nothing but a sentence of output.
14+
"""
15+
16+
from __future__ import annotations
17+
18+
import re
19+
20+
# A token that looks like a config/doc filename. High precision: if the entry
21+
# names such a file, that file — not memory — is the source of truth.
22+
_FILE_REF = re.compile(
23+
r"\b[\w./-]+\.(?:md|markdown|ya?ml|json|toml|ini|cfg|conf|txt|env)\b",
24+
re.IGNORECASE,
25+
)
26+
27+
# Value-assignment grammar: a directive verb steering a number ("set ... to 100",
28+
# "raise ... 12"). Bounded gap so it stays a local phrase, not a whole paragraph.
29+
_DIRECTIVE_VERB = re.compile(
30+
r"\b(?:set|change|update|increase|decrease|raise|lower|bump|cap|adjust)\b[^.\n]{0,40}\d+",
31+
re.IGNORECASE,
32+
)
33+
34+
# A limit/threshold word adjacent to a number, in either order. This is what
35+
# catches the rephrased-as-preference evasion ("limit of 100").
36+
_LIMIT_NUM = re.compile(
37+
r"\b(?:limit|max(?:imum)?|min(?:imum)?|threshold|cap|quota)\b[^.\n]{0,20}\d+"
38+
r"|\d+[^.\n]{0,20}\b(?:limit|max(?:imum)?|min(?:imum)?|threshold|cap|quota)\b",
39+
re.IGNORECASE,
40+
)
41+
42+
# Prescriptive imperatives — the grammar of a rule rather than a fact.
43+
_IMPERATIVE = re.compile(r"\b(?:always|never|must)\b", re.IGNORECASE)
44+
45+
46+
def routing_signals(content: str) -> list[str]:
47+
"""Return the structural signals tripped by ``content``, in stable order.
48+
49+
``"file_ref"`` — the entry names a config/doc file (that file is the likely
50+
source of truth). ``"directive"`` — the entry reads like a rule or a
51+
value-assignment rather than a standing fact. Empty list means nothing
52+
tripped. Order is deterministic (``file_ref`` before ``directive``) and each
53+
signal appears at most once, so callers can log it verbatim.
54+
"""
55+
text = content or ""
56+
signals: list[str] = []
57+
if _FILE_REF.search(text):
58+
signals.append("file_ref")
59+
if _DIRECTIVE_VERB.search(text) or _LIMIT_NUM.search(text) or _IMPERATIVE.search(text):
60+
signals.append("directive")
61+
return signals

src/pythinker_code/ui/shell/prompt.py

Lines changed: 25 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -401,14 +401,15 @@ def get_completions(
401401
typed_lower = typed.lower()
402402
seen: set[str] = set()
403403

404-
def emit(cmd: SlashCommand[Any]) -> Iterable[Completion]:
404+
def emit(cmd: SlashCommand[Any], label: str | None = None) -> Iterable[Completion]:
405405
if cmd.name in seen:
406406
return
407407
seen.add(cmd.name)
408+
shown = label or cmd.name
408409
yield Completion(
409-
text=f"/{cmd.name}",
410+
text=f"/{shown}",
410411
start_position=-len(token),
411-
display=f"/{cmd.name}",
412+
display=f"/{shown}",
412413
display_meta=self._display_meta(cmd),
413414
)
414415

@@ -417,35 +418,39 @@ def emit(cmd: SlashCommand[Any]) -> Iterable[Completion]:
417418
yield from emit(cmd)
418419
return
419420

420-
def match_tier(cmd: SlashCommand[Any]) -> int | None:
421-
"""Lower tier = stronger match. Name matches rank above alias matches
422-
so typing toward a command name (e.g. ``/report`` → ``/reports``)
423-
wins over a command that only matches via an exact alias."""
421+
def match_tier(cmd: SlashCommand[Any]) -> tuple[int, str] | None:
422+
"""Return ``(tier, label)`` or ``None``. Lower tier = stronger match.
423+
Name matches rank above alias matches so typing toward a command name
424+
(e.g. ``/report`` → ``/reports``) wins over a command that only matches
425+
via an exact alias. For alias-only matches the label is the matched
426+
alias, so the menu surfaces what the user typed toward (e.g. ``/res``
427+
→ ``/resume``) rather than the differently-named command (``/sessions``)."""
424428
name_lower = cmd.name.lower()
425429
if name_lower == typed_lower:
426-
return 0
430+
return (0, cmd.name)
427431
if name_lower.startswith(typed_lower):
428-
return 1
429-
alias_prefix = False
432+
return (1, cmd.name)
433+
alias_prefix: str | None = None
430434
for alias in cmd.aliases:
431435
alias_lower = alias.lower()
432436
if alias_lower == typed_lower:
433-
return 2
434-
if alias_lower.startswith(typed_lower):
435-
alias_prefix = True
436-
return 3 if alias_prefix else None
437+
return (2, alias)
438+
if alias_prefix is None and alias_lower.startswith(typed_lower):
439+
alias_prefix = alias
440+
return (3, alias_prefix) if alias_prefix is not None else None
437441

438442
# Rank by (match tier, command-name length, name): the closest, shortest
439443
# command name surfaces first within each tier.
440-
matched: list[tuple[int, int, str, SlashCommand[Any]]] = []
444+
matched: list[tuple[int, int, str, str, SlashCommand[Any]]] = []
441445
for cmd in self._available_commands:
442-
tier = match_tier(cmd)
443-
if tier is not None:
444-
matched.append((tier, len(cmd.name), cmd.name, cmd))
446+
result = match_tier(cmd)
447+
if result is not None:
448+
tier, label = result
449+
matched.append((tier, len(cmd.name), cmd.name, label, cmd))
445450
matched.sort(key=lambda item: (item[0], item[1], item[2]))
446451

447-
for _, _, _, cmd in matched:
448-
yield from emit(cmd)
452+
for _, _, _, label, cmd in matched:
453+
yield from emit(cmd, label)
449454

450455
def _disabled_during_task(self, cmd: SlashCommand[Any]) -> bool:
451456
"""True when a running turn blocks this shell-level command."""

tests/core/test_project_memory.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,36 @@ async def test_remove_deletes_matching_entry(tmp_path, monkeypatch):
239239
assert not r.ok and "No entry matched" in r.message
240240

241241

242+
async def test_remove_by_index(tmp_path, monkeypatch):
243+
"""An entry can be deleted by its 0-based index (from `list`) without
244+
guessing a substring — the fix for stale/imprecise old_text matches."""
245+
store = _store(tmp_path, monkeypatch)
246+
await store.add("memory", "uses pytest")
247+
await store.add("memory", "uses ruff")
248+
249+
r = await store.remove("memory", "", index=0)
250+
assert r.ok and await store.read_entries("memory") == ["uses ruff"]
251+
252+
253+
async def test_replace_by_index(tmp_path, monkeypatch):
254+
store = _store(tmp_path, monkeypatch)
255+
await store.add("memory", "uses pytest")
256+
await store.add("memory", "uses ruff")
257+
258+
r = await store.replace("memory", "", "uses ruff + biome", index=1)
259+
assert r.ok and await store.read_entries("memory") == ["uses pytest", "uses ruff + biome"]
260+
261+
262+
async def test_index_out_of_range_errors_with_inventory(tmp_path, monkeypatch):
263+
store = _store(tmp_path, monkeypatch)
264+
await store.add("memory", "uses pytest")
265+
266+
r = await store.remove("memory", "", index=5)
267+
assert not r.ok
268+
assert "index 5" in r.message
269+
assert "uses pytest" in r.message # inventory guides the retry
270+
271+
242272
async def test_snapshot_builds_block_with_priority_and_budget(tmp_path, monkeypatch):
243273
monkeypatch.setenv("PYTHINKER_SHARE_DIR", str(tmp_path / "share"))
244274
from pythinker_code.project_memory import ProjectMemoryStore

0 commit comments

Comments
 (0)