feat(memory): DPLAN-0207 P1 — unified entry schema (key_learnings dict→numbered list, sort-by-number rollover guardrail)

All 4 .trinity entry types now numbered+dated, list-shaped, newest-first. Rollover trims oldest by number; normalizer self-heals ordering (fixes S229 where rollover archived the newest key_learning). Backward-compatible: un-migrated dict key_learnings skip gracefully. @memory self-migrated to schema 3.0.0. 955 tests, seedgo 100%. Cross-branch migration = P2. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-13 15:35:43 -07:00
parent a8d05e2602
commit 7cf319b4cd
10 changed files with 416 additions and 19 deletions
@@ -13,6 +13,19 @@ PyPI version — not the changelog header.

 ### Changed

+- **Unified memory entry schema — Phase 1 (DPLAN-0207).** All four `.trinity`
+  entry types (`key_learnings`, `sessions`, `todos`, `observations`) move to one
+  shape: numbered + dated, list-shaped, newest-first. `key_learnings` converts
+  from a dict to a numbered list; the rollover extractor now trims the **oldest
+  by number from the tail**, and the schema normalizer self-heals ordering by
+  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.
+
 - **Memory config relocated to the json-home and unified behind one
  self-healing loader (FPLAN-0271).** `memory.config.json` moved from the loose
  tracked `config/` dir into the gitignored `memory_json/custom_config/`
@@ -705,6 +705,56 @@
      "file": "tests/test_config_loader.py",
      "standard": "meta",
      "reason": "Test file — META block present at lines 1-7; hook false-positive on test file format."
+    },
+    {
+      "file": "tests/test_handlers.py",
+      "standard": "architecture",
+      "reason": "Test file — lives in tests/ by design, not in 3-layer apps/ structure."
+    },
+    {
+      "file": "tests/test_handlers.py",
+      "standard": "documentation",
+      "reason": "Test file — test helper functions (fake_write, _make_v2_data, etc.) don't require docstrings."
+    },
+    {
+      "file": "tests/test_handlers.py",
+      "standard": "encapsulation",
+      "reason": "Test file — direct handler imports are correct for unit testing handler internals."
+    },
+    {
+      "file": "tests/test_handlers.py",
+      "standard": "meta",
+      "reason": "Test file — META block present at lines 1-7; hook false-positive on test file format."
+    },
+    {
+      "file": "tests/test_unified_schema.py",
+      "standard": "architecture",
+      "reason": "Test file — lives in tests/ by design, not in 3-layer apps/ structure."
+    },
+    {
+      "file": "tests/test_unified_schema.py",
+      "standard": "encapsulation",
+      "reason": "Test file — direct handler imports are correct for unit testing handler internals."
+    },
+    {
+      "file": "tests/test_unified_schema.py",
+      "standard": "meta",
+      "reason": "Test file — META block present at lines 1-7; hook false-positive on test file format."
+    },
+    {
+      "file": "tools/migrate_entries.py",
+      "standard": "architecture",
+      "reason": "Standalone utility script in tools/ — intentionally outside 3-layer apps/ structure."
+    },
+    {
+      "file": "tools/migrate_entries.py",
+      "standard": "silent_catch",
+      "reason": "Standalone script — cannot import prax logger. Errors reported via stderr print + result dict."
+    },
+    {
+      "file": "tools/migrate_entries.py",
+      "standard": "debug_print",
+      "reason": "Standalone script — print(stderr) is the error reporting mechanism. No prax available."
    }
  ],
  "notes": {
@@ -88,7 +88,7 @@ DEFAULT_CONFIG: dict[str, Any] = {
            "key_learnings": {
                "file": "local.json",
                "container": "key_learnings",
-                "kind": "dict",
+                "kind": "list",
                "field": "value",
                "max_chars": 200,
            },
@@ -286,17 +286,15 @@ def _extract_items_v2(file_path: Path, data: Dict[str, Any]) -> Dict[str, Any]:
            data["sessions"] = sessions[:-excess]  # keep newest
            all_extracted.extend(extracted_sessions)

-    # Extract from key_learnings dict (first keys are oldest in insertion order)
+    # Extract from key_learnings list (sorted newest-first; oldest at end)
    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) and len(key_learnings) >= max_key_learnings:
            excess = max(len(key_learnings) - max_key_learnings, 1)
-            keys_list = list(key_learnings.keys())
-            keys_to_extract = keys_list[:excess]  # oldest (first inserted)
-            for k in keys_to_extract:
-                all_extracted.append({"_type": "key_learning", "key": k, "value": key_learnings[k]})
-                del data["key_learnings"][k]
+            extracted_kl = key_learnings[-excess:]  # oldest from end
+            data["key_learnings"] = key_learnings[:-excess]  # keep newest
+            all_extracted.extend(extracted_kl)

    # Extract from observations array (if v2 observations file)
    max_observations = limits.get("max_observations")
@@ -242,8 +242,8 @@ def extract_text_from_memories(memories: List[Dict]) -> List[str]:
        elif "summary" in memory:
            # Sessions type (v2) - summary field
            text = str(memory["summary"])
-        elif "_type" in memory and memory["_type"] == "key_learning":
-            # Key learnings (v2) - key:value pair
+        elif "key" in memory and "value" in memory:
+            # Key learnings (unified) - key:value pair
            text = f"{memory.get('key', '')}: {memory.get('value', '')}"
        elif "content" in memory:
            text = str(memory["content"])
@@ -141,6 +141,17 @@ def normalize_memory_file(file_path: Path, dry_run: bool = False) -> Dict[str, A
        if "status" in metadata:
            _strip_orphan_keys(metadata["status"], set(tmpl_status.keys()), "status", changes)

+    # Sort list entries newest-first by number (self-heal guardrail)
+    for container_name in ("sessions", "key_learnings", "todos", "observations"):
+        container = data.get(container_name)
+        if isinstance(container, list) and len(container) > 1:
+            has_numbers = all(isinstance(e, dict) and "number" in e for e in container)
+            if has_numbers:
+                sorted_entries = sorted(container, key=lambda e: e["number"], reverse=True)
+                if sorted_entries != container:
+                    data[container_name] = sorted_entries
+                    changes.append(f"{container_name}: re-sorted by number (newest-first)")
+
    # Write if changes made and not dry run
    if changes and not dry_run:
        try:
@@ -219,7 +219,7 @@ def _apply_template_to_local(current: dict, template: dict, branch_name: str) ->
    if "key_learnings" not in data:
        active = data.get("active_tasks", {})
        if not isinstance(active, dict) or "key_learnings" not in active:
-            data["key_learnings"] = {}
+            data["key_learnings"] = []
            changes.append("key_learnings: added (empty)")

    # Todos: add if missing (operational list, not rolled over)
@@ -26,7 +26,7 @@
      "last_health_check": "{{DATE}}"
    }
  },
-  "key_learnings": {},
+  "key_learnings": [],
  "todos": [],
  "sessions": [
    {
@@ -176,7 +176,10 @@ class TestExtractItemsV2:
            {"session_number": i, "date": f"2026-01-{i:02d}", "summary": f"Session {i}"}
            for i in range(1, num_sessions + 1)
        ]
-        key_learnings = {f"learning_{i}": f"value_{i}" for i in range(1, num_learnings + 1)}
+        key_learnings = [
+            {"number": num_learnings - i + 1, "date": f"2026-01-{i:02d}", "key": f"learning_{i}", "value": f"value_{i}"}
+            for i in range(1, num_learnings + 1)
+        ]
        return {
            "document_metadata": {
                "schema_version": "2.0.0",
@@ -266,8 +269,8 @@ class TestExtractItemsV2:
        extracted_numbers = [s["session_number"] for s in result["extracted"]]
        assert extracted_numbers == [4, 5]

-    def test_extracts_oldest_key_learnings_by_insertion_order(self, monkeypatch, tmp_path):
-        """First-inserted keys are oldest and should be extracted first."""
+    def test_extracts_oldest_key_learnings_from_end(self, monkeypatch, tmp_path):
+        """Lowest-numbered entries (oldest, at end) should be extracted."""
        ext, _ = _import_extractor(monkeypatch)
        data = self._make_v2_data(num_sessions=0, num_learnings=5, max_sessions=100, max_learnings=3)

@@ -281,10 +284,12 @@ class TestExtractItemsV2:
        with patch.object(ext, "_write_memory_file", side_effect=fake_write):
            result = ext._extract_items_v2(mem_file, data)

-        remaining_keys = list(data["key_learnings"].keys())
-        assert remaining_keys == ["learning_3", "learning_4", "learning_5"]
+        # Kept entries should be the first 3 (newest = highest numbers)
+        kept_keys = [e["key"] for e in data["key_learnings"]]
+        assert kept_keys == ["learning_1", "learning_2", "learning_3"]
+        # Extracted should be the last 2 (oldest = lowest numbers)
        extracted_keys = [e["key"] for e in result["extracted"]]
-        assert extracted_keys == ["learning_1", "learning_2"]
+        assert extracted_keys == ["learning_4", "learning_5"]


 class TestUpdateMetadata:
@@ -0,0 +1,320 @@
+# ===================AIPASS====================
+# META DATA HEADER
+# Name: tests/test_unified_schema.py
+# Date: 2026-06-13
+# Version: 1.0.0
+# Category: memory/tests
+# =============================================
+
+"""
+Tests for FPLAN-0272: unified entry schema changes.
+
+Covers:
+  - normalize.py: number-sort self-heal guardrail (sort, skip, no-op)
+  - extractor.py: key_learnings list trimming (oldest from end, under-limit skip)
+  - entry_limits.py: list-kind key_learnings char-limit enforcement via changed_entries
+"""
+
+import importlib
+import json
+import sys
+from pathlib import Path
+from typing import Any
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+
+# ---------------------------------------------------------------------------
+# Import helpers
+# ---------------------------------------------------------------------------
+
+
+def _import_normalize(monkeypatch):
+    """Import normalize with mocked infrastructure dependencies."""
+    mock_json_handler = MagicMock()
+    mock_json_handler.log_operation = MagicMock(return_value=True)
+
+    json_pkg = MagicMock()
+    json_pkg.json_handler = mock_json_handler
+
+    monkeypatch.setitem(sys.modules, "aipass.memory.apps.handlers.json", json_pkg)
+    monkeypatch.setitem(sys.modules, "aipass.memory.apps.handlers.json.json_handler", mock_json_handler)
+
+    sys.modules.pop("aipass.memory.apps.handlers.schema.normalize", None)
+    parent = sys.modules.get("aipass.memory.apps.handlers.schema")
+    if parent is not None and hasattr(parent, "normalize"):
+        delattr(parent, "normalize")
+
+    from aipass.memory.apps.handlers.schema import normalize
+
+    return normalize, {
+        "json_handler": mock_json_handler,
+    }
+
+
+def _import_extractor(monkeypatch):
+    """Import extractor with mocked infrastructure dependencies."""
+    mock_json_handler = MagicMock()
+    mock_json_handler.log_operation = MagicMock(return_value=True)
+    mock_memory_files = MagicMock()
+    mock_memory_files.read_memory_file_data = MagicMock(return_value=None)
+    mock_memory_files.write_memory_file_simple = MagicMock()
+
+    json_pkg = MagicMock()
+    json_pkg.json_handler = mock_json_handler
+
+    monkeypatch.setitem(sys.modules, "aipass.memory.apps.handlers.json", json_pkg)
+    monkeypatch.setitem(sys.modules, "aipass.memory.apps.handlers.json.json_handler", mock_json_handler)
+    monkeypatch.setitem(sys.modules, "aipass.memory.apps.handlers.json.memory_files", mock_memory_files)
+
+    sys.modules.pop("aipass.memory.apps.handlers.rollover.extractor", None)
+    parent = sys.modules.get("aipass.memory.apps.handlers.rollover")
+    if parent is not None and hasattr(parent, "extractor"):
+        delattr(parent, "extractor")
+
+    from aipass.memory.apps.handlers.rollover import extractor
+
+    return extractor, {
+        "json_handler": mock_json_handler,
+        "memory_files": mock_memory_files,
+    }
+
+
+@pytest.fixture(autouse=True)
+def _fresh_entry_limits_modules(monkeypatch):
+    """Drop cached entry_limits modules so each test gets fresh imports."""
+    sys.modules.pop("aipass.memory.apps.handlers.json", None)
+    sys.modules.pop("aipass.memory.apps.handlers.json.json_handler", None)
+    sys.modules.pop("aipass.memory.apps.handlers.json.config_loader", None)
+    sys.modules.pop("aipass.memory.apps.handlers.json.entry_limits", None)
+    yield
+
+
+def _get_entry_limits():
+    """Import and return the entry_limits module."""
+    return importlib.import_module("aipass.memory.apps.handlers.json.entry_limits")
+
+
+# ---------------------------------------------------------------------------
+# Helpers
+# ---------------------------------------------------------------------------
+
+
+def _write_json(path: Path, data: dict) -> None:
+    path.write_text(json.dumps(data, indent=2), encoding="utf-8")
+
+
+# ===========================================================================
+# 1. Normalizer: number-sort self-heal guardrail
+# ===========================================================================
+
+
+class TestNormalizerNumberSort:
+    """Tests for the number-sort normalizer in normalize.py."""
+
+    def test_sorts_entries_by_number_descending(self, monkeypatch, tmp_path):
+        """Feed out-of-order entries with number fields -> verify re-sorted newest-first."""
+        norm, _ = _import_normalize(monkeypatch)
+        f = tmp_path / "test.local.json"
+        _write_json(
+            f,
+            {
+                "document_metadata": {
+                    "limits": {"max_sessions": 20},
+                    "status": {"last_health_check": "2026-06-13"},
+                },
+                "sessions": [
+                    {"number": 2, "date": "2026-01-02", "summary": "Second"},
+                    {"number": 5, "date": "2026-01-05", "summary": "Fifth"},
+                    {"number": 1, "date": "2026-01-01", "summary": "First"},
+                    {"number": 4, "date": "2026-01-04", "summary": "Fourth"},
+                    {"number": 3, "date": "2026-01-03", "summary": "Third"},
+                ],
+            },
+        )
+
+        result = norm.normalize_memory_file(f)
+
+        assert result["success"] is True
+        data = json.loads(f.read_text(encoding="utf-8"))
+        numbers = [e["number"] for e in data["sessions"]]
+        assert numbers == [5, 4, 3, 2, 1], f"Expected descending order, got {numbers}"
+        assert any("re-sorted" in c for c in result["changes"])
+
+    def test_skips_sort_when_no_numbers(self, monkeypatch, tmp_path):
+        """Entries without number field -> no sort applied."""
+        norm, _ = _import_normalize(monkeypatch)
+        f = tmp_path / "test.local.json"
+        original_sessions = [
+            {"date": "2026-01-03", "summary": "Third"},
+            {"date": "2026-01-01", "summary": "First"},
+            {"date": "2026-01-02", "summary": "Second"},
+        ]
+        _write_json(
+            f,
+            {
+                "document_metadata": {
+                    "limits": {"max_sessions": 20},
+                    "status": {"last_health_check": "2026-06-13"},
+                },
+                "sessions": original_sessions,
+            },
+        )
+
+        result = norm.normalize_memory_file(f)
+
+        assert result["success"] is True
+        data = json.loads(f.read_text(encoding="utf-8"))
+        # Order should be unchanged since no number fields exist
+        summaries = [e["summary"] for e in data["sessions"]]
+        assert summaries == ["Third", "First", "Second"]
+        assert not any("re-sorted" in c for c in result["changes"])
+
+    def test_no_change_when_already_sorted(self, monkeypatch, tmp_path):
+        """Already-sorted entries (descending by number) -> no changes reported."""
+        norm, _ = _import_normalize(monkeypatch)
+        f = tmp_path / "test.local.json"
+        _write_json(
+            f,
+            {
+                "document_metadata": {
+                    "limits": {"max_sessions": 20},
+                    "status": {"last_health_check": "2026-06-13"},
+                },
+                "sessions": [
+                    {"number": 5, "date": "2026-01-05", "summary": "Fifth"},
+                    {"number": 4, "date": "2026-01-04", "summary": "Fourth"},
+                    {"number": 3, "date": "2026-01-03", "summary": "Third"},
+                    {"number": 2, "date": "2026-01-02", "summary": "Second"},
+                    {"number": 1, "date": "2026-01-01", "summary": "First"},
+                ],
+            },
+        )
+
+        result = norm.normalize_memory_file(f)
+
+        assert result["success"] is True
+        assert result["changes"] == []
+
+
+# ===========================================================================
+# 2. Extractor: key_learnings list trimming
+# ===========================================================================
+
+
+class TestExtractorKeyLearningsList:
+    """Tests for key_learnings list extraction in extractor.py."""
+
+    def _make_kl_data(self, num_kl: int, max_kl: int) -> dict[str, Any]:
+        """Build v2 memory data with key_learnings as a list (newest-first by number)."""
+        key_learnings = [
+            {
+                "number": num_kl - i,
+                "date": f"2026-01-{(i + 1):02d}",
+                "key": f"learning_{num_kl - i}",
+                "value": f"value_{num_kl - i}",
+            }
+            for i in range(num_kl)
+        ]
+        return {
+            "document_metadata": {
+                "schema_version": "2.0.0",
+                "limits": {
+                    "max_sessions": 100,
+                    "max_key_learnings": max_kl,
+                },
+                "status": {"current_lines": 100},
+            },
+            "sessions": [],
+            "key_learnings": key_learnings,
+        }
+
+    def test_kl_list_trims_oldest_from_end(self, monkeypatch, tmp_path):
+        """List with 5 key_learnings, max 3 -> extracts 2 oldest (lowest numbers at end), keeps 3 newest."""
+        ext, _ = _import_extractor(monkeypatch)
+        data = self._make_kl_data(num_kl=5, max_kl=3)
+
+        mem_file = tmp_path / ".trinity" / "local.json"
+        mem_file.parent.mkdir(parents=True)
+        mem_file.write_text(json.dumps(data, indent=2), encoding="utf-8")
+
+        def fake_write(fp, d):
+            """Write JSON data to file, bypassing mocked memory_files."""
+            fp.write_text(json.dumps(d, indent=2), encoding="utf-8")
+
+        with patch.object(ext, "_write_memory_file", side_effect=fake_write):
+            result = ext._extract_items_v2(mem_file, data)
+
+        assert result["success"] is True
+        assert result["extracted_count"] == 2
+
+        # Kept entries: the first 3 (newest, highest numbers)
+        kept_numbers = [e["number"] for e in data["key_learnings"]]
+        assert kept_numbers == [5, 4, 3]
+
+        # Extracted entries: the last 2 (oldest, lowest numbers)
+        extracted_numbers = [e["number"] for e in result["extracted"]]
+        assert extracted_numbers == [2, 1]
+
+    def test_kl_list_under_limit_no_trim(self, monkeypatch, tmp_path):
+        """List with 2 key_learnings, max 5 -> skipped, no extraction."""
+        ext, _ = _import_extractor(monkeypatch)
+        data = self._make_kl_data(num_kl=2, max_kl=5)
+
+        mem_file = tmp_path / ".trinity" / "local.json"
+        mem_file.parent.mkdir(parents=True)
+        mem_file.write_text(json.dumps(data, indent=2), encoding="utf-8")
+
+        result = ext._extract_items_v2(mem_file, data)
+
+        assert result["success"] is True
+        assert result.get("skipped") is True
+        # All entries should still be present
+        assert len(data["key_learnings"]) == 2
+
+
+# ===========================================================================
+# 3. Entry limits: list-kind key_learnings char-limit enforcement
+# ===========================================================================
+
+
+class TestListKeyLearningCharLimit:
+    """Tests for key_learnings as kind='list' in changed_entries."""
+
+    def test_list_key_learning_over_char_limit(self):
+        """changed_entries with a new key_learning entry where value exceeds 200 chars -> violation."""
+        mod = _get_entry_limits()
+
+        # key_learnings as a list with kind="list"
+        limits: dict[str, Any] = {
+            "enabled": True,
+            "enforce": False,
+            "entry_types": {
+                "key_learnings": {
+                    "file": "local.json",
+                    "container": "key_learnings",
+                    "kind": "list",
+                    "field": "value",
+                    "max_chars": 200,
+                },
+            },
+        }
+
+        before: dict[str, Any] = {"key_learnings": []}
+        fat_value = "x" * 250
+        after: dict[str, Any] = {
+            "key_learnings": [
+                {"number": 1, "key": "new_learning", "value": fat_value},
+            ],
+        }
+
+        result = mod.changed_entries(before, after, limits)
+
+        assert len(result) == 1
+        assert result[0]["entry_type"] == "key_learnings"
+        assert result[0]["container"] == "key_learnings"
+        assert result[0]["key"] == "0"
+        assert result[0]["length"] == 250
+        assert result[0]["cap"] == 200
+        assert result[0]["over_by"] == 50