Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 20 additions & 8 deletions src/aelfrice/feedback.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,15 +85,20 @@ def apply_feedback(
source: str,
now: str | None = None,
propagate: bool = True,
update_posterior: bool = True,
) -> FeedbackResult:
"""Apply one feedback event to one belief.

1. Resolve the belief; raise ValueError if missing.
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
Expand Down Expand Up @@ -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(
Expand All @@ -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.
Expand Down
33 changes: 28 additions & 5 deletions src/aelfrice/hook_search.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -31,6 +34,7 @@
"""
from __future__ import annotations

import os
import sys
import traceback
from typing import IO, Final, Iterable
Expand All @@ -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.

Expand Down Expand Up @@ -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:
Expand All @@ -108,6 +130,7 @@ def record_retrieval(
b.id,
valence,
source,
update_posterior=update_posterior,
)
written += 1
stamped_ids.append(b.id)
Expand Down
34 changes: 33 additions & 1 deletion tests/regression/test_hook_to_feedback_history_end_to_end.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
Expand Down
32 changes: 32 additions & 0 deletions tests/test_feedback_apply.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
22 changes: 21 additions & 1 deletion tests/test_hook_search.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
Loading