From 2f327d85d76b5627aa6da2e59dcecc04c5e92b74 Mon Sep 17 00:00:00 2001 From: AIOSAI Date: Sat, 13 Jun 2026 16:06:00 -0700 Subject: [PATCH] =?UTF-8?q?fix(memory):=20DPLAN-0207=20P1=20hotfix=20?= =?UTF-8?q?=E2=80=94=20detector=20+=20learnings=20manager=20list-aware=20(?= =?UTF-8?q?key=5Flearnings)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- CHANGELOG.md | 12 +- .../memory/apps/handlers/learnings/manager.py | 132 ++++++++++-------- .../memory/apps/handlers/monitor/detector.py | 4 +- .../apps/handlers/rollover/extractor.py | 6 +- .../memory/apps/handlers/templates/differ.py | 2 +- src/aipass/memory/tests/test_detector.py | 46 ++++++ .../memory/tests/test_manager_vectorize.py | 39 +++++- 7 files changed, 174 insertions(+), 67 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index beeb12ee..f825187f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/src/aipass/memory/apps/handlers/learnings/manager.py b/src/aipass/memory/apps/handlers/learnings/manager.py index 6a769a30..62824ae3 100644 --- a/src/aipass/memory/apps/handlers/learnings/manager.py +++ b/src/aipass/memory/apps/handlers/learnings/manager.py @@ -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) diff --git a/src/aipass/memory/apps/handlers/monitor/detector.py b/src/aipass/memory/apps/handlers/monitor/detector.py index d16db1ba..5eb75bf6 100644 --- a/src/aipass/memory/apps/handlers/monitor/detector.py +++ b/src/aipass/memory/apps/handlers/monitor/detector.py @@ -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") diff --git a/src/aipass/memory/apps/handlers/rollover/extractor.py b/src/aipass/memory/apps/handlers/rollover/extractor.py index 44e0a2bb..0ea37458 100644 --- a/src/aipass/memory/apps/handlers/rollover/extractor.py +++ b/src/aipass/memory/apps/handlers/rollover/extractor.py @@ -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: diff --git a/src/aipass/memory/apps/handlers/templates/differ.py b/src/aipass/memory/apps/handlers/templates/differ.py index 747816b3..2a656f5d 100644 --- a/src/aipass/memory/apps/handlers/templates/differ.py +++ b/src/aipass/memory/apps/handlers/templates/differ.py @@ -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) diff --git a/src/aipass/memory/tests/test_detector.py b/src/aipass/memory/tests/test_detector.py index 82cebcb5..700eb804 100644 --- a/src/aipass/memory/tests/test_detector.py +++ b/src/aipass/memory/tests/test_detector.py @@ -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 diff --git a/src/aipass/memory/tests/test_manager_vectorize.py b/src/aipass/memory/tests/test_manager_vectorize.py index d750268b..b64b1134 100644 --- a/src/aipass/memory/tests/test_manager_vectorize.py +++ b/src/aipass/memory/tests/test_manager_vectorize.py @@ -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