Merge pull request #486 from Gentleman-Programming/fix/pi-mem-review-chrome

feat(pi): add native mem_review support

Author: Alan-TheGentleman

DOCS.md

@@ -142,6 +142,15 @@ Engram is local-first: local SQLite is authoritative; cloud features are optiona
   - `200` when deleted
   - `404` when observation does not exist
 
+### Review
+
+- `GET /review` — List observations due for local review. Query: `?project=X&limit=N`
+- `POST /review/mark_reviewed` — Reset one observation's local review cycle. Body: `{observation_id}`; legacy `{id}` is accepted.
+  - `200` with the refreshed observation payload when marked reviewed
+  - `400` when `observation_id`/`id` is missing or the JSON body is invalid
+  - `404` when the observation does not exist
+  - Local-only: updating `review_after` does not enqueue a sync mutation or propagate to other machines.
+
 ### Search
 
 - `GET /search` — FTS5 search. Query: `?q=QUERY&type=TYPE&project=PROJECT&scope=SCOPE&limit=N`

internal/server/server.go

@@ -204,6 +204,10 @@ func (s *Server) routes() {
 	s.mux.HandleFunc("GET /timeline", s.handleTimeline)
 	s.mux.HandleFunc("GET /observations/{id}", s.handleGetObservation)
 
+	// Lifecycle review
+	s.mux.HandleFunc("GET /review", s.handleReviewList)
+	s.mux.HandleFunc("POST /review/mark_reviewed", s.handleReviewMarkReviewed)
+
 	// Prompts
 	s.mux.HandleFunc("POST /prompts", s.handleAddPrompt)
 	s.mux.HandleFunc("GET /prompts/recent", s.handleRecentPrompts)
@@ -508,6 +512,80 @@ func (s *Server) handleTimeline(w http.ResponseWriter, r *http.Request) {
 	jsonResponse(w, http.StatusOK, result)
 }
 
+func (s *Server) handleReviewList(w http.ResponseWriter, r *http.Request) {
+	project := r.URL.Query().Get("project")
+	limit := queryInt(r, "limit", 10)
+
+	observations, err := s.store.ObservationsNeedingReview(project, limit)
+	if err != nil {
+		jsonError(w, http.StatusInternalServerError, err.Error())
+		return
+	}
+
+	structured := make([]map[string]any, 0, len(observations))
+	for _, obs := range observations {
+		structured = append(structured, reviewObservationPayload(obs))
+	}
+
+	jsonResponse(w, http.StatusOK, map[string]any{
+		"observations": structured,
+		"count":        len(structured),
+	})
+}
+
+func (s *Server) handleReviewMarkReviewed(w http.ResponseWriter, r *http.Request) {
+	var body struct {
+		ObservationID int64 `json:"observation_id"`
+		ID            int64 `json:"id"`
+	}
+	if err := json.NewDecoder(r.Body).Decode(&body); err != nil {
+		jsonError(w, http.StatusBadRequest, "invalid json: "+err.Error())
+		return
+	}
+
+	id := body.ObservationID
+	if id == 0 {
+		id = body.ID
+	}
+	if id == 0 {
+		jsonError(w, http.StatusBadRequest, "observation_id is required")
+		return
+	}
+
+	if err := s.store.MarkReviewed(id); err != nil {
+		if errors.Is(err, store.ErrObservationNotFound) {
+			jsonError(w, http.StatusNotFound, err.Error())
+			return
+		}
+		jsonError(w, http.StatusInternalServerError, err.Error())
+		return
+	}
+	obs, err := s.store.GetObservation(id)
+	if err != nil {
+		jsonError(w, http.StatusInternalServerError, "marked reviewed but failed to reload observation: "+err.Error())
+		return
+	}
+
+	jsonResponse(w, http.StatusOK, reviewObservationPayload(*obs))
+}
+
+func reviewObservationPayload(obs store.Observation) map[string]any {
+	payload := map[string]any{
+		"id":      obs.ID,
+		"sync_id": obs.SyncID,
+		"title":   obs.Title,
+		"type":    obs.Type,
+		"state":   obs.State(),
+	}
+	if obs.Project != nil {
+		payload["project"] = *obs.Project
+	}
+	if obs.ReviewAfter != nil {
+		payload["review_after"] = *obs.ReviewAfter
+	}
+	return payload
+}
+
 // ─── Prompts ─────────────────────────────────────────────────────────────────
 
 func (s *Server) handleAddPrompt(w http.ResponseWriter, r *http.Request) {

internal/server/server_test.go

@@ -231,6 +231,108 @@ func TestAdditionalServerErrorBranches(t *testing.T) {
 	}
 }
 
+func TestHandleReviewListAndMarkReviewed(t *testing.T) {
+	st := newServerTestStore(t)
+	srv := New(st, 0)
+	h := srv.Handler()
+
+	if err := st.CreateSession("s-http-review", "engram", "/tmp/engram"); err != nil {
+		t.Fatalf("create session: %v", err)
+	}
+	id, err := st.AddObservation(store.AddObservationParams{SessionID: "s-http-review", Type: "decision", Title: "Review me", Content: "Needs lifecycle review", Project: "engram"})
+	if err != nil {
+		t.Fatalf("add observation: %v", err)
+	}
+	past := time.Now().UTC().Add(-time.Hour).Format("2006-01-02 15:04:05")
+	if _, err := st.DB().Exec(`UPDATE observations SET review_after = ? WHERE id = ?`, past, id); err != nil {
+		t.Fatalf("backdate review_after: %v", err)
+	}
+
+	listReq := httptest.NewRequest(http.MethodGet, "/review?project=engram&limit=5", nil)
+	listRec := httptest.NewRecorder()
+	h.ServeHTTP(listRec, listReq)
+	if listRec.Code != http.StatusOK {
+		t.Fatalf("expected review list 200, got %d body=%s", listRec.Code, listRec.Body.String())
+	}
+	var listBody map[string]any
+	if err := json.NewDecoder(listRec.Body).Decode(&listBody); err != nil {
+		t.Fatalf("decode review list: %v", err)
+	}
+	observations, ok := listBody["observations"].([]any)
+	if !ok || len(observations) != 1 {
+		t.Fatalf("expected one review observation, got %#v", listBody["observations"])
+	}
+	entry, _ := observations[0].(map[string]any)
+	if entry["state"] != store.ObservationStateNeedsReview {
+		t.Fatalf("expected needs_review state, got %v", entry["state"])
+	}
+
+	markReq := httptest.NewRequest(http.MethodPost, "/review/mark_reviewed", strings.NewReader(fmt.Sprintf(`{"observation_id":%d}`, id)))
+	markReq.Header.Set("Content-Type", "application/json")
+	markRec := httptest.NewRecorder()
+	h.ServeHTTP(markRec, markReq)
+	if markRec.Code != http.StatusOK {
+		t.Fatalf("expected mark reviewed 200, got %d body=%s", markRec.Code, markRec.Body.String())
+	}
+	var markBody map[string]any
+	if err := json.NewDecoder(markRec.Body).Decode(&markBody); err != nil {
+		t.Fatalf("decode mark reviewed: %v", err)
+	}
+	if markBody["state"] != store.ObservationStateActive {
+		t.Fatalf("expected active after mark_reviewed, got %v", markBody["state"])
+	}
+}
+
+func TestHandleReviewMarkReviewedAcceptsIDAlias(t *testing.T) {
+	st := newServerTestStore(t)
+	srv := New(st, 0)
+	h := srv.Handler()
+
+	if err := st.CreateSession("s-http-review-alias", "engram", "/tmp/engram"); err != nil {
+		t.Fatalf("create session: %v", err)
+	}
+	id, err := st.AddObservation(store.AddObservationParams{SessionID: "s-http-review-alias", Type: "decision", Title: "Review alias", Content: "Needs lifecycle review", Project: "engram"})
+	if err != nil {
+		t.Fatalf("add observation: %v", err)
+	}
+	past := time.Now().UTC().Add(-time.Hour).Format("2006-01-02 15:04:05")
+	if _, err := st.DB().Exec(`UPDATE observations SET review_after = ? WHERE id = ?`, past, id); err != nil {
+		t.Fatalf("backdate review_after: %v", err)
+	}
+
+	req := httptest.NewRequest(http.MethodPost, "/review/mark_reviewed", strings.NewReader(fmt.Sprintf(`{"id":%d}`, id)))
+	req.Header.Set("Content-Type", "application/json")
+	rec := httptest.NewRecorder()
+	h.ServeHTTP(rec, req)
+	if rec.Code != http.StatusOK {
+		t.Fatalf("expected mark reviewed alias 200, got %d body=%s", rec.Code, rec.Body.String())
+	}
+}
+
+func TestHandleReviewMarkReviewedRequiresObservationID(t *testing.T) {
+	srv := New(newServerTestStore(t), 0)
+	req := httptest.NewRequest(http.MethodPost, "/review/mark_reviewed", strings.NewReader(`{}`))
+	req.Header.Set("Content-Type", "application/json")
+	rec := httptest.NewRecorder()
+	srv.Handler().ServeHTTP(rec, req)
+
+	if rec.Code != http.StatusBadRequest {
+		t.Fatalf("expected 400 when observation_id is missing, got %d", rec.Code)
+	}
+}
+
+func TestHandleReviewMarkReviewedReturnsNotFoundForUnknownObservation(t *testing.T) {
+	srv := New(newServerTestStore(t), 0)
+	req := httptest.NewRequest(http.MethodPost, "/review/mark_reviewed", strings.NewReader(`{"observation_id":999999}`))
+	req.Header.Set("Content-Type", "application/json")
+	rec := httptest.NewRecorder()
+	srv.Handler().ServeHTTP(rec, req)
+
+	if rec.Code != http.StatusNotFound {
+		t.Fatalf("expected 404 for unknown observation, got %d body=%s", rec.Code, rec.Body.String())
+	}
+}
+
 func TestExportHonorsProjectQueryScope(t *testing.T) {
 	st := newServerTestStore(t)
 	srv := New(st, 0)

plugin/pi/README.md

@@ -99,17 +99,29 @@ Pi events/tools -> gentle-engram extension -> ENGRAM_URL / engram serve -> SQLit
 Pi MCP tools   -> pi-mcp-adapter -> ENGRAM_BIN / engram mcp -> SQLite
 ```
 
-Pi-native compact tools use the same HTTP server path as event capture, including project detection, diagnostics, passive capture, and conflict-judgment tools such as `mem_current_project`, `mem_doctor`, `mem_capture_passive`, `mem_judge`, and `mem_compare`. MCP tools remain a separate stdio path, so direct MCP usage still needs an Engram binary even when `ENGRAM_URL` points at a remote HTTP server. Engram MCP direct tools are not enabled by default in Pi to avoid duplicate raw `engram_mem_*` tool rows.
+Pi-native compact tools use the same HTTP server path as event capture, including project detection, diagnostics, passive capture, lifecycle review, and conflict-judgment tools such as `mem_current_project`, `mem_doctor`, `mem_capture_passive`, `mem_review`, `mem_judge`, and `mem_compare`. MCP tools remain a separate stdio path, so direct MCP usage still needs an Engram binary even when `ENGRAM_URL` points at a remote HTTP server. Engram MCP direct tools are not enabled by default in Pi to avoid duplicate raw `engram_mem_*` tool rows.
 
 ## Compact memory tool rendering
 
-`gentle-engram` owns the Pi chrome for Engram memory tools by registering compact Pi-native `mem_*` tools in the companion package. When tools such as `mem_search`, `mem_context`, `mem_save`, `mem_session_summary`, `mem_get_observation`, `mem_judge`, and `mem_doctor` run in Pi, the default collapsed view stays compact:
+`gentle-engram` owns the Pi chrome for Engram memory tools by registering compact Pi-native `mem_*` tools in the companion package. When tools such as `mem_search`, `mem_context`, `mem_save`, `mem_session_summary`, `mem_get_observation`, `mem_review`, `mem_judge`, and `mem_doctor` run in Pi, the default collapsed view stays compact:
 
 ```text
 🧠 search “auth model” …
 ↳ ✓ 4 results
 ```
 
+For lifecycle review, `mem_review` keeps the collapsed output explicit without exposing raw tool payloads:
+
+```text
+🧠 review list “engram” limit 10 …
+↳ ✓ 3 need review
+
+🧠 review mark_reviewed #42 …
+↳ ✓ reviewed #42
+```
+
+`action=list` shows memories whose local `review_after` timestamp is due. `action=mark_reviewed` asks Engram core to reset that observation's local review clock according to its memory type. That review reset is local-only today: it updates the local lifecycle metadata but is not treated as a cloud/git sync mutation until the sync wire format carries lifecycle review fields.
+
 Normal memory activity also updates the status bar with short progress/result text such as `🧠 engram · search…` and `🧠 engram · ✓ 4 results`. The extension does not use notifications for normal memory operations.
 
 When a tool call fails because Engram cannot determine which project to use, the status bar shows an actionable label instead of the generic `error`:

plugin/pi/index.ts

@@ -38,6 +38,7 @@ const ENGRAM_TOOLS = [
   "mem_current_project",
   "mem_doctor",
   "mem_capture_passive",
+  "mem_review",
   "mem_judge",
   "mem_compare",
 ] as const;
@@ -488,6 +489,13 @@ const MEMORY_TOOL_SCHEMAS: Record<string, ReturnType<typeof Type.Object>> = {
     session_id: optionalString("Session ID to associate with"),
     source: optionalString("Source identifier, e.g. subagent-stop or session-end"),
   }),
+  mem_review: Type.Object({
+    action: Type.String({ description: "Action: list | mark_reviewed" }),
+    project: optionalString("Optional project filter for action=list"),
+    limit: optionalNumber("Max results for action=list"),
+    observation_id: optionalNumber("Observation id for action=mark_reviewed"),
+    id: optionalNumber("Alias for observation_id"),
+  }),
   mem_judge: Type.Object({
     judgment_id: Type.String({ description: "The relation judgment_id returned by mem_save candidates" }),
     relation: Type.String({ description: "Verdict: related | compatible | scoped | conflicts_with | supersedes | not_conflict" }),
@@ -640,6 +648,19 @@ async function callMemoryTool(toolName: string, params: Record<string, unknown>,
           source: params.source || "pi-tool",
         },
       });
+    case "mem_review": {
+      const action = String(params.action || "").trim();
+      if (action === "list") {
+        return engramFetch(`/review${queryString({ project: params.project, limit: params.limit })}`);
+      }
+      if (action === "mark_reviewed") {
+        return engramFetch("/review/mark_reviewed", {
+          method: "POST",
+          body: { observation_id: params.observation_id || params.id },
+        });
+      }
+      throw new Error("action must be one of: list, mark_reviewed");
+    }
     case "mem_judge":
       return engramFetch("/conflicts/judge", {
         method: "POST",

plugin/pi/memory-tool-chrome.js

@@ -17,6 +17,7 @@ const TOOL_LABELS = {
   mem_capture_passive: "capture passive",
   mem_judge: "judge",
   mem_compare: "compare",
+  mem_review: "review",
 };
 
 const ARG_KEYS = {
@@ -38,6 +39,7 @@ const ARG_KEYS = {
   mem_capture_passive: ["source", "content"],
   mem_judge: ["judgment_id", "relation"],
   mem_compare: ["memory_id_a", "memory_id_b"],
+  mem_review: ["action", "project", "limit", "observation_id", "id"],
 };
 
 export const SUPPORTED_MEMORY_TOOLS = Object.freeze(Object.keys(TOOL_LABELS));
@@ -58,6 +60,8 @@ function quote(value) {
 }
 
 export function compactToolArg(toolName, args = {}) {
+  if (toolName === "mem_review") return compactReviewArg(args);
+
   const keys = ARG_KEYS[toolName] ?? [];
   for (const key of keys) {
     const value = args?.[key];
@@ -68,6 +72,19 @@ export function compactToolArg(toolName, args = {}) {
   return "";
 }
 
+function compactReviewArg(args = {}) {
+  const parts = [];
+  if (args.action !== undefined && args.action !== null && args.action !== "") parts.push(String(args.action));
+
+  const id = args.observation_id ?? args.id;
+  if (id !== undefined && id !== null && id !== "") parts.push(`#${id}`);
+
+  if (args.project !== undefined && args.project !== null && args.project !== "") parts.push(quote(args.project));
+  if (args.limit !== undefined && args.limit !== null && args.limit !== "") parts.push(`limit ${args.limit}`);
+
+  return parts.join(" ");
+}
+
 function firstTextContent(result) {
   const block = result?.content?.find?.((entry) => entry?.type === "text" && typeof entry.text === "string");
   return block?.text ?? "";
@@ -83,6 +100,7 @@ function countItems(value) {
   if (Array.isArray(value?.observations)) return value.observations.length;
   if (Array.isArray(value?.sessions)) return value.sessions.length;
   if (Array.isArray(value?.prompts)) return value.prompts.length;
+  if (typeof value?.count === "number") return value.count;
   return undefined;
 }
 
@@ -112,6 +130,11 @@ export function compactResultStatus(toolName, result, options = {}) {
   if (toolName === "mem_capture_passive") return `✓ captured ${data?.saved ?? count ?? 0}`;
   if (toolName === "mem_judge") return data?.relation?.sync_id ? `✓ judged ${data.relation.sync_id}` : "✓ judged";
   if (toolName === "mem_compare") return data?.sync_id ? `✓ ${data.sync_id}` : "✓ compared";
+  if (toolName === "mem_review") {
+    if (count !== undefined) return `✓ ${count} need${count === 1 ? "s" : ""} review`;
+    const id = data?.id ?? data?.observation_id ?? data?.observation?.id;
+    return id ? `✓ reviewed #${id}` : "✓ reviewed";
+  }
   return "✓ done";
 }
 

plugin/pi/test/index-source.test.mjs

@@ -27,3 +27,14 @@ test("ambiguous_project error maps to actionable status label, not generic 'erro
   // The bare '· error' hardcoded string should no longer be present in the catch block
   assert.doesNotMatch(source, /setStatus\?\.\("engram",\s*`🧠 \$\{project\} · error`\)/);
 });
+
+test("mem_review is registered as a Pi-native executable memory tool", () => {
+  assert.match(source, /const ENGRAM_TOOLS = \[[\s\S]*"mem_review"/);
+  assert.match(source, /mem_review: Type\.Object\(\{[\s\S]*action: Type\.String\(\{ description: "Action: list \| mark_reviewed" \}\)/);
+  assert.match(source, /mem_review: Type\.Object\(\{[\s\S]*observation_id: optionalNumber\("Observation id for action=mark_reviewed"\)/);
+  assert.match(source, /mem_review: Type\.Object\(\{[\s\S]*id: optionalNumber\("Alias for observation_id"\)/);
+  assert.match(source, /case "mem_review":[\s\S]*action === "list"[\s\S]*engramFetch\(`\/review\$\{queryString\(\{ project: params\.project, limit: params\.limit \}\)\}`\)/);
+  assert.match(source, /case "mem_review":[\s\S]*action === "mark_reviewed"[\s\S]*engramFetch\("\/review\/mark_reviewed"/);
+  assert.match(source, /case "mem_review":[\s\S]*body: \{ observation_id: params\.observation_id \|\| params\.id \}/);
+  assert.match(source, /for \(const toolName of ENGRAM_TOOLS\)[\s\S]*executeMemoryTool\(toolName/);
+});