diff --git a/CHANGELOG.md b/CHANGELOG.md index f8de604d..8ba3aa9d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,8 @@ installable release; see the roadmap in [README.md](README.md). ### Added +- **Sentiment-feedback hook lane wired into UserPromptSubmit** ([#606](https://github.com/robotrocketscience/aelfrice/issues/606)). The v2.0 `sentiment_feedback` module (regex-only detector, 12 positive + 12 negative patterns, length-guard at 200 chars) is now reached by a live hook. At each `UserPromptSubmit` the new `apply_sentiment_feedback(prompt, session_id)` resolves the most-recent prior UPS audit row for the same `session_id`, projects its `beliefs[*].id` list, and — if `detect_sentiment` matches — calls `apply_sentiment_to_pending` against that set before this turn's retrieval runs, so demoted/promoted posteriors are reflected in the hits returned for the corrective prompt. Default off; opt-in via `[feedback] sentiment_from_prose = true` in `.aelfrice.toml` or `AELFRICE_FEEDBACK_SENTIMENT_FROM_PROSE=1` (existing `sentiment_feedback.is_enabled` resolver). One new hook-audit tag `sentiment_feedback` records pattern + matched_text + valence + applied belief ids per fire (distinct from the per-belief `feedback_history` rows, which already carry `source = sentiment_inferred`). Single-session window — cross-session propagation is explicit follow-up work. Fail-soft end-to-end: any internal error returns 0 and never surfaces into the UPS hook contract. Design memo: `docs/v3_sentiment_feedback_hook.md`. + - **HRR persistence — split-format save/load on `HRRStructIndex`** ([#553](https://github.com/robotrocketscience/aelfrice/issues/553)). `HRRStructIndex.save(path)` now writes a per-store directory containing `struct.npy` (the `(N, dim)` float64 matrix, mmap-able) plus `meta.npz` (the small metadata blob: belief ids, role/id vectors, dim, seed, layout version) instead of a single bundled `.npz`. Writes are atomic via temp-file + `os.replace` so a reader process never observes a partial write. `HRRStructIndex.load(path)` auto-detects the layout: a directory with the two split-format files loads the new layout; a file path falls through to the v1.7 bundled `.npz` reader and emits a one-shot module-logger deprecation warning (legacy stores still load — this is a substrate landing, not a breaking change). New `mmap=False` keyword on `load()` requests `np.load(mmap_mode='r')` on the struct matrix; default off keeps existing callers byte-stable. The split layout is the substrate that makes persistence-default-ON viable per `docs/feature-hrr-integration.md` — `np.load(.npz, mmap_mode='r')` is silently ignored per the numpy docs, so mmap requires this format change. Sub-pieces (`HRRStructIndexCache` mmap read path, ephemeral-path auto-disable, `aelf doctor` rows, `AELFRICE_HRR_PERSIST` opt-out flag) ship as separate atomic PRs per the spec's "Coordination" section. - **`merge-train` label-driven serialized merger** ([#602](https://github.com/robotrocketscience/aelfrice/issues/602)). New `.github/workflows/merge-train.yml` watches for the `ready-to-merge` label on PRs and FF-pushes the branch to `main` after verifying (a) the head SHA still matches what the labeling event saw (no race with a mid-queue force-push), (b) the branch is fast-forward on current `main`, (c) every commit between main and head is GPG/SSH-signed, and (d) all required check-runs on the head SHA have completed without failure (10-minute poll, configurable). Concurrency-1 via the `merge-train` group: labeled events queue rather than race, eliminating the merge-thrash documented in #602 (PR #591 cycled `attn:merge-conflict` four times in 30 minutes on 2026-05-10, PR #540 eight times in 9 minutes — both before this shipped). Hard constraint: the bot has no signing key, so it cannot rebase on the author's behalf — `required_signatures` on `main` would reject any commit it added. If a PR is behind main when its slot comes up, the bot unlabels and posts a comment requesting a local rebase. Under serialization this is rare: main only moves when the bot is merging, so a freshly-rebased author who labels promptly is FF when their slot is processed. New `CONTRIBUTING.md` § "Merging — the `ready-to-merge` label" documents the operator-facing flow. Second half of the merge-thrash mitigation; pairs with the PR-size soft-cap advisory below. diff --git a/docs/v3_sentiment_feedback_hook.md b/docs/v3_sentiment_feedback_hook.md new file mode 100644 index 00000000..52189bb6 --- /dev/null +++ b/docs/v3_sentiment_feedback_hook.md @@ -0,0 +1,150 @@ +# v3.0 spec: sentiment-feedback hook production wire-up (#606) + +Spec for issue [#606](https://github.com/robotrocketscience/aelfrice/issues/606). +Production wire-up of `aelfrice.sentiment_feedback` (shipped at v2.0 per #193) into a +live `UserPromptSubmit` hook lane. Module is pure today; nothing calls it on a +real hook fire. + +## What's being decided + +1. Hook lane. +2. Which retrieval window receives the sentiment-driven α/β bump. +3. Audit-log surface for sentiment fires. +4. Opt-in flag scope (one flag or two). +5. Privacy posture re: existing transcript-ingest opt-out. + +## Decisions + +### 1. Hook lane: `UserPromptSubmit` + +The corrective prompt ("no, that's wrong", "fix it") arrives in user prompt N +*after* the assistant has acted on the retrieval block from prompt N-1. The +right time to apply the correction is at UPS for prompt N, before the next +retrieval fires — that way the bumped posteriors are already reflected in the +hits this prompt returns. + +`Stop` would fire after the assistant's turn, before the user has had a chance +to react. Wrong lane. + +### 2. Retrieval window: the previous UPS audit record for the same `session_id` + +The `hook_audit.jsonl` already records every UPS retrieval with structured +`beliefs[*].id`. The sentiment lane reads that JSONL, filters by +`hook == "user_prompt_submit"` AND `session_id == `, takes the +most-recent prior row, and extracts the belief ids. + +Boundary cases: + +- **No prior UPS row for session.** First prompt; nothing to bump. No-op. +- **Prior UPS row has empty `beliefs`.** Retrieval returned nothing; no-op. +- **Audit disabled by config.** No prior beliefs visible to the sentiment + hook either; no-op. Surfaced in `aelf health` so the operator can see why + the lane is silent. + +Single-session only. Cross-session sentiment propagation is out of scope per +#606. + +### 3. Audit surface: new `hook_audit` tag `"sentiment_feedback"` + +One JSONL row per sentiment fire. Fields: + +- `hook = "sentiment_feedback"` +- `session_id` +- `prompt_prefix` (≤ `AUDIT_PROMPT_PREFIX_CAP`) +- `pattern` (named match, e.g. `"wrong"`, `"i_told_you"`) +- `matched_text` (literal substring; bounded by regex shape, typically ≤ 30 + chars) +- `sentiment` (`"positive"` | `"negative"`) +- `valence` (signed float passed to `apply_feedback`) +- `escalated` (bool — set when `detect_correction_frequency` fires) +- `belief_ids` (list of ids actually bumped) +- `n_beliefs` (count) + +The existing `feedback_history` table still gets one row per affected belief +via `apply_feedback`, with `source = sentiment_inferred` (module-level +constant — unchanged from v2.0). The hook-audit row is the *event-level* +record; `feedback_history` is the *belief-level* record. Both exist; they +serve different queries. + +### 4. Opt-in: single flag + +`[feedback] sentiment_from_prose = true` in `.aelfrice.toml` (or +`AELFRICE_FEEDBACK_SENTIMENT_FROM_PROSE=1`). The existing +`sentiment_feedback.is_enabled()` already resolves this. The hook calls +`is_enabled(config)` and short-circuits when false. No new config key; the +module flag is the hook flag. + +Programmatic use of `detect_sentiment` / `apply_sentiment_to_pending` from +non-hook code paths is unchanged — those functions are pure and have no +config-flag dependency. + +### 5. Privacy posture + +The UPS hook already reads every user prompt to do retrieval. The sentiment +lane does *more processing* on data already in the hook's hands — it does not +add a new data surface. + +What's stored that wasn't stored before: + +- One hook-audit row tagged `sentiment_feedback` per matched prompt + (`pattern` + `matched_text` ≤ ~30 chars, no raw prompt body). +- One `feedback_history` row per affected belief id. + +What's NOT stored: + +- Raw prompt content (cap at `AUDIT_PROMPT_PREFIX_CAP` already enforced for + UPS audit; same cap reused here). +- Anything that leaves the machine. No outbound calls. Stdlib regex only. + +**Transcript-ingest opt-out semantics.** The acceptance criterion in #606 says +the hook must respect the transcript-ingest opt-out. Interpretation: the +sentiment lane is gated by the same `[feedback] sentiment_from_prose` flag, +which is opt-in (default false). A user who has chosen not to install +transcript-ingest hooks has already implicitly opted out of all prose-side +analysis surfaces; the new lane requires explicit opt-in via the existing +config key, which they will not have set. No additional plumbing needed. + +`aelf health` surfaces enabled/disabled state via the existing +`_sentiment_from_prose_state` helper. No change to that surface for v3.0. + +## Determinism + +- Pure stdlib regex against the prompt string. +- Length guard at 200 chars rejects long pastes (unchanged from module). +- Same prompt + same prior-UPS audit row → byte-identical + `apply_feedback` calls + audit row. +- The PHILOSOPHY locked memory (`Avoid embeddings + non-determinism in + retrieval`) is preserved: the hot path is regex; no embedding lookup, + no LLM, no learned classifier. + +## Acceptance mapping (#606) + +| #606 AC | Where satisfied | +|---|---| +| 1. Spec memo + hook lane + decay policy | this memo | +| 2. Determinism property | §Determinism above; existing module + regex | +| 3. Privacy property | §5 Privacy posture above | +| 4. Two-session bench fixture | `tests/test_hook_sentiment_feedback.py::test_correction_lowers_subsequent_ranking` | +| 5. Audit row per fire | §3 above; new tag `sentiment_feedback` | + +## Out of scope + +- Cross-session sentiment propagation (sentiment in session A bumping + posteriors that surface for session B). +- Multi-language sentiment (English-only regex bank, unchanged from + v2.0 module). +- Ranked distribution (equal-weighted distribution preserved per the + v2.0 ratification: "matches the research-line behavior; ranked + distribution adds a knob without an evidence-gate"). +- Per-rank decay of the bump (every pending id receives the full + signal; if the prior turn returned 5 hits, all 5 get bumped equally). +- LLM-judged sentiment (deferred to the #605 PHILOSOPHY-determinism + decision; this lane stays regex-only regardless of that outcome). + +## Refs + +- `src/aelfrice/sentiment_feedback.py` — pure module (v2.0 shipped) +- `src/aelfrice/hook.py` — UPS hook entry-point (`user_prompt_submit`) +- `docs/v2_sentiment_feedback.md` — original v2.0 evaluation memo +- #193 — v2.0 evaluation gate (CLOSED COMPLETED 2026-05-03) +- #606 — this issue diff --git a/src/aelfrice/hook.py b/src/aelfrice/hook.py index db6a41ba..cae32b64 100644 --- a/src/aelfrice/hook.py +++ b/src/aelfrice/hook.py @@ -344,6 +344,7 @@ def _append_telemetry( AUDIT_HOOK_USER_PROMPT_SUBMIT: Final[str] = "user_prompt_submit" AUDIT_HOOK_SESSION_START: Final[str] = "session_start" +AUDIT_HOOK_SENTIMENT_FEEDBACK: Final[str] = "sentiment_feedback" @dataclass(frozen=True) @@ -699,6 +700,12 @@ def user_prompt_submit( else DEFAULT_HOOK_TOKEN_BUDGET ) config = load_user_prompt_submit_config(stderr=serr) + # #606: sentiment-feedback lane — apply correction signals from + # this prompt to the prior UPS turn's retrieved beliefs BEFORE + # this turn's retrieval, so demoted posteriors are reflected in + # the hits returned here. Default-off, fail-soft, opt-in via + # `[feedback] sentiment_from_prose = true` in `.aelfrice.toml`. + apply_sentiment_feedback(prompt, session_id, stderr=serr) retrieve_start = time.monotonic() hits = _retrieve(prompt, budget) if hits: @@ -912,6 +919,230 @@ def _open_store() -> MemoryStore: return MemoryStore(str(p)) +# --------------------------------------------------------------------------- +# Sentiment-feedback hook lane (#606) +# --------------------------------------------------------------------------- + + +def _load_aelfrice_toml( + start: Path | None = None, + *, + stderr: IO[str] | None = None, +) -> dict[str, Any]: + """Walk up from `start` looking for `.aelfrice.toml` and return the + full parsed mapping. Returns `{}` when no file is found, the file is + unreadable, or the TOML is malformed. Fail-soft: never raises. + + Used by the sentiment-feedback lane to resolve `[feedback]` config. + The two existing per-section loaders (`load_user_prompt_submit_config`, + `load_hook_audit_config`) are kept as-is so their typed-config return + contract is unchanged; this helper exists for callers that need the + whole document (e.g. modules with their own `is_enabled(config)` + surface like `sentiment_feedback.is_enabled`). + """ + serr: IO[str] = stderr if stderr is not None else sys.stderr + current = (start if start is not None else Path.cwd()).resolve() + seen: set[Path] = set() + while current not in seen: + seen.add(current) + candidate = current / _CONFIG_FILENAME + if candidate.is_file(): + try: + raw = candidate.read_bytes() + except OSError as exc: + print( + f"aelfrice hook: cannot read {candidate}: {exc}", + file=serr, + ) + return {} + try: + return cast( + dict[str, Any], + tomllib.loads(raw.decode("utf-8", errors="replace")), + ) + except tomllib.TOMLDecodeError as exc: + print( + f"aelfrice hook: malformed TOML in {candidate}: {exc}", + file=serr, + ) + return {} + parent = current.parent + if parent == current: + break + current = parent + return {} + + +def _load_prior_ups_belief_ids( + session_id: str, + *, + stderr: IO[str] | None = None, +) -> list[str]: + """Return the belief ids surfaced by the most-recent prior + UserPromptSubmit hook fire in `session_id`. + + Reads `hook_audit.jsonl` (and any rotated `.1` file), filters to UPS + rows for the matching session, and projects `beliefs[*].id` from the + final match. Returns `[]` when: + + - audit is disabled (file missing), + - the session has no prior UPS fires recorded, + - the most-recent prior fire returned zero beliefs, + - any I/O or JSON-shape error occurs (fail-soft). + + The rotated `.1` slot is also scanned so a session that crossed a + rotation boundary still surfaces its prior turn. Rotation is a rare + event (10 MB default cap) so the extra read is cheap. + """ + if not session_id: + return [] + try: + p = db_path() + if str(p) == ":memory:": + return [] + audit_path = _audit_path_for_db(p) + rotated = audit_path.with_name(audit_path.name + AUDIT_ROTATED_SUFFIX) + except Exception: + return [] + candidates: list[Path] = [] + if rotated.exists(): + candidates.append(rotated) + if audit_path.exists(): + candidates.append(audit_path) + if not candidates: + return [] + last_belief_ids: list[str] = [] + try: + for path in candidates: + for record in read_hook_audit(path): + if record.get("hook") != AUDIT_HOOK_USER_PROMPT_SUBMIT: + continue + if record.get("session_id") != session_id: + continue + beliefs_obj: Any = record.get("beliefs") + if not isinstance(beliefs_obj, list): + continue + ids: list[str] = [] + for b in beliefs_obj: + if not isinstance(b, dict): + continue + bid = b.get("id") + if isinstance(bid, str) and bid: + ids.append(bid) + last_belief_ids = ids + except (ValueError, OSError) as exc: + print( + f"aelfrice: prior-UPS audit scan failed (non-fatal): {exc}", + file=stderr if stderr is not None else sys.stderr, + ) + return [] + return last_belief_ids + + +def apply_sentiment_feedback( + prompt: str, + session_id: str | None, + *, + stderr: IO[str] | None = None, +) -> int: + """Detect sentiment in `prompt` and apply it to the prior UPS turn's + retrieved beliefs. + + Returns the number of beliefs whose posterior was updated. Returns + 0 on: + + - sentiment-from-prose disabled in config (default off), + - no sentiment signal detected in the prompt, + - no prior UPS fire in this session (or audit disabled), + - prior fire returned zero beliefs, + - any internal error (fail-soft). + + Always writes a `sentiment_feedback`-tagged hook-audit row when a + signal fires, even if zero beliefs are updated (e.g. all prior ids + have since been deleted) — the row records that the lane considered + the prompt. Disabled-by-config short-circuits before audit. + """ + serr: IO[str] = stderr if stderr is not None else sys.stderr + if not prompt or not session_id: + return 0 + try: + from aelfrice import sentiment_feedback as sf # noqa: PLC0415 + except Exception: # pragma: no cover — defensive + return 0 + try: + toml_cfg = _load_aelfrice_toml(stderr=serr) + if not sf.is_enabled(toml_cfg): + return 0 + signal = sf.detect_sentiment(prompt) + if signal is None: + return 0 + prior_ids = _load_prior_ups_belief_ids(session_id, stderr=serr) + if not prior_ids: + return 0 + store = _open_store() + try: + results = sf.apply_sentiment_to_pending( + store=store, + signal=signal, + pending_belief_ids=prior_ids, + ) + finally: + store.close() + applied_ids = [r.belief_id for r in results] + _write_sentiment_feedback_audit( + prompt=prompt, + session_id=session_id, + signal=signal, + applied_ids=applied_ids, + stderr=serr, + ) + return len(applied_ids) + except Exception as exc: + print( + f"aelfrice: sentiment-feedback hook failed (non-fatal): {exc}", + file=serr, + ) + return 0 + + +def _write_sentiment_feedback_audit( + *, + prompt: str, + session_id: str, + signal: "Any", + applied_ids: list[str], + stderr: IO[str] | None = None, +) -> None: + """Append one hook-audit row tagged `sentiment_feedback`. Fail-soft. + + Distinct from `_write_hook_audit_record`: the sentiment row carries + pattern/matched_text/valence/applied_ids — fields the UPS audit row + does not have. Reuses the same JSONL file + rotation policy. + """ + cfg = load_hook_audit_config(stderr=stderr) + if not cfg.enabled: + return + try: + p = db_path() + audit_path = _audit_path_for_db(p) + except Exception: + return + record: dict[str, object] = { + "ts": datetime.now(timezone.utc).strftime("%Y-%m-%dT%H:%M:%SZ"), + "hook": AUDIT_HOOK_SENTIMENT_FEEDBACK, + "session_id": session_id, + "prompt_prefix": prompt[:AUDIT_PROMPT_PREFIX_CAP], + "sentiment": signal.sentiment, + "pattern": signal.pattern, + "matched_text": signal.matched_text, + "valence": signal.valence, + "confidence": signal.confidence, + "belief_ids": applied_ids, + "n_beliefs": len(applied_ids), + } + _append_audit(audit_path, record, cfg.max_bytes, stderr=stderr) + + # --------------------------------------------------------------------------- # Session-start sub-block builder (#578) # --------------------------------------------------------------------------- diff --git a/tests/test_hook_sentiment_feedback.py b/tests/test_hook_sentiment_feedback.py new file mode 100644 index 00000000..d1da203d --- /dev/null +++ b/tests/test_hook_sentiment_feedback.py @@ -0,0 +1,491 @@ +"""Sentiment-feedback hook lane wired into UserPromptSubmit (#606).""" +from __future__ import annotations + +import io +import json +from pathlib import Path + +import pytest + +from aelfrice.hook import ( + AUDIT_FILENAME, + AUDIT_HOOK_SENTIMENT_FEEDBACK, + AUDIT_HOOK_USER_PROMPT_SUBMIT, + _audit_path_for_db, + _load_aelfrice_toml, + _load_prior_ups_belief_ids, + apply_sentiment_feedback, + read_hook_audit, + user_prompt_submit, +) +from aelfrice.models import BELIEF_FACTUAL, LOCK_NONE, Belief +from aelfrice.sentiment_feedback import ( + ENV_SENTIMENT, + SENTIMENT_INFERRED_SOURCE, +) +from aelfrice.store import MemoryStore + + +# --------------------------------------------------------------------------- +# fixtures + helpers +# --------------------------------------------------------------------------- + + +def _mk( + bid: str, + content: str, + *, + alpha: float = 1.0, + beta: float = 1.0, +) -> Belief: + return Belief( + id=bid, + content=content, + content_hash=f"h_{bid}", + alpha=alpha, + beta=beta, + type=BELIEF_FACTUAL, + lock_level=LOCK_NONE, + locked_at=None, + demotion_pressure=0, + created_at="2026-04-26T00:00:00Z", + last_retrieved_at=None, + ) + + +def _seed_db(db_path: Path, beliefs: list[Belief]) -> None: + store = MemoryStore(str(db_path)) + try: + for b in beliefs: + store.insert_belief(b) + finally: + store.close() + + +def _read_belief(db_path: Path, belief_id: str) -> Belief: + store = MemoryStore(str(db_path)) + try: + b = store.get_belief(belief_id) + assert b is not None, f"belief {belief_id} missing" + return b + finally: + store.close() + + +def _payload(prompt: str, session_id: str = "s1") -> str: + return json.dumps( + { + "session_id": session_id, + "transcript_path": "/dev/null", + "cwd": "/tmp", + "hook_event_name": "UserPromptSubmit", + "prompt": prompt, + } + ) + + +def _set_db(monkeypatch: pytest.MonkeyPatch, path: Path) -> None: + monkeypatch.setenv("AELFRICE_DB", str(path)) + + +def _enable_sentiment(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv(ENV_SENTIMENT, "1") + + +def _disable_sentiment(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv(ENV_SENTIMENT, raising=False) + + +# --------------------------------------------------------------------------- +# _load_aelfrice_toml +# --------------------------------------------------------------------------- + + +def test_load_toml_returns_empty_when_file_missing(tmp_path: Path) -> None: + assert _load_aelfrice_toml(start=tmp_path) == {} + + +def test_load_toml_returns_parsed_dict(tmp_path: Path) -> None: + (tmp_path / ".aelfrice.toml").write_text( + "[feedback]\nsentiment_from_prose = true\n", + encoding="utf-8", + ) + parsed = _load_aelfrice_toml(start=tmp_path) + assert parsed.get("feedback") == {"sentiment_from_prose": True} + + +def test_load_toml_returns_empty_on_malformed_toml(tmp_path: Path) -> None: + (tmp_path / ".aelfrice.toml").write_text("not = toml = at all", encoding="utf-8") + serr = io.StringIO() + assert _load_aelfrice_toml(start=tmp_path, stderr=serr) == {} + assert "malformed TOML" in serr.getvalue() + + +# --------------------------------------------------------------------------- +# _load_prior_ups_belief_ids +# --------------------------------------------------------------------------- + + +def test_prior_ups_returns_empty_when_audit_file_missing( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + _set_db(monkeypatch, tmp_path / "memory.db") + assert _load_prior_ups_belief_ids("s1") == [] + + +def test_prior_ups_returns_empty_when_session_id_blank( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + _set_db(monkeypatch, tmp_path / "memory.db") + assert _load_prior_ups_belief_ids("") == [] + + +def test_prior_ups_filters_by_session_and_takes_last( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + db = tmp_path / "memory.db" + _set_db(monkeypatch, db) + audit_path = _audit_path_for_db(db) + audit_path.parent.mkdir(parents=True, exist_ok=True) + rows = [ + { + "hook": AUDIT_HOOK_USER_PROMPT_SUBMIT, + "session_id": "s_other", + "beliefs": [{"id": "X1"}], + }, + { + "hook": AUDIT_HOOK_USER_PROMPT_SUBMIT, + "session_id": "s1", + "beliefs": [{"id": "A1"}, {"id": "A2"}], + }, + { + "hook": "session_start", + "session_id": "s1", + "beliefs": [{"id": "Z9"}], + }, + { + "hook": AUDIT_HOOK_USER_PROMPT_SUBMIT, + "session_id": "s1", + "beliefs": [{"id": "B1"}, {"id": "B2"}, {"id": "B3"}], + }, + ] + audit_path.write_text( + "\n".join(json.dumps(r) for r in rows) + "\n", encoding="utf-8" + ) + assert _load_prior_ups_belief_ids("s1") == ["B1", "B2", "B3"] + assert _load_prior_ups_belief_ids("s_other") == ["X1"] + assert _load_prior_ups_belief_ids("s_missing") == [] + + +def test_prior_ups_skips_non_dict_belief_entries( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + db = tmp_path / "memory.db" + _set_db(monkeypatch, db) + audit_path = _audit_path_for_db(db) + audit_path.parent.mkdir(parents=True, exist_ok=True) + audit_path.write_text( + json.dumps( + { + "hook": AUDIT_HOOK_USER_PROMPT_SUBMIT, + "session_id": "s1", + "beliefs": ["junk", {"no_id": True}, {"id": "OK"}, {"id": ""}], + } + ) + + "\n", + encoding="utf-8", + ) + assert _load_prior_ups_belief_ids("s1") == ["OK"] + + +# --------------------------------------------------------------------------- +# apply_sentiment_feedback +# --------------------------------------------------------------------------- + + +def test_apply_returns_zero_when_disabled( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + _disable_sentiment(monkeypatch) + db = tmp_path / "memory.db" + _seed_db(db, [_mk("F1", "kitchen has bananas")]) + _set_db(monkeypatch, db) + # Pre-write a UPS audit row so the lane has prior beliefs to bump + # if it were enabled — the disabled gate must short-circuit before + # this is read. + audit_path = _audit_path_for_db(db) + audit_path.parent.mkdir(parents=True, exist_ok=True) + audit_path.write_text( + json.dumps( + { + "hook": AUDIT_HOOK_USER_PROMPT_SUBMIT, + "session_id": "s1", + "beliefs": [{"id": "F1"}], + } + ) + + "\n", + encoding="utf-8", + ) + + n = apply_sentiment_feedback("no, wrong", "s1") + + assert n == 0 + # No sentiment_feedback row should have been written. + rows = read_hook_audit(audit_path) + assert not any(r.get("hook") == AUDIT_HOOK_SENTIMENT_FEEDBACK for r in rows) + # Belief posterior unchanged. + b = _read_belief(db, "F1") + assert b.alpha == 1.0 and b.beta == 1.0 + + +def test_apply_returns_zero_when_no_signal_detected( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + _enable_sentiment(monkeypatch) + db = tmp_path / "memory.db" + _seed_db(db, [_mk("F1", "kitchen has bananas")]) + _set_db(monkeypatch, db) + audit_path = _audit_path_for_db(db) + audit_path.parent.mkdir(parents=True, exist_ok=True) + audit_path.write_text( + json.dumps( + { + "hook": AUDIT_HOOK_USER_PROMPT_SUBMIT, + "session_id": "s1", + "beliefs": [{"id": "F1"}], + } + ) + + "\n", + encoding="utf-8", + ) + + n = apply_sentiment_feedback("show me the bananas", "s1") + + assert n == 0 + rows = read_hook_audit(audit_path) + assert not any(r.get("hook") == AUDIT_HOOK_SENTIMENT_FEEDBACK for r in rows) + + +def test_apply_returns_zero_when_no_prior_ups( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + _enable_sentiment(monkeypatch) + db = tmp_path / "memory.db" + _seed_db(db, [_mk("F1", "kitchen has bananas")]) + _set_db(monkeypatch, db) + # No audit file at all. + n = apply_sentiment_feedback("no, wrong", "s1") + assert n == 0 + + +def test_apply_demotes_prior_turn_beliefs_and_writes_audit( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + _enable_sentiment(monkeypatch) + db = tmp_path / "memory.db" + _seed_db( + db, + [ + _mk("F1", "the answer is purple"), + _mk("F2", "the answer is orange"), + ], + ) + _set_db(monkeypatch, db) + audit_path = _audit_path_for_db(db) + audit_path.parent.mkdir(parents=True, exist_ok=True) + audit_path.write_text( + json.dumps( + { + "hook": AUDIT_HOOK_USER_PROMPT_SUBMIT, + "session_id": "s1", + "beliefs": [{"id": "F1"}, {"id": "F2"}], + } + ) + + "\n", + encoding="utf-8", + ) + + n = apply_sentiment_feedback("no, that's wrong", "s1") + + assert n == 2 + # Both beliefs received negative feedback -> beta increased. + for bid in ("F1", "F2"): + b = _read_belief(db, bid) + assert b.beta > 1.0, f"{bid} beta should have grown from negative feedback" + assert b.alpha == 1.0 + # Sentiment audit row landed with all required fields. + rows = read_hook_audit(audit_path) + sf_rows = [r for r in rows if r.get("hook") == AUDIT_HOOK_SENTIMENT_FEEDBACK] + assert len(sf_rows) == 1 + row = sf_rows[0] + assert row["session_id"] == "s1" + assert row["sentiment"] == "negative" + assert row["pattern"] in {"wrong", "no"} + assert row["valence"] < 0 + assert sorted(row["belief_ids"]) == ["F1", "F2"] + assert row["n_beliefs"] == 2 + + +def test_apply_skips_belief_ids_that_no_longer_exist( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + _enable_sentiment(monkeypatch) + db = tmp_path / "memory.db" + _seed_db(db, [_mk("F1", "the answer is purple")]) + _set_db(monkeypatch, db) + audit_path = _audit_path_for_db(db) + audit_path.parent.mkdir(parents=True, exist_ok=True) + # Prior turn surfaced F1 (still present) AND F_GONE (since deleted). + audit_path.write_text( + json.dumps( + { + "hook": AUDIT_HOOK_USER_PROMPT_SUBMIT, + "session_id": "s1", + "beliefs": [{"id": "F1"}, {"id": "F_GONE"}], + } + ) + + "\n", + encoding="utf-8", + ) + + n = apply_sentiment_feedback("no, wrong", "s1") + + assert n == 1 # Only F1 was updatable. + rows = read_hook_audit(audit_path) + sf_rows = [r for r in rows if r.get("hook") == AUDIT_HOOK_SENTIMENT_FEEDBACK] + assert sf_rows[0]["belief_ids"] == ["F1"] + + +# --------------------------------------------------------------------------- +# integration: UPS hook end-to-end + two-session correction ranking (AC4) +# --------------------------------------------------------------------------- + + +def test_ups_hook_skips_sentiment_lane_when_disabled( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + _disable_sentiment(monkeypatch) + db = tmp_path / "memory.db" + _seed_db(db, [_mk("F1", "the answer is purple")]) + _set_db(monkeypatch, db) + + # Turn 1: prime the audit log with a UPS row. + user_prompt_submit( + stdin=io.StringIO(_payload("answer purple")), + stdout=io.StringIO(), + ) + # Turn 2: correction prompt. + user_prompt_submit( + stdin=io.StringIO(_payload("no, that's wrong")), + stdout=io.StringIO(), + ) + + b = _read_belief(db, "F1") + assert b.beta == 1.0, "lane was disabled, posterior must not move" + rows = read_hook_audit(_audit_path_for_db(db)) + assert not any(r.get("hook") == AUDIT_HOOK_SENTIMENT_FEEDBACK for r in rows) + + +def test_ups_hook_applies_sentiment_to_prior_turn_when_enabled( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + _enable_sentiment(monkeypatch) + db = tmp_path / "memory.db" + _seed_db(db, [_mk("F1", "the answer is purple")]) + _set_db(monkeypatch, db) + + # Turn 1: query that should retrieve F1; UPS writes its audit row. + user_prompt_submit( + stdin=io.StringIO(_payload("answer purple")), + stdout=io.StringIO(), + ) + pre = _read_belief(db, "F1") + # Turn 2: user says "no, that's wrong" — sentiment lane fires before + # this turn's retrieval and bumps F1's beta. + user_prompt_submit( + stdin=io.StringIO(_payload("no, that's wrong")), + stdout=io.StringIO(), + ) + post = _read_belief(db, "F1") + + assert post.beta > pre.beta, "negative sentiment must increase beta" + rows = read_hook_audit(_audit_path_for_db(db)) + sf_rows = [r for r in rows if r.get("hook") == AUDIT_HOOK_SENTIMENT_FEEDBACK] + assert len(sf_rows) == 1 + + +def test_correction_lowers_subsequent_ranking_across_sessions( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """AC4: session A applies a correction; session B replays the same + query; the corrected belief now ranks below an uncorrected sibling. + """ + _enable_sentiment(monkeypatch) + db = tmp_path / "memory.db" + # Both beliefs start at uniform prior (alpha=beta=1). + _seed_db( + db, + [ + _mk("WRONG", "purple bananas are the answer"), + _mk("OK", "purple bananas exist in the kitchen"), + ], + ) + _set_db(monkeypatch, db) + + # Session A: turn 1 surfaces only WRONG (prompt is narrower). + user_prompt_submit( + stdin=io.StringIO(_payload("answer", session_id="sA")), + stdout=io.StringIO(), + ) + # Session A: turn 2 — user says "no, wrong". Lane bumps WRONG only, + # because only WRONG was in sA's prior UPS retrieval set. + user_prompt_submit( + stdin=io.StringIO(_payload("no, that's wrong", session_id="sA")), + stdout=io.StringIO(), + ) + + # Session B: same DB; replay a query that surfaces both beliefs. + sout_b = io.StringIO() + user_prompt_submit( + stdin=io.StringIO(_payload("purple bananas", session_id="sB")), + stdout=sout_b, + ) + block = sout_b.getvalue() + + wrong_pos = block.find('id="WRONG"') + ok_pos = block.find('id="OK"') + assert wrong_pos != -1, "WRONG should still be retrieved" + assert ok_pos != -1, "OK should still be retrieved" + assert ok_pos < wrong_pos, ( + "the uncorrected belief must rank above the corrected one in session B" + ) + + # WRONG's posterior moved; OK's did not. + wrong = _read_belief(db, "WRONG") + ok = _read_belief(db, "OK") + assert wrong.beta > 1.0 + assert ok.beta == 1.0 + + +def test_feedback_history_row_uses_sentiment_inferred_source( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + _enable_sentiment(monkeypatch) + db = tmp_path / "memory.db" + _seed_db(db, [_mk("F1", "purple bananas in the kitchen")]) + _set_db(monkeypatch, db) + user_prompt_submit( + stdin=io.StringIO(_payload("purple bananas")), stdout=io.StringIO() + ) + user_prompt_submit( + stdin=io.StringIO(_payload("no, wrong")), stdout=io.StringIO() + ) + + store = MemoryStore(str(db)) + try: + events = store.list_feedback_events(belief_id="F1") + finally: + store.close() + assert any( + e.source == SENTIMENT_INFERRED_SOURCE for e in events + ), "feedback_history row should carry the sentiment_inferred source tag"