diff --git a/CLAUDE.md b/CLAUDE.md index 402254bc..1f711dda 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -259,6 +259,18 @@ The six kinds have confidence ceilings clamped on write and read: `observation` Writes are strict; reads are lenient. Invalid or legacy data remains readable as `unknown`, without promoting a coarse `source` string into a claim type. Corrections mark supersession without deleting history. Relevance retrieval returns only topical matches (never recent padding); explicit `mode="recent"` remains available. A populated store with no relevant hit does not fall back to raw notes. +### Person Memory (`src/pxh/people.py`) + +Facts a person literally stated about themselves, extracted **deterministically** +(regex, no model in the write path) into `state/people-{persona}.jsonl` as +provenance kind `report`. **Reflection must never read this store** — its output +reaches `/api/v1/public/thoughts`, the feed, the blog and Bluesky, so the +separate file *is* the privacy firewall, enforced by the filesystem rather than a +prompt. Pinned by `tests/test_people_invariants.py`; `people.py` and that test +are blacklisted from px-evolve because `mind.py` is a whitelisted target. Design +rationale, TTL policy and the bias-to-rejection matcher are in the module +docstring; the false-positive corpus in `tests/test_people.py` is the spec. + ### Autonomous Racing (px-race) ```bash diff --git a/docs/specs/2026-08-25-person-memory-minimal-design.md b/docs/specs/2026-08-25-person-memory-minimal-design.md new file mode 100644 index 00000000..708cc31d --- /dev/null +++ b/docs/specs/2026-08-25-person-memory-minimal-design.md @@ -0,0 +1,117 @@ +# Person Memory — Minimal Design (2026-08-25) + +The smallest thing that lets SPARK remember what the people it talks to have +literally told it about themselves, without any possibility of that memory +reaching the public pipeline, the performance personas, or a model's +imagination. Implementation: `src/pxh/people.py`. Structural invariants: +`tests/test_people_invariants.py`. False-positive corpus (the extraction +spec, in executable form): `tests/test_people.py`. + +## Scope and non-goals + +Three fact kinds, and only three: **stable preferences**, **stated +relationships**, **explicit first-person commitments**. Deliberately out of +scope, now and until a new design supersedes this one: embeddings, vector +stores, episodic memory, spatial memory, LLM-based extraction, additional +fact classes. A missed fact costs one turn of continuity; a fabricated or +leaked one is a robot telling a child something they never said, or telling +the internet something a child said in private. Every trade in this design +is made in that direction. + +## Store + +`state/people-{persona}.jsonl`, one JSON record per line, append-only, +trimmed to the last 2000 lines. Record shape is compatible with +`memory.py`'s records (`ts`, `subject`, `text`, `tags`, `importance`, +`source`, provenance block) so the existing relevance scorer can read it +unchanged when retrieval lands. Extra fields: `fact_kind`, `topic`, +`polarity`, `expires_ts`. + +**The separate file is the privacy firewall.** Reflection reads +`memories-{persona}.jsonl`, and reflection's output flows to +`thoughts-spark.jsonl` → `/api/v1/public/thoughts` → the site feed, the blog +and Bluesky. Person facts live in a file that `mind.py` never opens — the +same allowlist-by-construction discipline as `_REFLECTION_AWARENESS_KEYS`, +enforced by the filesystem and pinned by source-scan tests, not by prompt +prose. + +## Writer + +Deterministic regex extraction over clauses. **No model anywhere in the +write path** — a fact exists only because a human sentence asserted it. +Provenance kind is the hardcoded literal `report` (confidence ceiling 0.9); +no caller or model can choose a kind. The matcher is biased to rejection: +questions, hedges, conditionals, reported speech, second/third person, +hyperbole, deictic objects and negated intents are refused outright. +`tests/test_people.py`'s rejection corpus is the authority; patterns are +widened only against it, never to raise recall in the abstract. + +### Evidence minimisation + +Evidence stored with a fact is the **exact matched clause** plus the source +message reference — never the whole utterance. "I'm sad about school today, +but I really like dinosaurs" stores the dinosaur clause and nothing else; +the full message remains only in its source log (`obi_chat.jsonl`, the +conversation buffer), recoverable by id. A store whose stated purpose is +narrowly-scoped person facts must not accumulate unrelated private context +as a side effect of faithful provenance. + +### Identity threading + +A channel that has real event identity must thread it: obi-chat entries +carry an `id` and that id is the evidence reference (`obi_chat:`). +Voice conversation turns carry no id, so their reference is a content hash +of the utterance (`voice:turn:`) — an explicit fallback for +id-less channels, not an accepted normal path. + +### Who writes + +Exactly two call sites: `voice_loop.record_conversation_turn` (the user's +words only, SPARK persona only) and `api._append_obi_chat_api` (role `obi` +only, after the message is durably stored so evidence ids always name an +existing line). The persona gate lives inside `record_person_facts`, not at +the call sites: GREMLIN and VIXEN are refused at the writer, and a +per-persona filename alone would have been two stores, not a firewall. +SPARK's own replies are never facts about Obi. The writer never raises into +its caller. + +### Commitments expire; nothing is deleted + +Commitments get a days-not-weeks TTL (default 3, ceiling 10, tightened by a +named day, computed in Hobart time). Expiry filters at read time; records +stay on disk. Corrections use `provenance.supersedes` on the same +`(subject, fact_kind, topic)` — both records kept, one surfaced. + +## Operator seeding (stage 2) + +A small CLI writes operator-known facts through the same canonical +`append_person_facts` writer — never by editing the JSONL directly, never +via an LLM. Seed records are structurally distinguishable from +conversational extraction: `source`/`source_channel` is `operator_seed`, +the evidence names the operator as the asserting actor, and kind remains +`report`. **An operator-seeded fact must never render as "Obi told me"** — +attribution follows the record, and the record says who actually asserted +it. Optional expiry and supersession work exactly as above. Only benign, +stable facts are seeded; nothing sensitive (health, family conflict, +school support, private messages, location). + +## Retrieval (stage 5 — not in the writer PR) + +Injection into exactly two prompts: the SPARK voice prompt (persona == +spark) and the obi-chat prompt. Never GREMLIN/VIXEN; never reflection, +public chat, blog, or social. Retrieval returns zero when nothing is +relevant — no recent-padding, ever. Injected lines are compact, preserve +attribution ("Adrian told you that…" vs "Obi told you 2 days ago that…"), +and every injected statement is mechanically traceable to a stored record. + +## Enforcement + +- `tests/test_people_invariants.py`: source scans pin that `mind.py` has no + route to the store, no module reads it before retrieval lands, only the + two named call sites write, and the writer contains no model call. +- `src/pxh/people.py` and `tests/test_people_invariants.py` are blacklisted + from px-evolve: `mind.py` and `voice_loop.py` are whitelisted evolution + targets, so the module deciding whether a bridge exists must not be one + SPARK can propose editing. +- `tests/conftest.py` isolates the store autouse, so test utterances never + land fabricated facts in the live robot's `state/`. diff --git a/src/pxh/api.py b/src/pxh/api.py index 562911a0..4a9c3c66 100644 --- a/src/pxh/api.py +++ b/src/pxh/api.py @@ -32,6 +32,7 @@ from pydantic import BaseModel, Field, ValidationError, field_validator from starlette.middleware.base import BaseHTTPMiddleware +from . import people from .runtime_paths import resolve_heartbeat_read_path, resolve_sonar_live_read_path from .state import atomic_write, clear_quiet_mode, load_session, load_session_readonly, set_quiet_mode, update_session, tail_lines from .time import utc_timestamp @@ -1350,7 +1351,16 @@ def _append_obi_chat_api(entry: dict) -> None: lines = lines[-100:] atomic_write(path, "\n".join(lines) + "\n") except _FileLockTimeout: - pass # best-effort; the message will still be returned in the response + return # best-effort; the message will still be returned in the response + # Person-memory writer. Runs only after the message is durably stored, so a + # fact's evidence msg id always names a line that exists; the `obi` role + # gate is inside record_person_facts (SPARK's own replies are not facts + # about Obi), as is the guarantee that this never raises into the request. + people.record_person_facts(role=str(entry.get("role") or ""), + text=str(entry.get("text") or ""), + msg_id=str(entry.get("id") or "") or None, + ts=str(entry.get("ts") or "") or None, + channel="obi_chat") _PUBLIC_CHAT_EXECUTOR = ThreadPoolExecutor(max_workers=2) diff --git a/src/pxh/claude_session.py b/src/pxh/claude_session.py index 33ad39dd..ba837cc2 100644 --- a/src/pxh/claude_session.py +++ b/src/pxh/claude_session.py @@ -497,6 +497,14 @@ def run_claude_session( "tools/check_investigator_agent.py", "tests/test_agent_authority_invariant.py", ".claude/agents/spark-investigator.md", + # Person memory. The firewall keeping what a child said in private out of + # reflection — and therefore out of public thoughts, the blog and Bluesky — + # is that `mind.py` never opens `people-*.jsonl`. `mind.py` is a whitelisted + # evolution target, so the module that decides whether that bridge exists + # must not be one SPARK can propose editing, and neither must the test that + # checks it. Same reasoning as the policy and resident-only pairs above. + "src/pxh/people.py", + "tests/test_people_invariants.py", } BLACKLIST_PATTERNS = [ diff --git a/src/pxh/people.py b/src/pxh/people.py new file mode 100644 index 00000000..0a894212 --- /dev/null +++ b/src/pxh/people.py @@ -0,0 +1,407 @@ +"""Facts a person literally stated about themselves — the writer half of the +person-memory design (`docs/specs/2026-08-25-person-memory-minimal-design.md`). + +Store: ``state/people-{persona}.jsonl``, one record per line, shape-compatible +with ``memory.py``'s records so the existing relevance scorer can read it +unchanged when retrieval lands (step 4; this module deliberately ships without +an injection path). + +**A separate file is the contamination firewall, and it is the whole point.** +Reflection reads ``memories-{persona}.jsonl`` and its output flows to +``thoughts-spark.jsonl`` → ``/api/v1/public/thoughts`` → the site feed, the blog +and Bluesky. A person fact that reached reflection would reach all of those. It +cannot, because reflection never opens this file — the same +allowlist-by-construction discipline as ``mind._REFLECTION_AWARENESS_KEYS``, +except here the allowlist is the filesystem. Pinned by +``tests/test_people_invariants.py``; do not add a bridge. + +**There is no model in the write path.** Extraction is regex over sentences, so +a fact exists only if a human sentence asserted it — the strongest available +form of "no model-guessed facts". The provenance kind is hardcoded ``report`` +(ceiling 0.9): SPARK was told this, it did not see it and did not work it out. + +**Evidence is the matched clause, never the whole utterance.** Faithful +provenance does not require copying the message: "I'm sad about school today, +but I really like dinosaurs" must put *only* the dinosaur clause into this +store, or a benign match drags unrelated private material into a file whose +whole reason to exist is narrow scope. The record carries the exact asserted +span plus the source message id; the full original stays in its source log +(``obi_chat.jsonl``, the conversation buffer), recoverable by that id. Voice +turns have no ids, so their reference is a content hash of the utterance — +a deliberate fallback, not the normal path: a channel that has real event +identity (obi-chat does) must thread it. + +**The matcher is biased to rejection, on purpose.** It captures three kinds and +nothing else — stable preferences, stated relationships, and explicit +first-person commitments. Better to remember too little than to confidently +fossilise chatter: a missed fact costs one turn of continuity, a fabricated one +is a robot telling a child something they never said. Questions, hypotheticals, +reported speech, second/third-person statements, commands and deictic +("this song") objects are refused outright. ``tests/test_people.py`` carries the +false-positive corpus; widen the patterns only against it. + +**Commitments carry an aggressive TTL** because a stale promise recalled as +current is worse than forgetting it. Expiry is filtered at *read* time by +``read_people()``; nothing is ever deleted, matching ``provenance``'s +supersession-not-deletion posture. Corrections use ``provenance.supersedes``: +a new statement of the same ``(subject, fact_kind, topic)`` supersedes the last +one rather than duplicating it, so "my best friend is Sam" then "my best friend +is Mia" holds both records and surfaces one. + +**GREMLIN and VIXEN never write here.** ``record_person_facts`` is a no-op for +any persona other than SPARK (an empty persona string *is* SPARK — see +``voice_loop``). The performance characters do not get Obi's facts, and a +per-persona filename alone would not have stopped them acquiring their own. + +Reporting never raises into its caller, for ``health.py``'s reason: a memory +writer must not be able to kill the voice loop or the API request it hangs off. +""" +from __future__ import annotations + +import datetime as dt +import hashlib +import json +import os +import re +import sys +from pathlib import Path +from zoneinfo import ZoneInfo + +from filelock import FileLock + +from pxh import memory, provenance +from pxh.state import atomic_write +from pxh.time import utc_timestamp + +PROJECT_ROOT = Path(__file__).resolve().parent.parent.parent +HOBART_TZ = ZoneInfo("Australia/Hobart") + +FACT_KINDS = ("preference", "relationship", "commitment") +WRITER_PERSONAS = ("spark",) +DEFAULT_SUBJECT = "obi" +SPEAKER_ROLES = ("obi", "user") + +PEOPLE_LIMIT = 2000 +LOCK_TIMEOUT_S = 10 +MAX_FACT_CHARS = 120 +IMPORTANCE = 0.5 + +# Days, not weeks. A commitment recalled after its horizon is a robot insisting +# on a plan that already happened, so the default is short and the ceiling for +# an explicitly dated one is still inside a fortnight. +COMMITMENT_TTL_DAYS = 3 +COMMITMENT_MAX_TTL_DAYS = 10 + +# --- rejection ------------------------------------------------------------ +# Any sentence containing one of these is dropped whole, before matching. +# Hedges and conditionals ("if I had a dog", "I would never") are not assertions; +# reported speech ("my friend said her favourite is cats") is someone else's +# claim wearing a first-person sentence; interrogatives are requests, not facts. +_REJECT_SENTENCE = re.compile( + r"\b(?:if|would|wouldn't|could|should|might|may|maybe|perhaps|unless|" + r"suppose|imagine|pretend|wish|wishing|whenever|almost|nearly|" + r"said|say|says|saying|told|tells|telling|think|thinks|thought|reckon|" + r"reckons|ask|asks|asked|wonder|wondering|guess|" + r"what|when|where|which|who|whose|why|how|do you|did you|are you|" + r"can you|will you|have you)\b") + +# Deictic objects have no referent outside the moment ("I like this song"), so +# the fact would be unretrievable and, worse, wrong the next time it matched. +_DEICTIC_HEAD = re.compile(r"^(?:this|that|these|those|it|them|him|her|you|your)\b") + +# A commitment needs something concrete to be about. "I'm going to explode" is +# a mood; hyperbole that survives the referent test is refused by name. +_COMMIT_REFERENT = re.compile(r"\b(?:the|a|an|my|our|his|her|their)\s+[a-z]") +_HYPERBOLE = re.compile( + r"\b(?:explode|exploding|die|dying|kill|murder|scream|screaming|faint|melt|" + r"burst|vomit|throw up|be sick|cry forever|lose my mind|never speak)\b") +_NEGATED_INTENT = re.compile(r"^(?:not|never|no)\b") + +_WEEKDAYS = {"monday": 0, "tuesday": 1, "wednesday": 2, "thursday": 3, + "friday": 4, "saturday": 5, "sunday": 6} +_TEMPORAL = re.compile( + r"\b(?:today|tonight|tomorrow|this (?:week|weekend|afternoon|evening|arvo)|" + r"next (?:week|weekend)|" + "|".join(_WEEKDAYS) + r")\b") + +_RELATIONS = (r"best friend|friend|mum|mom|mother|dad|father|brother|sister|" + r"teacher|grandma|grandpa|nan|pop|cousin|coach") + +# A relationship fact names a person. "my friend is really nice" is a passing +# opinion wearing the same grammar, so the other half of the clause has to look +# like a name — at most three words, none of them a description. +_NAME_LIKE = re.compile(r"^[a-z][a-z'-]*(?: [a-z][a-z'-]*){0,2}$") +_NOT_A_NAME = re.compile( + r"\b(?:really|very|so|quite|pretty|too|the|a|an|nice|kind|cool|funny|good|" + r"great|best|mean|old|young|new|coming|here|there|away|back|sick|happy|sad|" + r"busy|angry|tired|late|early|right|wrong|annoying|silly|weird|boring|" + r"stupid|dumb|gross|loud|crazy|scary|naughty|rude|smelly|awesome|amazing|" + r"terrible|horrible)\b") + +# --- capture -------------------------------------------------------------- +_PREF_LIKE = re.compile( + r"^i (?:really |quite |always )?(?:like|love|adore|enjoy|prefer) (?P.+)$") +_PREF_DISLIKE = re.compile( + r"^i (?:really |quite |always )?(?:hate|dislike|don't like|do not like|" + r"don't enjoy|can't stand|cannot stand) (?P.+)$") +_PREF_FAV = re.compile( + r"^my (?:favourite|favorite) (?P[a-z ]{2,30}?) is (?P.+)$") +_REL_MINE = re.compile(rf"^my (?P{_RELATIONS}) is (?P.+)$") +_REL_THEIRS = re.compile(rf"^(?P[a-z][a-z'. -]{{1,30}}?) is my (?P{_RELATIONS})$") +_COMMIT_GOING = re.compile(r"^(?:i|we)(?:'m| am|'re| are) going to (?P.+)$") +_COMMIT_WILL = re.compile(r"^(?:i|we)(?:'ll| will) (?P.+)$") +_COMMIT_PROMISED = re.compile( + r"^i promised [a-z' -]{1,20} (?:i'd|i would|i will|to) (?P.+)$") + + +def _state_dir() -> Path: + return Path(os.environ.get("PX_STATE_DIR", PROJECT_ROOT / "state")) + + +def normalize_persona(persona: str | None) -> str: + """An empty persona is SPARK — `voice_loop` stores "" for the default.""" + slug = re.sub(r"[^a-z0-9_-]", "", (persona or "").lower().strip()) + return slug or "spark" + + +def people_file(persona: str = "spark") -> Path: + return _state_dir() / f"people-{normalize_persona(persona)}.jsonl" + + +# A conjunction followed by a first-person restart is a clause boundary even +# without punctuation ("I like dinosaurs but I hate broccoli"), while "fish and +# chips" stays whole because "chips" is not a restart. Leading conjunctions are +# shed so "…, but I really like dinosaurs" still hits the ^i anchor. +_CONJ_SPLIT = re.compile(r"\s+(?:but|and|so|then)\s+(?=(?:i|we|my)\b)", + re.IGNORECASE) +_LEAD_CONJ = re.compile(r"^(?:but|and|so|then|because|cause|cos)\s+", + re.IGNORECASE) + + +def _clauses(text: str) -> list[str]: + """Sentence/clause split. Commas are cut points too: a compound utterance + would otherwise hand a pattern an object that swallows the rest of it.""" + out: list[str] = [] + for chunk in re.split(r"[.!?;\n,]+", str(text or "")): + for piece in _CONJ_SPLIT.split(chunk): + piece = _LEAD_CONJ.sub("", piece).strip() + if piece: + out.append(piece) + return out + + +def _norm(value: str) -> str: + text = str(value or "").replace("’", "'").replace("‘", "'") + return re.sub(r"\s+", " ", text).strip(" \t'\"-") + + +def _commitment_expiry(obj: str, now: dt.datetime) -> str: + """Expiry for a commitment, derived from an explicit day when one is named. + + Hobart, never UTC: "Saturday" means the family's Saturday. The horizon is + the end of the named day plus a day of grace, capped — an undated promise + gets the short default rather than an open-ended one. + """ + local = now.astimezone(HOBART_TZ) + days = COMMITMENT_TTL_DAYS + if re.search(r"\b(?:today|tonight|this (?:afternoon|evening|arvo))\b", obj): + days = 1 + elif "tomorrow" in obj: + days = 2 + elif re.search(r"\bnext (?:week|weekend)\b", obj): + days = COMMITMENT_MAX_TTL_DAYS + else: + for name, index in _WEEKDAYS.items(): + if re.search(rf"\b{name}\b", obj): + ahead = (index - local.weekday()) % 7 or 7 + days = min(ahead + 1, COMMITMENT_MAX_TTL_DAYS) + break + midnight = local.replace(hour=0, minute=0, second=0, microsecond=0) + horizon = midnight + dt.timedelta(days=days) + return horizon.astimezone(dt.timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ") + + +def _match(low: str) -> tuple[str, str, str] | None: + """Return (fact_kind, topic, polarity) for a lowercased clause, else None. + + Every branch that cannot prove what it matched returns None. This function + is the whole precision budget of the feature. + """ + if _REJECT_SENTENCE.search(low): + return None + m = _PREF_FAV.match(low) + if m: + return ("preference", f"favourite:{_norm(m.group('topic'))}", "like") + for pattern, polarity in ((_PREF_LIKE, "like"), (_PREF_DISLIKE, "dislike")): + m = pattern.match(low) + if m: + obj = _norm(m.group("obj")) + if _DEICTIC_HEAD.match(obj) or len(obj) < 3: + return None + return ("preference", f"preference:{obj}", polarity) + for pattern in (_REL_MINE, _REL_THEIRS): + m = pattern.match(low) + if m: + obj = _norm(m.group("obj")) + if not _NAME_LIKE.match(obj) or _NOT_A_NAME.search(obj): + return None + return ("relationship", f"relation:{_norm(m.group('rel'))}", "") + for pattern in (_COMMIT_GOING, _COMMIT_WILL, _COMMIT_PROMISED): + m = pattern.match(low) + if m: + obj = _norm(m.group("obj")) + if _NEGATED_INTENT.match(obj) or _HYPERBOLE.search(obj): + return None + if not (_COMMIT_REFERENT.search(obj) or _TEMPORAL.search(obj)): + return None + return ("commitment", f"commitment:{obj}", "") + return None + + +def extract_person_facts(*, role: str, text: str, subject: str = DEFAULT_SUBJECT, + ts: str | None = None, msg_id: str | None = None, + channel: str = "conversation", + now: dt.datetime | None = None) -> list[dict]: + """Deterministically extract stated facts from one person's utterance. + + Strict about who is speaking: SPARK's own replies are not facts about Obi, + so a role outside `SPEAKER_ROLES` yields nothing. + """ + if str(role or "").lower().strip() not in SPEAKER_ROLES: + return [] + verbatim = _norm(text) + if not verbatim: + return [] + when = now or dt.datetime.now(dt.timezone.utc) + stamp_ts = ts or utc_timestamp() + ref = msg_id or f"turn:{hashlib.sha1(verbatim.encode('utf-8')).hexdigest()[:12]}" + out: list[dict] = [] + seen: set[str] = set() + for clause in _clauses(verbatim): + low = clause.lower() + hit = _match(low) + if hit is None: + continue + fact_kind, topic, polarity = hit + if topic in seen or len(clause) > MAX_FACT_CHARS: + continue + seen.add(topic) + record = { + "ts": stamp_ts, + "subject": str(subject or DEFAULT_SUBJECT).lower().strip(), + "fact_kind": fact_kind, + "topic": topic, + "polarity": polarity, + "text": clause, + "tags": sorted(memory._tokenize(clause)), + "importance": IMPORTANCE, + "source": "conversation", + "expires_ts": _commitment_expiry(low, when) if fact_kind == "commitment" else None, + } + # The kind is a literal here and nowhere else: no caller and no model + # gets to choose it, which is what makes this a `report` store by + # construction rather than by convention. Evidence is the matched + # clause, not `verbatim`: the rest of the utterance is not this fact's + # business, and the id already names the full source message. + provenance.stamp(record, "report", "conversation", + evidence=[f"{channel}:{ref}", clause]) + out.append(record) + return out + + +def load_people(persona: str = "spark") -> list[dict]: + f = people_file(persona) + if not f.exists(): + return [] + out: list[dict] = [] + try: + for line in f.read_text(encoding="utf-8").strip().splitlines(): + try: + rec = json.loads(line) + except json.JSONDecodeError: + continue + if isinstance(rec, dict) and rec.get("text"): + out.append(rec) + except (OSError, UnicodeDecodeError): + return [] + return out + + +def is_expired(record: dict, now: dt.datetime | None = None) -> bool: + """Lenient, like `provenance.read_provenance`: an unparseable expiry is not + an expiry. A corrupt line must not silently retire a fact.""" + raw = (record or {}).get("expires_ts") + if not raw: + return False + try: + when = dt.datetime.fromisoformat(str(raw).replace("Z", "+00:00")) + except (TypeError, ValueError): + return False + return (now or dt.datetime.now(dt.timezone.utc)) >= when + + +def read_people(persona: str = "spark", subject: str | None = None, + now: dt.datetime | None = None) -> list[dict]: + """Live facts: expiry filtered at read time, supersession annotated. + + Expired commitments are excluded, never deleted — the record stays on disk + so "you said you were going to the fair" remains answerable, it just stops + being surfaced as current. Superseded records are returned carrying + `superseded_by` so a caller can show the correction; retrieval drops them. + """ + records = provenance.apply_supersessions(load_people(persona)) + want = str(subject).lower().strip() if subject else None + return [r for r in records + if not is_expired(r, now) + and (want is None or str(r.get("subject", "")).lower() == want)] + + +def append_person_facts(records: list[dict], persona: str = "spark", + now: dt.datetime | None = None) -> list[dict]: + """Append, marking each new fact as superseding the last live one on the + same (subject, fact_kind, topic). Correction without deletion.""" + if not records: + return [] + live = read_people(persona, now=now) + latest: dict[tuple, dict] = {} + for rec in live: + if not provenance.is_superseded(rec): + latest[(rec.get("subject"), rec.get("fact_kind"), rec.get("topic"))] = rec + for rec in records: + prior = latest.get((rec.get("subject"), rec.get("fact_kind"), rec.get("topic"))) + if prior: + provenance.mark_supersedes(rec, prior) + f = people_file(persona) + f.parent.mkdir(parents=True, exist_ok=True) + with FileLock(str(f) + ".lock", timeout=LOCK_TIMEOUT_S): + with f.open("a", encoding="utf-8") as fh: + for rec in records: + fh.write(json.dumps(rec, ensure_ascii=False) + "\n") + try: + lines = f.read_text(encoding="utf-8").strip().splitlines() + if len(lines) > PEOPLE_LIMIT: + atomic_write(f, "\n".join(lines[-PEOPLE_LIMIT:]) + "\n") + except OSError: + pass + return records + + +def record_person_facts(*, role: str, text: str, persona: str = "spark", + subject: str = DEFAULT_SUBJECT, ts: str | None = None, + msg_id: str | None = None, + channel: str = "conversation") -> int: + """The one-line call-site hook. Never raises; returns how many facts landed. + + GREMLIN and VIXEN are refused here rather than at the filename: a + per-persona file would have given the performance characters their own + person store, which is not a firewall, it is two stores. + """ + try: + if normalize_persona(persona) not in WRITER_PERSONAS: + return 0 + facts = extract_person_facts(role=role, text=text, subject=subject, + ts=ts, msg_id=msg_id, channel=channel) + append_person_facts(facts, persona="spark") + return len(facts) + except Exception as exc: # broad: a memory writer must not kill its caller + print(f"[people] extraction failed: {exc}", file=sys.stderr) + return 0 diff --git a/src/pxh/voice_loop.py b/src/pxh/voice_loop.py index 8d1e5806..8225d106 100644 --- a/src/pxh/voice_loop.py +++ b/src/pxh/voice_loop.py @@ -14,7 +14,7 @@ from filelock import Timeout as FileLockTimeout -from pxh import policy, policy_context +from pxh import people, policy, policy_context from pxh.utils import clamp from pxh.spark_config import ANNOUNCE_ALLOWED_TARGETS, ANNOUNCE_MAX_CHARS @@ -346,6 +346,12 @@ def record_conversation_turn( ) -> None: """Append a turn and trim the buffer to the last max_turns, atomically. max_turns <= 0 disables the buffer (writes an empty file).""" + # Person-memory writer: the user's own words only, and only under the SPARK + # persona (refused inside record_person_facts, so GREMLIN/VIXEN cannot + # acquire a store by any route). Ahead of the early return, because turning + # the conversation buffer off is not a decision about long-term memory. + people.record_person_facts(role="user", text=user_text, persona=persona, + channel="voice") if max_turns <= 0: atomic_write(conversation_path(persona), "") return diff --git a/tests/conftest.py b/tests/conftest.py index 08db9614..5151b003 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -53,6 +53,28 @@ def _isolate_brain_mailbox(tmp_path, monkeypatch): monkeypatch.setattr(brain, "brain_root", lambda: tmp_path / "brain") +@pytest.fixture(autouse=True) +def _isolate_person_store(tmp_path, monkeypatch): + """Keep test utterances out of the live state/people-*.jsonl. + + The person writer hangs off two call sites that plenty of existing tests + already exercise incidentally — voice_loop.record_conversation_turn and + api._append_obi_chat_api. Without this, a fixture sentence that happens to + match a pattern lands a fabricated fact in the real store, and step 4's + retrieval would later read it back to a child as something they said. Same + #221 hazard as the health and mailbox fixtures. + + Repoints PROJECT_ROOT rather than PX_STATE_DIR, so a test that deliberately + sets PX_STATE_DIR still wins (people._state_dir prefers the env var) while + one that sets nothing can no longer reach the real directory. + """ + try: + from pxh import people + except ImportError: + return + monkeypatch.setattr(people, "PROJECT_ROOT", tmp_path / "people-root") + + @pytest.fixture(autouse=True) def _isolate_alive_heartbeat(tmp_path, monkeypatch): """Keep heartbeat reads off the live robot's /run/spark. diff --git a/tests/test_people.py b/tests/test_people.py new file mode 100644 index 00000000..568e575a --- /dev/null +++ b/tests/test_people.py @@ -0,0 +1,318 @@ +"""Tests for pxh.people — the deterministic person-fact writer. + +The false-positive corpus below is the load-bearing half. Recall can be widened +later against real family speech; precision cannot be recovered once SPARK has +told a child something they never said. Each rejection case names the trap it +represents, so a future widening that breaks one has to argue with the trap +rather than with a bare assertion. +""" +from __future__ import annotations + +import datetime as dt +import json + +import pytest + +from pxh import people, provenance + +NOW = dt.datetime(2026, 8, 25, 9, 0, tzinfo=dt.timezone.utc) # Tuesday, Hobart + + +@pytest.fixture(autouse=True) +def _isolated_state(tmp_path, monkeypatch): + monkeypatch.setenv("PX_STATE_DIR", str(tmp_path)) + + +def _facts(text, role="obi", **kw): + return people.extract_person_facts(role=role, text=text, msg_id="m1", + ts="2026-08-25T09:00:00Z", now=NOW, **kw) + + +# ── What is captured ─────────────────────────────────────────────────────── + +CAPTURE = [ + ("my favourite animal is the cuttlefish", "preference", "favourite:animal"), + ("my favorite colour is orange", "preference", "favourite:colour"), + ("I like dinosaurs", "preference", "preference:dinosaurs"), + ("I love the ocean", "preference", "preference:the ocean"), + ("I hate broccoli", "preference", "preference:broccoli"), + ("I don't like mushrooms", "preference", "preference:mushrooms"), + ("Mia is my best friend", "relationship", "relation:best friend"), + ("Mr Tan is my teacher", "relationship", "relation:teacher"), + ("my best friend is Sam", "relationship", "relation:best friend"), + ("I'm going to the school fair on Saturday", "commitment", None), + ("we're going to the beach on Saturday", "commitment", None), + ("I promised Dad I'd feed the cat", "commitment", None), + ("I'll show you the drawing tomorrow", "commitment", None), +] + + +@pytest.mark.parametrize("text,kind,topic", CAPTURE) +def test_captures_the_three_fact_kinds(text, kind, topic): + got = _facts(text) + assert len(got) == 1, f"{text!r} produced {got}" + assert got[0]["fact_kind"] == kind + if topic: + assert got[0]["topic"] == topic + + +def test_polarity_distinguishes_like_from_dislike(): + assert _facts("I like broccoli")[0]["polarity"] == "like" + assert _facts("I hate broccoli")[0]["polarity"] == "dislike" + + +# ── What must never be captured ──────────────────────────────────────────── + +REJECT = [ + ("do you like dogs?", "question — a request for SPARK's view, not an assertion"), + ("what is your favourite animal?", "question wearing the favourite-X grammar"), + ("who is your best friend", "interrogative without a question mark"), + ("if I had a dog I would call him Rex", "conditional — a fact about an imagined world"), + ("I would never eat broccoli", "hypothetical 'would never', not the flat 'I never'"), + ("maybe I like dinosaurs", "hedge — the speaker has not committed to it"), + ("I think I like dogs", "hedge verb wrapping a well-formed preference"), + ("I might go to the fair on Saturday", "modal — an option, not a commitment"), + ("I'm going to explode", "hyperbole with future-intent grammar"), + ("I'm going to die of boredom", "hyperbole that also carries a temporal-ish object"), + ("my friend said her favourite is cats", "reported speech — someone else's claim"), + ("Dad told me he hates mornings", "reported speech about a third party"), + ("you like trains", "second person — a claim about SPARK, not the speaker"), + ("Dad hates mornings", "third person — not the speaker's own stated fact"), + ("Sam is my friend's brother", "possessive chain — a relation of a relation, not of the speaker"), + ("go forward", "command to the robot"), + ("say something about dogs", "request for speech, not a stated fact"), + ("turn left and then stop", "compound command"), + ("I'm hungry", "transient bodily state"), + ("I'm tired today", "transient state with a temporal marker"), + ("I like this song", "deictic object — no referent survives the moment"), + ("I love that", "bare deictic object"), + ("I'll be back", "future intent with no concrete referent"), + ("I will not go to the fair", "negated intent"), + ("my friend is really nice", "passing opinion wearing relationship grammar"), + ("my brother is annoying", "adjective wearing name grammar — an opinion, not a name"), + ("I wish I liked broccoli", "counterfactual wish"), + ("I could eat the whole cake", "modal, not an intention"), + ("", "empty utterance"), + (" ", "whitespace-only utterance"), +] + + +@pytest.mark.parametrize("text,trap", REJECT) +def test_false_positive_corpus_produces_nothing(text, trap): + assert _facts(text) == [], trap + + +def test_sparks_own_reply_is_never_a_fact_about_obi(): + assert people.extract_person_facts(role="spark", text="I like dinosaurs", + msg_id="m1", now=NOW) == [] + + +def test_compound_utterance_does_not_swallow_the_rest_of_the_sentence(): + got = _facts("I like dinosaurs, my best friend is Sam") + assert {f["fact_kind"] for f in got} == {"preference", "relationship"} + assert all(len(f["text"]) < 40 for f in got) + + +def test_conjunction_restart_is_a_clause_boundary_but_objects_stay_whole(): + got = _facts("I like dinosaurs but I hate broccoli") + assert [(f["polarity"], f["topic"]) for f in got] == [ + ("like", "preference:dinosaurs"), ("dislike", "preference:broccoli")] + # "chips" is not a first-person restart, so the object survives intact. + assert _facts("I like fish and chips")[0]["topic"] == "preference:fish and chips" + + +def test_evidence_is_the_matched_clause_never_the_whole_utterance(): + """Pinned by review (2026-08-25): a benign match must not drag unrelated + private material into the store. The full message stays in its source log, + recoverable via the evidence message id — data minimisation and provenance + at the same time.""" + got = _facts("I'm sad about school today, but I really like dinosaurs.") + assert len(got) == 1 + assert got[0]["topic"] == "preference:dinosaurs" + people.append_person_facts(got, now=NOW) + raw = people.people_file().read_text(encoding="utf-8") + assert "dinosaurs" in raw + assert "sad" not in raw and "school" not in raw + + +# ── Record shape and provenance ──────────────────────────────────────────── + +def test_record_is_a_report_claim_with_traceable_evidence(): + rec = _facts("I like dinosaurs", subject="obi")[0] + prov = provenance.read_provenance(rec) + assert prov["kind"] == "report" + assert prov["confidence"] <= provenance.CONFIDENCE_CEILING["report"] + assert "obi_chat:m1" in prov["evidence"] or "conversation:m1" in prov["evidence"] + assert "I like dinosaurs" in prov["evidence"] + assert rec["subject"] == "obi" + assert rec["id"] and rec["ts"].endswith("Z") + assert rec["source"] == "conversation" + assert "dinosaurs" in rec["tags"] + + +def test_kind_is_hardcoded_and_not_a_caller_choice(): + """No parameter of the writer can produce anything but `report` — the + provenance kind is a literal, so no caller and no model can raise it.""" + import inspect + assert '"report"' in inspect.getsource(people.extract_person_facts) + params = inspect.signature(people.extract_person_facts).parameters + assert not any("kind" in p for p in params if p != "channel") + params = inspect.signature(people.record_person_facts).parameters + assert not any("kind" in p for p in params) + + +def test_voice_turn_without_a_message_id_still_gets_a_turn_reference(): + rec = people.extract_person_facts(role="user", text="I like dinosaurs", + channel="voice", now=NOW)[0] + refs = provenance.read_provenance(rec)["evidence"] + assert any(r.startswith("voice:turn:") for r in refs), refs + + +# ── Commitment TTL ───────────────────────────────────────────────────────── + +def _expiry(text): + return _facts(text)[0]["expires_ts"] + + +def test_preferences_and_relationships_never_expire(): + assert _facts("I like dinosaurs")[0]["expires_ts"] is None + assert _facts("Mia is my best friend")[0]["expires_ts"] is None + + +def test_commitment_ttl_is_days_not_weeks(): + horizon = dt.datetime.fromisoformat( + _expiry("I'm going to feed the cat").replace("Z", "+00:00")) + assert 0 < (horizon - NOW).days <= people.COMMITMENT_MAX_TTL_DAYS + + +def test_named_day_tightens_the_ttl_below_the_default(): + tomorrow = dt.datetime.fromisoformat( + _expiry("I'll show you the drawing tomorrow").replace("Z", "+00:00")) + default = dt.datetime.fromisoformat( + _expiry("I'm going to feed the cat").replace("Z", "+00:00")) + assert tomorrow < default + + +def test_expired_commitment_is_filtered_at_read_time_but_kept_on_disk(): + people.append_person_facts(_facts("I'm going to the fair tomorrow"), now=NOW) + later = NOW + dt.timedelta(days=30) + assert people.read_people(now=later) == [] + assert len(people.load_people()) == 1 # history, not deletion + + +def test_unparseable_expiry_reads_as_live(): + """Lenient reads: a corrupt line must not silently retire a fact.""" + assert people.is_expired({"expires_ts": "not-a-date"}, now=NOW) is False + + +# ── Supersession, not duplication ────────────────────────────────────────── + +def test_new_statement_supersedes_the_prior_one_on_the_same_topic(): + people.append_person_facts(_facts("my best friend is Sam"), now=NOW) + people.append_person_facts(_facts("my best friend is Mia"), now=NOW) + stored = people.read_people(now=NOW) + assert len(stored) == 2 # both kept + live = [r for r in stored if not provenance.is_superseded(r)] + assert [r["text"] for r in live] == ["my best friend is Mia"] + + +def test_a_correction_flips_polarity_without_deleting_the_old_belief(): + people.append_person_facts(_facts("I like broccoli"), now=NOW) + people.append_person_facts(_facts("I hate broccoli"), now=NOW) + live = [r for r in people.read_people(now=NOW) + if not provenance.is_superseded(r)] + assert [r["polarity"] for r in live] == ["dislike"] + + +def test_unrelated_topics_do_not_supersede_each_other(): + people.append_person_facts(_facts("I like dinosaurs"), now=NOW) + people.append_person_facts(_facts("I like cuttlefish"), now=NOW) + live = [r for r in people.read_people(now=NOW) + if not provenance.is_superseded(r)] + assert len(live) == 2 + + +# ── Store behaviour ──────────────────────────────────────────────────────── + +def test_store_is_a_separate_file_from_consolidated_memory(): + from pxh import memory + assert people.people_file("spark") != memory.memories_file("spark") + assert people.people_file("spark").name == "people-spark.jsonl" + + +def test_load_skips_malformed_lines(): + f = people.people_file() + f.parent.mkdir(parents=True, exist_ok=True) + f.write_text(json.dumps(_facts("I like dinosaurs")[0]) + "\n{broken\n", + encoding="utf-8") + assert len(people.load_people()) == 1 + + +def test_read_can_filter_by_subject(): + people.append_person_facts(_facts("I like dinosaurs", subject="obi"), now=NOW) + people.append_person_facts(_facts("I like coffee", subject="adrian"), now=NOW) + assert len(people.read_people(subject="obi", now=NOW)) == 1 + + +# ── Persona firewall ─────────────────────────────────────────────────────── + +@pytest.mark.parametrize("persona", ["gremlin", "vixen", "GREMLIN"]) +def test_personas_never_get_a_person_store(tmp_path, persona): + assert people.record_person_facts(role="user", text="I like dinosaurs", + persona=persona) == 0 + assert not list(tmp_path.glob("people-*.jsonl")) + + +def test_empty_persona_is_spark_because_voice_loop_stores_it_that_way(): + assert people.normalize_persona("") == "spark" + assert people.record_person_facts(role="user", text="I like dinosaurs", + persona="") == 1 + assert people.people_file("spark").exists() + + +def test_a_persona_slug_cannot_escape_the_state_dir(): + assert people.people_file("../../etc/passwd").name == "people-etcpasswd.jsonl" + + +# ── Failure posture ──────────────────────────────────────────────────────── + +def test_writer_never_raises_into_its_caller(monkeypatch, capsys): + monkeypatch.setattr(people, "append_person_facts", + lambda *a, **k: (_ for _ in ()).throw(OSError("disk full"))) + assert people.record_person_facts(role="obi", text="I like dinosaurs") == 0 + assert "people" in capsys.readouterr().err + + +# ── Call-site wiring ─────────────────────────────────────────────────────── + +def test_voice_loop_records_the_user_turn_for_the_spark_persona(monkeypatch): + from pxh import voice_loop + monkeypatch.setenv("PX_CONVERSATION_TURNS", "2") + voice_loop.record_conversation_turn("spark", "I like dinosaurs", "nice") + live = people.read_people(now=NOW) + assert [r["text"] for r in live] == ["I like dinosaurs"] + + +def test_voice_loop_does_not_record_sparks_own_reply(monkeypatch): + from pxh import voice_loop + voice_loop.record_conversation_turn("spark", "hello", "I like dinosaurs") + assert people.read_people(now=NOW) == [] + + +def test_voice_loop_records_nothing_under_a_persona(): + from pxh import voice_loop + voice_loop.record_conversation_turn("vixen", "I like dinosaurs", "hi") + assert people.load_people("spark") == [] + assert people.load_people("vixen") == [] + + +def test_obi_chat_append_records_obi_messages_only(tmp_path, monkeypatch): + from pxh import api + monkeypatch.setattr(api, "_public_state_dir", lambda: tmp_path) + api._append_obi_chat_api({"id": "abc123", "ts": "2026-08-25T09:00:00Z", + "role": "obi", "text": "I like dinosaurs"}) + api._append_obi_chat_api({"id": "def456", "ts": "2026-08-25T09:00:01Z", + "role": "spark", "text": "I like cuttlefish"}) + live = people.read_people(now=NOW) + assert [r["text"] for r in live] == ["I like dinosaurs"] + assert "obi_chat:abc123" in provenance.read_provenance(live[0])["evidence"] diff --git a/tests/test_people_invariants.py b/tests/test_people_invariants.py new file mode 100644 index 00000000..d2986010 --- /dev/null +++ b/tests/test_people_invariants.py @@ -0,0 +1,142 @@ +"""Structural invariants for the person store — the firewall, not the feature. + +`tests/test_people.py` checks what the writer extracts. This file checks the +property that has to hold even if every pattern in it is wrong: **reflection +cannot see this store.** Reflection's output lands in `thoughts-spark.jsonl`, +which is served at `/api/v1/public/thoughts` and forwarded to the site feed, +the blog and Bluesky. A fact a child stated in private has exactly one thing +between it and that pipeline, and it is not a prompt instruction — it is the +fact that `mind.reflection()` never opens the file. + +Same posture as `tests/test_policy_invariants.py`: a source scan, so a future +bridge fails a test rather than being noticed in review. `src/pxh/people.py` +and this file are blacklisted from px-evolve — `mind.py` and `voice_loop.py` +are both *whitelisted* self-evolution targets, so the module SPARK could edit +must not also be the module that decides whether the edit is allowed. +""" +from __future__ import annotations + +import ast +import inspect +from pathlib import Path + +import pytest + +from pxh import claude_session, memory, mind, people, voice_loop + +SRC = Path(mind.__file__).resolve().parent + +# Substrings that would mean reflection had acquired a route to the store. +# Deliberately specific rather than the bare word "people": `mind.py` legitimately +# says `rooms_with_people` about Frigate labels, and a scan that cries wolf on +# prose is a scan someone eventually deletes. +FORBIDDEN = ("pxh.people", "import people", "people_file", "read_people", + "load_people", "record_person_facts", "extract_person_facts", + "people-") + + +def _imported_modules(path: Path) -> set[str]: + tree = ast.parse(path.read_text(encoding="utf-8")) + names: set[str] = set() + for node in ast.walk(tree): + if isinstance(node, ast.Import): + names.update(a.name for a in node.names) + elif isinstance(node, ast.ImportFrom): + base = node.module or "" + names.add(base) + names.update(f"{base}.{a.name}" if base else a.name + for a in node.names) + return names + + +def test_reflection_context_builder_has_no_route_to_the_person_store(): + src = inspect.getsource(mind.reflection) + hits = [token for token in FORBIDDEN if token in src] + assert not hits, f"reflection() references the person store: {hits}" + + +def test_mind_does_not_import_people_anywhere(): + """Not just reflection: no import at all, so no future helper inside + `mind.py` can quietly become the bridge.""" + imported = _imported_modules(Path(mind.__file__)) + assert not {n for n in imported if n.endswith("people") or ".people" in n} + + +def test_mind_source_never_names_the_person_store(): + src = Path(mind.__file__).read_text(encoding="utf-8") + hits = [token for token in FORBIDDEN if token in src] + assert not hits, f"mind.py references the person store: {hits}" + + +def test_people_does_not_import_mind(): + """The other direction matters too — an import cycle is how a 'just for + logging' call into the cognitive loop starts.""" + imported = _imported_modules(Path(people.__file__)) + assert not {n for n in imported if n.endswith("mind") or ".mind" in n} + + +def test_person_store_is_physically_separate_from_consolidated_memory(): + """The firewall is the filesystem. If these two ever resolve to the same + file, reflection reads person facts on its next tick.""" + assert people.people_file("spark") != memory.memories_file("spark") + assert "people-" in people.people_file("spark").name + assert "memories-" in memory.memories_file("spark").name + + +def test_no_other_module_reads_the_person_store(): + """Step 3 is write-only. Retrieval (step 4) will add readers deliberately; + until then anything reading this store is an accident.""" + readers = set() + for path in sorted(SRC.glob("*.py")): + if path.name == "people.py": + continue + text = path.read_text(encoding="utf-8") + if "read_people" in text or "load_people" in text: + readers.add(path.name) + assert readers == set(), f"unexpected readers of the person store: {readers}" + + +def test_only_the_two_specified_call_sites_write_facts(): + writers = set() + for path in sorted(SRC.glob("*.py")): + if path.name == "people.py": + continue + if "record_person_facts" in path.read_text(encoding="utf-8"): + writers.add(path.name) + assert writers == {"api.py", "voice_loop.py"} + + +def test_the_writer_holds_no_model_call(): + """Resident-only Claude cuts both ways: the write path must contain no LLM + call of any kind, so a fact exists only because a human sentence asserted + it. Deterministic extraction is the strongest available form of that.""" + src = Path(people.__file__).read_text(encoding="utf-8") + for token in ("claude", "ask_brain", "call_llm", "ollama", "run_claude_session", + "subprocess"): + assert token not in src.lower(), token + + +@pytest.mark.parametrize("persona", ["gremlin", "vixen"]) +def test_the_performance_personas_are_refused_at_the_writer(persona, tmp_path, + monkeypatch): + """A per-persona filename alone is not a firewall — it is two stores. The + refusal lives in `record_person_facts`, above the filename.""" + monkeypatch.setenv("PX_STATE_DIR", str(tmp_path)) + assert people.record_person_facts(role="obi", text="I like dinosaurs", + persona=persona) == 0 + assert list(tmp_path.glob("people-*.jsonl")) == [] + assert people.WRITER_PERSONAS == ("spark",) + + +def test_voice_loop_passes_the_live_persona_through_to_the_gate(): + """The gate is only real if the call site hands it the actual persona — + a hardcoded "spark" here would let GREMLIN write under SPARK's name.""" + src = inspect.getsource(voice_loop.record_conversation_turn) + assert "record_person_facts" in src + assert "persona=persona" in src + + +def test_writer_and_its_invariants_are_blacklisted_from_self_evolution(): + for path in ("src/pxh/people.py", "tests/test_people_invariants.py"): + assert path in claude_session.BLACKLIST_FILES + assert not claude_session.file_in_whitelist(path)