fix(memory): DPLAN-0207 P1 hotfix — detector + learnings manager list-aware (key_learnings)
Migration surfaced two consumers that still counted key_learnings as a dict: detector v2 trigger (at-cap list invisible -> fell to v1 line-count) and learnings/manager (used by rollover + symbolic). Made list-aware + dual-mode; +5 regression tests (detector counts a LIST, manager round-trip) — the gap 955 tests missed. rollover check now shows '25/25 key_learnings' (v2), was '609/500 lines'. 960 tests; seedgo 99% (pre-existing unused-function on unwired add_learning, not a regression). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
7cf319b4cd
commit
2f327d85d7
+8
-4
@@ -21,10 +21,14 @@ PyPI version — not the changelog header.
|
||||
re-sorting on `number` — so an out-of-order write can never archive a fresh
|
||||
entry (the bug surfaced in S229, where rollover ate the *newest* key_learning
|
||||
instead of the oldest). Backward-compatible: un-migrated dict-shaped
|
||||
key_learnings skip cleanly, no crash. `@memory` self-migrated to
|
||||
`schema_version` 3.0.0 as the first specimen (955 tests, seedgo 100%).
|
||||
Cross-branch migration of all branches, plus `/memo`+`/prep` and @spawn
|
||||
template updates, follow in later phases.
|
||||
key_learnings skip cleanly, no crash. **All 17 branches migrated** to
|
||||
`schema_version` 3.0.0 (reversible per-file backups, no data loss). A
|
||||
follow-up made the rollover **detector** and the **learnings manager** (used
|
||||
by rollover + symbolic) list-aware — a live `rollover check` caught they still
|
||||
counted key_learnings as a dict, so an at-cap list was invisible to the
|
||||
detector (the 955 unit tests stayed green because none counted a *list*). 960
|
||||
tests; seedgo 99% (1 pre-existing unused-function on an unwired manager API).
|
||||
Remaining: `/memo`+`/prep` and @spawn template updates.
|
||||
|
||||
- **Memory config relocated to the json-home and unified behind one
|
||||
self-healing loader (FPLAN-0271).** `memory.config.json` moved from the loose
|
||||
|
||||
@@ -20,7 +20,8 @@ Purpose:
|
||||
historical data in searchable vector storage.
|
||||
|
||||
Format:
|
||||
key_learnings: dict with "name": "value... [2026-02-04]"
|
||||
key_learnings: list of {number, date, key, value} (v3, newest-first)
|
||||
or dict with "name": "value... [2026-02-04]" (legacy)
|
||||
recently_completed: list with "Task description [2026-02-04]"
|
||||
"""
|
||||
|
||||
@@ -161,36 +162,27 @@ def _find_learnings_location(data: Dict[str, Any]) -> Tuple[Dict[str, Any] | Non
|
||||
return (None, "")
|
||||
|
||||
|
||||
def _get_learnings(data: Dict[str, Any]) -> Dict[str, str]:
|
||||
def _get_learnings(data: Dict[str, Any]) -> list | Dict[str, str]:
|
||||
"""
|
||||
Get key_learnings from data regardless of location
|
||||
Get key_learnings from data regardless of location.
|
||||
|
||||
Args:
|
||||
data: Parsed JSON data
|
||||
|
||||
Returns:
|
||||
key_learnings dict, or empty dict if not found
|
||||
Returns list (v3 unified schema) or dict (legacy).
|
||||
"""
|
||||
parent, _ = _find_learnings_location(data)
|
||||
if parent is None:
|
||||
return {}
|
||||
return parent.get("key_learnings", {})
|
||||
return []
|
||||
kl = parent.get("key_learnings", [])
|
||||
return kl if isinstance(kl, (list, dict)) else []
|
||||
|
||||
|
||||
def _set_learnings(data: Dict[str, Any], learnings: Dict[str, str]) -> bool:
|
||||
def _set_learnings(data: Dict[str, Any], learnings: list | Dict[str, str]) -> bool:
|
||||
"""
|
||||
Set key_learnings in data at correct location
|
||||
Set key_learnings in data at correct location.
|
||||
|
||||
Args:
|
||||
data: Parsed JSON data
|
||||
learnings: New key_learnings dict
|
||||
|
||||
Returns:
|
||||
True if set successfully, False if no location found
|
||||
Accepts list (v3) or dict (legacy).
|
||||
"""
|
||||
parent, _ = _find_learnings_location(data)
|
||||
if parent is None:
|
||||
# Create at root level if not exists
|
||||
data["key_learnings"] = learnings
|
||||
return True
|
||||
parent["key_learnings"] = learnings
|
||||
@@ -516,11 +508,17 @@ def ensure_timestamps(file_path: Path) -> Dict[str, Any]:
|
||||
updated_count = 0
|
||||
today = datetime.now().strftime("%Y-%m-%d")
|
||||
|
||||
for key, value in learnings.items():
|
||||
_, timestamp = parse_timestamp(value)
|
||||
if timestamp is None:
|
||||
learnings[key] = add_timestamp(value, today)
|
||||
updated_count += 1
|
||||
if isinstance(learnings, list):
|
||||
for entry in learnings:
|
||||
if isinstance(entry, dict) and "date" not in entry:
|
||||
entry["date"] = today
|
||||
updated_count += 1
|
||||
else:
|
||||
for key, value in learnings.items():
|
||||
_, timestamp = parse_timestamp(value)
|
||||
if timestamp is None:
|
||||
learnings[key] = add_timestamp(value, today)
|
||||
updated_count += 1
|
||||
|
||||
if updated_count > 0:
|
||||
_set_learnings(data, learnings)
|
||||
@@ -575,30 +573,32 @@ def enforce_limit(file_path: Path) -> Dict[str, Any]:
|
||||
"message": "Under limit, no action needed",
|
||||
}
|
||||
|
||||
# Sort by age (oldest first)
|
||||
sorted_entries = sorted(
|
||||
learnings.items(),
|
||||
key=lambda x: get_entry_age(x[1]),
|
||||
reverse=True, # Oldest first
|
||||
)
|
||||
|
||||
# Calculate how many to remove
|
||||
to_remove_count = current_count - max_entries
|
||||
to_remove = sorted_entries[:to_remove_count]
|
||||
to_keep = sorted_entries[to_remove_count:]
|
||||
|
||||
# Extract branch name from filename
|
||||
parts = file_path.stem.split(".")
|
||||
branch_name = parts[0] if parts else "UNKNOWN"
|
||||
|
||||
# Vectorize before removing
|
||||
vectorize_result = _vectorize_learnings(branch_name, to_remove)
|
||||
to_remove_count = current_count - max_entries
|
||||
|
||||
# Continue even if vectorization fails - don't block removal
|
||||
# Caller (module) will log if needed based on vectorize_result['success']
|
||||
|
||||
# Update key_learnings with remaining entries
|
||||
_set_learnings(data, dict(to_keep))
|
||||
if isinstance(learnings, list):
|
||||
# v3 list: oldest entries are at the end (lowest number)
|
||||
to_remove_entries = learnings[-to_remove_count:]
|
||||
to_keep_entries = learnings[:-to_remove_count]
|
||||
to_vectorize = [(e.get("key", ""), e.get("value", "")) for e in to_remove_entries if isinstance(e, dict)]
|
||||
removed_keys = [e.get("key", "") for e in to_remove_entries if isinstance(e, dict)]
|
||||
vectorize_result = _vectorize_learnings(branch_name, to_vectorize)
|
||||
_set_learnings(data, to_keep_entries)
|
||||
else:
|
||||
# Legacy dict: sort by age, oldest first
|
||||
sorted_entries = sorted(
|
||||
learnings.items(),
|
||||
key=lambda x: get_entry_age(x[1]),
|
||||
reverse=True,
|
||||
)
|
||||
to_remove = sorted_entries[:to_remove_count]
|
||||
to_keep = sorted_entries[to_remove_count:]
|
||||
removed_keys = [k for k, _ in to_remove]
|
||||
vectorize_result = _vectorize_learnings(branch_name, to_remove)
|
||||
_set_learnings(data, dict(to_keep))
|
||||
|
||||
try:
|
||||
write_memory_file_simple(file_path, data)
|
||||
@@ -606,17 +606,16 @@ def enforce_limit(file_path: Path) -> Dict[str, Any]:
|
||||
logger.warning(f"[learnings_manager] Failed to write file: {e}")
|
||||
return {"success": False, "error": f"Failed to write file: {e}"}
|
||||
|
||||
json_handler.log_operation(
|
||||
"enforce_limit", {"removed": to_remove_count, "remaining": len(to_keep), "success": True}
|
||||
)
|
||||
remaining = current_count - to_remove_count
|
||||
json_handler.log_operation("enforce_limit", {"removed": to_remove_count, "remaining": remaining, "success": True})
|
||||
|
||||
return {
|
||||
"success": True,
|
||||
"removed": to_remove_count,
|
||||
"vectorized": vectorize_result.get("success", False),
|
||||
"remaining": len(to_keep),
|
||||
"remaining": remaining,
|
||||
"max": max_entries,
|
||||
"removed_keys": [k for k, _ in to_remove],
|
||||
"removed_keys": removed_keys,
|
||||
}
|
||||
|
||||
|
||||
@@ -781,16 +780,37 @@ def add_learning(file_path: Path, key: str, value: str) -> Dict[str, Any]:
|
||||
logger.warning(f"[learnings_manager] Failed to read file: {e}")
|
||||
return {"success": False, "error": f"Failed to read file: {e}"}
|
||||
|
||||
# Get existing learnings or create empty dict
|
||||
learnings = _get_learnings(data)
|
||||
if not learnings:
|
||||
learnings = {}
|
||||
|
||||
# Add with timestamp
|
||||
timestamped_value = add_timestamp(value)
|
||||
is_update = key in learnings
|
||||
learnings[key] = timestamped_value
|
||||
_set_learnings(data, learnings)
|
||||
if isinstance(learnings, list):
|
||||
# v3 list: find existing entry by key, or insert new at front
|
||||
is_update = False
|
||||
for entry in learnings:
|
||||
if isinstance(entry, dict) and entry.get("key") == key:
|
||||
entry["value"] = value
|
||||
entry["date"] = datetime.now().strftime("%Y-%m-%d")
|
||||
is_update = True
|
||||
break
|
||||
if not is_update:
|
||||
max_num = max((e.get("number", 0) for e in learnings if isinstance(e, dict)), default=0)
|
||||
learnings.insert(
|
||||
0,
|
||||
{
|
||||
"number": max_num + 1,
|
||||
"date": datetime.now().strftime("%Y-%m-%d"),
|
||||
"key": key,
|
||||
"value": value,
|
||||
},
|
||||
)
|
||||
timestamped_value = value
|
||||
_set_learnings(data, learnings)
|
||||
else:
|
||||
if not learnings:
|
||||
learnings = {}
|
||||
timestamped_value = add_timestamp(value)
|
||||
is_update = key in learnings
|
||||
learnings[key] = timestamped_value
|
||||
_set_learnings(data, learnings)
|
||||
|
||||
try:
|
||||
write_memory_file_simple(file_path, data)
|
||||
|
||||
@@ -286,8 +286,8 @@ def _should_rollover(file_path: Path) -> tuple[bool, int, int, str, str]:
|
||||
|
||||
max_key_learnings = limits.get("max_key_learnings")
|
||||
if max_key_learnings is not None:
|
||||
key_learnings = data.get("key_learnings", {})
|
||||
if isinstance(key_learnings, dict) and len(key_learnings) >= max_key_learnings:
|
||||
key_learnings = data.get("key_learnings", [])
|
||||
if isinstance(key_learnings, (list, dict)) and len(key_learnings) >= max_key_learnings:
|
||||
reasons.append(f"{len(key_learnings)}/{max_key_learnings} key_learnings")
|
||||
|
||||
max_observations = limits.get("max_observations")
|
||||
|
||||
@@ -10,7 +10,7 @@
|
||||
Memory Extraction Handler
|
||||
|
||||
Surgically extracts oldest items from memory files during rollover.
|
||||
Understands real JSON structure (sessions, observations arrays, key_learnings dict).
|
||||
Understands real JSON structure (sessions, observations, key_learnings arrays).
|
||||
|
||||
Purpose:
|
||||
v1 (schema <2.0.0): When file exceeds max_lines, extract oldest items from
|
||||
@@ -21,7 +21,7 @@ Purpose:
|
||||
Strategy:
|
||||
- Detect schema version from document_metadata
|
||||
- v1: line-count based extraction (legacy)
|
||||
- v2: entry-count based extraction (sessions array + key_learnings dict)
|
||||
- v2: entry-count based extraction (sessions + key_learnings + observations arrays)
|
||||
- Extract oldest items (FIFO)
|
||||
- Update document_metadata.status
|
||||
"""
|
||||
@@ -261,7 +261,7 @@ def _extract_items_v2(file_path: Path, data: Dict[str, Any]) -> Dict[str, Any]:
|
||||
"""
|
||||
Extract items from v2 format file (entry-count based).
|
||||
|
||||
Handles sessions (array, oldest at end) and key_learnings (dict, oldest first).
|
||||
Handles sessions, key_learnings, and observations (arrays, newest-first, oldest at end).
|
||||
Trims to max_sessions / max_key_learnings limits defined in document_metadata.
|
||||
|
||||
Args:
|
||||
|
||||
@@ -266,7 +266,7 @@ def diff_template_vs_branch(branch_path: str | Path) -> dict:
|
||||
if "key_learnings" not in current:
|
||||
active = current.get("active_tasks", {})
|
||||
if not isinstance(active, dict) or "key_learnings" not in active:
|
||||
file_diff["additions"].append("key_learnings: {} (missing)")
|
||||
file_diff["additions"].append("key_learnings: [] (missing)")
|
||||
|
||||
if file_diff["additions"] or file_diff["removals"] or file_diff["modifications"]:
|
||||
result["local"].append(file_diff)
|
||||
|
||||
@@ -303,6 +303,52 @@ class TestCheckSingleFile:
|
||||
assert result["success"] is True
|
||||
assert result["should_rollover"] is False
|
||||
|
||||
def test_v2_list_key_learnings_triggers_rollover(self, tmp_path: Path):
|
||||
"""List-shaped key_learnings at/over max_key_learnings triggers v2 rollover."""
|
||||
mem_file = tmp_path / "DEVPULSE.local.json"
|
||||
data = {
|
||||
"document_metadata": {
|
||||
"schema_version": "3.0.0",
|
||||
"limits": {"max_key_learnings": 3},
|
||||
},
|
||||
"key_learnings": [
|
||||
{"number": 3, "date": "2026-06-13", "key": "c", "value": "vc"},
|
||||
{"number": 2, "date": "2026-06-12", "key": "b", "value": "vb"},
|
||||
{"number": 1, "date": "2026-06-11", "key": "a", "value": "va"},
|
||||
],
|
||||
}
|
||||
mem_file.write_text(json.dumps(data, indent=2), encoding="utf-8")
|
||||
|
||||
from aipass.memory.apps.handlers.monitor.detector import check_single_file
|
||||
|
||||
result = check_single_file(mem_file)
|
||||
|
||||
assert result["success"] is True
|
||||
assert result["should_rollover"] is True
|
||||
assert "3/3 key_learnings" in result["trigger"].v2_reason
|
||||
|
||||
def test_v2_list_key_learnings_under_limit_no_trigger(self, tmp_path: Path):
|
||||
"""List-shaped key_learnings under limit does not trigger."""
|
||||
mem_file = tmp_path / "DRONE.local.json"
|
||||
data = {
|
||||
"document_metadata": {
|
||||
"schema_version": "3.0.0",
|
||||
"limits": {"max_key_learnings": 10},
|
||||
},
|
||||
"key_learnings": [
|
||||
{"number": 2, "date": "2026-06-13", "key": "b", "value": "vb"},
|
||||
{"number": 1, "date": "2026-06-12", "key": "a", "value": "va"},
|
||||
],
|
||||
}
|
||||
mem_file.write_text(json.dumps(data, indent=2), encoding="utf-8")
|
||||
|
||||
from aipass.memory.apps.handlers.monitor.detector import check_single_file
|
||||
|
||||
result = check_single_file(mem_file)
|
||||
|
||||
assert result["success"] is True
|
||||
assert result["should_rollover"] is False
|
||||
|
||||
|
||||
# ===========================================================================
|
||||
# _read_registry
|
||||
|
||||
@@ -86,7 +86,7 @@ class TestGetSetLearnings:
|
||||
mgr, _ = _import_manager(monkeypatch)
|
||||
data = {"something_else": True}
|
||||
result = mgr._get_learnings(data)
|
||||
assert result == {}
|
||||
assert result == []
|
||||
|
||||
def test_set_learnings_existing(self, monkeypatch):
|
||||
mgr, _ = _import_manager(monkeypatch)
|
||||
@@ -102,6 +102,43 @@ class TestGetSetLearnings:
|
||||
assert ok is True
|
||||
assert data["key_learnings"] == {"fresh": "entry"}
|
||||
|
||||
def test_get_learnings_list(self, monkeypatch):
|
||||
"""_get_learnings returns list when key_learnings is a list (v3)."""
|
||||
mgr, _ = _import_manager(monkeypatch)
|
||||
entries = [
|
||||
{"number": 2, "date": "2026-06-13", "key": "b", "value": "vb"},
|
||||
{"number": 1, "date": "2026-06-12", "key": "a", "value": "va"},
|
||||
]
|
||||
data = {"key_learnings": entries}
|
||||
result = mgr._get_learnings(data)
|
||||
assert isinstance(result, list)
|
||||
assert len(result) == 2
|
||||
assert result[0]["key"] == "b"
|
||||
|
||||
def test_set_learnings_list(self, monkeypatch):
|
||||
"""_set_learnings accepts and stores a list (v3)."""
|
||||
mgr, _ = _import_manager(monkeypatch)
|
||||
entries = [{"number": 1, "date": "2026-06-13", "key": "a", "value": "va"}]
|
||||
data = {"key_learnings": []}
|
||||
ok = mgr._set_learnings(data, entries)
|
||||
assert ok is True
|
||||
assert isinstance(data["key_learnings"], list)
|
||||
assert data["key_learnings"][0]["key"] == "a"
|
||||
|
||||
def test_get_set_list_roundtrip(self, monkeypatch):
|
||||
"""Round-trip: get list, modify, set back."""
|
||||
mgr, _ = _import_manager(monkeypatch)
|
||||
entries = [
|
||||
{"number": 2, "date": "2026-06-13", "key": "b", "value": "vb"},
|
||||
{"number": 1, "date": "2026-06-12", "key": "a", "value": "va"},
|
||||
]
|
||||
data = {"key_learnings": list(entries)}
|
||||
learnings = mgr._get_learnings(data)
|
||||
learnings.append({"number": 3, "date": "2026-06-14", "key": "c", "value": "vc"})
|
||||
mgr._set_learnings(data, learnings)
|
||||
assert len(data["key_learnings"]) == 3
|
||||
assert data["key_learnings"][2]["key"] == "c"
|
||||
|
||||
|
||||
# ===========================================================================
|
||||
# _find_recently_completed_location
|
||||
|
||||
Reference in New Issue
Block a user