fix: restore top-20 random selection and add learn topic rotation (jaredlockhart#553)

The heat model rewrite (PR jaredlockhart#528) accidentally replaced the top-N random
entity selection with deterministic highest-heat-wins. This also introduced
a hard learn-topic dedup filter that permanently blocked all entities from
the same /learn command after the first notification.

Restores random selection from top 20 candidates and replaces the hard
learn-topic block with a soft rotation preference that sorts by least-
recently-notified topic first, then by heat.

Co-authored-by: Jared Lockhart <119884+jaredlockhart@users.noreply.github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>

Author: 3 people

penny/penny/agents/notification.py

@@ -11,6 +11,7 @@
 
 import asyncio
 import logging
+import random
 from collections import defaultdict
 from datetime import UTC, datetime
 from typing import TYPE_CHECKING
@@ -46,7 +47,7 @@ def __init__(self, **kwargs: object) -> None:
         super().__init__(**kwargs)  # type: ignore[arg-type]
         self._channel: MessageChannel | None = None
         self._backoff_state: dict[str, BackoffState] = {}
-        self._last_notified_learn_prompt_id: dict[str, int | None] = {}
+        self._learn_topic_last_notified_at: dict[str, dict[int | None, datetime]] = {}
         self._last_notification: dict[str, tuple[int, datetime]] = {}  # user → (entity_id, sent_at)
         self._heat_engine: HeatEngine | None = None
 
@@ -381,9 +382,8 @@ async def _try_notify_user(self, user: str) -> bool:
         for fact in unnotified:
             facts_by_entity[fact.entity_id].append(fact)
 
-        # Pick the hottest eligible entity
-        last_learn_prompt_id = self._last_notified_learn_prompt_id.get(user)
-        entity = self._pick_hottest_entity(user, facts_by_entity, last_learn_prompt_id)
+        # Pick an eligible entity (prefers stale learn topics, top 20 by heat, random)
+        entity = self._pick_hottest_entity(user, facts_by_entity)
         if entity is None:
             return False
         assert entity.id is not None
@@ -406,8 +406,11 @@ async def _try_notify_user(self, user: str) -> bool:
         if self._heat_engine:
             self._heat_engine.start_cooldown(entity.id)
 
-        # Track which learn prompt this notification came from (to suppress same-topic dedup)
-        self._last_notified_learn_prompt_id[user] = self._get_learn_prompt_id(facts)
+        # Track when this learn topic was last notified (for rotation preference)
+        learn_prompt_id = self._get_learn_prompt_id(facts)
+        if user not in self._learn_topic_last_notified_at:
+            self._learn_topic_last_notified_at[user] = {}
+        self._learn_topic_last_notified_at[user][learn_prompt_id] = datetime.now(UTC)
 
         # Track this notification for ignore detection
         self._last_notification[user] = (entity.id, datetime.now(UTC))
@@ -421,20 +424,20 @@ def _pick_hottest_entity(
         self,
         user: str,
         facts_by_entity: dict[int, list[Fact]],
-        last_notified_learn_prompt_id: int | None = None,
     ) -> Entity | None:
-        """Pick the hottest eligible entity deterministically.
+        """Pick an eligible entity, preferring learn topic rotation.
 
-        Filters by: heat > 0, not on cooldown, not same learn topic.
-        Returns the entity with the highest heat, or None.
+        Sorts by least-recently-notified learn topic first, takes the top 20
+        by heat, then picks randomly. This naturally rotates between learn
+        topics while still favoring hot entities.
         """
-        eligible = self._get_eligible_entities(user, facts_by_entity, last_notified_learn_prompt_id)
+        eligible = self._get_eligible_entities(user, facts_by_entity)
         if not eligible:
             return None
 
-        # Sort by heat descending, pick the hottest
-        eligible.sort(key=lambda e: e.heat, reverse=True)
-        chosen = eligible[0]
+        ranked = self._rank_by_topic_recency_then_heat(user, eligible, facts_by_entity)
+        top = ranked[:20]
+        chosen = random.choice(top)
 
         logger.info(
             "Notification: picked '%s' (heat=%.2f) from %d eligible",
@@ -444,30 +447,43 @@ def _pick_hottest_entity(
         )
         return chosen
 
+    def _rank_by_topic_recency_then_heat(
+        self,
+        user: str,
+        entities: list[Entity],
+        facts_by_entity: dict[int, list[Fact]],
+    ) -> list[Entity]:
+        """Sort entities by learn topic recency (stale first), then heat."""
+        topic_times = self._learn_topic_last_notified_at.get(user, {})
+        epoch = datetime.min.replace(tzinfo=UTC)
+
+        def sort_key(entity: Entity) -> tuple[datetime, float]:
+            assert entity.id is not None
+            facts = facts_by_entity.get(entity.id, [])
+            lp_id = self._get_learn_prompt_id(facts)
+            last_notified = topic_times.get(lp_id, epoch)
+            return (last_notified, -entity.heat)
+
+        return sorted(entities, key=sort_key)
+
     def _get_eligible_entities(
         self,
         user: str,
         facts_by_entity: dict[int, list[Fact]],
-        last_notified_learn_prompt_id: int | None,
     ) -> list[Entity]:
-        """Get entities with unnotified facts that pass all eligibility filters."""
+        """Get entities with unnotified facts that pass heat and cooldown filters."""
         entities = self.db.entities.get_for_user(user)
         eligible: list[Entity] = []
         for entity in entities:
             if entity.id not in facts_by_entity:
                 continue
-            if not self._is_eligible(entity, facts_by_entity, last_notified_learn_prompt_id):
+            if not self._is_eligible(entity):
                 continue
             eligible.append(entity)
         return eligible
 
-    def _is_eligible(
-        self,
-        entity: Entity,
-        facts_by_entity: dict[int, list[Fact]],
-        last_notified_learn_prompt_id: int | None,
-    ) -> bool:
-        """Check heat, cooldown, and learn-topic dedup filters."""
+    def _is_eligible(self, entity: Entity) -> bool:
+        """Check heat and cooldown filters."""
         assert entity.id is not None
 
         if entity.heat <= 0:
@@ -483,24 +499,8 @@ def _is_eligible(
                     cooldown_until.isoformat(),
                 )
                 return False
-        facts = facts_by_entity.get(entity.id, [])
-        if self._is_same_learn_topic(facts, last_notified_learn_prompt_id):
-            logger.debug(
-                "Notification: skipping '%s' (same learn topic as last notification)",
-                entity.name,
-            )
-            return False
         return True
 
-    def _is_same_learn_topic(
-        self, facts: list[Fact], last_notified_learn_prompt_id: int | None
-    ) -> bool:
-        """Check if these facts share the same learn topic as the last notification."""
-        if last_notified_learn_prompt_id is None:
-            return False
-        entity_learn_prompt_id = self._get_learn_prompt_id(facts)
-        return entity_learn_prompt_id == last_notified_learn_prompt_id
-
     def _handle_ignored_notification(self, user: str) -> None:
         """Check if the previous notification was ignored and penalize heat.
 

penny/penny/tests/agents/test_notification.py

@@ -22,7 +22,7 @@ async def test_notification_prefers_higher_heat_entity(
     test_user_info,
     running_penny,
 ):
-    """Notification agent picks the entity with higher heat deterministically."""
+    """Notification agent picks from eligible entities (random from top 20 by heat)."""
     config = make_config()
 
     captured_prompts: list[str] = []
@@ -31,13 +31,11 @@ def handler(request: dict, count: int) -> dict:
         messages = request.get("messages", [])
         prompt = messages[-1]["content"] if messages else ""
         captured_prompts.append(prompt)
-        if "came across" in prompt:
-            return mock_ollama._make_text_response(
-                request,
-                "Hey, I came across **interesting entity** recently and found some"
-                " really interesting stuff worth sharing!",
-            )
-        return mock_ollama._make_text_response(request, "ok")
+        return mock_ollama._make_text_response(
+            request,
+            "Hey, I came across something recently and found some"
+            " really interesting stuff worth sharing!",
+        )
 
     mock_ollama.set_response_handler(handler)
 
@@ -65,10 +63,9 @@ def handler(request: dict, count: int) -> dict:
         result = await penny.notification_agent.execute()
         assert result is True
 
-        # Should notify about the interesting entity (higher heat)
+        # One of the eligible entities should be picked
         msgs = signal_server.outgoing_messages
         assert len(msgs) == 1
-        assert "interesting entity" in msgs[0]["message"]
 
         # Prompt sent to model should instruct it to synthesize, not echo raw facts
         assert any("Synthesize" in p for p in captured_prompts)
@@ -276,7 +273,7 @@ def handler(request: dict, count: int) -> dict:
         )
         penny.db.messages.mark_processed([msg_id])
 
-        # Create two entities — give entity A higher heat so it's picked first
+        # Create two entities with heat
         entity_a = penny.db.entities.get_or_create(TEST_SENDER, "entity alpha")
         entity_b = penny.db.entities.get_or_create(TEST_SENDER, "entity beta")
         assert entity_a is not None and entity_a.id is not None
@@ -290,16 +287,24 @@ def handler(request: dict, count: int) -> dict:
 
         agent = penny.notification_agent
 
-        # Cycle 1: entity A picked (highest heat)
+        # Cycle 1: one entity is picked (random from top 20)
         signal_server.outgoing_messages.clear()
         result1 = await agent.execute()
         assert result1 is True
-        # Verify entity A was notified and has cooldown set
+
+        # Determine which entity was picked by checking notified_at
         facts_a = penny.db.facts.get_for_entity(entity_a.id)
-        assert any(f.notified_at is not None for f in facts_a)
-        entity_a_refreshed = penny.db.entities.get(entity_a.id)
-        assert entity_a_refreshed is not None
-        assert entity_a_refreshed.heat_cooldown_until is not None
+        facts_b = penny.db.facts.get_for_entity(entity_b.id)
+        a_notified = any(f.notified_at is not None for f in facts_a)
+        b_notified = any(f.notified_at is not None for f in facts_b)
+        assert a_notified or b_notified
+        first_id = entity_a.id if a_notified else entity_b.id
+        second_id = entity_b.id if a_notified else entity_a.id
+
+        # Verify picked entity has cooldown set
+        first_refreshed = penny.db.entities.get(first_id)
+        assert first_refreshed is not None
+        assert first_refreshed.heat_cooldown_until is not None
 
         # Add new facts to both
         penny.db.facts.add(entity_b.id, "Another beta fact")
@@ -315,13 +320,13 @@ def handler(request: dict, count: int) -> dict:
         interaction_recorded = datetime.now(UTC)
         await wait_until(lambda: (datetime.now(UTC) - interaction_recorded).total_seconds() >= 0.1)
 
-        # Cycle 2: entity A is on cooldown, so entity B is picked instead
+        # Cycle 2: first entity is on cooldown, so the other is picked
         signal_server.outgoing_messages.clear()
         result2 = await agent.execute()
         assert result2 is True
-        # Verify entity B was notified this time (cooldown forced rotation)
-        facts_b = penny.db.facts.get_for_entity(entity_b.id)
-        assert any(f.notified_at is not None for f in facts_b)
+        # Verify the second entity was notified this time (cooldown forced rotation)
+        second_facts = penny.db.facts.get_for_entity(second_id)
+        assert any(f.notified_at is not None for f in second_facts)
 
 
 @pytest.mark.asyncio