diff --git a/src/aelfrice/feedback.py b/src/aelfrice/feedback.py index 0d605e8a1..174a12343 100644 --- a/src/aelfrice/feedback.py +++ b/src/aelfrice/feedback.py @@ -85,6 +85,7 @@ def apply_feedback( source: str, now: str | None = None, propagate: bool = True, + update_posterior: bool = True, ) -> FeedbackResult: """Apply one feedback event to one belief. @@ -92,8 +93,12 @@ def apply_feedback( 2. Reject zero valence: a no-update event is not a successful update, and pre-commit #5 says feedback_history records every successful update — so a zero call has no row to write. - 3. Bayesian-update alpha or beta by valence sign. - 4. Persist the new posterior on the belief row. + 3. Bayesian-update alpha or beta by valence sign — UNLESS + `update_posterior` is False, in which case the posterior is left + untouched (audit-only; #1086). Retrieval-exposure records an event + for the recurrence axis without being treated as truth-evidence. + 4. Persist the new posterior on the belief row (skipped when + `update_posterior` is False — nothing changed). 5. Append one row to feedback_history (created_at = `now` or UTC now). 6. Propagate the signal through outbound edges (#1058): each attenuated delta from `store.propagate_valence` is applied via a @@ -124,11 +129,18 @@ def apply_feedback( prior_alpha: float = b.alpha prior_beta: float = b.beta - new_alpha, new_beta = _bayesian_update(b, valence) - - b.alpha = new_alpha - b.beta = new_beta - store.update_belief(b) + if update_posterior: + new_alpha, new_beta = _bayesian_update(b, valence) + b.alpha = new_alpha + b.beta = new_beta + store.update_belief(b) + else: + # Audit-only (#1086): record the event so exposure frequency stays + # recoverable (the recurrence axis), but do NOT move the posterior. + # A retrieval is exposure, not endorsement; counting every surfacing + # as positive evidence inflated whatever recurs. The posterior is + # left exactly as-is; the feedback_history row is still written. + new_alpha, new_beta = prior_alpha, prior_beta timestamp: str = now if now is not None else _utc_now_iso() event_id: int = store.insert_feedback_event( @@ -149,7 +161,7 @@ def apply_feedback( source=source, ) - if propagate and _propagation_enabled(): + if update_posterior and propagate and _propagation_enabled(): deltas = store.propagate_valence(belief_id, valence) # Sorted for a deterministic feedback_history row order # regardless of edge-iteration order inside the BFS. diff --git a/src/aelfrice/hook_search.py b/src/aelfrice/hook_search.py index 7b544f174..7ebb4a0a8 100644 --- a/src/aelfrice/hook_search.py +++ b/src/aelfrice/hook_search.py @@ -12,11 +12,14 @@ to the agent". The README's Bayesian-memory claim depends on this loop closing. -2. Hook-driven valence is implicit and weaker than explicit user - feedback. A retrieval is exposure, not endorsement. Use a small - positive valence (`HOOK_RETRIEVAL_VALENCE = 0.1`) so a thousand - retrievals over a few months don't dominate the posterior the way a - handful of explicit user thumbs-up should. +2. A retrieval is exposure, not endorsement. Since #1086 the hook + records the exposure event to `feedback_history` (so surfacing + frequency stays recoverable) but does NOT move the Bayesian posterior + by default: counting every surfacing as positive evidence let whatever + recurs float above genuine knowledge. The legacy behaviour — a small + positive `HOOK_RETRIEVAL_VALENCE = 0.1` per retrieval — is restored by + setting `AELFRICE_EXPOSURE_UPDATES_POSTERIOR=1` (benchmark A/B and + rollback). The module exposes two functions: `search_for_prompt`, the hook's top-level call (retrieve + record), and `record_retrieval`, the audit @@ -31,6 +34,7 @@ """ from __future__ import annotations +import os import sys import traceback from typing import IO, Final, Iterable @@ -45,6 +49,23 @@ retrieval row. ARCHITECTURE.md commits this string publicly; downstream analysis (e.g. `SELECT ... WHERE source = 'hook'`) depends on it.""" +ENV_EXPOSURE_UPDATES_POSTERIOR: Final[str] = "AELFRICE_EXPOSURE_UPDATES_POSTERIOR" +"""Opt-in to the legacy behaviour where a hook retrieval moves the +Bayesian posterior. Default is OFF (#1086): a retrieval is exposure, not +endorsement, and counting every surfacing as positive evidence inflated +whatever recurs — measured on a real store, junk (session scaffolding, +fragments) accumulated MORE exposure than genuine knowledge and floated +above it. With the flag unset, hook retrievals are recorded to +feedback_history (so exposure frequency stays recoverable) but leave the +posterior untouched. Set to "1" to restore the pre-#1086 posterior update +(kept for benchmark A/B and rollback). Read per call so a flip is honoured +without restart.""" + + +def _exposure_updates_posterior() -> bool: + return os.environ.get(ENV_EXPOSURE_UPDATES_POSTERIOR, "0") == "1" + + HOOK_RETRIEVAL_VALENCE: Final[float] = 0.1 """Per-belief positive valence written for each hook-driven retrieval. @@ -99,6 +120,7 @@ def record_retrieval( needing to inspect the audit log. """ serr: IO[str] = stderr if stderr is not None else sys.stderr + update_posterior: bool = _exposure_updates_posterior() written: int = 0 stamped_ids: list[str] = [] for b in beliefs: @@ -108,6 +130,7 @@ def record_retrieval( b.id, valence, source, + update_posterior=update_posterior, ) written += 1 stamped_ids.append(b.id) diff --git a/tests/regression/test_hook_to_feedback_history_end_to_end.py b/tests/regression/test_hook_to_feedback_history_end_to_end.py index 965847443..1210ef796 100644 --- a/tests/regression/test_hook_to_feedback_history_end_to_end.py +++ b/tests/regression/test_hook_to_feedback_history_end_to_end.py @@ -99,9 +99,41 @@ def test_hook_fire_writes_feedback_rows_tagged_hook( assert {e.belief_id for e in hook_events} >= {"F1", "F2"} -def test_hook_fire_increments_alpha_not_beta( +def test_hook_fire_records_exposure_without_moving_posterior( tmp_path: Path, monkeypatch: pytest.MonkeyPatch ) -> None: + """#1086: firing the hook records the retrieval to feedback_history + (recurrence axis) but leaves the posterior untouched by default — + exposure is not endorsement.""" + db = tmp_path / "memory.db" + s = MemoryStore(str(db)) + s.insert_belief(_mk("F1", "the kitchen is full of bananas")) + s.close() + _set_db(monkeypatch, db) + + user_prompt_submit( + stdin=io.StringIO(_payload("are there bananas in the kitchen")), + stdout=io.StringIO(), + ) + + s2 = MemoryStore(str(db)) + try: + b = s2.get_belief("F1") + events = s2.count_feedback_events() + finally: + s2.close() + assert b is not None + assert b.alpha == 1.0 # unchanged — audit-only + assert b.beta == 1.0 + assert events >= 1 # exposure still recorded + + +def test_hook_fire_legacy_flag_increments_alpha( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """AELFRICE_EXPOSURE_UPDATES_POSTERIOR=1 restores the pre-#1086 + behaviour: a hook fire bumps alpha by HOOK_RETRIEVAL_VALENCE.""" + monkeypatch.setenv("AELFRICE_EXPOSURE_UPDATES_POSTERIOR", "1") db = tmp_path / "memory.db" s = MemoryStore(str(db)) s.insert_belief(_mk("F1", "the kitchen is full of bananas")) diff --git a/tests/test_feedback_apply.py b/tests/test_feedback_apply.py index 9f0182887..d0271e34d 100644 --- a/tests/test_feedback_apply.py +++ b/tests/test_feedback_apply.py @@ -214,3 +214,35 @@ def test_unknown_belief_id_does_not_write_history() -> None: with pytest.raises(ValueError): apply_feedback(s, "nonexistent", valence=1.0, source="user") assert s.count_feedback_events() == 0 + + +# --- Audit-only exposure (update_posterior=False, #1086) ----------------- + + +def test_update_posterior_false_leaves_alpha_beta_unchanged() -> None: + """A retrieval is exposure, not endorsement: audit-only feedback must + not move the Bayesian posterior.""" + s = _store_with(_mk(alpha=2.0, beta=3.0)) + apply_feedback(s, "b1", valence=0.1, source="hook", update_posterior=False) + got = s.get_belief("b1") + assert got is not None + assert (got.alpha, got.beta) == (2.0, 3.0) + + +def test_update_posterior_false_still_writes_audit_row() -> None: + """Exposure is recorded to feedback_history so surfacing frequency + stays recoverable for the recurrence axis.""" + s = _store_with(_mk()) + apply_feedback(s, "b1", valence=0.1, source="hook", update_posterior=False) + assert s.count_feedback_events() == 1 + + +def test_update_posterior_false_result_reports_no_change() -> None: + res = apply_feedback( + s := _store_with(_mk(alpha=2.0, beta=3.0)), + "b1", valence=0.1, source="hook", update_posterior=False, + ) + assert res.new_alpha == res.prior_alpha == 2.0 + assert res.new_beta == res.prior_beta == 3.0 + assert res.propagated == [] # propagation is a posterior effect — skipped + s.close() diff --git a/tests/test_hook_search.py b/tests/test_hook_search.py index 58888fcb7..c7cf2fe48 100644 --- a/tests/test_hook_search.py +++ b/tests/test_hook_search.py @@ -101,7 +101,27 @@ def test_record_retrieval_empty_iterable_writes_no_rows() -> None: assert s.count_feedback_events() == 0 -def test_record_retrieval_updates_alpha_not_beta() -> None: +def test_record_retrieval_audit_only_does_not_move_posterior() -> None: + """#1086: a hook retrieval is exposure, not endorsement. By default it + records an audit row for the recurrence axis but leaves the Bayesian + posterior untouched — counting every surfacing as evidence let whatever + recurs float above genuine knowledge.""" + s = _seed(_mk("b1")) + written = record_retrieval(s, [s.get_belief("b1")]) # type: ignore[list-item] + assert written == 1 + b = s.get_belief("b1") + assert b is not None + assert b.alpha == 1.0 # unchanged + assert b.beta == 1.0 + assert s.count_feedback_events() == 1 # audit row still written + + +def test_record_retrieval_legacy_flag_updates_alpha( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """AELFRICE_EXPOSURE_UPDATES_POSTERIOR=1 restores the pre-#1086 + behaviour: a hook retrieval bumps alpha by HOOK_RETRIEVAL_VALENCE.""" + monkeypatch.setenv("AELFRICE_EXPOSURE_UPDATES_POSTERIOR", "1") s = _seed(_mk("b1")) record_retrieval(s, [s.get_belief("b1")]) # type: ignore[list-item] b = s.get_belief("b1")