mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-07-25 15:37:53 +00:00
fix(memory): coerce stored confidence in the three remaining raw reads (#4034)
* fix(memory): coerce stored confidence in the three remaining raw reads `_coerce_source_confidence` exists because `memory.json` is user-editable and written across versions: a `confidence` that is null, a str, a bool, out of range, or non-finite still has to read back as a usable score. Consolidation already routes every stored read through it, and #4023 did the same for the max_facts trim. Three reads still take the field raw. `_build_staleness_section` formats it with `f"{conf:.2f}"` — a str raises ValueError, None raises TypeError. `_do_update_memory_sync`'s `except Exception` swallows that into `return False`, aborting the whole memory-update cycle permanently, since the offending fact is then never rewritten. The staleness cap's `sort(key=lambda f: f.get("confidence", 0))` ranks the facts it is about to delete. A str raises the same way; the values that don't raise mis-rank instead. `true`/`inf` outrank a genuine 0.9 and push it into the removal slot; an absent key ranks 0 and is deleted first; `nan` compares false against everything, so which fact dies depends on where the corrupted one happens to sit in the file. `search_memory_facts` — the `memory_search` tool, added in #4023 — ranks results the same raw way and then truncates to `limit`, so the same mis-ranking hands the model the wrong facts, or fails the tool call outright on a str. All three now read through the helper, so an unusable stored confidence ranks as unknown (0.5): neither kept ahead of a real score nor evicted before one. The two sorts run in opposite directions, and 0.5 is load-bearing in both. * test(memory): anchor the confidence delta at every coerced read The str-based tests raise on main, so they cannot go red for any stored confidence that mis-ranks without raising. `true`/`false`/`inf` and an absent key never reached the except handler at all: bool subclasses int, so `true` ranked 1.0; `inf` outranked every real score; an absent key ranked 0. Silent fact loss in the staleness cap, wrong results out of `memory_search` — no log line either way. Parametrize both ranking sorts over the four non-raising inputs that fall to the 0.5 default, each pitted against a genuine neighbour chosen so the survivor flips. The sorts run in opposite directions, so the one matrix covers eviction from both ends. `nan` is excluded from those: its old rank is undefined rather than pinned to an end of the order, so one fixed input order happens to yield the correct survivor. Its real property is order-independence, asserted across both fact orders. The staleness prompt formatter is pinned by rendered value, not merely by not raising: 1.5 → 1.00, -0.3 → 0.00, and inf/nan/true/None → 0.50, matching the consolidation prompt that reads the same field through the same helper.
This commit is contained in:
parent
b53c1ae0e0
commit
e32456d337
@ -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(
|
||||
|
||||
@ -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()
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user