diff --git a/backend/tests/test_memory_staleness_review.py b/backend/tests/test_memory_staleness_review.py index 186850002..e353e4d75 100644 --- a/backend/tests/test_memory_staleness_review.py +++ b/backend/tests/test_memory_staleness_review.py @@ -12,6 +12,8 @@ Covers: from datetime import UTC, datetime, timedelta from unittest.mock import MagicMock, patch +import pytest + from deerflow.agents.memory.updater import ( MemoryUpdater, _build_staleness_section, @@ -24,6 +26,10 @@ from deerflow.config.memory_config import MemoryConfig # ── Helpers ──────────────────────────────────────────────────────────────── +_ABSENT = object() +"""Sentinel: the fact carries no ``confidence`` key at all.""" + + def _memory_config(**overrides: object) -> MemoryConfig: config = MemoryConfig() for key, value in overrides.items(): @@ -244,6 +250,65 @@ class TestBuildStalenessSection: assert 'pref<"erences>' not in section assert "pref<"erences>" in section + @pytest.mark.parametrize("stored_confidence", ["0.9", None, "high", ""]) + def test_non_float_confidence_does_not_raise(self, stored_confidence): + """A stored ``confidence`` that is not a float must not abort the update. + + ``memory.json`` is user-editable and written across versions, which is why + ``_coerce_source_confidence`` exists. Formatting it raw raises ValueError on + a str and TypeError on None; ``_do_update_memory_sync``'s ``except Exception`` + turns that into a silent ``return False`` that aborts the whole memory-update + cycle -- permanently, since the offending fact is then never rewritten. + """ + fact = _make_fact("fact_x", "Some fact", "knowledge", 0.9, days_ago=120) + fact["confidence"] = stored_confidence + + section = _build_staleness_section([fact], 90) + + assert "fact_x" in section + assert "Some fact" in section + + @pytest.mark.parametrize( + ("stored_confidence", "rendered"), + [ + ("0.9", "0.90"), + ("high", "0.50"), + (None, "0.50"), + (True, "0.50"), + (1.5, "1.00"), + (-0.3, "0.00"), + (float("inf"), "0.50"), + (float("nan"), "0.50"), + ], + ) + def test_confidence_is_normalised_like_every_other_stored_read(self, stored_confidence, rendered): + """Pins the mapping, not just the absence of a crash. + + ``0.5`` is this module's default for an unknown confidence + (``create_memory_fact``, ``_normalize_memory_update_fact``, + ``_coerce_source_confidence``). Before this change the staleness prompt + rendered ``1.5`` as ``1.50``, ``inf`` as ``inf``, and a ``True`` as ``1.00`` + -- disagreeing with the consolidation prompt, which reads the same field + through the same helper. + """ + fact = _make_fact("fact_x", "Some fact", "knowledge", 0.9, days_ago=120) + fact["confidence"] = stored_confidence + + section = _build_staleness_section([fact], 90) + + assert f"| {rendered} |" in section + + def test_missing_confidence_key_renders_unknown_not_zero(self): + """An absent key is *unknown* (0.50), not *worthless* (0.00). + + The staleness cap removes the lowest-confidence facts first, so ranking an + unreadable confidence at 0.00 would make that fact the first one deleted. + """ + fact = _make_fact("fact_x", "Some fact", "knowledge", 0.9, days_ago=120) + del fact["confidence"] + + assert "| 0.50 |" in _build_staleness_section([fact], 90) + # ── _apply_updates with staleness removals ───────────────────────────────── @@ -353,6 +418,121 @@ class TestApplyUpdatesStaleness: assert "fact_mid" in remaining_ids assert "fact_low1" in remaining_ids + def test_safety_cap_sort_survives_non_float_stored_confidence(self): + """The cap's ranking sort reads stored confidence and must coerce it. + + ``sort(key=lambda f: f.get("confidence", 0))`` compares a str against a + float and raises ``TypeError``, which the caller swallows into an aborted + update. Fixing only the prompt formatter would move this crash rather than + remove it, so the sort is pinned here too. ``"0.95"`` must rank like 0.95. + """ + updater = MemoryUpdater() + current_memory = _make_memory( + [ + _make_fact("fact_str_high", confidence="0.95", days_ago=100), + _make_fact("fact_mid", confidence=0.80, days_ago=100), + _make_fact("fact_low", confidence=0.60, days_ago=100), + ] + ) + update_data = { + "user": {}, + "history": {}, + "newFacts": [], + "factsToRemove": [], + "staleFactsToRemove": [ + {"id": "fact_str_high", "reason": "outdated"}, + {"id": "fact_mid", "reason": "outdated"}, + {"id": "fact_low", "reason": "outdated"}, + ], + } + + with patch( + "deerflow.agents.memory.updater.get_memory_config", + return_value=_memory_config(max_facts=100, staleness_max_removals_per_cycle=1), + ): + result = updater._apply_updates(current_memory, update_data) + + # Only the single lowest-confidence fact is removed; the string "0.95" + # ranks as the highest and survives. + remaining_ids = {f["id"] for f in result["facts"]} + assert remaining_ids == {"fact_str_high", "fact_mid"} + + @pytest.mark.parametrize( + ("stored_confidence", "rival_confidence", "survivor"), + [ + (_ABSENT, 0.1, "fact_x"), + (False, 0.1, "fact_x"), + (True, 0.9, "fact_rival"), + (float("inf"), 0.9, "fact_rival"), + ], + ) + def test_safety_cap_ranks_unusable_confidence_as_unknown(self, stored_confidence, rival_confidence, survivor): + """The cap deletes the lowest-ranked fact, so a mis-ranked one deletes its neighbour. + + Mirrors the max_facts trim's delta set with the sort reversed: here a + ``true``/``inf`` fact ranked *above* a genuine 0.9 and pushed it into + the removal slot. None of these raised under the old key, so the + string-coercion test above passes unchanged for all four. + """ + fact_x = _make_fact("fact_x", days_ago=100) + if stored_confidence is _ABSENT: + del fact_x["confidence"] + else: + fact_x["confidence"] = stored_confidence + update_data = { + "user": {}, + "history": {}, + "newFacts": [], + "factsToRemove": [], + "staleFactsToRemove": [ + {"id": "fact_x", "reason": "outdated"}, + {"id": "fact_rival", "reason": "outdated"}, + ], + } + + with patch( + "deerflow.agents.memory.updater.get_memory_config", + return_value=_memory_config(max_facts=100, staleness_max_removals_per_cycle=1), + ): + result = MemoryUpdater()._apply_updates( + _make_memory([fact_x, _make_fact("fact_rival", confidence=rival_confidence, days_ago=100)]), + update_data, + ) + + assert [f["id"] for f in result["facts"]] == [survivor] + + def test_safety_cap_with_nan_confidence_is_order_independent(self): + """Under the raw key the cap deleted either fact depending on their file order. + + ``nan`` compares false against every score, so ``sort`` leaves the pair + untouched: with the corrupted fact stored *second*, the genuine 0.9 one + landed in the removal slot instead. + """ + update_data = { + "user": {}, + "history": {}, + "newFacts": [], + "factsToRemove": [], + "staleFactsToRemove": [ + {"id": "fact_nan", "reason": "outdated"}, + {"id": "fact_rival", "reason": "outdated"}, + ], + } + + survivors = [] + for nan_first in (True, False): + nan_fact = _make_fact("fact_nan", confidence=float("nan"), days_ago=100) + rival = _make_fact("fact_rival", confidence=0.9, days_ago=100) + facts = [nan_fact, rival] if nan_first else [rival, nan_fact] + with patch( + "deerflow.agents.memory.updater.get_memory_config", + return_value=_memory_config(max_facts=100, staleness_max_removals_per_cycle=1), + ): + result = MemoryUpdater()._apply_updates(_make_memory(facts), update_data) + survivors.append(result["facts"][0]["id"]) + + assert survivors == ["fact_rival", "fact_rival"] + def test_empty_stale_removals_no_effect(self): updater = MemoryUpdater() current_memory = _make_memory( diff --git a/backend/tests/test_memory_updater.py b/backend/tests/test_memory_updater.py index 4c065ba0d..fe1ceebc6 100644 --- a/backend/tests/test_memory_updater.py +++ b/backend/tests/test_memory_updater.py @@ -2,6 +2,8 @@ import asyncio import threading from unittest.mock import AsyncMock, MagicMock, patch +import pytest + from deerflow.agents.memory.prompt import format_conversation_for_update from deerflow.agents.memory.updater import ( MemoryUpdater, @@ -14,6 +16,7 @@ from deerflow.agents.memory.updater import ( create_memory_fact_with_created_fact, delete_memory_fact, import_memory_data, + search_memory_facts, update_memory_fact, ) from deerflow.config.memory_config import MemoryConfig @@ -38,6 +41,10 @@ def _make_memory(facts: list[dict[str, object]] | None = None) -> dict[str, obje } +_ABSENT = object() +"""Sentinel: the fact carries no ``confidence`` key at all.""" + + def _memory_config(**overrides: object) -> MemoryConfig: config = MemoryConfig() for key, value in overrides.items(): @@ -255,6 +262,88 @@ def test_apply_updates_preserves_threshold_and_max_facts_trimming() -> None: assert result["facts"][1]["source"] == "thread-9" +def _searchable_fact(fact_id: str, confidence: object = _ABSENT) -> dict[str, object]: + fact: dict[str, object] = { + "id": fact_id, + "content": f"deploy runbook {fact_id}", + "category": "context", + "createdAt": "2026-03-18T00:00:00Z", + "source": "t", + } + if confidence is not _ABSENT: + fact["confidence"] = confidence + return fact + + +def test_search_memory_facts_sort_survives_non_float_stored_confidence() -> None: + """``memory_search`` ranks stored facts by confidence and must coerce it. + + ``sort(key=lambda f: f.get("confidence", 0))`` compares a str against a float + and raises ``TypeError``, which surfaces to the model as a failed tool call. + The stored ``"0.95"`` must rank as 0.95 and lead the results. + """ + facts = [ + _searchable_fact("f_low", 0.10), + _searchable_fact("f_str", "0.95"), + _searchable_fact("f_mid", 0.50), + ] + + with patch("deerflow.agents.memory.updater.get_memory_data", return_value=_make_memory(facts=facts)): + result = search_memory_facts("deploy runbook") + + assert [fact["id"] for fact in result] == ["f_str", "f_mid", "f_low"] + + +@pytest.mark.parametrize( + ("stored_confidence", "rival_confidence", "top"), + [ + # Ranked at the bottom by the old ``f.get("confidence", 0)`` key ... + (_ABSENT, 0.1, "f_x"), + (False, 0.1, "f_x"), + # ... and at the very top, because ``bool`` subclasses ``int`` and + # ``inf`` compares above every real score. (``nan`` also falls to the + # default, but its old ranking was order-dependent rather than pinned + # to an end — see the order-independence test below.) + (True, 0.9, "f_rival"), + (float("inf"), 0.9, "f_rival"), + ], +) +def test_search_memory_facts_ranks_unusable_confidence_as_unknown(stored_confidence, rival_confidence, top) -> None: + """Every value that falls to the 0.5 default must rank as *unknown*, not best or worst. + + The old key never raised on these — it silently mis-ranked them, so the + string-coercion test above cannot go red for any of them. ``true``/``inf`` + outranked a genuine 0.9 and pushed the better fact out of a capped result + set; a missing key ranked below a genuine 0.1 and dropped itself. ``limit`` + turns either mis-ranking into a wrong answer for the model. + """ + facts = [_searchable_fact("f_x", stored_confidence), _searchable_fact("f_rival", rival_confidence)] + + with patch("deerflow.agents.memory.updater.get_memory_data", return_value=_make_memory(facts=facts)): + result = search_memory_facts("deploy runbook", limit=1) + + assert [fact["id"] for fact in result] == [top] + + +def test_search_memory_facts_with_nan_confidence_is_order_independent() -> None: + """``nan`` compares false against everything, so a raw key leaves the sort undefined. + + Which fact the model gets back then depends on where the corrupted one happens + to sit in ``memory.json`` — the same file answers the same query differently + across two runs. Coercing ``nan`` to the 0.5 default restores a total order. + """ + tops = [] + for nan_first in (True, False): + nan_fact = _searchable_fact("f_nan", float("nan")) + rival = _searchable_fact("f_rival", 0.9) + facts = [nan_fact, rival] if nan_first else [rival, nan_fact] + with patch("deerflow.agents.memory.updater.get_memory_data", return_value=_make_memory(facts=facts)): + result = search_memory_facts("deploy runbook", limit=1) + tops.append(result[0]["id"]) + + assert tops == ["f_rival", "f_rival"] + + def test_apply_updates_preserves_source_error() -> None: updater = MemoryUpdater() current_memory = _make_memory()