diff --git a/mcp_server/core/change_remediation.py b/mcp_server/core/change_remediation.py new file mode 100644 index 00000000..12ebe3d4 --- /dev/null +++ b/mcp_server/core/change_remediation.py @@ -0,0 +1,50 @@ +"""Remediation policy for memories impacted by a code change (fleet-watch #110). + +Detection (``handlers/consolidation/memory_staleness_pass.py``) marks an impacted +memory stale — it points at the bug. Remediation goes one step further: it +decides HOW to make the memory correct again, safely, so a commit that +invalidates a memory also repairs it. + +Two classes, because auto-rewriting prose from a diff risks fabrication: + + - CODE-DERIVED memories (written by codebase ingestion — ``agent_context == + 'codebase'``) can be refreshed *mechanically*: re-ingesting the changed file + with ``codebase_analyze`` (incremental, content-hash tracked) supersedes the + old AST-derived fact with the current one. Action: ``REINGEST``. + - HAND-AUTHORED memories (decisions, lessons) must NOT be silently rewritten; + a machine cannot re-derive an author's intent from a diff. Action: + ``FLAG_STALE`` — mark stale and surface for a human/LLM to re-author. + +Pure: a memory dict in, an action out. Callers own the I/O (the re-ingest call, +the stale mark). ``agent_context == 'codebase'`` is the same marker +``handlers/codebase_analyze_helpers.py`` and ``handlers/change_impact.py`` use to +scope code-derived rows, reused here — not a new classification signal. +""" + +from __future__ import annotations + +from enum import Enum + + +class Remediation(str, Enum): + REINGEST = "reingest" + FLAG_STALE = "flag_stale" + + +def is_code_derived(memory: dict) -> bool: + """True when the memory was produced by codebase ingestion. + + Primary marker: ``agent_context == 'codebase'`` (the scope predicate the + codebase-analyze read/write paths already use). A codebase content-hash tag + (``codebase_analyze``'s incremental HASH_TAG) is accepted as a fallback for + rows written before the context was consistently stamped. + """ + if str(memory.get("agent_context", "")).strip().lower() == "codebase": + return True + tags = {str(t).lower() for t in (memory.get("tags") or [])} + return "codebase" in tags or any(t.startswith("hash:") for t in tags) + + +def classify_remediation(memory: dict) -> Remediation: + """REINGEST a code-derived memory; FLAG_STALE a hand-authored one.""" + return Remediation.REINGEST if is_code_derived(memory) else Remediation.FLAG_STALE diff --git a/mcp_server/handlers/consolidation/change_remediation_pass.py b/mcp_server/handlers/consolidation/change_remediation_pass.py new file mode 100644 index 00000000..fd3bdb2a --- /dev/null +++ b/mcp_server/handlers/consolidation/change_remediation_pass.py @@ -0,0 +1,58 @@ +"""Apply the change-remediation policy to diff-impacted memories (#110). + +The sibling of ``memory_staleness_pass`` (detection): where that marks stale, +this *repairs*. Given the memories a commit impacted (from ``change_impact``'s +matcher ∩ the diff) — each carrying the subset of its file refs that changed — +it applies ``core.change_remediation``: + + - code-derived memories → collect their changed files and re-ingest them once + (``reingest_fn``); ``codebase_analyze`` supersedes the stale AST facts. + - hand-authored memories → ``mark_memory_stale`` and leave the re-authoring to + a human/LLM (never silently rewritten). + +Composition root: pure policy (``classify_remediation``) + injected re-ingest +callback + injected store. No direct I/O here, so it unit-tests with fakes; the +real wiring (``codebase_analyze`` as ``reingest_fn``, the commit diff as the +impact source) is done by the caller and validated against AP + a real codebase. +""" + +from __future__ import annotations + +import logging +from typing import Callable, Protocol + +from mcp_server.core.change_remediation import Remediation, classify_remediation + +logger = logging.getLogger(__name__) + +ReingestFn = Callable[[list[str]], None] + + +class _RemediationStore(Protocol): + def mark_memory_stale(self, memory_id: int, stale: bool = True) -> None: ... + + +def remediate_impacted( + impacted: list[dict], + store: _RemediationStore, + reingest_fn: ReingestFn, +) -> dict[str, int]: + """Repair diff-impacted memories per the remediation policy. + + Each ``impacted`` item is a memory dict plus ``changed_refs`` — the subset + of its file references that appear in the commit diff. Returns counts. + """ + reingest_paths: set[str] = set() + counts = {"reingest_memories": 0, "flagged_stale": 0, "reingest_paths": 0} + for mem in impacted: + if classify_remediation(mem) is Remediation.REINGEST: + reingest_paths.update(mem.get("changed_refs") or []) + counts["reingest_memories"] += 1 + else: + store.mark_memory_stale(mem["id"], True) + counts["flagged_stale"] += 1 + if reingest_paths: + reingest_fn(sorted(reingest_paths)) + counts["reingest_paths"] = len(reingest_paths) + logger.info("change remediation: %s", counts) + return counts diff --git a/tests_py/core/test_change_remediation.py b/tests_py/core/test_change_remediation.py new file mode 100644 index 00000000..8a35d9f2 --- /dev/null +++ b/tests_py/core/test_change_remediation.py @@ -0,0 +1,29 @@ +"""Unit tests for the pure change-remediation classifier (#110).""" + +from __future__ import annotations + +from mcp_server.core.change_remediation import ( + Remediation, + classify_remediation, + is_code_derived, +) + + +def test_code_derived_by_agent_context() -> None: + assert is_code_derived({"agent_context": "codebase"}) + assert classify_remediation({"agent_context": "codebase"}) is Remediation.REINGEST + + +def test_code_derived_by_tags() -> None: + assert is_code_derived({"tags": ["codebase"]}) + assert is_code_derived({"tags": ["hash:abc123"]}) + + +def test_hand_authored_is_flag_stale() -> None: + mem = {"agent_context": "", "tags": ["decision"], "content": "we chose X"} + assert not is_code_derived(mem) + assert classify_remediation(mem) is Remediation.FLAG_STALE + + +def test_missing_fields_default_to_flag_stale() -> None: + assert classify_remediation({}) is Remediation.FLAG_STALE diff --git a/tests_py/handlers/consolidation/test_change_remediation_pass.py b/tests_py/handlers/consolidation/test_change_remediation_pass.py new file mode 100644 index 00000000..e7a0a22b --- /dev/null +++ b/tests_py/handlers/consolidation/test_change_remediation_pass.py @@ -0,0 +1,73 @@ +"""Unit tests for the change-remediation orchestrator (#110). + +DB-free: a fake store records mark_memory_stale calls, a fake callback records +the paths handed to re-ingestion. Verifies the code-derived→reingest / +hand-authored→flag-stale split, path dedup, and the no-op case. +""" + +from __future__ import annotations + +from mcp_server.handlers.consolidation.change_remediation_pass import ( + remediate_impacted, +) + + +class _FakeStore: + def __init__(self) -> None: + self.marked: list[tuple[int, bool]] = [] + + def mark_memory_stale(self, memory_id: int, stale: bool = True) -> None: + self.marked.append((memory_id, stale)) + + +def _recorder(): + calls: list[list[str]] = [] + return calls, lambda paths: calls.append(paths) + + +def test_splits_reingest_and_flag_stale() -> None: + impacted = [ + {"id": 1, "agent_context": "codebase", "changed_refs": ["src/a.py"]}, + { + "id": 2, + "agent_context": "", + "tags": ["decision"], + "changed_refs": ["src/a.py"], + }, + ] + store = _FakeStore() + calls, reingest = _recorder() + + counts = remediate_impacted(impacted, store, reingest) + + assert counts == {"reingest_memories": 1, "flagged_stale": 1, "reingest_paths": 1} + assert calls == [["src/a.py"]] # only the code-derived memory's ref + assert store.marked == [(2, True)] # only the hand-authored memory flagged + + +def test_dedups_reingest_paths_across_memories() -> None: + impacted = [ + { + "id": 1, + "agent_context": "codebase", + "changed_refs": ["src/a.py", "src/b.py"], + }, + {"id": 2, "agent_context": "codebase", "changed_refs": ["src/b.py"]}, + ] + store = _FakeStore() + calls, reingest = _recorder() + + counts = remediate_impacted(impacted, store, reingest) + + assert counts["reingest_memories"] == 2 + assert calls == [["src/a.py", "src/b.py"]] # sorted + deduped + assert store.marked == [] + + +def test_empty_impacted_is_noop() -> None: + store = _FakeStore() + calls, reingest = _recorder() + counts = remediate_impacted([], store, reingest) + assert counts == {"reingest_memories": 0, "flagged_stale": 0, "reingest_paths": 0} + assert calls == [] + assert store.marked == []