From f69e93c90a831884adc69f5b3dea6323face6963 Mon Sep 17 00:00:00 2001 From: aditya Date: Thu, 30 Jul 2026 18:03:54 +0530 Subject: [PATCH] Add reviewed memory correction path --- README.md | 192 ++++++++++++++++++++++++++++++++++++++ s13code/routes.py | 7 +- s13code/runtime.py | 57 +++++++++-- tests/test_s13_runtime.py | 87 +++++++++++++++++ 4 files changed, 333 insertions(+), 10 deletions(-) diff --git a/README.md b/README.md index 7cf7872..97df5f4 100644 --- a/README.md +++ b/README.md @@ -124,6 +124,198 @@ Add one subsection to this README in the same pull request. It must contain: Do not commit `.env`, credentials, personal memory, generated databases, unrestricted local paths, benchmark output containing private data, or provider responses containing secrets. Use synthetic identities in every proof. +## Student contribution: reviewed memory corrections + +This PR adds a reviewed correction path for durable memory. When a user says `Correction:` with a supported fact key, the runtime canonicalizes the new fact, finds the matching current fact inside the same memory scope, writes the replacement with `supersedes_id`, preserves the old record as `superseded`, and keeps stale same-key episode text out of answer evidence. If the correction is ambiguous, the graph records `review_required` and does not promote a fact. Synthetic identities only: tenant `acme`, project `family`, user `rohan`. + +### Exact API requests + +```json +{ + "write": { + "tenant_id": "acme", + "project_id": "family", + "user_id": "rohan", + "prompt": "My mom's birthday is 15 May 2026. Remember that." + }, + "correct": { + "tenant_id": "acme", + "project_id": "family", + "user_id": "rohan", + "prompt": "Correction: my mom's birthday is 16 May 2026. Remember that." + }, + "recall": { + "tenant_id": "acme", + "project_id": "family", + "user_id": "rohan", + "prompt": "When is mom's birthday?" + }, + "cross_scope_attack": { + "tenant_id": "beta", + "project_id": "family", + "user_id": "rohan", + "prompt": "Correction: my mom's birthday is 17 May 2026. Remember that." + }, + "ambiguous_attack": { + "tenant_id": "acme", + "project_id": "family", + "user_id": "rohan", + "prompt": "Correction: replace it with next Friday. Remember that." + } +} +``` + +### Extension graph and trace + +The correction write produced: + +```json +{ + "new_text": "Mom's birthday is 16 May 2026.", + "supersedes_id_matches_old": true, + "metadata": { + "promotion": "explicit_user_request", + "correction": true, + "fact_key": "mom_birthday" + } +} +``` + +The recall run graph was `recall -> answer`. Ordered event trace: + +```text +23 run_started +24 graph_patched add=['recall'] connect=[] finish=False +25 task_started:recall +26 task_succeeded:recall +27 graph_patched add=['answer'] connect=[['recall', 'answer']] finish=False +28 task_started:answer +29 task_succeeded:answer +30 graph_patched add=[] connect=[] finish=True +``` + +Actual final result: + +```text +You told me it is 16 May 2026. [source: api://agent/runs] +``` + +Evidence and provider/agent assignments: + +```json +{ + "history": [ + { + "text": "Mom's birthday is 16 May 2026.", + "status": "current", + "supersedes_id": "", + "sources": ["api://agent/runs"] + }, + { + "text": "Mom's birthday is 15 May 2026.", + "status": "superseded", + "supersedes_id": null, + "sources": ["api://agent/runs"] + } + ], + "agents": { + "recall": { + "agent": "memory_recall", + "skill": "memory_recall", + "state": "succeeded", + "provider": null, + "model": null + }, + "answer": { + "agent": "answer_with_evidence", + "skill": "answer_with_evidence", + "state": "succeeded", + "provider": "deterministic_gateway", + "model": "stub-v1" + } + } +} +``` + +### Floor reproduction + +Test suite: + +```text +$ uv run ruff check . +All checks passed! + +$ uv run pytest -q +47 passed, 1 warning in 1.87s + +$ cd ../S13Proof +$ PYTHONPATH=../S13Code uv run pytest -q +3 passed in 1.15s +``` + +Non-browser benchmark cases retained: + +| Case | What it proves | Graph/evidence | Final result | +|---|---|---|---| +| `birthday_write` | live expansion plus durable write | `recall + remember`, then `reminder`, then `answer`; fact source `api://agent/runs` | `Remembered mom's birthday and created two calendar artifacts.` | +| `birthday_read` | durable-memory round trip | `recall -> answer`; retrieved current fact `Mom's birthday is 15 May 2026.` | `You told me mom's birthday is 15 May 2026. [source: api://agent/runs]` | +| `corpus_index` | live graph outcome expansion | `list_directory -> index_1..index_5 -> answer`; files under `sandbox/papers/` | `Indexed the discovered paper files into semantic chunks.` | +| `corpus_cot` | semantic document query | `recall -> answer`; document chunks from `sandbox/papers/react.txt` and `sandbox/papers/cot.txt` | `The indexed evidence says chain-of-thought prompting elicits intermediate reasoning steps, while ReAct combines reasoning traces with actions. [source: file://papers/cot.txt]` | + +A2A waiting/resume proof: + +```text +run_started +graph_patched add=['remote_specialist'] wait=['remote_specialist'] +a2a_task_completed:remote_specialist +graph_patched resume=['remote_specialist'] +run_resumed +task_started:remote_specialist +task_succeeded:remote_specialist +graph_patched finish=True +``` + +Remote artifact: + +```text +An Agent Card advertises capabilities and transports; it grants no local-memory authority. +The coordinator must send only explicitly authorized task context and validate the returned artifact. +``` + +Honest limitation exposed by the floor trace: the semantic indexing floor used one heading-fallback chunk per arXiv abstract fixture in this local deterministic run. That proves scoped retrieval and provenance, but it does not exercise fine-grained live LLM boundary decisions unless Ollama/`S13_LIVE_SEMANTIC_CHUNKING=1` is enabled. + +### Adversarial failure and fix + +Before the fix, `Correction: my mom's birthday is 16 May 2026. Remember that.` was promoted as another current fact with no `supersedes_id`; the regression failed with `KeyError: 'supersedes_id'`. A pronoun-only correction such as `Correction: replace it with next Friday. Remember that.` could also become an unreviewed fact. After the fix: + +```json +{ + "cross_scope_supersedes_id": null, + "ambiguous_result": { + "fact": null, + "review_required": true, + "reason": "ambiguous_correction: no supported fact key found" + } +} +``` + +### Reproduce from a fresh checkout + +```bash +uv sync --locked --dev +uv run ruff check . +uv run pytest -q + +cd ../S13Proof +PYTHONPATH=../S13Code uv run pytest -q +PYTHONPATH=../S13Code uv run python run_a2a_proof.py --output a2a-proof.json + +# With glc_v3 and S13Code running as documented above: +uv run python run_benchmark.py \ + --base-url http://127.0.0.1:8113 \ + --output benchmark.md +``` + ## License MIT. See `LICENSE`. diff --git a/s13code/routes.py b/s13code/routes.py index 1570088..d3c30e6 100644 --- a/s13code/routes.py +++ b/s13code/routes.py @@ -49,6 +49,7 @@ class SearchBody(ScopeBody): query: str = Field(min_length=1, max_length=20_000) limit: int = Field(default=5, ge=1, le=50) kinds: list[MemoryKind] | None = None + include_history: bool = False @router.post("/runs") @@ -90,10 +91,12 @@ async def document(body: IndexBody, request: Request): @router.post("/memory/search") async def memory_search(body: SearchBody, request: Request): hits = request.app.state.s13_runtime.memory.recall(body.query, body.scope(), - kinds=body.kinds, limit=body.limit) + kinds=body.kinds, limit=body.limit, + include_history=body.include_history) return {"query": body.query, "hits": [{"id": hit.id, "kind": hit.kind.value, "text": hit.text, "sources": [source.uri for source in hit.sources], - "metadata": hit.metadata} for hit in hits]} + "metadata": hit.metadata, "status": hit.status, + "supersedes_id": hit.supersedes_id} for hit in hits]} @router.get("/runs/{run_id}") diff --git a/s13code/runtime.py b/s13code/runtime.py index 45c16e3..01877eb 100644 --- a/s13code/runtime.py +++ b/s13code/runtime.py @@ -23,6 +23,30 @@ TextLLM = Callable[[str, str], Awaitable[dict[str, Any]]] +def _strip_correction_prefix(text: str) -> tuple[str, bool]: + corrected = re.sub(r"^\s*correction\s*:\s*", "", text, flags=re.I) + return corrected, corrected != text + + +def _canonical_fact_text(text: str) -> str: + birthday = re.search(r"(?:my\s+)?mom(?:'s)?\s+birthday\s+is\s+(\d{1,2}\s+[A-Za-z]+\s+\d{4})", + text, re.I) + if birthday: + return f"Mom's birthday is {birthday.group(1)}." + return text.strip() + + +def _fact_key(text: str) -> str | None: + lowered = text.lower() + if "mom" in lowered and "birthday" in lowered: + return "mom_birthday" + if "budget" in lowered: + return "budget" + if "quota" in lowered: + return "quota" + return None + + def _work_intent(prompt: str) -> tuple[str, list[TaskSpec]]: """Choose the first useful frontier from the non-browser skill surface. @@ -240,7 +264,13 @@ async def recall(task: TaskSpec) -> dict[str, Any]: # answer. Episodes remain useful context but must not become # the apparent source of the user's own birthday/preference. hits = sorted(hits, key=lambda hit: 0 if hit.kind is MemoryKind.FACT else - (1 if hit.kind is not MemoryKind.EPISODE else 2))[:5] + (1 if hit.kind is not MemoryKind.EPISODE else 2)) + current_fact_keys = {_fact_key(hit.text) for hit in hits if hit.kind is MemoryKind.FACT} + current_fact_keys.discard(None) + if current_fact_keys: + hits = [hit for hit in hits + if hit.kind is MemoryKind.FACT or _fact_key(hit.text) not in current_fact_keys] + hits = hits[:5] return {"hits": [{"id": hit.id, "kind": hit.kind.value, "text": hit.text, "sources": [source.uri for source in hit.sources]} for hit in hits]} @@ -319,17 +349,28 @@ async def answer(_: TaskSpec) -> dict[str, Any]: "evidence_count": len(evidence)} async def remember_explicit(task: TaskSpec) -> dict[str, Any]: - fact_text = task.input["text"] - birthday = re.search(r"(?:my\s+)?mom(?:'s)?\s+birthday\s+is\s+(\d{1,2}\s+[A-Za-z]+\s+\d{4})", - fact_text, re.I) - if birthday: - fact_text = f"Mom's birthday is {birthday.group(1)}." + fact_text, is_correction = _strip_correction_prefix(task.input["text"]) + fact_text = _canonical_fact_text(fact_text) + key = _fact_key(fact_text) + if is_correction and key is None: + return {"fact": None, "review_required": True, + "reason": "ambiguous_correction: no supported fact key found"} + supersedes_id = None + if is_correction: + candidates = runtime.memory.recall(fact_text, scope, kinds=[MemoryKind.FACT], limit=16) + for candidate in candidates: + if _fact_key(candidate.text) == key: + supersedes_id = candidate.id + break record = runtime.memory.write(MemoryRecord( MemoryKind.FACT, scope, fact_text, [user_source], - Principal("gateway", "gateway"), metadata={"run_id": run_id, "promotion": "explicit_user_request"}, + Principal("gateway", "gateway"), metadata={"run_id": run_id, "promotion": "explicit_user_request", + "correction": is_correction, "fact_key": key}, + supersedes_id=supersedes_id, )) return {"fact": {"id": record.id, "kind": record.kind.value, "text": record.text, - "sources": [source.uri for source in record.sources]}} + "sources": [source.uri for source in record.sources], + "supersedes_id": record.supersedes_id, "metadata": record.metadata}} async def run_search(task: TaskSpec) -> dict[str, Any]: return await web_search(task.input["query"], max_results=int(task.input.get("max_results", 3))) diff --git a/tests/test_s13_runtime.py b/tests/test_s13_runtime.py index 63d6261..ff0f805 100644 --- a/tests/test_s13_runtime.py +++ b/tests/test_s13_runtime.py @@ -68,6 +68,93 @@ async def fake_gateway(_app, prompt: str, _system: str): assert any(hit["kind"] == "fact" and "15 May 2026" in hit["text"] for hit in hits) +def test_explicit_correction_supersedes_matching_fact_and_preserves_history(app_client, monkeypatch): + app_client.app.state.s13_runtime.memory.embedder = DeterministicEmbedder(256) + + async def fake_gateway(_app, prompt: str, _system: str): + if "When is mom's birthday?" in prompt: + assert "16 May 2026" in prompt + assert "15 May 2026" not in prompt + return {"text": "You told me it is 16 May 2026. [source: chat://birthday/2]", + "provider": "fake", "model": "fake"} + return {"text": "Updated.", "provider": "fake", "model": "fake"} + + monkeypatch.setattr(agent_route, "gateway_text_llm", fake_gateway) + scope = {"tenant_id": "acme", "project_id": "family", "user_id": "rohan"} + + first = app_client.post("/v1/agent/runs", json={**scope, + "prompt": "My mom's birthday is 15 May 2026. Remember that."}) + assert first.status_code == 200 + old_id = first.json()["graph"]["nodes"]["remember"]["result"]["fact"]["id"] + + correction = app_client.post("/v1/agent/runs", json={**scope, + "prompt": "Correction: my mom's birthday is 16 May 2026. Remember that."}) + assert correction.status_code == 200 + corrected = correction.json()["graph"]["nodes"]["remember"]["result"]["fact"] + assert corrected["supersedes_id"] == old_id + + answer = app_client.post("/v1/agent/runs", json={**scope, "prompt": "When is mom's birthday?"}) + assert answer.json()["answer"].startswith("You told me it is 16 May 2026") + + history = app_client.post("/v1/agent/memory/search", json={**scope, + "query": "mom birthday", "include_history": True, "kinds": ["fact"], "limit": 10}) + assert history.status_code == 200 + statuses = {record["id"]: record["status"] for record in history.json()["hits"]} + assert statuses[old_id] == "superseded" + assert statuses[corrected["id"]] == "current" + + +def test_correction_cannot_supersede_a_different_tenant_fact(app_client, monkeypatch): + app_client.app.state.s13_runtime.memory.embedder = DeterministicEmbedder(256) + + async def fake_gateway(_app, _prompt: str, _system: str): + return {"text": "ok", "provider": "fake", "model": "fake"} + + monkeypatch.setattr(agent_route, "gateway_text_llm", fake_gateway) + acme = {"tenant_id": "acme", "project_id": "family", "user_id": "rohan"} + beta = {"tenant_id": "beta", "project_id": "family", "user_id": "rohan"} + + first = app_client.post("/v1/agent/runs", json={**acme, + "prompt": "My mom's birthday is 15 May 2026. Remember that."}) + old_id = first.json()["graph"]["nodes"]["remember"]["result"]["fact"]["id"] + + correction = app_client.post("/v1/agent/runs", json={**beta, + "prompt": "Correction: my mom's birthday is 16 May 2026. Remember that."}) + assert correction.status_code == 200 + corrected = correction.json()["graph"]["nodes"]["remember"]["result"]["fact"] + assert corrected["supersedes_id"] is None + + acme_history = app_client.post("/v1/agent/memory/search", json={**acme, + "query": "mom birthday", "include_history": True, "kinds": ["fact"], "limit": 10}) + records = {record["id"]: record for record in acme_history.json()["hits"]} + assert records[old_id]["status"] == "current" + + +def test_ambiguous_correction_requires_review_and_does_not_write_memory(app_client, monkeypatch): + app_client.app.state.s13_runtime.memory.embedder = DeterministicEmbedder(256) + + async def fake_gateway(_app, prompt: str, _system: str): + assert "replace it with next Friday" not in prompt + return {"text": "No authorized durable memory matched.", "provider": "fake", "model": "fake"} + + monkeypatch.setattr(agent_route, "gateway_text_llm", fake_gateway) + scope = {"tenant_id": "acme", "project_id": "family", "user_id": "rohan"} + before = app_client.post("/v1/agent/memory/search", json={**scope, + "query": "replace Friday", "include_history": True, "kinds": ["fact"], "limit": 10}) + assert before.status_code == 200 + + response = app_client.post("/v1/agent/runs", json={**scope, + "prompt": "Correction: replace it with next Friday. Remember that."}) + assert response.status_code == 200 + remember = response.json()["graph"]["nodes"]["remember"]["result"] + assert remember["fact"] is None + assert remember["review_required"] is True + assert "ambiguous_correction" in remember["reason"] + after = app_client.post("/v1/agent/memory/search", json={**scope, + "query": "replace Friday", "include_history": True, "kinds": ["fact"], "limit": 10}) + assert after.json()["hits"] == before.json()["hits"] + + def test_http_resume_replays_persisted_run_context(app_client, monkeypatch): app_client.app.state.s13_runtime.memory.embedder = DeterministicEmbedder(128)