fix(hindsight): dropped punctuation-only assistant turns before retain

prepareRetentionTranscript only skipped messages whose stripped content trimmed to the empty string, so single-character assistant turns like ".", "...", or pure whitespace survived as `[role: assistant]\n.\n[assistant:end]` blocks. Repeated across a session (each tool-call-only or thinking-only turn produces one) these polluted the Hindsight bank, wasted retain tokens, and degraded recall relevance. extractMessages and flattenMessagesForRecall had the same gap, only checking length/trim truthiness. Add hasSubstantiveContent in hindsight/content.ts (a single \p{L}/\p{N} test) and apply it from all three call sites so retain, recall, and pre-compaction inputs require at least one letter or digit per message. Single-char real replies ("y", "4", "ok") still pass. Fixes #1806
2026-06-04 00:56:35 +00:00
parent b97ddeac94
commit 662b995cc9
5 changed files with 60 additions and 7 deletions
@@ -14,6 +14,7 @@

 - Fixed selector dialogs (the `ask` tool, hook prompts) collapsing to a single visible option on shorter terminals when options carried long descriptions: the highlighted option's wrapped description consumed the entire row budget, hiding every other option and making the menu feel unnavigable (down moved the lone visible entry, left/right did nothing). When the fully-expanded list overflows, `HookSelectorComponent` now renders a compact list — every option label stays on screen and only the highlighted option expands its description, truncated to the remaining rows — so the whole menu is always visible and the detail pane follows the cursor.
 - Fixed `read` failing with "Path not found" on web URLs whose scheme `//` collapsed to a single `/` (e.g. `https:/github.com/...`), which happens when a URL is routed through Node's `path.normalize`/`path.resolve`. The fetch URL recognizer now accepts a single-slash scheme and repairs it back to `//` before fetching, so collapsed URLs resolve instead of falling through to filesystem lookup.
+- Fixed Hindsight retain (and the shared mnemopi/Hindsight recall paths) framing assistant turns whose only content was punctuation/whitespace — most commonly the lone `.` some providers emit for tool-call-only or thinking-only turns — into `[role: assistant]\n.\n[assistant:end]` blocks that polluted the bank, wasted retain tokens, and degraded recall. `prepareRetentionTranscript`, `extractMessages`, and `flattenMessagesForRecall` now require at least one letter or digit per message via a shared `hasSubstantiveContent` predicate ([#1806](https://github.com/can1357/oh-my-pi/issues/1806)).

 ## [15.8.3] - 2026-06-03
 ### Fixed
@@ -15,7 +15,7 @@ import type { AgentSession } from "../session/agent-session";
 import { computeBankScope } from "./bank";
 import { createHindsightClient } from "./client";
 import { isHindsightConfigured, loadHindsightConfig } from "./config";
-import type { HindsightMessage } from "./content";
+import { hasSubstantiveContent, type HindsightMessage } from "./content";
 import { HindsightSessionState } from "./state";

 const STATIC_INSTRUCTIONS = [
@@ -181,7 +181,7 @@ function flattenMessagesForRecall(messages: AgentMessage[]): HindsightMessage[]
 		if (msg.role === "user") {
 			const content = msg.content;
 			if (typeof content === "string") {
-				if (content.trim()) out.push({ role: "user", content });
+				if (hasSubstantiveContent(content)) out.push({ role: "user", content });
 				continue;
 			}
 			if (Array.isArray(content)) {
@@ -189,7 +189,7 @@ function flattenMessagesForRecall(messages: AgentMessage[]): HindsightMessage[]
 					.filter((b): b is { type: "text"; text: string } => !!b && (b as { type?: unknown }).type === "text")
 					.map(b => b.text)
 					.join("\n");
-				if (text.trim()) out.push({ role: "user", content: text });
+				if (hasSubstantiveContent(text)) out.push({ role: "user", content: text });
 			}
 			continue;
 		}
@@ -198,7 +198,7 @@ function flattenMessagesForRecall(messages: AgentMessage[]): HindsightMessage[]
 				.filter((b): b is { type: "text"; text: string } => b.type === "text")
 				.map(b => b.text)
 				.join("\n");
-			if (text.trim()) out.push({ role: "assistant", content: text });
+			if (hasSubstantiveContent(text)) out.push({ role: "assistant", content: text });
 		}
 	}
 	return out;
@@ -43,6 +43,22 @@ export function stripMemoryTags(content: string): string {
 		.replace(LEGACY_RELEVANT_MEMORIES_REGEX, "");
 }

+// At least one letter or digit means the message carries a token a retriever
+// can actually match on. Punctuation/whitespace-only strings (e.g. the lone
+// `.` some providers emit for tool-call-only or thinking-only assistant turns)
+// are dropped before retain/recall touches them — see issue #1806.
+const SUBSTANTIVE_CHAR_RE = /[\p{L}\p{N}]/u;
+
+/**
+ * True when `content` carries at least one letter or digit. Used by retain
+ * and recall paths to drop placeholder assistant turns ("." / "..." / pure
+ * whitespace) that would otherwise pollute the bank and waste tokens on
+ * embeddings with no semantic content.
+ */
+export function hasSubstantiveContent(content: string): boolean {
+	return SUBSTANTIVE_CHAR_RE.test(content);
+}
+
 /** Format recall results into a bullet list for context injection. */
 export function formatMemories(results: RecallResultLike[]): string {
 	if (results.length === 0) return "";
@@ -197,7 +213,7 @@ export function prepareRetentionTranscript(
 	const parts: string[] = [];
 	for (const msg of targetMessages) {
 		const content = stripMemoryTags(msg.content).trim();
-		if (!content) continue;
+		if (!hasSubstantiveContent(content)) continue;
 		parts.push(`[role: ${msg.role}]\n${content}\n[${msg.role}:end]`);
 	}

@@ -9,7 +9,7 @@

 import type { AssistantMessage } from "@oh-my-pi/pi-ai";
 import type { SessionEntry } from "../session/session-manager";
-import type { HindsightMessage } from "./content";
+import { type HindsightMessage, hasSubstantiveContent } from "./content";

 export interface ReadonlySessionManagerLike {
 	getEntries(): SessionEntry[];
@@ -39,7 +39,7 @@ export function extractMessages(sessionManager: ReadonlySessionManagerLike): Hin
 		if (role !== "user" && role !== "assistant") continue;

 		const text = role === "user" ? extractUserText(msg) : extractAssistantText(msg as AssistantMessage);
-		if (text.length === 0) continue;
+		if (!hasSubstantiveContent(text)) continue;
 		messages.push({ role, content: text });
 	}

@@ -3,6 +3,7 @@ import {
 	composeRecallQuery,
 	formatCurrentTime,
 	formatMemories,
+	hasSubstantiveContent,
 	type HindsightMessage,
 	prepareRetentionTranscript,
 	sliceLastTurnsByUserBoundary,
@@ -182,6 +183,41 @@ describe("prepareRetentionTranscript", () => {
 		const empty = prepareRetentionTranscript([{ role: "user", content: "<memories>x</memories>" }], true);
 		expect(empty.transcript).toBeNull();
 	});
+
+	it("skips punctuation-only assistant turns so retain never stores `.` noise (#1806)", () => {
+		const messages: HindsightMessage[] = [
+			{ role: "user", content: "explain how transformers work" },
+			{ role: "assistant", content: "." },
+			{ role: "user", content: "now ssh into the server" },
+			{ role: "assistant", content: "..." },
+			{ role: "user", content: "any more updates?" },
+			{ role: "assistant", content: "  \n\t" },
+			{ role: "user", content: "ok keep going" },
+			{ role: "assistant", content: "done — here are the results" },
+		];
+		const { transcript, messageCount } = prepareRetentionTranscript(messages, true);
+		expect(messageCount).toBe(5);
+		expect(transcript).not.toContain("[role: assistant]\n.\n[assistant:end]");
+		expect(transcript).not.toContain("[role: assistant]\n...\n[assistant:end]");
+		expect(transcript).toContain("done — here are the results");
+	});
+});
+
+describe("hasSubstantiveContent", () => {
+	it("treats letter/digit-bearing strings as substantive", () => {
+		expect(hasSubstantiveContent("ok")).toBe(true);
+		expect(hasSubstantiveContent("y")).toBe(true);
+		expect(hasSubstantiveContent("4")).toBe(true);
+		expect(hasSubstantiveContent("こんにちは")).toBe(true);
+	});
+
+	it("rejects whitespace and punctuation-only strings", () => {
+		expect(hasSubstantiveContent("")).toBe(false);
+		expect(hasSubstantiveContent(".")).toBe(false);
+		expect(hasSubstantiveContent("...")).toBe(false);
+		expect(hasSubstantiveContent(" \t\n")).toBe(false);
+		expect(hasSubstantiveContent("— ! ?")).toBe(false);
+	});
 });

 describe("formatMemories", () => {