refactor: break up search_memories() into focused helpers

Extract _build_memory_filter(), _fetch_memory_candidates(),
_row_to_entry(), and _score_and_rank() from the 82-line
search_memories() function for better readability and testability.

Fixes #554

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

src/infrastructure/hands/shell.py

@@ -144,6 +144,65 @@ class ShellHand:
 
         return None
 
+    @staticmethod
+    def _build_run_env(env: dict | None) -> dict:
+        """Merge *env* overrides into a copy of the current environment."""
+        import os
+
+        run_env = os.environ.copy()
+        if env:
+            run_env.update(env)
+        return run_env
+
+    async def _execute_subprocess(
+        self,
+        command: str,
+        effective_timeout: int,
+        cwd: str | None,
+        run_env: dict,
+        start: float,
+    ) -> ShellResult:
+        """Run *command* as a subprocess with timeout enforcement."""
+        proc = await asyncio.create_subprocess_shell(
+            command,
+            stdout=asyncio.subprocess.PIPE,
+            stderr=asyncio.subprocess.PIPE,
+            cwd=cwd,
+            env=run_env,
+        )
+
+        try:
+            stdout_bytes, stderr_bytes = await asyncio.wait_for(
+                proc.communicate(), timeout=effective_timeout
+            )
+        except TimeoutError:
+            proc.kill()
+            await proc.wait()
+            latency = (time.time() - start) * 1000
+            logger.warning("Shell command timed out after %ds: %s", effective_timeout, command)
+            return ShellResult(
+                command=command,
+                success=False,
+                exit_code=-1,
+                error=f"Command timed out after {effective_timeout}s",
+                latency_ms=latency,
+                timed_out=True,
+            )
+
+        latency = (time.time() - start) * 1000
+        exit_code = proc.returncode if proc.returncode is not None else -1
+        stdout = stdout_bytes.decode("utf-8", errors="replace").strip()
+        stderr = stderr_bytes.decode("utf-8", errors="replace").strip()
+
+        return ShellResult(
+            command=command,
+            success=exit_code == 0,
+            exit_code=exit_code,
+            stdout=stdout,
+            stderr=stderr,
+            latency_ms=latency,
+        )
+
     async def run(
         self,
         command: str,
@@ -164,7 +223,6 @@ class ShellHand:
         """
         start = time.time()
 
-        # Validate
         validation_error = self._validate_command(command)
         if validation_error:
             return ShellResult(
@@ -178,52 +236,8 @@ class ShellHand:
         cwd = working_dir or self._working_dir
 
         try:
-            import os
-
-            run_env = os.environ.copy()
-            if env:
-                run_env.update(env)
-
-            proc = await asyncio.create_subprocess_shell(
-                command,
-                stdout=asyncio.subprocess.PIPE,
-                stderr=asyncio.subprocess.PIPE,
-                cwd=cwd,
-                env=run_env,
-            )
-
-            try:
-                stdout_bytes, stderr_bytes = await asyncio.wait_for(
-                    proc.communicate(), timeout=effective_timeout
-                )
-            except TimeoutError:
-                proc.kill()
-                await proc.wait()
-                latency = (time.time() - start) * 1000
-                logger.warning("Shell command timed out after %ds: %s", effective_timeout, command)
-                return ShellResult(
-                    command=command,
-                    success=False,
-                    exit_code=-1,
-                    error=f"Command timed out after {effective_timeout}s",
-                    latency_ms=latency,
-                    timed_out=True,
-                )
-
-            latency = (time.time() - start) * 1000
-            exit_code = proc.returncode if proc.returncode is not None else -1
-            stdout = stdout_bytes.decode("utf-8", errors="replace").strip()
-            stderr = stderr_bytes.decode("utf-8", errors="replace").strip()
-
-            return ShellResult(
-                command=command,
-                success=exit_code == 0,
-                exit_code=exit_code,
-                stdout=stdout,
-                stderr=stderr,
-                latency_ms=latency,
-            )
-
+            run_env = self._build_run_env(env)
+            return await self._execute_subprocess(command, effective_timeout, cwd, run_env, start)
         except Exception as exc:
             latency = (time.time() - start) * 1000
             logger.warning("Shell command failed: %s — %s", command, exc)

src/timmy/memory_system.py

@@ -98,6 +98,73 @@ def _get_table_columns(conn: sqlite3.Connection, table_name: str) -> set[str]:
     return {row[1] for row in cursor.fetchall()}
 
 
+def _migrate_episodes(conn: sqlite3.Connection) -> None:
+    """Migrate episodes table rows into the unified memories table."""
+    logger.info("Migration: Converting episodes table to memories")
+    try:
+        cols = _get_table_columns(conn, "episodes")
+        context_type_col = "context_type" if "context_type" in cols else "'conversation'"
+
+        conn.execute(f"""
+            INSERT INTO memories (
+                id, content, memory_type, source, embedding,
+                metadata, agent_id, task_id, session_id,
+                created_at, access_count, last_accessed
+            )
+            SELECT
+                id, content,
+                COALESCE({context_type_col}, 'conversation'),
+                COALESCE(source, 'agent'),
+                embedding,
+                metadata, agent_id, task_id, session_id,
+                COALESCE(timestamp, datetime('now')), 0, NULL
+            FROM episodes
+        """)
+        conn.execute("DROP TABLE episodes")
+        logger.info("Migration: Migrated episodes to memories")
+    except sqlite3.Error as exc:
+        logger.warning("Migration: Failed to migrate episodes: %s", exc)
+
+
+def _migrate_chunks(conn: sqlite3.Connection) -> None:
+    """Migrate chunks table rows into the unified memories table."""
+    logger.info("Migration: Converting chunks table to memories")
+    try:
+        cols = _get_table_columns(conn, "chunks")
+
+        id_col = "id" if "id" in cols else "CAST(rowid AS TEXT)"
+        content_col = "content" if "content" in cols else "text"
+        source_col = (
+            "filepath" if "filepath" in cols else ("source" if "source" in cols else "'vault'")
+        )
+        embedding_col = "embedding" if "embedding" in cols else "NULL"
+        created_col = "created_at" if "created_at" in cols else "datetime('now')"
+
+        conn.execute(f"""
+            INSERT INTO memories (
+                id, content, memory_type, source, embedding,
+                created_at, access_count
+            )
+            SELECT
+                {id_col}, {content_col}, 'vault_chunk', {source_col},
+                {embedding_col}, {created_col}, 0
+            FROM chunks
+        """)
+        conn.execute("DROP TABLE chunks")
+        logger.info("Migration: Migrated chunks to memories")
+    except sqlite3.Error as exc:
+        logger.warning("Migration: Failed to migrate chunks: %s", exc)
+
+
+def _drop_legacy_table(conn: sqlite3.Connection, table: str) -> None:
+    """Drop a legacy table if it exists."""
+    try:
+        conn.execute(f"DROP TABLE {table}")  # noqa: S608
+        logger.info("Migration: Dropped old %s table", table)
+    except sqlite3.Error as exc:
+        logger.warning("Migration: Failed to drop %s: %s", table, exc)
+
+
 def _migrate_schema(conn: sqlite3.Connection) -> None:
     """Migrate from old three-table schema to unified memories table.
 
@@ -110,78 +177,16 @@ def _migrate_schema(conn: sqlite3.Connection) -> None:
     tables = {row[0] for row in cursor.fetchall()}
 
     has_memories = "memories" in tables
-    has_episodes = "episodes" in tables
-    has_chunks = "chunks" in tables
-    has_facts = "facts" in tables
 
-    # Check if we need to migrate (old schema exists)
-    if not has_memories and (has_episodes or has_chunks or has_facts):
+    if not has_memories and (tables & {"episodes", "chunks", "facts"}):
         logger.info("Migration: Creating unified memories table")
-        # Schema will be created by _ensure_schema above
 
-    # Migrate episodes -> memories
-    if has_episodes and has_memories:
-        logger.info("Migration: Converting episodes table to memories")
-        try:
-            cols = _get_table_columns(conn, "episodes")
-            context_type_col = "context_type" if "context_type" in cols else "'conversation'"
-
-            conn.execute(f"""
-                INSERT INTO memories (
-                    id, content, memory_type, source, embedding,
-                    metadata, agent_id, task_id, session_id,
-                    created_at, access_count, last_accessed
-                )
-                SELECT 
-                    id, content, 
-                    COALESCE({context_type_col}, 'conversation'),
-                    COALESCE(source, 'agent'),
-                    embedding,
-                    metadata, agent_id, task_id, session_id,
-                    COALESCE(timestamp, datetime('now')), 0, NULL
-                FROM episodes
-            """)
-            conn.execute("DROP TABLE episodes")
-            logger.info("Migration: Migrated episodes to memories")
-        except sqlite3.Error as exc:
-            logger.warning("Migration: Failed to migrate episodes: %s", exc)
-
-    # Migrate chunks -> memories as vault_chunk
-    if has_chunks and has_memories:
-        logger.info("Migration: Converting chunks table to memories")
-        try:
-            cols = _get_table_columns(conn, "chunks")
-
-            id_col = "id" if "id" in cols else "CAST(rowid AS TEXT)"
-            content_col = "content" if "content" in cols else "text"
-            source_col = (
-                "filepath" if "filepath" in cols else ("source" if "source" in cols else "'vault'")
-            )
-            embedding_col = "embedding" if "embedding" in cols else "NULL"
-            created_col = "created_at" if "created_at" in cols else "datetime('now')"
-
-            conn.execute(f"""
-                INSERT INTO memories (
-                    id, content, memory_type, source, embedding,
-                    created_at, access_count
-                )
-                SELECT 
-                    {id_col}, {content_col}, 'vault_chunk', {source_col},
-                    {embedding_col}, {created_col}, 0
-                FROM chunks
-            """)
-            conn.execute("DROP TABLE chunks")
-            logger.info("Migration: Migrated chunks to memories")
-        except sqlite3.Error as exc:
-            logger.warning("Migration: Failed to migrate chunks: %s", exc)
-
-    # Drop old tables
-    if has_facts:
-        try:
-            conn.execute("DROP TABLE facts")
-            logger.info("Migration: Dropped old facts table")
-        except sqlite3.Error as exc:
-            logger.warning("Migration: Failed to drop facts: %s", exc)
+    if "episodes" in tables and has_memories:
+        _migrate_episodes(conn)
+    if "chunks" in tables and has_memories:
+        _migrate_chunks(conn)
+    if "facts" in tables:
+        _drop_legacy_table(conn, "facts")
 
     conn.commit()
 
@@ -298,6 +303,86 @@ def store_memory(
     return entry
 
 
+def _build_memory_filter(
+    context_type: str | None,
+    agent_id: str | None,
+    session_id: str | None,
+) -> tuple[str, list]:
+    """Build WHERE clause and params for memory queries."""
+    conditions: list[str] = []
+    params: list = []
+
+    if context_type:
+        conditions.append("memory_type = ?")
+        params.append(context_type)
+    if agent_id:
+        conditions.append("agent_id = ?")
+        params.append(agent_id)
+    if session_id:
+        conditions.append("session_id = ?")
+        params.append(session_id)
+
+    where_clause = "WHERE " + " AND ".join(conditions) if conditions else ""
+    return where_clause, params
+
+
+def _fetch_memory_candidates(
+    where_clause: str, params: list, candidate_limit: int
+) -> list[sqlite3.Row]:
+    """Fetch candidate memory rows from the database."""
+    query_sql = f"""
+        SELECT * FROM memories
+        {where_clause}
+        ORDER BY created_at DESC
+        LIMIT ?
+    """
+    params.append(candidate_limit)
+
+    with get_connection() as conn:
+        return conn.execute(query_sql, params).fetchall()
+
+
+def _row_to_entry(row: sqlite3.Row) -> MemoryEntry:
+    """Convert a database row to a MemoryEntry."""
+    return MemoryEntry(
+        id=row["id"],
+        content=row["content"],
+        source=row["source"],
+        context_type=row["memory_type"],  # DB column -> API field
+        agent_id=row["agent_id"],
+        task_id=row["task_id"],
+        session_id=row["session_id"],
+        metadata=json.loads(row["metadata"]) if row["metadata"] else None,
+        embedding=json.loads(row["embedding"]) if row["embedding"] else None,
+        timestamp=row["created_at"],
+    )
+
+
+def _score_and_rank(
+    rows: list[sqlite3.Row],
+    query: str,
+    query_embedding: list[float],
+    min_relevance: float,
+    limit: int,
+) -> list[MemoryEntry]:
+    """Score candidates by similarity and return top results."""
+    results = []
+    for row in rows:
+        entry = _row_to_entry(row)
+
+        if entry.embedding:
+            score = cosine_similarity(query_embedding, entry.embedding)
+        else:
+            score = _keyword_overlap(query, entry.content)
+
+        entry.relevance_score = score
+        if score >= min_relevance:
+            results.append(entry)
+
+    results.sort(key=lambda x: x.relevance_score or 0, reverse=True)
+    return results[:limit]
+
+
 def search_memories(
     query: str,
     limit: int = 10,
@@ -320,66 +405,9 @@ def search_memories(
         List of MemoryEntry objects sorted by relevance
     """
     query_embedding = embed_text(query)
-
-    # Build query with filters
-    conditions = []
-    params = []
-
-    if context_type:
-        conditions.append("memory_type = ?")
-        params.append(context_type)
-    if agent_id:
-        conditions.append("agent_id = ?")
-        params.append(agent_id)
-    if session_id:
-        conditions.append("session_id = ?")
-        params.append(session_id)
-
-    where_clause = "WHERE " + " AND ".join(conditions) if conditions else ""
-
-    # Fetch candidates (we'll do in-memory similarity for now)
-    query_sql = f"""
-        SELECT * FROM memories
-        {where_clause}
-        ORDER BY created_at DESC
-        LIMIT ?
-    """
-    params.append(limit * 3)  # Get more candidates for ranking
-
-    with get_connection() as conn:
-        rows = conn.execute(query_sql, params).fetchall()
-
-    # Compute similarity scores
-    results = []
-    for row in rows:
-        entry = MemoryEntry(
-            id=row["id"],
-            content=row["content"],
-            source=row["source"],
-            context_type=row["memory_type"],  # DB column -> API field
-            agent_id=row["agent_id"],
-            task_id=row["task_id"],
-            session_id=row["session_id"],
-            metadata=json.loads(row["metadata"]) if row["metadata"] else None,
-            embedding=json.loads(row["embedding"]) if row["embedding"] else None,
-            timestamp=row["created_at"],
-        )
-
-        if entry.embedding:
-            score = cosine_similarity(query_embedding, entry.embedding)
-            entry.relevance_score = score
-            if score >= min_relevance:
-                results.append(entry)
-        else:
-            # Fallback: check for keyword overlap
-            score = _keyword_overlap(query, entry.content)
-            entry.relevance_score = score
-            if score >= min_relevance:
-                results.append(entry)
-
-    # Sort by relevance and return top results
-    results.sort(key=lambda x: x.relevance_score or 0, reverse=True)
-    return results[:limit]
+    where_clause, params = _build_memory_filter(context_type, agent_id, session_id)
+    rows = _fetch_memory_candidates(where_clause, params, limit * 3)
+    return _score_and_rank(rows, query, query_embedding, min_relevance, limit)
 
 
 def delete_memory(memory_id: str) -> bool:

src/timmy/thinking.py

@@ -772,23 +772,10 @@ class ThinkingEngine:
         except Exception as exc:
             logger.debug("Thought issue filing skipped: %s", exc)
 
-    def _gather_system_snapshot(self) -> str:
-        """Gather lightweight real system state for grounding thoughts in reality.
+    # ── System snapshot helpers ────────────────────────────────────────────
 
-        Returns a short multi-line string with current time, thought count,
-        recent chat activity, and task queue status. Never crashes — every
-        section is independently try/excepted.
-        """
-        parts: list[str] = []
-
-        # Current local time
-        now = datetime.now().astimezone()
-        tz = now.strftime("%Z") or "UTC"
-        parts.append(
-            f"Local time: {now.strftime('%I:%M %p').lstrip('0')} {tz}, {now.strftime('%A %B %d')}"
-        )
-
-        # Thought count today (cheap DB query)
+    def _snap_thought_count(self, now: datetime) -> str | None:
+        """Return today's thought count, or *None* on failure."""
         try:
             today_start = now.replace(hour=0, minute=0, second=0, microsecond=0)
             with _get_conn(self._db_path) as conn:
@@ -796,66 +783,94 @@ class ThinkingEngine:
                     "SELECT COUNT(*) as c FROM thoughts WHERE created_at >= ?",
                     (today_start.isoformat(),),
                 ).fetchone()["c"]
-            parts.append(f"Thoughts today: {count}")
+            return f"Thoughts today: {count}"
         except Exception as exc:
             logger.debug("Thought count query failed: %s", exc)
-            pass
+            return None
 
-        # Recent chat activity (in-memory, no I/O)
+    def _snap_chat_activity(self) -> list[str]:
+        """Return chat-activity lines (in-memory, no I/O)."""
         try:
             from infrastructure.chat_store import message_log
 
             messages = message_log.all()
             if messages:
-                parts.append(f"Chat messages this session: {len(messages)}")
                 last = messages[-1]
-                parts.append(f'Last chat ({last.role}): "{last.content[:80]}"')
-            else:
-                parts.append("No chat messages this session")
+                return [
+                    f"Chat messages this session: {len(messages)}",
+                    f'Last chat ({last.role}): "{last.content[:80]}"',
+                ]
+            return ["No chat messages this session"]
         except Exception as exc:
             logger.debug("Chat activity query failed: %s", exc)
-            pass
+            return []
 
-        # Task queue (lightweight DB query)
+    def _snap_task_queue(self) -> str | None:
+        """Return a one-line task queue summary, or *None*."""
         try:
             from swarm.task_queue.models import get_task_summary_for_briefing
 
-            summary = get_task_summary_for_briefing()
-            running = summary.get("running", 0)
-            pending = summary.get("pending_approval", 0)
-            done = summary.get("completed", 0)
-            failed = summary.get("failed", 0)
+            s = get_task_summary_for_briefing()
+            running, pending = s.get("running", 0), s.get("pending_approval", 0)
+            done, failed = s.get("completed", 0), s.get("failed", 0)
             if running or pending or done or failed:
-                parts.append(
+                return (
                     f"Tasks: {running} running, {pending} pending, "
                     f"{done} completed, {failed} failed"
                 )
         except Exception as exc:
             logger.debug("Task queue query failed: %s", exc)
-            pass
+        return None
 
-        # Workspace updates (file-based communication with Hermes)
+    def _snap_workspace(self) -> list[str]:
+        """Return workspace-update lines (file-based Hermes comms)."""
         try:
             from timmy.workspace import workspace_monitor
 
             updates = workspace_monitor.get_pending_updates()
+            lines: list[str] = []
             new_corr = updates.get("new_correspondence")
-            new_inbox = updates.get("new_inbox_files", [])
-
             if new_corr:
-                # Count entries (assuming each entry starts with a timestamp or header)
-                line_count = len([line for line in new_corr.splitlines() if line.strip()])
-                parts.append(
+                line_count = len([ln for ln in new_corr.splitlines() if ln.strip()])
+                lines.append(
                     f"Workspace: {line_count} new correspondence entries (latest from: Hermes)"
                 )
+            new_inbox = updates.get("new_inbox_files", [])
             if new_inbox:
                 files_str = ", ".join(new_inbox[:5])
                 if len(new_inbox) > 5:
                     files_str += f", ... (+{len(new_inbox) - 5} more)"
-                parts.append(f"Workspace: {len(new_inbox)} new inbox files: {files_str}")
+                lines.append(f"Workspace: {len(new_inbox)} new inbox files: {files_str}")
+            return lines
         except Exception as exc:
             logger.debug("Workspace check failed: %s", exc)
-            pass
+            return []
+
+    def _gather_system_snapshot(self) -> str:
+        """Gather lightweight real system state for grounding thoughts in reality.
+
+        Returns a short multi-line string with current time, thought count,
+        recent chat activity, and task queue status. Never crashes — every
+        section is independently try/excepted.
+        """
+        now = datetime.now().astimezone()
+        tz = now.strftime("%Z") or "UTC"
+
+        parts: list[str] = [
+            f"Local time: {now.strftime('%I:%M %p').lstrip('0')} {tz}, {now.strftime('%A %B %d')}"
+        ]
+
+        thought_line = self._snap_thought_count(now)
+        if thought_line:
+            parts.append(thought_line)
+
+        parts.extend(self._snap_chat_activity())
+
+        task_line = self._snap_task_queue()
+        if task_line:
+            parts.append(task_line)
+
+        parts.extend(self._snap_workspace())
 
         return "\n".join(parts) if parts else ""