Compare commits
30 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 484e290d9a | |||
| 573b1840ef | |||
| 466892de97 | |||
| 609ee5aa7e | |||
| 47062ed207 | |||
| 29200e49db | |||
| b24238ab24 | |||
| 9508a8197b | |||
| d40d9a039a | |||
| 200cd73924 | |||
| bd4b8936e4 | |||
| b8d8bb8a57 | |||
| ec3d4229e3 | |||
| bae2e75625 | |||
| b02bf0a6df | |||
| 038f3cea91 | |||
| 61acd85954 | |||
| 9034a5bcec | |||
| fe18a8371d | |||
| c329d15f7f | |||
| 0a50557d3d | |||
| 8c0cedb3db | |||
| e16ade4725 | |||
| b8ccc88b58 | |||
| 0b1600e524 | |||
| ef3a438048 | |||
| 2a843c29f2 | |||
| 46cac94aa9 | |||
| 6f093e3f6f | |||
| 1cd387c59f |
@@ -195,6 +195,10 @@ OBSIDIAN_E2E_PASSWORD=
|
||||
OBSIDIAN_DEVICE_NAME=simbarag
|
||||
# Set to true to run continuous sync in the background
|
||||
OBSIDIAN_CONTINUOUS_SYNC=false
|
||||
# bidirectional: notes and journal tasks Simba writes sync back to your devices.
|
||||
# pull-only: download only, Simba's writes stay in the container's copy.
|
||||
# mirror-remote: download only and revert local changes.
|
||||
OBSIDIAN_SYNC_MODE=bidirectional
|
||||
OBSIDIAN_SYNC_INTERVAL=60
|
||||
# Local path to Obsidian vault (where files are synced)
|
||||
OBSIDIAN_VAULT_PATH=/app/data/obsidian
|
||||
@@ -204,6 +208,9 @@ OBSIDIAN_VAULT_NAME=
|
||||
# LangGraph checkpoint store (SQLite). Must be on a persistent volume.
|
||||
# Both blue and green containers share this volume via docker-compose.
|
||||
CHECKPOINT_DB_PATH=/app/checkpoints/checkpoints.db
|
||||
# Checkpoints keep a full message-list snapshot per turn; this bounds how many
|
||||
# are kept per conversation thread (older ones are pruned after each turn).
|
||||
CHECKPOINT_KEEP_PER_THREAD=20
|
||||
|
||||
# Beli restaurant recommendations
|
||||
# Beli account credentials for each profile are stored encrypted in the database.
|
||||
|
||||
@@ -34,11 +34,13 @@ jobs:
|
||||
working-directory: raggr-frontend
|
||||
steps:
|
||||
- uses: actions/checkout@v4.2.2
|
||||
# No `cache: yarn` here: setup-node requires yarn to already be on PATH
|
||||
# for that, and the Node it installs doesn't ship one.
|
||||
- uses: actions/setup-node@v4.4.0
|
||||
with:
|
||||
node-version: '22.14.0'
|
||||
cache: yarn
|
||||
cache-dependency-path: raggr-frontend/yarn.lock
|
||||
- name: Install Yarn (lockfile v1)
|
||||
run: npm install --global yarn@1.22.22
|
||||
- name: Install locked frontend dependencies
|
||||
run: yarn install --frozen-lockfile
|
||||
- name: Renderer tests
|
||||
|
||||
@@ -34,21 +34,10 @@ logs:
|
||||
|
||||
# Vector store reindexing
|
||||
reindex:
|
||||
$(COMPOSE) exec $(RAGGR) python -c "\
|
||||
import asyncio; \
|
||||
from blueprints.rag.logic import delete_all_documents, index_documents, index_obsidian_documents; \
|
||||
delete_all_documents(); \
|
||||
asyncio.run(index_documents()); \
|
||||
asyncio.run(index_obsidian_documents()); \
|
||||
print('Reindex complete')"
|
||||
$(COMPOSE) exec $(RAGGR) python -m scripts.manage_vectorstore reindex
|
||||
|
||||
reindex-obsidian:
|
||||
$(COMPOSE) exec $(RAGGR) python -c "\
|
||||
import asyncio; \
|
||||
from blueprints.rag.logic import delete_documents_by_metadata, index_obsidian_documents; \
|
||||
delete_documents_by_metadata('source', 'obsidian'); \
|
||||
asyncio.run(index_obsidian_documents()); \
|
||||
print('Obsidian reindex complete')"
|
||||
$(COMPOSE) exec $(RAGGR) python -m scripts.manage_vectorstore reindex-obsidian
|
||||
|
||||
# YNAB mirror: delta sync, or full re-download
|
||||
ynab-sync:
|
||||
|
||||
@@ -32,6 +32,7 @@ from utils.s3_client import get_image as s3_get_image
|
||||
from utils.s3_client import upload_file as s3_upload_file
|
||||
from utils.s3_client import upload_image as s3_upload_image
|
||||
|
||||
from .checkpoints import delete_thread_checkpoints
|
||||
from .logic import (
|
||||
add_message_to_conversation,
|
||||
persist_agent_tool_messages,
|
||||
@@ -137,9 +138,24 @@ conversation_blueprint = Blueprint(
|
||||
|
||||
async def _get_owned_conversation(conversation_id, user_id) -> Conversation | None:
|
||||
"""Fetch a conversation only if it belongs to the given user."""
|
||||
try:
|
||||
conversation_id = uuid.UUID(str(conversation_id))
|
||||
except ValueError:
|
||||
return None
|
||||
return await Conversation.get_or_none(id=conversation_id, user_id=user_id)
|
||||
|
||||
|
||||
async def _owns_storage_key(key: str, user_id) -> bool:
|
||||
"""True if `key` is stored under a conversation the user owns.
|
||||
|
||||
Every uploaded object lives under conversations/<conversation id>/.
|
||||
"""
|
||||
parts = key.split("/")
|
||||
if len(parts) < 3 or parts[0] != "conversations" or ".." in parts:
|
||||
return False
|
||||
return await _get_owned_conversation(parts[1], user_id) is not None
|
||||
|
||||
|
||||
def _build_user_message(
|
||||
query_text: str,
|
||||
image_block: dict | None = None,
|
||||
@@ -220,6 +236,8 @@ async def upload_image():
|
||||
|
||||
if not file or not conversation_id:
|
||||
return jsonify({"error": "file and conversation_id are required"}), 400
|
||||
if await _get_owned_conversation(conversation_id, current_user_uuid) is None:
|
||||
return jsonify({"error": "conversation not found"}), 404
|
||||
|
||||
file_bytes = file.read()
|
||||
content_type = file.content_type or "image/jpeg"
|
||||
@@ -325,6 +343,8 @@ async def upload_attachment():
|
||||
@conversation_blueprint.get("/image/<path:image_key>")
|
||||
@jwt_access_required
|
||||
async def serve_image(image_key: str):
|
||||
if not await _owns_storage_key(image_key, get_jwt_identity()):
|
||||
return jsonify({"error": "image not found"}), 404
|
||||
url = await s3_presigned_url(image_key)
|
||||
return jsonify({"url": url})
|
||||
|
||||
@@ -758,5 +778,11 @@ async def delete_conversation(conversation_id: str):
|
||||
store = getattr(current_app, "conversation_summary_store", None)
|
||||
if store is not None:
|
||||
await store.delete(str(user_uuid), str(conversation.id))
|
||||
thread_id = str(conversation.id)
|
||||
await conversation.delete()
|
||||
checkpointer = getattr(
|
||||
getattr(current_app, "main_agent", None), "checkpointer", None
|
||||
)
|
||||
if checkpointer is not None:
|
||||
await delete_thread_checkpoints(checkpointer, thread_id)
|
||||
return jsonify({"success": True})
|
||||
|
||||
@@ -23,6 +23,7 @@ from blueprints.conversation.artifacts_util import (
|
||||
note_title,
|
||||
obsidian_source_ref,
|
||||
search_documents_artifact,
|
||||
search_results_model_context,
|
||||
truncate,
|
||||
unique,
|
||||
)
|
||||
@@ -162,7 +163,10 @@ async def simba_search(
|
||||
)
|
||||
except ValueError as exc:
|
||||
return str(exc), None
|
||||
return serialized, search_documents_artifact(query, "paperless", docs)
|
||||
artifact = search_documents_artifact(query, "paperless", docs)
|
||||
if artifact is not None:
|
||||
artifact["model_context"] = search_results_model_context(docs, "paperless")
|
||||
return serialized, artifact
|
||||
|
||||
|
||||
MEALIE_NOT_CONFIGURED = "Mealie integration is not configured. Please set MEALIE_BASE_URL and MEALIE_API_TOKEN environment variables."
|
||||
@@ -488,7 +492,10 @@ async def obsidian_search_notes(
|
||||
serialized, docs = await query_vector_store(
|
||||
query=query, source="obsidian", **kwargs
|
||||
)
|
||||
return serialized, search_documents_artifact(query, "obsidian", docs)
|
||||
artifact = search_documents_artifact(query, "obsidian", docs)
|
||||
if artifact is not None:
|
||||
artifact["model_context"] = search_results_model_context(docs, "obsidian")
|
||||
return serialized, artifact
|
||||
|
||||
except Exception as e:
|
||||
return f"Error searching Obsidian notes: {str(e)}", None
|
||||
|
||||
@@ -143,43 +143,64 @@ def vault_relative_path(filepath: str) -> str:
|
||||
return filepath
|
||||
|
||||
|
||||
def _result_identity(doc, source: str) -> dict:
|
||||
"""Identifying fields shared by the UI snippet and the model's full-text view."""
|
||||
from blueprints.rag.retrieval import citation_url
|
||||
|
||||
meta = doc.metadata or {}
|
||||
result = {}
|
||||
if url := citation_url(doc):
|
||||
result["url"] = url
|
||||
result["source_id"] = str(doc.id)
|
||||
if headings := [meta[k] for k in ("h1", "h2", "h3") if meta.get(k)]:
|
||||
result["headings"] = headings
|
||||
if source == "obsidian":
|
||||
filepath = meta.get("filepath") or ""
|
||||
path = vault_relative_path(filepath) if filepath else ""
|
||||
result["title"] = note_title(path, meta) if path else "Untitled note"
|
||||
if path:
|
||||
result["path"] = path
|
||||
created = meta.get("created_at")
|
||||
if isinstance(created, str) and created:
|
||||
result["date"] = created[:10]
|
||||
else:
|
||||
filename = meta.get("filename") or ""
|
||||
result["title"] = Path(filename).stem or filename or "Document"
|
||||
if filename:
|
||||
result["filename"] = filename
|
||||
if meta.get("document_type"):
|
||||
result["document_type"] = str(meta["document_type"])
|
||||
created = epoch_to_date(meta.get("created_date"))
|
||||
if created:
|
||||
result["date"] = created
|
||||
return result
|
||||
|
||||
|
||||
def search_documents_artifact(query: str, source: str, docs) -> dict | None:
|
||||
"""Build a `search.documents` artifact from vector-store Documents."""
|
||||
if not docs:
|
||||
return None
|
||||
results = []
|
||||
from blueprints.rag.retrieval import citation_url
|
||||
|
||||
for doc in docs:
|
||||
meta = doc.metadata or {}
|
||||
result = {"snippet": snippet(doc.page_content)}
|
||||
if url := citation_url(doc):
|
||||
result["url"] = url
|
||||
result["source_id"] = str(doc.id)
|
||||
if headings := [meta[k] for k in ("h1", "h2", "h3") if meta.get(k)]:
|
||||
result["headings"] = headings
|
||||
if source == "obsidian":
|
||||
filepath = meta.get("filepath") or ""
|
||||
path = vault_relative_path(filepath) if filepath else ""
|
||||
result["title"] = note_title(path, meta) if path else "Untitled note"
|
||||
if path:
|
||||
result["path"] = path
|
||||
created = meta.get("created_at")
|
||||
if isinstance(created, str) and created:
|
||||
result["date"] = created[:10]
|
||||
else:
|
||||
filename = meta.get("filename") or ""
|
||||
result["title"] = Path(filename).stem or filename or "Document"
|
||||
if filename:
|
||||
result["filename"] = filename
|
||||
if meta.get("document_type"):
|
||||
result["document_type"] = str(meta["document_type"])
|
||||
created = epoch_to_date(meta.get("created_date"))
|
||||
if created:
|
||||
result["date"] = created
|
||||
results.append(result)
|
||||
results = [
|
||||
{"snippet": snippet(doc.page_content), **_result_identity(doc, source)}
|
||||
for doc in docs
|
||||
]
|
||||
return make_artifact(
|
||||
"search.documents",
|
||||
f"Search: {query}",
|
||||
{"query": query, "source": source, "results": results},
|
||||
)
|
||||
|
||||
|
||||
def search_results_model_context(docs, source: str) -> dict:
|
||||
"""Full chunk text for the model, keyed like `search_documents_artifact`'s results.
|
||||
|
||||
Set as a tool's `artifact["model_context"]` so that under compaction (see
|
||||
compact_tool_result in blueprints/conversation/context.py) the model still
|
||||
sees the full chunk text it retrieved, not the UI's 400-char snippets.
|
||||
"""
|
||||
return {
|
||||
"results": [
|
||||
{**_result_identity(doc, source), "content": doc.page_content}
|
||||
for doc in docs
|
||||
]
|
||||
}
|
||||
|
||||
@@ -0,0 +1,87 @@
|
||||
"""Keeps the LangGraph checkpoint DB from growing without bound.
|
||||
|
||||
Every `AsyncSqliteSaver` row stores a full snapshot of the graph's channel
|
||||
values (the complete message list so far), not a delta — so an unpruned
|
||||
thread's checkpoints grow roughly linearly with turn count.
|
||||
"""
|
||||
|
||||
import logging
|
||||
import os
|
||||
|
||||
DEFAULT_KEEP_PER_THREAD = 20
|
||||
|
||||
|
||||
def keep_per_thread() -> int:
|
||||
raw = os.getenv("CHECKPOINT_KEEP_PER_THREAD", str(DEFAULT_KEEP_PER_THREAD))
|
||||
try:
|
||||
value = int(raw)
|
||||
except ValueError:
|
||||
value = -1
|
||||
if value < 1:
|
||||
logging.warning(
|
||||
"CHECKPOINT_KEEP_PER_THREAD=%r is invalid; falling back to %d",
|
||||
raw,
|
||||
DEFAULT_KEEP_PER_THREAD,
|
||||
)
|
||||
return DEFAULT_KEEP_PER_THREAD
|
||||
return value
|
||||
|
||||
|
||||
async def prune_thread_checkpoints(
|
||||
checkpointer, thread_id: str, keep: int | None = None
|
||||
) -> None:
|
||||
"""Keep only the newest `keep` checkpoints (and their writes) for a thread.
|
||||
|
||||
Safe to call after every turn: each checkpoint row is a self-contained
|
||||
channel snapshot rather than a delta, and `aget_tuple`/`aget_state` only
|
||||
ever read the single newest row for a (thread_id, checkpoint_ns) pair, so
|
||||
dropping older rows never affects state reads. Reuses the saver's own
|
||||
lock so this never races its aput/aget_tuple calls.
|
||||
"""
|
||||
keep = keep_per_thread() if keep is None else keep
|
||||
thread_id = str(thread_id)
|
||||
conn = checkpointer.conn
|
||||
async with checkpointer.lock, conn.cursor() as cur:
|
||||
await cur.execute(
|
||||
"SELECT DISTINCT checkpoint_ns FROM checkpoints WHERE thread_id = ?",
|
||||
(thread_id,),
|
||||
)
|
||||
namespaces = [row[0] for row in await cur.fetchall()]
|
||||
for ns in namespaces:
|
||||
await cur.execute(
|
||||
"""
|
||||
DELETE FROM checkpoints
|
||||
WHERE thread_id = ? AND checkpoint_ns = ?
|
||||
AND checkpoint_id NOT IN (
|
||||
SELECT checkpoint_id FROM checkpoints
|
||||
WHERE thread_id = ? AND checkpoint_ns = ?
|
||||
ORDER BY checkpoint_id DESC
|
||||
LIMIT ?
|
||||
)
|
||||
""",
|
||||
(thread_id, ns, thread_id, ns, keep),
|
||||
)
|
||||
await cur.execute(
|
||||
"""
|
||||
DELETE FROM writes
|
||||
WHERE thread_id = ? AND checkpoint_ns = ?
|
||||
AND checkpoint_id NOT IN (
|
||||
SELECT checkpoint_id FROM checkpoints
|
||||
WHERE thread_id = ? AND checkpoint_ns = ?
|
||||
)
|
||||
""",
|
||||
(thread_id, ns, thread_id, ns),
|
||||
)
|
||||
await conn.commit()
|
||||
|
||||
|
||||
async def delete_thread_checkpoints(checkpointer, thread_id: str) -> None:
|
||||
"""Delete a conversation's checkpoint thread; never raises.
|
||||
|
||||
Called from conversation delete, which must still succeed even if the
|
||||
checkpoint store is unreachable or the thread is already gone.
|
||||
"""
|
||||
try:
|
||||
await checkpointer.adelete_thread(str(thread_id))
|
||||
except Exception:
|
||||
logging.exception("Failed to delete checkpoint thread %s", thread_id)
|
||||
@@ -1,4 +1,6 @@
|
||||
SIMBA_SYSTEM_PROMPT = """You are Simba, Ryan's helpful personal assistant. You're named after his orange cat. You have a warm, friendly personality with a light cat-themed touch, but your priority is always being genuinely useful — give thorough, detailed answers and think things through carefully. When asked about Simba the cat, you speak as him in first person. For everything else, you're just a great assistant who happens to have a cat's name.
|
||||
SIMBA_SYSTEM_PROMPT = """You are Simba, Ryan's helpful personal assistant. You're named after his orange cat. You have a warm, friendly personality with a light cat-themed touch. When asked about Simba the cat, you speak as him in first person. For everything else, you're just a great assistant who happens to have a cat's name.
|
||||
|
||||
Answer directly and concisely by default: lead with the answer, use short paragraphs or a compact list, and skip restating the question or adding filler sign-offs. Expand with more detail only when the user asks for it or the task genuinely calls for it, such as multi-step plans, comparisons, or explanations they requested. Be accurate: check tool results before stating numbers or dates.
|
||||
|
||||
SIMBA FACTS (as of January 2026):
|
||||
- Name: Simba
|
||||
@@ -34,46 +36,4 @@ IMPORTANT: When users ask factual questions about Simba's health, medical histor
|
||||
|
||||
SOURCE CITATIONS: When using retrieved documents, cite the provided Citation URLs with Markdown links. Use only URLs returned by tools; never invent source IDs or references. A citation opens the exact indexed section, which may differ from a newer original. Document date filters refer to indexed document metadata, not necessarily the date of every event described in the document.
|
||||
|
||||
BUDGET & FINANCE (YNAB Integration):
|
||||
You have access to Ryan's budget data through YNAB (You Need A Budget). When users ask about financial matters, use the appropriate YNAB tools:
|
||||
- Use ynab_budget_summary for a month's overview (Ready to Assign, income, spent, overspending, net worth)
|
||||
- Use ynab_account_balances for account and credit card balances
|
||||
- Use ynab_search_transactions to find specific purchases or spending at particular stores
|
||||
- Use ynab_spending_report for totals by category, payee, or account, and group_by='month' for trends
|
||||
- Use ynab_category_budget for what's assigned/available per category in a month
|
||||
- Use ynab_recent_changes for what was added, edited, or re-assigned recently
|
||||
Always use these tools when asked about budgets, spending, transactions, or financial health.
|
||||
|
||||
MEAL PLANNING (Mealie Integration):
|
||||
You have access to Ryan's meal plans, recipes, and shopping lists through Mealie. When users ask about food or meal planning, use the appropriate Mealie tools:
|
||||
- Use mealie_todays_meals to check what's planned for today
|
||||
- Use mealie_meal_plans to see the week's meal schedule or a custom date range
|
||||
- Use mealie_get_recipe to get full recipe details (ingredients, instructions)
|
||||
- Use mealie_shopping_lists to check what groceries are needed
|
||||
- Use mealie_create_meal_plan to add meals to the plan
|
||||
Always use these tools when asked about meals, recipes, what's for dinner, or shopping lists.
|
||||
|
||||
HOME INVENTORY (Homebox Integration):
|
||||
If homebox_* tools are available, use them for questions about household items: where something is stored, what's in a location, warranties, and maintenance. Search before creating items to avoid duplicates, and use homebox_set_item to edit an existing item (name, description, notes, quantity, purchase info, custom fields) instead of creating a new one.
|
||||
|
||||
WIKI (BookStack Integration):
|
||||
If bookstack_* tools are available, use them to look up and write household documentation in the BookStack wiki. Search before creating books, chapters, or pages to avoid duplicates, and prefer updating an existing page over creating a near-copy. Write page content as markdown.
|
||||
|
||||
NOTES & RESEARCH (Obsidian Integration):
|
||||
You have access to Ryan's Obsidian vault through the Obsidian integration. When users ask about research, personal notes, or information that might be stored in markdown files, use the appropriate Obsidian tools:
|
||||
- Use obsidian_search_notes to search through your vault for relevant information
|
||||
- Use obsidian_read_note to read the full content of a specific note by path
|
||||
- Use obsidian_create_note to save new findings, ideas, or research to your vault
|
||||
- Use obsidian_create_task to create task notes with due dates
|
||||
Always use these tools when users ask about notes, research, ideas, tasks, or when you want to save information for future reference.
|
||||
|
||||
DAILY JOURNAL (Task Tracking):
|
||||
You have access to Ryan's daily journal notes. Each note lives at 50 - Journal/YYYY/MM/YYYY-MM-DD.md and has two sections: tasks and log.
|
||||
- Use journal_get_today to read today's full daily note (tasks + log)
|
||||
- Use journal_get_tasks to list tasks (done/pending) for today or a specific date
|
||||
- Use journal_add_task to add a new task to today's (or a given date's) note
|
||||
- Use journal_complete_task to check off a task as done
|
||||
Use these tools when Ryan asks about today's tasks, wants to add something to his list, or wants to mark a task complete.
|
||||
|
||||
USER MEMORY:
|
||||
You can remember facts about the user across conversations using the save_user_memory tool. When a user explicitly asks you to remember something, or shares a meaningful preference or personal fact, save it. Saved memories will automatically appear at the end of this prompt in future conversations under "USER MEMORIES"."""
|
||||
INTEGRATIONS: Tool descriptions cover what each tool does and its parameters; rely on those rather than this list. Use ynab_* tools for budget, spending, transaction, or financial questions. Use mealie_* tools for meals, recipes, or shopping lists. If homebox_* tools are available, use them for household items (location, warranties, maintenance); search before creating to avoid duplicates, and edit an existing item with homebox_set_item instead of creating a new one. If bookstack_* tools are available, use them for household wiki documentation; search before creating books/chapters/pages, prefer updating an existing page over a near-duplicate, and write content as markdown. Use obsidian_* tools for vault notes and research, and journal_* tools for the daily note's tasks and log (one note per day at 50 - Journal/YYYY/MM/YYYY-MM-DD.md). Use save_user_memory when the user explicitly asks you to remember something or shares a durable preference; saved memories appear below in future conversations under "USER MEMORIES"."""
|
||||
|
||||
@@ -13,6 +13,7 @@ from langchain_core.messages import convert_to_messages
|
||||
from quart import current_app
|
||||
|
||||
from metrics import AGENT_REQUESTS_TOTAL, AGENT_RESPONSE_SECONDS
|
||||
from .checkpoints import prune_thread_checkpoints
|
||||
from .logic import get_agent_for
|
||||
from .memory import build_system_prompt_with_memories
|
||||
|
||||
@@ -119,6 +120,18 @@ def _record(turn: PreparedTurn, started: float, result: str) -> None:
|
||||
)
|
||||
|
||||
|
||||
async def _prune_after_turn(turn: PreparedTurn) -> None:
|
||||
checkpointer = getattr(turn.agent, "checkpointer", None)
|
||||
if checkpointer is None:
|
||||
return
|
||||
try:
|
||||
await prune_thread_checkpoints(
|
||||
checkpointer, turn.config["configurable"]["thread_id"]
|
||||
)
|
||||
except Exception:
|
||||
logging.exception("Failed to prune checkpoints")
|
||||
|
||||
|
||||
async def invoke_prepared(turn: PreparedTurn) -> dict:
|
||||
started = time.monotonic()
|
||||
result = "success"
|
||||
@@ -132,6 +145,7 @@ async def invoke_prepared(turn: PreparedTurn) -> dict:
|
||||
raise
|
||||
finally:
|
||||
_record(turn, started, result)
|
||||
await _prune_after_turn(turn)
|
||||
|
||||
|
||||
async def invoke_agent_for_user(
|
||||
@@ -169,3 +183,4 @@ async def stream_prepared(turn: PreparedTurn):
|
||||
raise
|
||||
finally:
|
||||
_record(turn, started, result)
|
||||
await _prune_after_turn(turn)
|
||||
|
||||
+149
-54
@@ -1,4 +1,6 @@
|
||||
import asyncio
|
||||
import datetime
|
||||
import json
|
||||
import logging
|
||||
import os
|
||||
import re
|
||||
@@ -16,7 +18,7 @@ from langchain_text_splitters import (
|
||||
from sqlalchemy import create_engine, text
|
||||
|
||||
from .fetchers import PaperlessNGXService
|
||||
from .retrieval import build_filter, citation_url, document_dates
|
||||
from .retrieval import build_filter, citation_url, document_date, document_dates
|
||||
from utils.obsidian_service import ObsidianService
|
||||
|
||||
# Load environment variables
|
||||
@@ -200,9 +202,13 @@ def _sanitize_documents(documents: list[Document]) -> list[Document]:
|
||||
return [doc for doc in documents if doc.page_content]
|
||||
|
||||
|
||||
async def index_documents():
|
||||
"""Index Paperless-NGX documents into vector store."""
|
||||
documents = await fetch_documents_from_paperless_ngx()
|
||||
async def index_documents(documents: list[Document] | None = None):
|
||||
"""Index Paperless-NGX documents into vector store.
|
||||
|
||||
Pass already-fetched `documents` to index them without fetching again.
|
||||
"""
|
||||
if documents is None:
|
||||
documents = await fetch_documents_from_paperless_ngx()
|
||||
|
||||
splits = text_splitter.split_documents(documents)
|
||||
splits = _sanitize_documents(splits)
|
||||
@@ -325,6 +331,57 @@ def _load_obsidian_index_cache() -> dict[str, float]:
|
||||
return {row[0]: row[1] for row in result if row[0] is not None}
|
||||
|
||||
|
||||
def _walk_vault_mtimes(obsidian_service: ObsidianService) -> dict[str, float]:
|
||||
"""Walk the vault and stat every file. Blocking; run via asyncio.to_thread."""
|
||||
return {
|
||||
str(md_path): md_path.stat().st_mtime
|
||||
for md_path in obsidian_service.walk_vault()
|
||||
}
|
||||
|
||||
|
||||
def _read_obsidian_document(
|
||||
obsidian_service: ObsidianService, filepath: str, now: float
|
||||
) -> Document | None:
|
||||
"""Read and parse one vault file into a Document, or None on error."""
|
||||
try:
|
||||
with open(filepath, "r", encoding="utf-8") as f:
|
||||
content = f.read()
|
||||
parsed = obsidian_service.parse_markdown(content, filepath)
|
||||
metadata = {
|
||||
"source": "obsidian",
|
||||
"filepath": parsed["filepath"],
|
||||
"folder": str(Path(parsed["filepath"]).parent)
|
||||
if parsed["filepath"]
|
||||
else "",
|
||||
"tags": parsed["tags"],
|
||||
"created_at": parsed["metadata"].get("created_at"),
|
||||
"indexed_at": now,
|
||||
**{
|
||||
k: v
|
||||
for k, v in parsed["metadata"].items()
|
||||
if k not in ["created_at", "created_by"]
|
||||
},
|
||||
}
|
||||
return Document(
|
||||
page_content=parsed["content"], metadata=_make_serializable(metadata)
|
||||
)
|
||||
except Exception as e:
|
||||
logger.warning(f"Error reading {filepath}: {e}")
|
||||
return None
|
||||
|
||||
|
||||
def _read_obsidian_documents(
|
||||
obsidian_service: ObsidianService, filepaths: list[str], now: float
|
||||
) -> list[Document]:
|
||||
"""Read and parse a batch of vault files. Blocking; run via asyncio.to_thread."""
|
||||
documents = []
|
||||
for filepath in filepaths:
|
||||
document = _read_obsidian_document(obsidian_service, filepath, now)
|
||||
if document is not None:
|
||||
documents.append(document)
|
||||
return documents
|
||||
|
||||
|
||||
async def sync_obsidian_documents() -> dict[str, int]:
|
||||
"""Incrementally sync Obsidian documents to pgvector.
|
||||
|
||||
@@ -340,69 +397,52 @@ async def sync_obsidian_documents() -> dict[str, int]:
|
||||
|
||||
# Load cache from DB on first run
|
||||
if not _obsidian_index_cache:
|
||||
_obsidian_index_cache = _load_obsidian_index_cache()
|
||||
_obsidian_index_cache = await asyncio.to_thread(_load_obsidian_index_cache)
|
||||
|
||||
# Build map of current vault files -> mtime
|
||||
vault_files: dict[str, float] = {}
|
||||
for md_path in obsidian_service.walk_vault():
|
||||
vault_files[str(md_path)] = md_path.stat().st_mtime
|
||||
vault_files = await asyncio.to_thread(_walk_vault_mtimes, obsidian_service)
|
||||
|
||||
added = 0
|
||||
updated = 0
|
||||
deleted = 0
|
||||
|
||||
# Find files to add or update
|
||||
files_to_index: list[str] = []
|
||||
stale_filepaths: list[str] = []
|
||||
for filepath, mtime in vault_files.items():
|
||||
indexed_at = _obsidian_index_cache.get(filepath)
|
||||
if indexed_at is None:
|
||||
files_to_index.append(filepath)
|
||||
added += 1
|
||||
elif mtime > indexed_at:
|
||||
# Delete old chunks first
|
||||
delete_documents_by_metadata("filepath", filepath)
|
||||
stale_filepaths.append(filepath)
|
||||
files_to_index.append(filepath)
|
||||
updated += 1
|
||||
|
||||
# Find deleted files (in cache but not on disk)
|
||||
for filepath in list(_obsidian_index_cache):
|
||||
if filepath not in vault_files:
|
||||
delete_documents_by_metadata("filepath", filepath)
|
||||
del _obsidian_index_cache[filepath]
|
||||
deleted += 1
|
||||
# Find deleted files (in cache but not on disk). Don't drop them from the
|
||||
# cache yet: if the delete below raises, we want the next sync to still
|
||||
# know about them and retry, rather than leaving their chunks searchable
|
||||
# forever with no record that they need cleaning up.
|
||||
removed_filepaths = [fp for fp in _obsidian_index_cache if fp not in vault_files]
|
||||
stale_filepaths.extend(removed_filepaths)
|
||||
deleted = len(removed_filepaths)
|
||||
|
||||
# Delete old chunks for updated/removed files before re-indexing, in one
|
||||
# transaction instead of a round-trip per file. Left uncaught: a failure
|
||||
# here must stop the sync so the cache isn't marked clean for work that
|
||||
# didn't happen.
|
||||
if stale_filepaths:
|
||||
await asyncio.to_thread(delete_documents_by_filepaths, stale_filepaths)
|
||||
|
||||
# Only now that the delete has succeeded is it safe to forget these files.
|
||||
for filepath in removed_filepaths:
|
||||
del _obsidian_index_cache[filepath]
|
||||
|
||||
# Index new/changed files
|
||||
if files_to_index:
|
||||
now = time.time()
|
||||
documents = []
|
||||
for filepath in files_to_index:
|
||||
try:
|
||||
with open(filepath, "r", encoding="utf-8") as f:
|
||||
content = f.read()
|
||||
parsed = obsidian_service.parse_markdown(content, filepath)
|
||||
metadata = {
|
||||
"source": "obsidian",
|
||||
"filepath": parsed["filepath"],
|
||||
"folder": str(Path(parsed["filepath"]).parent)
|
||||
if parsed["filepath"]
|
||||
else "",
|
||||
"tags": parsed["tags"],
|
||||
"created_at": parsed["metadata"].get("created_at"),
|
||||
"indexed_at": now,
|
||||
**{
|
||||
k: v
|
||||
for k, v in parsed["metadata"].items()
|
||||
if k not in ["created_at", "created_by"]
|
||||
},
|
||||
}
|
||||
document = Document(
|
||||
page_content=parsed["content"],
|
||||
metadata=_make_serializable(metadata),
|
||||
)
|
||||
documents.append(document)
|
||||
except Exception as e:
|
||||
logger.warning(f"Error reading {filepath}: {e}")
|
||||
continue
|
||||
documents = await asyncio.to_thread(
|
||||
_read_obsidian_documents, obsidian_service, files_to_index, now
|
||||
)
|
||||
|
||||
if documents:
|
||||
splits = []
|
||||
@@ -462,7 +502,7 @@ async def query_vector_store(
|
||||
]
|
||||
serialized = "\n\n".join(
|
||||
(
|
||||
f"Source: {doc.metadata}\n"
|
||||
f"{_chunk_header(doc)}\n"
|
||||
+ (f"Citation: {citation_url(doc)}\n" if citation_url(doc) else "")
|
||||
+ f"Content: {doc.page_content}"
|
||||
)
|
||||
@@ -471,6 +511,28 @@ async def query_vector_store(
|
||||
return serialized, retrieved_docs
|
||||
|
||||
|
||||
def _chunk_title(doc: Document) -> str:
|
||||
from blueprints.conversation.artifacts_util import note_title, vault_relative_path
|
||||
|
||||
meta = doc.metadata or {}
|
||||
if meta.get("source") == "obsidian":
|
||||
path = vault_relative_path(meta.get("filepath") or "")
|
||||
return note_title(path, meta) if path else "Untitled note"
|
||||
filename = meta.get("filename") or ""
|
||||
return Path(filename).stem or filename or "Document"
|
||||
|
||||
|
||||
def _chunk_header(doc: Document) -> str:
|
||||
"""Compact per-chunk header (title, date, source) replacing the raw metadata dict."""
|
||||
meta = doc.metadata or {}
|
||||
parts = [f"Title: {_chunk_title(doc)}"]
|
||||
if date := document_date(meta):
|
||||
parts.append(f"Date: {date}")
|
||||
if source := meta.get("source"):
|
||||
parts.append(f"Source: {source}")
|
||||
return " | ".join(parts)
|
||||
|
||||
|
||||
def delete_all_documents():
|
||||
"""Delete all documents from the vector store collection."""
|
||||
collection_id = _get_collection_id()
|
||||
@@ -485,6 +547,27 @@ def delete_all_documents():
|
||||
conn.commit()
|
||||
|
||||
|
||||
def _delete_by_containment(conn, collection_id, filter_dict: dict) -> None:
|
||||
"""Delete rows whose cmetadata contains filter_dict.
|
||||
|
||||
Uses JSONB containment (`@>`) rather than `cmetadata->>key = value` so the
|
||||
delete can use the GIN index langchain_postgres creates on cmetadata
|
||||
(ix_cmetadata_gin, jsonb_path_ops); `->>` text extraction can't use it.
|
||||
|
||||
Unlike `->>`, which coerces to text before comparing, `@>` matches on the
|
||||
stored JSON type: the string "123" does not match the number 123. Callers
|
||||
must pass values with the same type as what was stored (today's only
|
||||
caller, filepath/source strings, is fine).
|
||||
"""
|
||||
conn.execute(
|
||||
text(
|
||||
"DELETE FROM langchain_pg_embedding "
|
||||
"WHERE collection_id = :cid AND cmetadata @> CAST(:filter AS jsonb)"
|
||||
),
|
||||
{"cid": collection_id, "filter": json.dumps(filter_dict)},
|
||||
)
|
||||
|
||||
|
||||
def delete_documents_by_metadata(key: str, value: str):
|
||||
"""Delete documents matching a metadata key/value pair."""
|
||||
collection_id = _get_collection_id()
|
||||
@@ -492,13 +575,25 @@ def delete_documents_by_metadata(key: str, value: str):
|
||||
return
|
||||
engine = _get_engine()
|
||||
with engine.connect() as conn:
|
||||
conn.execute(
|
||||
text(
|
||||
"DELETE FROM langchain_pg_embedding "
|
||||
"WHERE collection_id = :cid AND cmetadata->>:key = :value"
|
||||
),
|
||||
{"cid": collection_id, "key": key, "value": value},
|
||||
)
|
||||
_delete_by_containment(conn, collection_id, {key: value})
|
||||
conn.commit()
|
||||
|
||||
|
||||
def delete_documents_by_filepaths(filepaths: list[str]) -> None:
|
||||
"""Delete all chunks for several obsidian filepaths in one transaction.
|
||||
|
||||
One containment delete per path, committed together, so a sync pass with
|
||||
many stale files doesn't pay a commit round-trip per file.
|
||||
"""
|
||||
if not filepaths:
|
||||
return
|
||||
collection_id = _get_collection_id()
|
||||
if not collection_id:
|
||||
return
|
||||
engine = _get_engine()
|
||||
with engine.connect() as conn:
|
||||
for filepath in filepaths:
|
||||
_delete_by_containment(conn, collection_id, {"filepath": filepath})
|
||||
conn.commit()
|
||||
|
||||
|
||||
|
||||
@@ -225,6 +225,14 @@ def validate_configuration(env: Mapping[str, str], *, production: bool = True) -
|
||||
problem(flag, "signature validation must remain enabled in production")
|
||||
for name in ("METRICS_TOKEN", "CRON_SECRET", "EMAIL_HMAC_SECRET"):
|
||||
secret(name, minimum=32)
|
||||
if env.get("OBSIDIAN_SYNC_MODE", "bidirectional") not in {
|
||||
"bidirectional",
|
||||
"pull-only",
|
||||
"mirror-remote",
|
||||
}:
|
||||
problem(
|
||||
"OBSIDIAN_SYNC_MODE", "must be bidirectional, pull-only or mirror-remote"
|
||||
)
|
||||
if env.get("OBSIDIAN_CONTINUOUS_SYNC") == "true":
|
||||
for name in ("OBSIDIAN_EMAIL", "OBSIDIAN_PASSWORD", "OBSIDIAN_VAULT_ID"):
|
||||
if not value(name):
|
||||
|
||||
@@ -11,6 +11,7 @@ x-raggr-common: &raggr-common
|
||||
- PROMPT_BUDGET_TOKENS=${PROMPT_BUDGET_TOKENS:-16000}
|
||||
- RESPONSE_RESERVE_TOKENS=${RESPONSE_RESERVE_TOKENS:-4096}
|
||||
- CHECKPOINT_DB_PATH=${CHECKPOINT_DB_PATH:-/app/checkpoints/checkpoints.db}
|
||||
- CHECKPOINT_KEEP_PER_THREAD=${CHECKPOINT_KEEP_PER_THREAD:-20}
|
||||
- CONVERSATION_SUMMARY_DB_PATH=${CONVERSATION_SUMMARY_DB_PATH:-${CHECKPOINT_DB_PATH:-/app/checkpoints/checkpoints.db}.summaries.db}
|
||||
- CHAT_RUN_LEASE_SECONDS=${CHAT_RUN_LEASE_SECONDS:-30}
|
||||
- CHAT_RUN_MAX_SECONDS=${CHAT_RUN_MAX_SECONDS:-1800}
|
||||
@@ -60,6 +61,7 @@ x-raggr-common: &raggr-common
|
||||
- OBSIDIAN_E2E_PASSWORD=${OBSIDIAN_E2E_PASSWORD}
|
||||
- OBSIDIAN_DEVICE_NAME=${OBSIDIAN_DEVICE_NAME}
|
||||
- OBSIDIAN_CONTINUOUS_SYNC=${OBSIDIAN_CONTINUOUS_SYNC:-false}
|
||||
- OBSIDIAN_SYNC_MODE=${OBSIDIAN_SYNC_MODE:-bidirectional}
|
||||
- OBSIDIAN_VAULT_PATH=${OBSIDIAN_VAULT_PATH:-/app/data/obsidian}
|
||||
- OBSIDIAN_VAULT_NAME=${OBSIDIAN_VAULT_NAME:-}
|
||||
- S3_ENDPOINT_URL=${S3_ENDPOINT_URL}
|
||||
|
||||
+1
-1
@@ -39,7 +39,7 @@ make logs
|
||||
make migrate-history
|
||||
```
|
||||
|
||||
Startup validates configuration, runs `aerich upgrade`, optionally sets up Obsidian pull-only sync, and replaces the shell with the Python process. **A migration failure stops startup; the application is never launched.** Obsidian login/setup failures still log a warning and allow the application to continue.
|
||||
Startup validates configuration, runs `aerich upgrade`, optionally starts Obsidian sync in the background (`OBSIDIAN_SYNC_MODE`, default `bidirectional` so notes and tasks Simba writes reach your devices; containers take turns via a lock file in the shared vault), and replaces the shell with the Python process. **A migration failure stops startup; the application is never launched.** Obsidian login/setup failures still log a warning and allow the application to continue.
|
||||
|
||||
An empty database without Aerich metadata may need explicit initialization. Inspect its state first. Run the migration tool in a one-off container because the app cannot start until migrations pass:
|
||||
|
||||
|
||||
@@ -0,0 +1,618 @@
|
||||
# Local LLM efficiency tickets
|
||||
|
||||
Work items for making SimbaRAG's agent answer faster on the local llama-server
|
||||
without losing answer quality. Each ticket is written to be picked up cold: it
|
||||
says what to change, where, why, how to test it, and what "done" means.
|
||||
|
||||
Do the tickets in order. EFF-1 is a prerequisite for measuring EFF-2 and EFF-4,
|
||||
and EFF-3 is the safety net that tells you whether EFF-2 made answers worse.
|
||||
|
||||
---
|
||||
|
||||
## Background: read this first
|
||||
|
||||
### The model server
|
||||
|
||||
- llama-server runs on a separate GPU box at `http://192.168.1.7:8085`
|
||||
(OpenAI-compatible API under `/v1`). The `.env` / `.env.example` addresses
|
||||
are stale; do not trust them.
|
||||
- Model: `Qwen3.8-27B-UD-Q4_K_XL.gguf` (unsloth GGUF) on one RTX 3090 (24 GB),
|
||||
llama.cpp build `b10884` (unsloth fork), vision enabled.
|
||||
- As of 2026-10-12 the server runs with:
|
||||
`--parallel 2` (two slots, 32,768 tokens each), KV cache `q8_0`/`q8_0`,
|
||||
flash attention on, and **MTP speculative decoding**
|
||||
(`--spec-type draft-mtp --spec-draft-n-max 3`).
|
||||
- The server-side config is **not** in this repo (it is a systemd unit on the
|
||||
GPU box). Do not try to change it. Tickets here are app-side only.
|
||||
|
||||
### What we measured (so you understand the goal)
|
||||
|
||||
- Before MTP: ~34 tokens/s generation. After MTP: ~84 tokens/s on tool-call
|
||||
output, ~57 tokens/s on prose.
|
||||
- A real chat turn: 7k-13.5k prompt tokens, ~4 model calls per turn,
|
||||
0.8-3 s prompt processing per call, **~16 s before the first answer
|
||||
token**. Prompt processing is NOT the bottleneck. Generated tokens are:
|
||||
every model call first generates hidden "thinking" tokens, then either a
|
||||
tool call or the answer. With 4 calls per turn, that thinking adds up.
|
||||
- Hard-capping thinking hurts reasoning. Same arithmetic question
|
||||
("cat eats 3/8 cup twice a day, 14-cup bag opened Sept 3, when does it run
|
||||
out?", correct answer **Sept 21**):
|
||||
|
||||
| Request setting | Time thinking | Answer |
|
||||
|---|---|---|
|
||||
| `reasoning_effort: "low"` (what the app sends today) | 5.9 s | Sept 21 (correct) |
|
||||
| + `reasoning_budget_tokens: 64` | 0.97 s | Sept 20 (wrong) |
|
||||
| `chat_template_kwargs: {"enable_thinking": false}` | 0.14 s | Sept 28 (wrong) |
|
||||
|
||||
So the goal is NOT "turn thinking off". It is "turn thinking off or down only
|
||||
on calls where it is useless, and keep it where the model actually reasons".
|
||||
|
||||
### llama-server request fields you will use
|
||||
|
||||
All verified against the live server on build b10884:
|
||||
|
||||
| Field (top level of the JSON request body) | Effect |
|
||||
|---|---|
|
||||
| `reasoning_effort: "low" \| "medium" \| "xhigh"` | Soft hint. The chat template adds a sentence like "Reasoning effort is set to low" to the system prompt. The model usually but not always obeys. The app already sends `"low"` (see `model_kwargs` in `blueprints/conversation/agents.py`). |
|
||||
| `reasoning_budget_tokens: <int>` | **Hard cap** on thinking tokens for this request. When hit, llama-server forces the thinking block closed and the model must answer. (`thinking_budget_tokens` is an alias; use `reasoning_budget_tokens`.) |
|
||||
| `chat_template_kwargs: {"enable_thinking": false}` | No thinking at all for this request. The template pre-fills an empty `<think></think>` block. |
|
||||
|
||||
These are NOT OpenAI API parameters. The Python `openai` client (which
|
||||
LangChain's `ChatOpenAI` uses under the hood) rejects unknown keyword
|
||||
arguments, so they must be sent via **`extra_body`**, e.g.
|
||||
`llama_chat.bind_tools(tools, extra_body={"reasoning_budget_tokens": 512})`.
|
||||
|
||||
Also note: llama-server's `usage` block does **not** include
|
||||
`completion_tokens_details.reasoning_tokens`. That is why `reasoning_tokens`
|
||||
in the app's `stream_request` log line is always `null`. Don't try to "fix"
|
||||
that in the app; it is a server limitation. Use `output_tokens` per call
|
||||
instead (thinking tokens are included in it).
|
||||
|
||||
### How the agent is wired (the parts you will touch)
|
||||
|
||||
- `blueprints/conversation/agents.py`
|
||||
- Near the top: `llama_chat = ChatOpenAI(base_url=..., model_kwargs={"reasoning_effort": "low"}, max_tokens=RESPONSE_RESERVE_TOKENS, ...)`
|
||||
and `openai_fallback = ChatOpenAI(model="gpt-5-mini", ...)`.
|
||||
- `build_agent()` at the bottom of the file. Inside it, `routed_model(messages, config)`
|
||||
runs **once per model call**. It picks the tools, binds them, and returns
|
||||
`prompt | bound_model`. **This is the hook for per-call settings.**
|
||||
- `TOOL_ROUTING_MODE` env var (default `static`):
|
||||
- `static`: every tool the user may use is bound on every call
|
||||
(`router.select_static`). There is no `discover_tool_groups` tool.
|
||||
- `dynamic`: `router.select(messages)` picks tool groups from keywords in
|
||||
the user's message, and the model can call `discover_tool_groups` to
|
||||
unlock more groups on the next call.
|
||||
- Every ticket below must work in **both** modes. Tests should cover both
|
||||
where it matters.
|
||||
- `blueprints/conversation/tool_routing.py`: `ToolRouter`, `DISCOVERY_NAME = "discover_tool_groups"`,
|
||||
`GROUP_TERMS` keyword lists, `select()` / `select_static()`.
|
||||
- `blueprints/conversation/__init__.py`: the streaming endpoint. `_chat_run_stream`
|
||||
→ `event_generator` tracks every model call in the `model_calls` dict
|
||||
(keyed by LangChain `run_id`) from `on_chat_model_start` / `_stream` / `_end`
|
||||
events, and `_log_summary()` writes one JSON log line with
|
||||
`"event": "stream_request"` per user turn. `_model_call_summary()` builds the
|
||||
per-call dict (`prefill_ms`, `ms`, `prompt_tokens`, `output_tokens`, ...).
|
||||
- `blueprints/conversation/context.py`: prompt budgeting (`CONTEXT_WINDOW_TOKENS`,
|
||||
`PROMPT_BUDGET_TOKENS`, `RESPONSE_RESERVE_TOKENS=4096`). You should not need
|
||||
to change it.
|
||||
|
||||
### The message sequence inside one user turn
|
||||
|
||||
LangGraph's ReAct agent loops: model call → (tools run) → model call → ...
|
||||
until the model answers without tool calls. The `messages` list that
|
||||
`routed_model` receives grows like this:
|
||||
|
||||
```
|
||||
... earlier turns ...
|
||||
HumanMessage("how much did we spend on groceries last month?") <- call 1 sees this as the last message
|
||||
AIMessage(tool_calls=[ynab_spending_report(...)]) <- call 1 output
|
||||
ToolMessage(name="ynab_spending_report", content="{...}") <- call 2 sees this as the last message
|
||||
AIMessage("You spent $612 on groceries...") <- call 2 output (final answer)
|
||||
```
|
||||
|
||||
In `dynamic` mode there can be a discovery hop:
|
||||
|
||||
```
|
||||
HumanMessage("...")
|
||||
AIMessage(tool_calls=[discover_tool_groups(groups=["finance"])])
|
||||
ToolMessage(name="discover_tool_groups", content='{"enabled_tool_groups": ["finance"], ...}')
|
||||
AIMessage(tool_calls=[ynab_spending_report(...)]) <- this call needs no real thinking
|
||||
ToolMessage(name="ynab_spending_report", ...)
|
||||
AIMessage("final answer")
|
||||
```
|
||||
|
||||
The model can also make **several tool calls in one AIMessage** (parallel
|
||||
calls), which produces several `ToolMessage`s in a row. `ToolMessage.name` is
|
||||
set to the tool's name by LangChain/LangGraph for both success and error
|
||||
results. `ToolMessage.status` is `"success"` or `"error"`.
|
||||
|
||||
### Ground rules for every ticket
|
||||
|
||||
1. **One ticket = one branch = one PR.** Branch from up-to-date `main`, e.g.
|
||||
`git checkout main && git pull && git checkout -b feat/eff-1-call-kind-logging`.
|
||||
2. Line numbers drift. Find code by function/constant name, not by line number.
|
||||
3. Match surrounding style: plain functions, short docstrings, comments only
|
||||
where the "why" isn't obvious. Don't reformat unrelated code.
|
||||
4. Run the unit tests before pushing:
|
||||
`.venv/bin/pytest tests/unit -q` (or `make test`, which needs `uv`).
|
||||
All tests must pass, including ones you didn't touch.
|
||||
5. If you add an environment variable you must add it to **all four** of:
|
||||
`.env.example`, `docker-compose.yml` (the `environment:` list of the
|
||||
raggr services), `config/validation.py` (defaults/validation), and the
|
||||
real `.env` if present. Prefer module constants unless the ticket says
|
||||
otherwise; fewer knobs is better.
|
||||
6. Do not write DB migrations by hand. None of these tickets should need a
|
||||
migration; if you think one does, stop and ask.
|
||||
7. PRs go to Gitea, not GitHub: `git push -u origin <branch>` then
|
||||
`tea pr create -t "title" -d "body"`.
|
||||
8. Never point tests at the live llama-server. Unit tests must run offline.
|
||||
Only EFF-3's script talks to the real server, and it is not a pytest test.
|
||||
9. Don't commit `.DS_Store`, `chroma_db/`, `chromadb/`, `.planning/`.
|
||||
10. **You probably cannot reach the llama-server or production.** It lives on
|
||||
Ryan's home LAN (`192.168.1.7`), and production logs/DB are on his server.
|
||||
Don't try to work around that, and don't block on it. Every step below
|
||||
that needs the live server or prod data is marked **[Ryan runs]**: write
|
||||
the code so Ryan can run that step with one copy-pasteable command, put
|
||||
that exact command in the PR description under a "For Ryan to run"
|
||||
heading, and finish the rest of the ticket. If you *can* reach
|
||||
`http://192.168.1.7:8085/health` (e.g. running locally on his network),
|
||||
you may run those steps yourself; they only generate text and change
|
||||
nothing on the server.
|
||||
|
||||
---
|
||||
|
||||
## EFF-1: Classify each model call and log the classification
|
||||
|
||||
**Size:** small. **Depends on:** nothing. **Blocks:** EFF-2, EFF-4.
|
||||
|
||||
### Why
|
||||
|
||||
We want to know where the generated tokens go: is it the first call of a turn,
|
||||
the "now call the tool you just unlocked" call after `discover_tool_groups`,
|
||||
or the call that writes the answer from tool results? Today the
|
||||
`stream_request` log has per-call `output_tokens` and `ms` but nothing says
|
||||
which kind of call each one was. EFF-2 also needs this exact classification to
|
||||
pick thinking settings, so it lives in one shared function.
|
||||
|
||||
### What to build
|
||||
|
||||
1. **New module** `blueprints/conversation/thinking.py` with:
|
||||
|
||||
```python
|
||||
"""Per-call classification used to size the model's hidden thinking."""
|
||||
|
||||
from langchain_core.messages import HumanMessage, ToolMessage
|
||||
|
||||
from blueprints.conversation.tool_routing import DISCOVERY_NAME
|
||||
|
||||
FIRST = "first" # last message is the user's question
|
||||
AFTER_DISCOVERY = "after_discovery" # only discover_tool_groups results since the model last spoke
|
||||
AFTER_TOOLS = "after_tools" # real tool results to read (or anything else)
|
||||
|
||||
|
||||
def classify_call(messages) -> str:
|
||||
...
|
||||
```
|
||||
|
||||
Rules, in this order:
|
||||
- Collect the **trailing run** of `ToolMessage`s at the end of `messages`
|
||||
(walk backwards from the end while `isinstance(m, ToolMessage)`; stop at
|
||||
the first non-ToolMessage).
|
||||
- If that run is non-empty **and every** message in it has
|
||||
`m.name == DISCOVERY_NAME` **and** `m.status == "success"` → return
|
||||
`AFTER_DISCOVERY`.
|
||||
- Mixed run (discovery + a real tool in the same parallel batch) →
|
||||
`AFTER_TOOLS`, because there is real data to read.
|
||||
- A discovery result with `status == "error"` → `AFTER_TOOLS` (something
|
||||
went wrong; let the model think).
|
||||
- Else, if `messages` is non-empty and `messages[-1]` is a `HumanMessage`
|
||||
→ `FIRST`.
|
||||
- Else → `AFTER_TOOLS`. This is the safe default (it gets the most
|
||||
thinking in EFF-2), so anything unexpected, including an empty list,
|
||||
lands here.
|
||||
- `messages` may contain a `SystemMessage` at the start; it does not matter
|
||||
for these rules. `messages` might also be passed as a LangGraph state
|
||||
dict `{"messages": [...]}` in some code paths. Look at how
|
||||
`_build_prompt` in `agents.py` handles that
|
||||
(`state["messages"] if isinstance(state, dict) else state`) and do the
|
||||
same at the top of `classify_call`.
|
||||
|
||||
2. **Attach the classification to each model call** in `routed_model`
|
||||
(`blueprints/conversation/agents.py`, inside `build_agent`):
|
||||
- Compute `kind = classify_call(messages)`.
|
||||
- Attach it as LangChain run metadata so it shows up in the streaming
|
||||
events: wrap the returned chain with
|
||||
`.with_config(metadata={"call_kind": kind})`. Example:
|
||||
|
||||
```python
|
||||
chain = RunnableLambda(partial(_build_prompt, tool_schemas=schemas)) | bound
|
||||
return chain.with_config(metadata={"call_kind": kind})
|
||||
```
|
||||
|
||||
LangChain merges config metadata into child runs, so the
|
||||
`on_chat_model_start` event for this call should carry
|
||||
`event["metadata"]["call_kind"]`. **Verify this** in a test (see below);
|
||||
if metadata does not propagate in the installed LangChain version, attach
|
||||
it to `bound` instead (`bound.with_config(...)`), and re-check.
|
||||
|
||||
3. **Record it in the log line** in `blueprints/conversation/__init__.py`:
|
||||
- In the `on_chat_model_start` branch where `model_calls[event["run_id"]] = {...}`
|
||||
is created, add `"call_kind": metadata.get("call_kind")`.
|
||||
- In `_model_call_summary()`, add `"call_kind": call.get("call_kind")` to
|
||||
the returned dict.
|
||||
- Do not change any other field names in the log line; dashboards or
|
||||
scripts may read them.
|
||||
|
||||
### Tests (new file `tests/unit/test_thinking.py`)
|
||||
|
||||
Write a pure-function test for each rule. Build messages with
|
||||
`HumanMessage`, `AIMessage(content="", tool_calls=[{"id": "c1", "name": "...", "args": {}}])`,
|
||||
`ToolMessage(content="...", tool_call_id="c1", name="...", status="success")`.
|
||||
|
||||
- `[HumanMessage]` → `FIRST`.
|
||||
- `[Human, AI(tool_call discover), Tool(name=discover, success)]` → `AFTER_DISCOVERY`.
|
||||
- `[Human, AI(discover + ynab), Tool(discover), Tool(ynab)]` → `AFTER_TOOLS` (mixed parallel batch).
|
||||
- `[Human, AI(discover), Tool(discover, status="error")]` → `AFTER_TOOLS`.
|
||||
- `[Human, AI(ynab), Tool(ynab)]` → `AFTER_TOOLS`.
|
||||
- Two discovery results in a row, both success → `AFTER_DISCOVERY`.
|
||||
- An older discovery earlier in the turn but the last messages are a real
|
||||
tool result → `AFTER_TOOLS` (only the trailing run counts).
|
||||
- A previous turn ended in a tool result, then a new `HumanMessage` → `FIRST`.
|
||||
- `[]` → `AFTER_TOOLS`.
|
||||
- The state-dict form `{"messages": [HumanMessage(...)]}` → `FIRST`.
|
||||
|
||||
Plus one wiring test that the metadata reaches the streaming events. Look at
|
||||
`tests/unit/test_tool_routing.py`: `import_agents()` imports `agents.py` with
|
||||
all external services stubbed, and
|
||||
`test_real_graph_binds_discovered_schemas_and_accounts_same_subset` builds the
|
||||
real graph with a `FakeModel`. Copy that pattern: build the agent, run
|
||||
`agent.astream_events(..., version="v2")` (or whatever the app uses; check
|
||||
`_chat_run_stream`), collect `on_chat_model_start` events, and assert their
|
||||
`metadata["call_kind"]` values are `["first", "after_discovery", ...]` for a
|
||||
dynamic-mode run. If the `FakeModel` there is a plain `RunnableLambda` (not a
|
||||
chat model), you may not get `on_chat_model_start` events at all. In that
|
||||
case use `langchain_core.language_models.fake_chat_models.GenericFakeChatModel`
|
||||
(or `FakeMessagesListChatModel`) with a `bind_tools` that returns itself, so
|
||||
real chat-model events fire. Keep the test offline.
|
||||
|
||||
### Done when
|
||||
|
||||
- `classify_call` exists with the rules above and the tests pass.
|
||||
- **[Ryan runs]** After deploying, a `stream_request` log line has a
|
||||
`call_kind` on every entry in `model_calls`:
|
||||
`make logs | grep stream_request | tail -5`.
|
||||
- No behavior change for users (this ticket only adds logging).
|
||||
|
||||
---
|
||||
|
||||
## EFF-2: Size thinking per call kind (the actual speedup)
|
||||
|
||||
**Size:** small-medium. **Depends on:** EFF-1 (uses `classify_call`).
|
||||
Ideally EFF-3 exists so you can measure before/after.
|
||||
|
||||
### Why
|
||||
|
||||
Thinking is the biggest chunk of time before the first answer token, but the
|
||||
experiment in the Background section shows that killing it everywhere makes
|
||||
math and date answers wrong. Some calls don't need it:
|
||||
|
||||
- **`AFTER_DISCOVERY`** (dynamic mode only): the model just unlocked a tool
|
||||
group; all it has to do now is call the tool. Thinking here is waste.
|
||||
- **`FIRST`**: usually "pick a tool and fill in arguments", but sometimes the
|
||||
model answers directly (chit-chat, or a math question like the cat-food one)
|
||||
with no tool. So we cap it moderately instead of turning it off.
|
||||
- **`AFTER_TOOLS`**: the model reads tool results and writes the answer
|
||||
(YNAB totals, comparing dates). This is where reasoning matters. We only
|
||||
add a generous cap so one runaway call can't burn the whole 4,096-token
|
||||
`max_tokens` on thinking and leave nothing for the answer.
|
||||
|
||||
### What to build
|
||||
|
||||
1. In `blueprints/conversation/thinking.py` (from EFF-1), add:
|
||||
|
||||
```python
|
||||
FIRST_CALL_THINKING_BUDGET = 512
|
||||
AFTER_TOOLS_THINKING_BUDGET = 1024
|
||||
|
||||
|
||||
def thinking_options(kind: str) -> dict:
|
||||
"""llama-server request fields (sent via extra_body) for one call kind."""
|
||||
if kind == AFTER_DISCOVERY:
|
||||
return {"chat_template_kwargs": {"enable_thinking": False}}
|
||||
if kind == FIRST:
|
||||
return {"reasoning_budget_tokens": FIRST_CALL_THINKING_BUDGET}
|
||||
return {"reasoning_budget_tokens": AFTER_TOOLS_THINKING_BUDGET}
|
||||
```
|
||||
|
||||
Keep these as module constants, not environment variables (see ground rule 5).
|
||||
|
||||
2. In `routed_model` (`agents.py`, inside `build_agent`), pass the options to
|
||||
the **llama** model only:
|
||||
|
||||
```python
|
||||
kind = classify_call(messages) # already added in EFF-1
|
||||
fallback = openai_fallback.bind_tools(selected)
|
||||
bound = (
|
||||
llama_chat.bind_tools(selected, extra_body=thinking_options(kind)).with_fallbacks([fallback])
|
||||
if llama_chat
|
||||
else fallback
|
||||
)
|
||||
```
|
||||
|
||||
- **Do not** pass `extra_body` to `openai_fallback`. OpenAI's API rejects
|
||||
unknown fields like `reasoning_budget_tokens`, and the fallback is what
|
||||
saves us when the GPU box is down.
|
||||
- Keep the existing `model_kwargs={"reasoning_effort": "low"}` on
|
||||
`llama_chat`. The soft hint and the hard cap work together.
|
||||
- `bind_tools(tools, **kwargs)` forwards extra kwargs into the request.
|
||||
Confirm with a test (below) that `extra_body` ends up in the bound
|
||||
kwargs. If `ChatOpenAI` in the installed version does not accept
|
||||
`extra_body` through `bind_tools`, use
|
||||
`llama_chat.bind_tools(selected).bind(extra_body=...)`.
|
||||
|
||||
3. Add the options to the log line so we can correlate: in `routed_model`,
|
||||
extend the EFF-1 metadata to
|
||||
`metadata={"call_kind": kind, "thinking": thinking_options(kind)}` and copy
|
||||
`"thinking"` into `model_calls[...]` / `_model_call_summary()` the same way
|
||||
as `call_kind`. (If the fallback model handled the call, the field is still
|
||||
logged; that's fine, `provider` tells you which model ran.)
|
||||
|
||||
### Things that will break and how to fix them
|
||||
|
||||
- `tests/unit/test_tool_routing.py` has fake models with
|
||||
`def bind_tools(self, tools):` (two places). They will now get an
|
||||
unexpected `extra_body=` keyword and raise `TypeError`. Change those fakes to
|
||||
`def bind_tools(self, tools, **kwargs):`. Don't change what the tests assert.
|
||||
- If you used `.bind(extra_body=...)` instead, the fakes need a `.bind`
|
||||
method, or make `bind_tools` return a real `Runnable` (they currently return
|
||||
`RunnableLambda`, which has `.bind`). Check before assuming.
|
||||
|
||||
### Tests (add to `tests/unit/test_thinking.py`)
|
||||
|
||||
- `thinking_options(FIRST) == {"reasoning_budget_tokens": 512}`.
|
||||
- `thinking_options(AFTER_DISCOVERY) == {"chat_template_kwargs": {"enable_thinking": False}}`.
|
||||
- `thinking_options(AFTER_TOOLS) == {"reasoning_budget_tokens": 1024}`.
|
||||
- `thinking_options("something unknown") == {"reasoning_budget_tokens": 1024}`
|
||||
(safe default).
|
||||
- Wiring test, modeled on `test_real_graph_binds_discovered_schemas_and_accounts_same_subset`:
|
||||
a fake llama model whose `bind_tools(self, tools, **kwargs)` records
|
||||
`kwargs`. Run a dynamic-mode turn that does discovery → tool → answer and
|
||||
assert the recorded `extra_body` values were, in order:
|
||||
`{"reasoning_budget_tokens": 512}`, `{"chat_template_kwargs": {"enable_thinking": False}}`,
|
||||
`{"reasoning_budget_tokens": 1024}`.
|
||||
- Same wiring test in static mode (no discovery): expect `512` then `1024`.
|
||||
- Fallback test: with the primary fake set to fail, assert the fallback
|
||||
fake's `bind_tools` was **never** given `extra_body`.
|
||||
|
||||
### Manual check against the real server (**[Ryan runs]**, optional, read-only, not a test)
|
||||
|
||||
You can confirm llama-server honors the fields with curl. This only generates
|
||||
text; it changes nothing on the server:
|
||||
|
||||
```bash
|
||||
curl -s http://192.168.1.7:8085/v1/chat/completions -H 'content-type: application/json' -d '{
|
||||
"model": "qwen3.8-27b", "max_tokens": 1024, "reasoning_effort": "low",
|
||||
"reasoning_budget_tokens": 64,
|
||||
"messages": [{"role": "user", "content": "What is 17*23?"}]
|
||||
}' | python3 -c 'import json,sys; d=json.load(sys.stdin); m=d["choices"][0]["message"]; print(len(m.get("reasoning_content") or ""), d["timings"]["predicted_n"], m["content"])'
|
||||
```
|
||||
|
||||
A small `reasoning_content` length and `predicted_n` around the budget means
|
||||
the cap works.
|
||||
|
||||
### Done when
|
||||
|
||||
- Tests above pass, and the whole unit suite passes.
|
||||
- **[Ryan runs]** EFF-3's evaluation shows: median time-to-first-answer-token lower than
|
||||
before, **no drop** in tool-choice accuracy, **zero** malformed tool calls,
|
||||
and no new wrong answers on the arithmetic/date cases. If the arithmetic
|
||||
cases regress, raise `FIRST_CALL_THINKING_BUDGET` (try 1024) before anything
|
||||
else, and say so in the PR.
|
||||
- The PR description includes the exact commands for Ryan to produce a
|
||||
before/after comparison: EFF-3 run on `main` with `--label before`, then on
|
||||
this branch with `--label after`, then the compare command. If EFF-3 isn't
|
||||
merged yet, instead give him the log query for ~10 real chats (`ttft_ms` and
|
||||
per-call `output_tokens` from `stream_request` lines, grouped by
|
||||
`call_kind`). Ryan merges only after those numbers look right, so don't
|
||||
merge it yourself.
|
||||
|
||||
---
|
||||
|
||||
## EFF-3: Model-level A/B evaluation script
|
||||
|
||||
**Size:** medium. **Depends on:** nothing (but EFF-1/2 make it more useful).
|
||||
|
||||
### Why
|
||||
|
||||
`scripts/evaluate_context.py` only tests history trimming. It never calls a
|
||||
model, so it can't tell us whether a model or server change made answers
|
||||
slower or worse. We need a repeatable script that sends realistic agent
|
||||
requests to whatever `LLAMA_SERVER_URL` points at and reports speed and
|
||||
correctness, so any change (EFF-2, new llama-server flags, a different model)
|
||||
can be judged with numbers.
|
||||
|
||||
### What to build
|
||||
|
||||
1. **Fixture file** `tests/fixtures/model_eval/cases.json`: a list of cases.
|
||||
Each case is one **single model call** (not a full agent loop; that would
|
||||
need the DB, Paperless, YNAB, etc.). Shape:
|
||||
|
||||
```json
|
||||
{
|
||||
"name": "ynab_groceries_last_month",
|
||||
"kind": "tool_choice",
|
||||
"messages": [
|
||||
{"role": "user", "content": "How much did we spend on groceries last month?"}
|
||||
],
|
||||
"expect_tool": "ynab_spending_report",
|
||||
"expect_args_subset": {"category": "Groceries"}
|
||||
}
|
||||
```
|
||||
|
||||
Three kinds of case:
|
||||
- `"tool_choice"`: the user message alone; pass if the model's first
|
||||
response calls `expect_tool`, and `expect_args_subset` (optional) keys
|
||||
match the call's arguments (string compare, case-insensitive).
|
||||
- `"answer"`: messages include a canned assistant tool call **and** a canned
|
||||
tool result (role `"tool"`, realistic JSON copied from a real tool's
|
||||
output shape; look at the YNAB tools in `blueprints/ynab/tools.py` and
|
||||
the artifact shapes in `tests/fixtures/context/scenarios.json`); pass if
|
||||
the final answer text contains every string in `expect_contains`
|
||||
(case-insensitive) and calls no tool.
|
||||
- `"direct"`: no tool should be called; pass if the answer contains
|
||||
`expect_contains`. Include the cat-food question from the Background
|
||||
section (`expect_contains: ["21"]`; the correct date is Sept 21).
|
||||
|
||||
Write **at least 25 cases** covering: pets/Simba (`simba_search`,
|
||||
`simbakit_*`), meals (`mealie_*`), notes/journal (`obsidian_*`,
|
||||
`journal_*`), YNAB spending/balances (`ynab_*`), restaurants (`beli_*`),
|
||||
web search, date questions ("what's on Friday"), 2-3 follow-up questions
|
||||
that only make sense with a previous turn ("what about last month?"; put
|
||||
the previous turn in `messages`), at least 3 `"answer"` cases with
|
||||
arithmetic over tool results, and 2 `"direct"` reasoning questions. Use
|
||||
the real tool names: get them from `agents.py` (`@tool` functions) and
|
||||
`blueprints/ynab/tools.py`. No real personal data in fixtures; invent
|
||||
plausible numbers.
|
||||
|
||||
2. **Script** `scripts/evaluate_model.py`, run as
|
||||
`.venv/bin/python -m scripts.evaluate_model [--cases PATH] [--repeat N] [--label NAME] [--out results.json]`:
|
||||
- Reads `LLAMA_SERVER_URL` and `LLAMA_MODEL_NAME` from the environment
|
||||
(`load_dotenv()` like the rest of the app). Allow `--base-url` to
|
||||
override, because the `.env` value is stale; the real server is
|
||||
`http://192.168.1.7:8085/v1`.
|
||||
- For the system prompt and tool schemas, use the **real ones** so the
|
||||
eval matches production: `SIMBA_SYSTEM_PROMPT` from
|
||||
`blueprints/conversation/prompts.py`, and tool schemas via
|
||||
`convert_to_openai_tool(t)` for the tools in `agents.tools` (plus YNAB
|
||||
tools if they're in that list). Importing `agents.py` initializes
|
||||
services; use the same stubbing trick as `import_agents()` in
|
||||
`tests/unit/test_tool_routing.py` so the import works offline. Bind all
|
||||
tools (static mode) unless `--routing dynamic` is given, in which case
|
||||
use `ToolRouter(...).select(messages)` to pick them like production does.
|
||||
- If `blueprints/conversation/thinking.py` exists (EFF-2 merged), apply
|
||||
`thinking_options(classify_call(...))` per case so the eval reflects
|
||||
production; add `--no-thinking-policy` to compare against today's
|
||||
behavior.
|
||||
- Call the server with the plain `openai` client (`OpenAI(base_url=..., api_key="not-needed")`),
|
||||
**streaming**, with `stream_options={"include_usage": True}`,
|
||||
`reasoning_effort="low"`, `max_tokens=4096`, and any thinking fields in
|
||||
`extra_body`. Streaming lets you measure:
|
||||
- `ttft_ms`: time from request to the first **content or tool-call**
|
||||
delta (ignore deltas that only carry `reasoning_content`).
|
||||
- `total_ms`, `completion_tokens` and `prompt_tokens` (from `usage`).
|
||||
- llama-server also sends a non-standard `timings` object on the final
|
||||
chunk with `predicted_per_second`, `prompt_per_second`, `draft_n`,
|
||||
`draft_n_accepted`. Read it from the raw chunk
|
||||
(`chunk.model_extra.get("timings")` with the openai client) and record
|
||||
it if present; don't fail if absent.
|
||||
- Grade each case per its `kind` (rules above). A tool call whose
|
||||
arguments aren't valid JSON counts as **malformed**: record it
|
||||
separately, it's the worst failure.
|
||||
- Output: a table to stdout (one row per case: name, pass/fail, ttft_ms,
|
||||
total_ms, completion_tokens, tok/s, draft acceptance) and a summary:
|
||||
pass rate per kind, malformed count, median and p90 of ttft_ms and
|
||||
total_ms, median completion_tokens. With `--out`, write all raw
|
||||
results plus the summary as JSON, including `--label`, the base URL,
|
||||
the model name, and a timestamp.
|
||||
- `--repeat N` runs each case N times (default 1). Sampling is random
|
||||
(server default temperature 1.0), so use `--repeat 3` for real
|
||||
comparisons and report pass rate as passes/(cases×N).
|
||||
- Add `scripts/compare_model_eval.py a.json b.json` (or a `--compare`
|
||||
flag) that prints the two summaries side by side with deltas.
|
||||
- Exit code 0 always (it's a report, not a gate), except exit 2 on
|
||||
connection errors.
|
||||
|
||||
3. **Docs:** add a short section to `docs/efficiency/README.md` (create it)
|
||||
explaining how to run it, with an example command and how to read the
|
||||
output. One paragraph plus commands is enough.
|
||||
|
||||
### Tests
|
||||
|
||||
The script itself talks to a live server, so it is **not** a unit test. But
|
||||
add `tests/unit/test_model_eval.py` for the pure parts:
|
||||
- The grading function for each kind, including malformed JSON arguments and
|
||||
case-insensitive substring matching.
|
||||
- Loading `cases.json` validates required keys per kind (fail fast with a
|
||||
clear message naming the bad case).
|
||||
- The summary math (median/p90) on a small hand-made list.
|
||||
Keep these offline: no `OpenAI` client construction in tests.
|
||||
|
||||
### Done when
|
||||
|
||||
- The unit tests pass.
|
||||
- The script runs end to end against a **fake** server so you can prove it
|
||||
works without the LAN: add `--base-url` support and test it manually
|
||||
against a tiny local stub (a ~40-line `http.server` or `aiohttp` script
|
||||
that returns a canned streaming chat-completion with one tool call, a
|
||||
`usage` chunk and a `timings` object). Don't commit the stub unless it's
|
||||
clean enough to live in `tests/fixtures/model_eval/` and is used by a test.
|
||||
- **[Ryan runs]** The PR description ends with the real command for Ryan:
|
||||
`.venv/bin/python -m scripts.evaluate_model --base-url http://192.168.1.7:8085/v1 --repeat 3 --label baseline --out /tmp/baseline.json`
|
||||
|
||||
---
|
||||
|
||||
## EFF-4: Fewer discovery round-trips in dynamic routing mode
|
||||
|
||||
**Size:** small. **Depends on:** EFF-1 (to measure). **Only matters if
|
||||
production runs `TOOL_ROUTING_MODE=dynamic`.** You can't see the production
|
||||
`.env`; ask Ryan which mode production uses before starting. If production is `static`, skip this
|
||||
ticket and note that in the PR list.
|
||||
|
||||
### Why
|
||||
|
||||
Every `discover_tool_groups` call is a full extra model round-trip: prompt
|
||||
processing + generated tokens + the tool call. And because the template puts
|
||||
the tool definitions at the very top of the prompt, the next call has a
|
||||
different tool list, so llama-server's prompt cache misses and it reprocesses
|
||||
the whole prompt. Even with EFF-2 making that call cheaper, it's cheapest not
|
||||
to make it. Most discovery calls happen because the user's words didn't match
|
||||
`GROUP_TERMS` in `tool_routing.py`.
|
||||
|
||||
### What to do
|
||||
|
||||
1. **Collect data first. [Ryan runs]** After EFF-1 is deployed for a few days, pull the
|
||||
user messages that led to discovery. The `stream_request` log has
|
||||
`tools` (tool spans, with names) and `model_calls` (with `call_kind`).
|
||||
A turn where a `discover_tool_groups` tool span appears is a miss. The
|
||||
user's message text is in the DB (`conversation` messages), matched by
|
||||
the `conv` id in the log line. Write a small script (Ryan runs it on the
|
||||
server; commit it under `scripts/` only if it's clean and reusable) that
|
||||
lists, per missed turn: the
|
||||
user message and the groups the model asked for.
|
||||
2. For each miss, add the missing everyday words to the right group's string
|
||||
in `GROUP_TERMS`. Example: if "what did the paycheck cover this month?"
|
||||
triggered discovery of `finance`, add `paycheck paychecks` to the
|
||||
`finance` string. Before adding a word, ask: "would this word show up in
|
||||
questions that have nothing to do with this group?" If yes, don't add it,
|
||||
because extra tools on every call cost prompt tokens and confuse the model.
|
||||
Rules of thumb:
|
||||
- Add nouns and verbs specific to the domain ("groceries", "paycheck",
|
||||
"litter"); avoid generic words ("left", "show", "get", "much").
|
||||
- A word can be in two groups if it really belongs to both.
|
||||
- Remember matching is on whole lowercase words (`words()` splits on
|
||||
non-alphanumerics), so add plural forms separately ("bill bills").
|
||||
3. Add a test per new word family in `tests/unit/test_tool_routing.py` in the
|
||||
existing style: `names(router, [HumanMessage(content="<the real user phrasing>")])`
|
||||
includes the expected tool.
|
||||
|
||||
### Done when
|
||||
|
||||
- Every miss you collected now routes without discovery (tests prove it).
|
||||
- No existing routing test changed its expectations.
|
||||
- PR description lists the misses found and the words added.
|
||||
|
||||
---
|
||||
|
||||
## Manual / ops follow-ups (for Ryan, not for Codex)
|
||||
|
||||
These need the GPU box and are listed so they aren't forgotten:
|
||||
|
||||
- Try `--spec-draft-n-max 4` (tool-call output accepted ~92% of drafts at 3;
|
||||
depth 4 sometimes wins on JSON-heavy output). Compare with EFF-3,
|
||||
`--repeat 3`.
|
||||
- Try `--spec-type draft-mtp,ngram-mod` (n-gram drafting could help when the
|
||||
model copies IDs from tool results into arguments). Unverified; A/B with
|
||||
EFF-3.
|
||||
- Record the final llama-server unit (flags and why) in `docs/deployment.md`
|
||||
so the server config isn't only on the GPU box.
|
||||
- Old unit is backed up at `/etc/systemd/system/llama-server.service.bak`.
|
||||
@@ -0,0 +1,432 @@
|
||||
# Immich MCP: technical design
|
||||
|
||||
Status: proposed · Owner: Ryan · Date: 2026-10-13
|
||||
|
||||
## 1. Summary
|
||||
|
||||
Give Simba (and Claude Code) a small set of tools for the household Immich
|
||||
library so a request like *"my Maine trip, here are the photos"* turns into
|
||||
either a link to the album that already exists or a confirmed, one-step
|
||||
album creation. The tools live in a stdio MCP server we write and keep in
|
||||
this repository (`immich_mcp/`). raggr loads it the same way it loads
|
||||
Homebox and BookStack; Claude Code can register the same server with
|
||||
`claude mcp add`.
|
||||
|
||||
Work ships in two phases:
|
||||
|
||||
- **Phase 1, read-only.** Find albums, search photos by date, place and person,
|
||||
CLIP search, and spot photos missing from an existing album.
|
||||
- **Phase 2, writes.** Create an album and add photos to an album, behind a
|
||||
server-side switch (`IMMICH_ENABLE_WRITES=true`) and a count check that
|
||||
stops the model from writing something the user did not confirm.
|
||||
|
||||
There are no delete, archive, trash, metadata-edit or sharing tools in either phase.
|
||||
|
||||
## 2. Goals and non-goals
|
||||
|
||||
### Goals
|
||||
|
||||
1. **"Do I already have an album for X?"** returns the album name, photo count,
|
||||
date span and a clickable link, in one tool call.
|
||||
2. **"Photos from my Maine trip"** with no album returns a summary the user can
|
||||
check (how many, which days, which towns, a few sample filenames) without
|
||||
moving hundreds of asset IDs through the model's context.
|
||||
3. **"Make an album for it"** creates the album with exactly the photos the user
|
||||
saw in that summary, or does nothing and explains why.
|
||||
4. **"Is the Maine album complete?"** lists photos taken during the album's dates
|
||||
that are not in it, such as camera imports or a partner's phone, and can add them after confirmation.
|
||||
5. The same server works from Claude Code on the laptop.
|
||||
6. Tool results stay small enough for the local Qwen model's 32k context: each
|
||||
result is at most about 2,500 characters, and all six tool schemas together
|
||||
cost at most about 1,500 tokens.
|
||||
|
||||
### Non-goals
|
||||
|
||||
- Deleting, trashing, archiving, hiding, or editing assets or albums. Removing photos from an album is out too.
|
||||
- Showing images inline in chat. The desktop client's `photo` artifact
|
||||
(`docs/desktop-client.md`) can build on this later using the asset links
|
||||
these tools return.
|
||||
- Shared links, inviting album users, face naming, tagging.
|
||||
- Replacing `scripts/index_immich.py`, the one-off script that describes Simba photos into the
|
||||
vector store. It is left untouched; retiring it is an open question (§13).
|
||||
- Non-admin access. Photos are household-private, so like Homebox and BookStack
|
||||
these tools exist only on the admin agent.
|
||||
|
||||
## 3. Example conversations (acceptance scenarios)
|
||||
|
||||
These double as the manual verification script for the final ticket.
|
||||
|
||||
**A. The album already exists**
|
||||
|
||||
> **User:** my maine trip, here are the photos
|
||||
> **Simba** → `immich_find_albums(query="maine")`
|
||||
> → `[{"name": "Maine 2026", "asset_count": 412, "start": "2026-08-02", "end": "2026-08-09", "url": "https://photos.example/albums/…"}]`
|
||||
> **Simba:** You already have **Maine 2026**: 412 photos, Aug 2–9. [link]
|
||||
|
||||
**B. No album yet**
|
||||
|
||||
> **User:** find my maine trip photos
|
||||
> `immich_find_albums("maine")` → `[]`
|
||||
> `immich_search_photos(state="Maine")` → total 398; clusters: Aug 2–9 2026 (391), Jul 14 2023 (7); top places Bar Harbor, Portland
|
||||
> **Simba:** I found 391 photos from Aug 2–9, 2026, mostly Bar Harbor and Portland, plus 7 from a day in July 2023. Want an album for the August trip?
|
||||
> **User:** yes, call it Maine 2026
|
||||
> `immich_search_photos(taken_after="2026-08-02", taken_before="2026-08-09")` → total 455 (this adds the drive up and photos with no GPS)
|
||||
> **Simba:** Counting everything from Aug 2–9, not only photos tagged in Maine, gives 455 (64 have no location or were taken on the way). Create **Maine 2026** with all 455?
|
||||
> **User:** yes
|
||||
> `immich_create_album(name="Maine 2026", criteria={taken_after, taken_before}, expected_count=455)` → created, 455 added, link
|
||||
|
||||
**C. The album exists but is incomplete**
|
||||
|
||||
> **User:** is the maine album missing anything?
|
||||
> `immich_find_albums("maine")` → Maine 2026, Aug 2–9
|
||||
> `immich_album_gaps(album_id=…)` → 37 photos taken Aug 1–10 are not in the album (mostly Aug 5, Canon EOS R6)
|
||||
> **Simba:** 37 photos from those days aren't in it, mostly camera shots from Aug 5. Add them?
|
||||
> `immich_add_to_album(album_id, criteria={taken_after: 2026-08-01, taken_before: 2026-08-10}, expected_new_count=37)`
|
||||
|
||||
**D. Content search**
|
||||
|
||||
> **User:** pictures of simba in the snow
|
||||
> `immich_smart_search(query="orange cat in snow", person="Simba")` → top 10, with dates and links
|
||||
|
||||
## 4. Approach and alternatives
|
||||
|
||||
| Option | Pros | Cons |
|
||||
|---|---|---|
|
||||
| **A. Our own stdio MCP server in this repo (chosen)** | One implementation serves raggr and Claude Code; we control tool names, descriptions and output size; write safety is enforced in code we own; no new runtime dependency (`mcp` 1.30 is already locked through `langchain-mcp-adapters`); fits the Homebox/BookStack loading pattern | We maintain it (≈500 lines plus tests) |
|
||||
| B. Third-party Immich MCP server (several exist: lidless-labs `immich-mcp`, `barryw/ImmichMCP`, `okopok/immich-mcp`, …), pinned and allowlisted like Homebox/BookStack | No code to write | Not evaluated in depth. Generic API-wrapping servers return per-asset JSON (a 400-photo trip won't fit in 32k) and add assets by ID list (the model would have to copy hundreds of UUIDs). Our key, which needs album write permissions, would go to a young third-party package. Tool descriptions we can't edit cost context. |
|
||||
| C. Plain LangChain tools in `utils/immich_service.py` (like Mealie/YNAB) | Simplest wiring | Simba only; Claude Code would need a second implementation |
|
||||
|
||||
Option A keeps option C's control and adds reuse from Claude Code. Most of
|
||||
the work goes into compact summaries and write guards, and no off-the-shelf
|
||||
server gives us either.
|
||||
|
||||
## 5. Architecture
|
||||
|
||||
```
|
||||
┌───────────── raggr (admin agent) ─────────────┐
|
||||
│ app.py lifespan │
|
||||
│ load_immich_tools() ─┐ │
|
||||
│ │ langchain-mcp-adapters│
|
||||
│ wrap_admin_only() │ (new stdio session │
|
||||
│ prefix immich_* │ per tool call) │
|
||||
└────────────────────────┼───────────────────────┘
|
||||
│ stdio
|
||||
Claude Code ── claude mcp add ──────┤
|
||||
▼
|
||||
┌──────── immich_mcp (python -m immich_mcp) ─────┐
|
||||
│ server.py FastMCP tools (6) │
|
||||
│ criteria.py PhotoCriteria → Immich search body │
|
||||
│ summarize.py clusters / places / samples │
|
||||
│ client.py ImmichClient (httpx, method+path │
|
||||
│ allowlist, no DELETE) │
|
||||
└────────────────────────┬───────────────────────┘
|
||||
│ HTTPS, x-api-key
|
||||
▼
|
||||
Immich server (/api)
|
||||
```
|
||||
|
||||
**Process lifetime.** `MultiServerMCPClient.get_tools()` without a session
|
||||
creates tools that open a **new stdio session for every call**
|
||||
(`langchain_mcp_adapters/tools.py`). The server therefore must be
|
||||
**stateless between calls**: no in-memory selections, caches or tokens. Write
|
||||
tools take the same *criteria* as the preview and resolve them again, and
|
||||
the `expected_count` check (§8) catches any drift. Starting a process costs
|
||||
roughly 0.5–1 s per call. That is acceptable for this use, and IMM-04 measures it.
|
||||
|
||||
## 6. Package layout
|
||||
|
||||
New top-level package (copied into the image by a new `COPY immich_mcp ./immich_mcp`
|
||||
line, because the Dockerfile copies directories explicitly):
|
||||
|
||||
```
|
||||
immich_mcp/
|
||||
__init__.py # docstring only
|
||||
__main__.py # `python -m immich_mcp` → server.main()
|
||||
config.py # env parsing (§10), ImmichConfig dataclass
|
||||
client.py # ImmichClient: async httpx, allowlisted endpoints only
|
||||
criteria.py # PhotoCriteria (pydantic), date/time-zone handling, request bodies
|
||||
summarize.py # pure functions: clusters, places, samples, histogram
|
||||
server.py # FastMCP("immich"), tool definitions, JSON output shaping
|
||||
tests/unit/
|
||||
test_immich_client.py
|
||||
test_immich_criteria.py
|
||||
test_immich_summarize.py
|
||||
test_immich_server.py # tools via an in-memory MCP session
|
||||
test_immich_mcp.py # raggr loader + admin gate (mirrors test_homebox_mcp.py)
|
||||
```
|
||||
|
||||
raggr-side changes:
|
||||
|
||||
- `utils/mcp_tools.py`: `load_stdio_tools(..., args: list[str] | None = None,
|
||||
cwd: str | None = None)`. Today `args` is hard-coded to `[]` and no `cwd` is passed.
|
||||
`StdioConnection` already supports both. Homebox and BookStack keep the defaults.
|
||||
- `utils/immich_mcp.py`: `IMMICH_TOOL_ALLOWLIST`, `select_immich_tools`,
|
||||
`load_immich_tools()` (mirrors `utils/homebox_mcp.py`). The command defaults to
|
||||
`sys.executable` (the app's venv interpreter) with `args=["-m", "immich_mcp"]`
|
||||
and `cwd` set to the repo root (`Path(__file__).resolve().parents[1]`), so
|
||||
`-m` finds the package whatever the parent's working directory is. The subprocess
|
||||
env is only the `IMMICH_*` variables plus `PATH` (`load_stdio_tools` behaviour),
|
||||
so no other secrets leak into it. `IMMICH_MCP_COMMAND` overrides the
|
||||
command for debugging. Note the two names: `utils/immich_mcp.py` is raggr's
|
||||
*loader*, and top-level `immich_mcp/` is the *server*. Neither imports the other.
|
||||
- `app.py`: add `await load_immich_tools()` to `admin_tools`.
|
||||
- `blueprints/conversation/tool_routing.py`: add a `"photos"` group with the terms
|
||||
`immich photo photos picture pictures pic pics album albums trip trips vacation`,
|
||||
map the prefix `("immich_", "photos")`, and add `"immich"` to the admin prefix
|
||||
tuple in `ToolRouter.__init__`.
|
||||
- `blueprints/conversation/tool_labels.py`: `("immich_", "Looking through photos…")`.
|
||||
- `raggr-frontend/src/components/ActivityTimeline.tsx`: `["immich", Images]` (lucide).
|
||||
- `blueprints/conversation/prompts.py`: one sentence in INTEGRATIONS (§9).
|
||||
- `blueprints/integrations/__init__.py`: admin row `("Immich", ("IMMICH_URL", "IMMICH_API_KEY"))`.
|
||||
- `config/validation.py`: `IMMICH_URL` and `IMMICH_PUBLIC_URL` in the URL list;
|
||||
`IMMICH_ENABLE_WRITES` in the true/false list.
|
||||
- `.env.example`, `docker-compose.yml`: env vars (§10). `IMMICH_URL` and
|
||||
`IMMICH_API_KEY` already exist in `.env.example` for the indexer, so reuse them.
|
||||
|
||||
## 7. Tool specifications
|
||||
|
||||
Every tool returns **compact JSON** (`json.dumps(..., separators=(",", ":"),
|
||||
ensure_ascii=False)`), at most `MAX_RESULT_CHARS = 2500`. Dates in output are
|
||||
calendar dates (`YYYY-MM-DD`): a photo's date is its own local day (§7.4), and
|
||||
album `start`/`end` (UTC timestamps in Immich) are converted to
|
||||
`IMMICH_TIMEZONE`. If a result would go over the cap, lists are cut down (samples
|
||||
first, then places, then clusters) and `"trimmed": true` is set; the result is
|
||||
never cut mid-JSON. Links are built
|
||||
from `IMMICH_PUBLIC_URL`: albums at `/albums/{id}`, photos at `/photos/{id}`.
|
||||
Errors come back as `{"error": "<plain sentence>"}` and never raise, so the
|
||||
model can tell the user what went wrong.
|
||||
|
||||
### 7.1 Shared input: `PhotoCriteria`
|
||||
|
||||
| Field | Type | Meaning |
|
||||
|---|---|---|
|
||||
| `taken_after` | `YYYY-MM-DD` | inclusive, local day start |
|
||||
| `taken_before` | `YYYY-MM-DD` | **inclusive**, local day end (converted to the next day 00:00 local, exclusive) |
|
||||
| `state` | str | reverse-geocoded state/province, e.g. "Maine" |
|
||||
| `city` | str | e.g. "Bar Harbor" |
|
||||
| `country` | str | e.g. "United States of America" |
|
||||
| `person` | str | Immich person name, resolved via `GET /search/person?name=`; several matches → error that lists them |
|
||||
| `media` | `"photo" \| "video" \| "all"` | default `"all"` |
|
||||
| `favorites_only` | bool | default false |
|
||||
|
||||
**The fields are flat tool parameters, never a nested object.** Every tool that
|
||||
takes criteria declares these eight parameters directly. A nested `criteria`
|
||||
object would produce `$defs`/`$ref` in the tool schema, which the local model's
|
||||
llama-server grammar handles poorly, and it costs more schema tokens. In the
|
||||
examples in §3, `criteria={…}` is shorthand for passing these parameters.
|
||||
|
||||
For `immich_search_photos` and both write tools, at least one field must be
|
||||
set, so a request can never match the whole library. (`immich_smart_search` is
|
||||
already scoped by its query.)
|
||||
`taken_before < taken_after` is an error. Place names are matched
|
||||
case-insensitively: before searching, the server looks the value up in
|
||||
`GET /search/suggestions?type=state|city|country` and uses Immich's exact
|
||||
spelling. If nothing matches, it returns the three closest suggestions
|
||||
(`difflib.get_close_matches`) instead of searching.
|
||||
|
||||
### 7.2 Phase 1: read-only
|
||||
|
||||
**`immich_find_albums(query: str = "", limit: int = 10)`**
|
||||
Lists albums (`GET /albums`, both owned and shared-with-me) and ranks them by
|
||||
case-insensitive substring match, then `difflib` ratio ≥ 0.6. An empty query
|
||||
returns the most recently updated. Output per album: `id`, `name`,
|
||||
`asset_count`, `start`, `end`, `shared`, `owned` (false for albums shared with
|
||||
the key's user; adding to those may fail), `url`. Why match locally: Immich's `name` filter
|
||||
is an exact match, and "maine" should find "Maine 2026" and "Acadia / Maine".
|
||||
|
||||
**`immich_search_photos(criteria…, samples: int = 5)`**
|
||||
1. `POST /search/statistics` gets the exact `total`.
|
||||
2. Pages `POST /search/metadata` (size 1000, `withExif: true`) up to
|
||||
`MAX_SCAN = 5000` assets.
|
||||
3. Returns `total`, `scanned`, `truncated`, `first`, `last`,
|
||||
`clusters` (§7.4, at most 8), `top_places` (city/state with counts, at most 5),
|
||||
`no_location` (count with no city/state), `by_media` (photo/video counts),
|
||||
`samples` (evenly spaced across the scan: date, city, filename, url), and
|
||||
`criteria` (the normalized criteria, echoed so the model can pass it
|
||||
straight to a write tool).
|
||||
|
||||
**`immich_smart_search(query: str, criteria… (all optional), limit: int = 10)`**
|
||||
`POST /search/smart` (CLIP), with the same filters. Returns the top `limit` (at most 25)
|
||||
results in rank order: date, city, filename, url. There is no total, because
|
||||
smart search ranks results rather than filtering them. Use it to find content
|
||||
("in the snow", "sunset at the beach"), not to collect a trip.
|
||||
|
||||
**`immich_album_gaps(album_id: str, pad_days: int = 1)`**
|
||||
Loads the album (`GET /albums/{id}` for `startDate`/`endDate`). It then collects
|
||||
the IDs already in the album (`POST /search/metadata` with `albumIds=[id]`,
|
||||
paged) and the photos in `[start − pad, end + pad]` (local dates), and returns the
|
||||
search summary from §7.2 computed over the photos **not** in the album. It also
|
||||
returns `criteria` for that window and `missing_count`, ready for
|
||||
`immich_add_to_album`. Returns an error if the album has no dates (it's empty).
|
||||
`missing_count` must be computed exactly as `immich_add_to_album` computes
|
||||
`expected_new_count`; both go through one shared helper,
|
||||
`resolve_missing(album_id, criteria)`. If the window has more than `MAX_SCAN`
|
||||
photos, `missing_count` is marked `truncated`, and a later add with that
|
||||
count fails the check. That's the safe direction.
|
||||
|
||||
### 7.3 Phase 2: writes (only registered when `IMMICH_ENABLE_WRITES=true`)
|
||||
|
||||
**`immich_create_album(name: str, expected_count: int, <criteria fields>, description: str = "")`**
|
||||
1. If any album already has this exact name (case-insensitive), return an error that names it
|
||||
and suggests `immich_add_to_album`. Never create a duplicate.
|
||||
2. Resolve the criteria to asset IDs (full paging, no `MAX_SCAN` limit, but
|
||||
refuse if `total > MAX_WRITE = 2000`).
|
||||
3. If `len(ids) != expected_count`, return `{"error": …, "actual_count": n}`
|
||||
and write nothing.
|
||||
4. `POST /albums` with `albumName`, `description`, and `assetIds` for the first
|
||||
500. Add the rest with `PUT /albums/{id}/assets` in chunks of 500.
|
||||
5. Return `id`, `name`, `added`, `url`.
|
||||
|
||||
**`immich_add_to_album(album_id: str, expected_new_count: int, <criteria fields>)`**
|
||||
1. Resolve the criteria to IDs (same `MAX_WRITE` limit) and subtract the IDs already in the album.
|
||||
2. If the count of new IDs differs from `expected_new_count`, return an error and write nothing.
|
||||
3. `PUT /albums/{id}/assets` in chunks of 500. Count per-ID results from
|
||||
`BulkIdResponseDto`: `success`, `duplicate`, other errors (with reasons).
|
||||
4. Return `added`, `already_present`, `failed` (at most 5 reasons), `url`.
|
||||
|
||||
### 7.4 Trip clustering (`summarize.cluster_by_day`)
|
||||
|
||||
Group scanned assets by the photographer's local day: the date part of
|
||||
`localDateTime`, taken **as-is**. Immich stores it as a "timezone-agnostic
|
||||
timestamp" of the wall-clock time, so converting it would shift days. If
|
||||
`localDateTime` is missing, fall back to `fileCreatedAt` converted to
|
||||
`IMMICH_TIMEZONE`. Sort the
|
||||
days and start a new cluster wherever the gap between consecutive days is more than
|
||||
`gap_days = 2` (one quiet day in the middle of a trip does not split it). Each
|
||||
cluster reports `start`, `end`, `count`, and its top city/state. Clusters are sorted
|
||||
by count descending; keep 8. This is how "my Maine trip" becomes
|
||||
"Aug 2–9, 2026" rather than "every photo ever taken in Maine". It's a pure function,
|
||||
tested with hand-built fixtures.
|
||||
|
||||
## 8. Write safety
|
||||
|
||||
These layers stack; any one of them blocks an unwanted write.
|
||||
|
||||
1. **Not registered by default.** Write tools are defined only when
|
||||
`IMMICH_ENABLE_WRITES=true`. Phase 1 can be deployed and used before
|
||||
anyone decides to allow writes.
|
||||
2. **Admin-only.** The tools are compiled only into `app.admin_agent`, and each call
|
||||
re-checks `is_admin` through `wrap_admin_only` (`utils/mcp_tools.py`).
|
||||
3. **Confirmed count.** `expected_count` / `expected_new_count` must equal the
|
||||
number of photos the server resolves at write time. The model can only know the
|
||||
number by having shown a search result, and the prompt (§9) tells it to get a yes
|
||||
from the user first. If the library changed in between, the check fails.
|
||||
4. **Bounded size.** `MAX_WRITE = 2000` per call.
|
||||
5. **No destructive endpoints in the client.** `ImmichClient` exposes named
|
||||
methods only, and its request helper accepts only the methods and path patterns
|
||||
in `ALLOWED_ENDPOINTS`. A unit test asserts that no `DELETE`, `PATCH`, or
|
||||
`/trash`, `/assets` (bulk) write is reachable.
|
||||
6. **A scoped API key.** Immich API keys carry permissions. The key needs only
|
||||
`album.read`, `asset.read`, `asset.statistics`, `person.read`, and for Phase 2
|
||||
`album.create` and `albumAsset.create`. Do not give it `all`.
|
||||
7. **No duplicate names.** See `immich_create_album` step 1.
|
||||
8. **Writes add and never remove.** At worst a write adds wrong photos to an
|
||||
album. That is fixable in the Immich UI and loses no data.
|
||||
|
||||
## 9. Prompt and routing
|
||||
|
||||
Append to INTEGRATIONS in `blueprints/conversation/prompts.py`:
|
||||
|
||||
> If immich_* tools are available, use them for photos and albums: call
|
||||
> immich_find_albums before searching or creating; for a trip, search by place,
|
||||
> read the date clusters, then search the whole date range so photos without
|
||||
> GPS are included; show the count and dates and get an explicit yes before
|
||||
> immich_create_album or immich_add_to_album, passing the confirmed count.
|
||||
|
||||
Routing: the `"photos"` group (§6) makes these tools discoverable in
|
||||
`TOOL_ROUTING_MODE=dynamic`. In static mode they are always bound for admins, like
|
||||
Homebox.
|
||||
|
||||
## 10. Configuration
|
||||
|
||||
| Variable | Required | Default | Notes |
|
||||
|---|---|---|---|
|
||||
| `IMMICH_URL` | yes | – | Base URL the **server** uses (LAN/compose network). Already in `.env.example`. |
|
||||
| `IMMICH_API_KEY` | yes | – | Scoped key (§8.6). Already in `.env.example`. |
|
||||
| `IMMICH_PUBLIC_URL` | no | `IMMICH_URL` | Base for links shown to the user. |
|
||||
| `IMMICH_TIMEZONE` | no | `TZ`, then `America/New_York` | Local-day interpretation (§7.1, §7.4). |
|
||||
| `IMMICH_ENABLE_WRITES` | no | `false` | Registers the Phase 2 tools. |
|
||||
| `IMMICH_MCP_COMMAND` | no | `sys.executable -m immich_mcp` | Override for debugging. |
|
||||
|
||||
All of them go in `.env`, `.env.example` and the `raggr` service environment in
|
||||
`docker-compose.yml` (project rule). If `IMMICH_URL` or `IMMICH_API_KEY` is unset,
|
||||
`load_immich_tools()` logs at info level and returns `[]`, like Homebox.
|
||||
|
||||
Claude Code registration (documented in IMM-07):
|
||||
|
||||
```bash
|
||||
claude mcp add immich \
|
||||
-e IMMICH_URL=http://192.168.1.5:2283 -e IMMICH_API_KEY=… \
|
||||
-e IMMICH_PUBLIC_URL=https://photos.… -e IMMICH_ENABLE_WRITES=true \
|
||||
-- uv run --directory ~/Programs/raggr python -m immich_mcp
|
||||
```
|
||||
|
||||
## 11. Immich API compatibility
|
||||
|
||||
Verified against the upstream OpenAPI spec at `3.3.0-rc.0`:
|
||||
|
||||
- Endpoints used: `GET /server/version`, `GET /albums`, `GET /albums/{id}`,
|
||||
`POST /albums`, `PUT /albums/{id}/assets`, `POST /search/metadata`,
|
||||
`POST /search/smart`, `POST /search/statistics`, `GET /search/suggestions`,
|
||||
`GET /search/person`.
|
||||
- **Deprecation.** In v3.2.0 Immich deprecated the flat search fields
|
||||
(`takenAfter`, `takenBefore`, `state`, `city`, `country`, `personIds`, `albumIds`,
|
||||
`type`, `page`, …) in favour of a nested `filter` object (`SearchFilter`, with
|
||||
`DateFilter {gte, lt}`, `StringFilterNullable {eq, in}`, `IdsFilter {any, all, none}`)
|
||||
and `cursor` paging. The flat fields still work in 3.3, and they are the
|
||||
only option before 3.2. Mapping: `takenAfter/takenBefore` → `filter.takenAt
|
||||
{gte, lt}`, `state/city/country` → `{eq}`, `personIds` → `{all}`, `type` →
|
||||
`{eq}`, `isFavorite` → `{eq: true}`, `albumIds` → `{any}`, and in `album_gaps`, `{none}`.
|
||||
- **Decision.** Build request bodies in a single function,
|
||||
`criteria.to_search_body(criteria, version)`. Use the flat fields below 3.2 and
|
||||
`filter`/`cursor` from 3.2 on, choosing by `GET /server/version` (read once per
|
||||
process; a process lives for one call, so this is one extra request per call, or
|
||||
`IMMICH_API_STYLE=legacy|filter` to skip it). Golden-body tests pin both shapes.
|
||||
The `filter` style also gives `album_gaps` a server-side `albumIds: {none: [id]}`,
|
||||
so it doesn't need to diff locally.
|
||||
- Production's Immich version is not yet known (it was unreachable from the design
|
||||
machine). IMM-01 records it. If it is below 2.0, stop and revisit this design.
|
||||
|
||||
## 12. Testing
|
||||
|
||||
- **Pure units:** criteria validation, local-day to UTC conversion
|
||||
(DST edges, inclusive `taken_before`), request-body golden tests for both API
|
||||
styles, clustering fixtures (single trip, quiet day inside, two trips, empty),
|
||||
sample spacing, output caps (a large fixture stays at most 2500 chars).
|
||||
- **Client:** `httpx.MockTransport`. Checks the endpoint allowlist (DELETE raises),
|
||||
paging stops at `MAX_SCAN`, auth header, and that HTTP errors map to readable error strings.
|
||||
- **Tools:** call the FastMCP server in-process with
|
||||
`mcp.shared.memory.create_connected_server_and_client_session` against the mock
|
||||
transport. Covers scenarios A–D from §3, the `expected_count` mismatch (no write
|
||||
happens), the duplicate-name refusal, write tools absent when the flag is off, and the
|
||||
schema token budget (sum of `len(json.dumps(schema)) / 4` is at most 1500).
|
||||
- **raggr wiring:** mirror `tests/unit/test_homebox_mcp.py` (allowlist, admin
|
||||
gate, missing env returns `[]`, prefix), plus `test_tool_routing.py` cases for the
|
||||
`photos` group and admin hiding.
|
||||
- **Live (manual, IMM-08):** scenarios A–D against production with a scoped key.
|
||||
|
||||
## 13. Open questions
|
||||
|
||||
1. **Production Immich version** and whether `/search/statistics` and the
|
||||
`filter` style are available (IMM-01).
|
||||
2. **Partner photos.** Immich search covers partners with "show in timeline"
|
||||
enabled. Can the key's user add a partner's asset to their own album? If not,
|
||||
`add` returns per-ID errors, which we report. Verify in IMM-01.
|
||||
3. **Time zone of typed dates.** Clustering uses each photo's own local day
|
||||
(§7.4), but `taken_after`/`taken_before` filter on UTC `takenAt`, interpreted
|
||||
in `IMMICH_TIMEZONE`. For a trip three hours west, a photo at 11 pm local
|
||||
on the last day falls outside the window. `album_gaps` pads by a day, and
|
||||
users can widen a range. A precise fix (filtering on `localDateTime`) is not
|
||||
in the API. Accepted for now.
|
||||
4. **Retire `scripts/index_immich.py`?** Once smart search is live, the vector-store
|
||||
copy of Simba photo descriptions may no longer be needed. Decide after Phase 1 has run for a while.
|
||||
5. **Sharing.** Should "send the Maine album to Mom" create a shared link? Not now;
|
||||
it would need `sharedLink.create` and a separate design.
|
||||
|
||||
## 14. Rollout
|
||||
|
||||
1. Merge Phase 1 with `IMMICH_ENABLE_WRITES` unset. Deploy and run scenarios A, B (read
|
||||
part), C (read part) and D.
|
||||
2. Create the Phase 2 key permissions, set `IMMICH_ENABLE_WRITES=true`, redeploy,
|
||||
and run B and C end to end on a throwaway album name. Delete the test album by hand in Immich.
|
||||
3. Register the server in Claude Code.
|
||||
@@ -0,0 +1,789 @@
|
||||
# Immich MCP: tickets
|
||||
|
||||
Source of truth: [`design.md`](design.md). Section references like "§7.2"
|
||||
point there. When a ticket and the design disagree, the design wins: stop
|
||||
and ask the conductor rather than guessing.
|
||||
|
||||
Line numbers are from `main` at `bae2e75` (2026-10-13). They will drift, so
|
||||
search for the quoted code when they don't match.
|
||||
|
||||
---
|
||||
|
||||
## 1. How this work is run (Herdr rules)
|
||||
|
||||
All of these tickets are built with the **Herdr fan-out workflow** (`/herdr-fanout`).
|
||||
The roles do not change from ticket to ticket.
|
||||
|
||||
### Roles
|
||||
|
||||
- **Conductor pane** (the main Claude session, normally Opus). It plans, hands
|
||||
out work, reviews, and ships. It does **not** write the feature code for
|
||||
worker tickets. The exceptions are tickets marked *Owner: conductor*, and a
|
||||
second-round review that still finds a real bug, which the conductor fixes directly.
|
||||
- **Worker panes** (sub panes, one per ticket, smaller model:
|
||||
`claude --model sonnet`). Each one implements exactly one ticket in its own git
|
||||
worktree and then reports back. Workers never ship.
|
||||
|
||||
### Conductor duties, in order
|
||||
|
||||
1. **Preconditions.** `test "$HERDR_ENV" = 1`; read `CLAUDE.md`; `git fetch`; note
|
||||
`origin/main`. Pick each ticket's **base branch**: `main` once its
|
||||
dependencies are merged. A ticket is never stacked on an unmerged branch
|
||||
unless this file says so.
|
||||
2. **Plan file per ticket.** Copy the ticket section below into
|
||||
`<scratchpad>/plans/<ticket-id>.md`, add the current line anchors, and paste
|
||||
the **worker ground rules** (below) at the bottom.
|
||||
3. **Worktree + pane per ticket.**
|
||||
```bash
|
||||
git worktree add -b <branch> <scratchpad>/wt-<ticket-id> origin/main
|
||||
ln -s ~/Programs/raggr/.venv <scratchpad>/wt-<ticket-id>/.venv
|
||||
ln -s ~/Programs/raggr/raggr-frontend/node_modules <scratchpad>/wt-<ticket-id>/raggr-frontend/node_modules # only IMM-05
|
||||
herdr pane run <pane> "claude --model sonnet \"Implement the plan in <plan path>. Read it fully first. Your worktree is \$PWD.\""
|
||||
```
|
||||
Open one tab per wave, split into one pane per ticket (`--no-focus`, `--cwd`
|
||||
set to the worktree). Keep the ticket→pane mapping in `<scratchpad>/panes.md`.
|
||||
4. **Watch.** Run a Monitor loop over `herdr agent get <pane>` that reports when a
|
||||
pane becomes `idle|done|blocked`. Review each pane as soon as it finishes;
|
||||
don't wait for the whole wave.
|
||||
5. **Review twice per ticket.**
|
||||
- Read the worker's final report from its transcript (newest
|
||||
`~/.claude/projects/<cwd-slug>/*.jsonl`). Pane scrollback is too short to rely on.
|
||||
- Read the diff yourself: `git -C <wt> diff origin/main`, plus new files. Check it
|
||||
against the ticket's acceptance criteria and the design. Look at correctness,
|
||||
edge cases, security (the write guards in §8), anything changed silently,
|
||||
scope creep, and whether each test **fails without the change** (spot-check by
|
||||
reverting the fix).
|
||||
- Run the suite yourself in the worktree; a worker saying "tests pass" is not enough.
|
||||
- Send feedback as a numbered list with `file:line` and the fix you expect:
|
||||
`herdr pane run <pane> "<feedback>"`. Confirm the pane flips to `working`.
|
||||
- **Round two:** confirm every round-one item was actually fixed. If round two
|
||||
still finds a real bug, the conductor fixes it directly. There is no round three.
|
||||
6. **Ship, one PR per ticket.** In the worktree: re-run tests and lint; `git fetch`;
|
||||
rebase onto `origin/main` if it moved and re-test. Then **confirm nothing main gained
|
||||
was reverted** (for files main changed since your base, diff and check main's
|
||||
changes are still there). Commit with a message that explains *why*, plus the
|
||||
session's attribution lines. Then `git push -u origin <branch>` and
|
||||
`tea pr create -t "<title>" -d "<body>"`. The body covers the problem, the change,
|
||||
what review caught, and test numbers. Merge in ticket order when the user has
|
||||
asked for merging (`tea pr merge --style merge <n>`, after CI is green). If a
|
||||
merge is blocked, list the PRs and their order for the user and don't work around it.
|
||||
7. **Clean up.** Exit the workers, `herdr pane close` / `herdr tab close`,
|
||||
`git worktree remove` each worktree, and delete merged local branches.
|
||||
8. **Report.** Give a table of ticket → PR → state, what each review round
|
||||
caught, and anything left undone.
|
||||
|
||||
### Ownership of this file (sheepdog rule)
|
||||
|
||||
Several panes run at once, and each opens its own PR, so this file would be a
|
||||
merge-conflict magnet if everyone edited it. One pane owns it:
|
||||
|
||||
- **The conductor is the sheepdog, and the only writer of `tickets.md`.**
|
||||
Workers never edit it, and ticket PRs never touch it. A worker's diff that touches
|
||||
this file is sent back in review.
|
||||
- The conductor records status **once per wave**, after that wave's PRs merge.
|
||||
It makes one small commit on `docs/immich-mcp-tickets-wave-<n>` (tick checkboxes,
|
||||
add the PR numbers to the order table, file follow-ups as IMM-09+) and merges that
|
||||
PR before starting the next wave. No two branches edit the file at the same time,
|
||||
so there's nothing to race on.
|
||||
- Workers who find something ticket-worthy (a bug outside their scope, a design gap)
|
||||
put it under "open doubts" in their report. The conductor decides whether it becomes a ticket.
|
||||
- Live notes during a wave (pane mapping, review state) go in `<scratchpad>/panes.md`,
|
||||
not here.
|
||||
|
||||
### Worker ground rules (paste into every plan)
|
||||
|
||||
- Work **only** inside your worktree (your cwd). Don't touch other worktrees or
|
||||
`~/Programs/raggr`. No `git stash`, commits, pushes or PRs; the conductor ships.
|
||||
- Read `CLAUDE.md` and `docs/immich-mcp/design.md` first. Match the surrounding
|
||||
style: comment density, naming, and async everywhere.
|
||||
- Change only the files your ticket lists. If you think another file needs to
|
||||
change, say so in your report instead of editing it.
|
||||
- **Never edit `docs/immich-mcp/tickets.md`.** The conductor owns it (sheepdog rule).
|
||||
- Run `.venv/bin/pytest tests/unit -q` and `uvx ruff==0.8.2 check .`; both must
|
||||
pass. For IMM-05 also run `cd raggr-frontend && yarn tsc --noEmit`.
|
||||
- Never hand-write DB migrations (none of these tickets need one). If you think
|
||||
one is needed, stop and report.
|
||||
- New env vars go in **three places**: `.env.example`, the `x-raggr-common`
|
||||
environment block in `docker-compose.yml`, and the conductor's note to update `.env`.
|
||||
- Don't call a real Immich server from tests. Use `httpx.MockTransport` or stubs.
|
||||
- When you're done, reply with: files changed, what you did, the test and lint output
|
||||
(counts), and any open doubts. Feedback will arrive in this pane. Address it and
|
||||
reply in the same format.
|
||||
|
||||
### Repo conventions the conductor enforces
|
||||
|
||||
- Gitea with `tea` (not `gh`). PR flow: push, then `tea pr create`. "Kermit" means commit.
|
||||
- One logical change per branch/PR. Branch names follow the tickets below.
|
||||
- CI runs `ruff check .`, `pytest tests/unit`, `python -m scripts.evaluate_context`,
|
||||
and the frontend typecheck and build. All must be green before merging.
|
||||
- Never commit `.DS_Store`, `chroma_db/`, `chromadb/`, `.planning/`.
|
||||
|
||||
---
|
||||
|
||||
## 2. Ticket order
|
||||
|
||||
```
|
||||
IMM-01 (conductor: make keys, check Maine/partner facts)
|
||||
│ (must finish before IMM-04 merges)
|
||||
▼
|
||||
┌───────── Wave 1 ─────────┐
|
||||
│ IMM-02 server foundation │──┐
|
||||
│ IMM-03 summarize (pure) │──┤
|
||||
└──────────────────────────┘ │
|
||||
▼ ▼
|
||||
┌───────── Wave 2 ─────────────────────┐
|
||||
│ IMM-04 read tools (needs 02 + 03) │
|
||||
│ IMM-05 raggr wiring (needs 02) │
|
||||
└──────────────────────────────────────┘
|
||||
▼
|
||||
Wave 3: IMM-06 write tools (needs 04)
|
||||
▼
|
||||
Wave 4: IMM-07 docs + Claude Code (needs 05 + 06)
|
||||
▼
|
||||
Wave 5: IMM-08 live verification + rollout (conductor; needs everything deployed)
|
||||
```
|
||||
|
||||
| Order | Ticket | Owner | Depends on | Runs alongside | Merge order |
|
||||
|---|---|---|---|---|---|
|
||||
| 0 | IMM-01 Create scoped API keys, check Maine/partner facts (version already known: 3.3.1) | conductor | – | Wave 1 | (no PR; findings go in the wave-1 status commit) |
|
||||
| 1 | IMM-02 Server foundation: config, client, criteria, package skeleton | worker | – | IMM-03 | 1st |
|
||||
| 1 | IMM-03 Trip summaries: clustering, places, samples (pure functions) | worker | – | IMM-02 | 2nd (rebase on IMM-02) |
|
||||
| 2 | IMM-04 Phase 1 read tools in the MCP server | worker | 02, 03 | IMM-05 | 3rd |
|
||||
| 2 | IMM-05 Wire the server into raggr (admin agent, routing, UI, config) | worker | 02 | IMM-04 | 4th |
|
||||
| 3 | IMM-06 Phase 2 write tools behind `IMMICH_ENABLE_WRITES` | worker | 04 | – | 5th |
|
||||
| 4 | IMM-07 Usage docs, Claude Code registration, design follow-ups | worker | 05, 06 | – | 6th |
|
||||
| 5 | IMM-08 Live verification and rollout | conductor | all merged + deployed | – | – |
|
||||
|
||||
Why this order:
|
||||
|
||||
- **IMM-02 and IMM-03 don't share files.** IMM-03 works on plain dicts and never
|
||||
imports the client. IMM-03 must **not** create `immich_mcp/__init__.py`; IMM-02 owns
|
||||
it. Until then the package imports fine as a namespace package. Merge IMM-02 first,
|
||||
then rebase IMM-03.
|
||||
- **IMM-04 and IMM-05 don't share files.** IMM-04 only touches `immich_mcp/server.py` and
|
||||
its test. IMM-05 only touches raggr. Tool names are fixed by the design (§7), so
|
||||
IMM-05 can allowlist them before they exist. Its tests use stubs, as
|
||||
`test_homebox_mcp.py` does.
|
||||
- **IMM-06 waits for IMM-04** because both edit `immich_mcp/server.py`.
|
||||
- **IMM-07 comes last among the code tickets** so the docs describe what was merged,
|
||||
not what was planned.
|
||||
- **Phase 1 can deploy after IMM-05** with `IMMICH_ENABLE_WRITES` unset. Scenarios A,
|
||||
D, and the read halves of B and C work from then on.
|
||||
|
||||
---
|
||||
|
||||
## 3. Tickets
|
||||
|
||||
### [ ] IMM-01: Verify production Immich and create scoped API keys
|
||||
|
||||
- **Owner:** conductor (needs the Immich UI and an API key; no worker, no PR)
|
||||
- **Depends on:** nothing · **Blocks:** IMM-04 merge, IMM-08
|
||||
- **Why:** The API changed shape in v3.2.0 (§11), and the design has an open
|
||||
question (§13.2) that only the real server can answer.
|
||||
|
||||
**Already known (checked 2026-10-13):**
|
||||
|
||||
- `GET https://immich.ryanjchen.com/api/server/version` →
|
||||
`{"major":3,"minor":3,"patch":1}`. **Immich 3.3.1**, so the `filter` API style
|
||||
(nested `filter` plus `cursor` paging) is available and is the **default**. The legacy
|
||||
flat fields still work on 3.3.x, so keep both code paths and their golden tests.
|
||||
Production doesn't need `IMMICH_API_STYLE` set.
|
||||
- **Public URL:** `IMMICH_PUBLIC_URL=https://immich.ryanjchen.com`. Links in tool
|
||||
output use this. `IMMICH_URL` can stay on the LAN/compose address
|
||||
(`http://192.168.1.5:2283`) for the server's own traffic. If that address isn't
|
||||
reachable from the raggr container, use the public URL for both.
|
||||
|
||||
**Steps**
|
||||
|
||||
1. ~~Get the server version~~: done above (3.3.1). Re-run
|
||||
`curl -s https://immich.ryanjchen.com/api/server/version` right before IMM-08
|
||||
in case Immich was upgraded in between; a 4.x major means re-checking §11.
|
||||
2. In Immich → Account Settings → API Keys, create two keys for **Ryan's user**:
|
||||
- `simba-immich-read` with exactly `album.read`, `asset.read`,
|
||||
`asset.statistics`, `person.read`.
|
||||
- `simba-immich-write`: the same four plus `album.create` and `albumAsset.create`.
|
||||
Do **not** grant `all`, any `*.delete`, `album.update`, `album.share`,
|
||||
`asset.update` or `asset.share`.
|
||||
Store both in the password manager. Production `.env` starts with the **read**
|
||||
key (Phase 1).
|
||||
3. Using the read key, check the facts the design depends on (curl or httpie):
|
||||
- `GET /api/search/suggestions?type=state` contains `"Maine"` (confirms the
|
||||
reverse-geocoding spelling).
|
||||
- `POST /api/search/statistics` with `{"filter":{"state":{"eq":"Maine"}}}`
|
||||
returns a total, and the same total as legacy `{"state":"Maine"}`, which proves
|
||||
both paths agree on 3.3.1.
|
||||
- `GET /api/albums` returns albums including the existing Maine album. Record
|
||||
its name, `assetCount`, `startDate` and `endDate`; IMM-08 uses them.
|
||||
- **Partner photos (§13.2):** if a partner shares with Ryan with "show in
|
||||
timeline" on, check whether partner assets come back from
|
||||
`POST /api/search/metadata`. Write the answer down. Adding them to an album
|
||||
gets tested in IMM-08 with the write key.
|
||||
4. Check which `IMMICH_URL` the raggr container can reach:
|
||||
`docker compose exec raggr-<color> curl -s $IMMICH_URL/api/server/ping`.
|
||||
5. Put the results (partner answer, Maine album facts, chosen `IMMICH_URL`) in
|
||||
`<scratchpad>/imm-01-findings.md`, paste them into the IMM-04 and IMM-07
|
||||
plan files, and post them to the user. The conductor records them in the
|
||||
wave-1 status commit (sheepdog rule).
|
||||
|
||||
**Done when:** both keys exist with exactly the listed permissions; the Maine facts
|
||||
and the partner answer are recorded; the container-reachable `IMMICH_URL` is known.
|
||||
|
||||
---
|
||||
|
||||
### [ ] IMM-02: Server foundation (config, client, criteria, package skeleton)
|
||||
|
||||
- **Owner:** worker · **Branch:** `feat/immich-mcp-foundation` · **Base:** `origin/main`
|
||||
- **Depends on:** nothing · **Runs alongside:** IMM-03 · **Blocks:** IMM-03 merge, IMM-04, IMM-05
|
||||
- **PR title:** "Add the Immich MCP server package: config, API client and search criteria"
|
||||
- **Design:** §5, §6, §7.1, §8.5, §10, §11
|
||||
|
||||
**Goal.** Build everything the tools need except the tools themselves. When this
|
||||
ticket is done, `python -m immich_mcp` starts a FastMCP stdio server called
|
||||
`immich` with **no tools yet**, and the HTTP client and criteria model are fully
|
||||
tested.
|
||||
|
||||
**Files to create**
|
||||
|
||||
| File | Contents |
|
||||
|---|---|
|
||||
| `immich_mcp/__init__.py` | Module docstring only: what the package is, and that it runs as `python -m immich_mcp` over stdio. |
|
||||
| `immich_mcp/__main__.py` | `from immich_mcp.server import main` then `main()`. |
|
||||
| `immich_mcp/config.py` | Frozen `ImmichConfig` dataclass and `load_config(env=os.environ)`. |
|
||||
| `immich_mcp/client.py` | `ImmichClient` and `ImmichError`. |
|
||||
| `immich_mcp/criteria.py` | `PhotoCriteria`, `CriteriaError`, `local_day_bounds`, `to_search_body`, `to_statistics_body`, `resolve_place`. |
|
||||
| `immich_mcp/server.py` | `mcp = FastMCP("immich")`, `main()` that calls `mcp.run()` (stdio). No tools. |
|
||||
| `tests/unit/test_immich_config.py` | Config tests. |
|
||||
| `tests/unit/test_immich_client.py` | Client tests. |
|
||||
| `tests/unit/test_immich_criteria.py` | Criteria and body tests. |
|
||||
|
||||
**Files to modify**
|
||||
|
||||
- `Dockerfile`: add `COPY immich_mcp ./immich_mcp` after `COPY utils ./utils`
|
||||
(line 53). Without it the module is missing from the image; the Dockerfile copies
|
||||
directories explicitly.
|
||||
|
||||
**Do not change:** anything under `utils/`, `blueprints/` or `app.py`, or any existing test.
|
||||
|
||||
**Details**
|
||||
|
||||
1. **`config.py`**
|
||||
- Fields: `url` (from `IMMICH_URL`, trailing `/` stripped), `api_key`
|
||||
(`IMMICH_API_KEY`), `public_url` (`IMMICH_PUBLIC_URL`, default `url`),
|
||||
`timezone` (`ZoneInfo(IMMICH_TIMEZONE or TZ or "America/New_York")`),
|
||||
`writes_enabled` (`IMMICH_ENABLE_WRITES == "true"`), and `api_style`
|
||||
(`IMMICH_API_STYLE` in `{"legacy", "filter"}`, or `None` meaning auto-detect).
|
||||
- Missing `url` or `api_key` raises `ValueError` with a message that names the
|
||||
variable but **never prints the key**.
|
||||
- Constants live here: `MAX_RESULT_CHARS = 2500`, `MAX_SCAN = 5000`,
|
||||
`MAX_WRITE = 2000`, `PAGE_SIZE = 1000`, `WRITE_CHUNK = 500`.
|
||||
2. **`client.py`**
|
||||
- `ImmichClient(config, transport: httpx.AsyncBaseTransport | None = None)`
|
||||
wraps one `httpx.AsyncClient(base_url=f"{url}/api", headers={"x-api-key": key,
|
||||
"Accept": "application/json"}, timeout=30)`. Tests inject the transport. It is
|
||||
an async context manager.
|
||||
- `ALLOWED_ENDPOINTS: tuple[tuple[str, re.Pattern], ...]`, exactly:
|
||||
`GET /server/version`, `GET /albums`, `GET /albums/{uuid}`,
|
||||
`GET /search/suggestions`, `GET /search/person`, `POST /search/metadata`,
|
||||
`POST /search/smart`, `POST /search/statistics`, `POST /albums`,
|
||||
`PUT /albums/{uuid}/assets`. A private `_request(method, path, **kw)` raises
|
||||
`PermissionError` for anything else **before** sending. All public methods go
|
||||
through it.
|
||||
- Public methods (all `async`, returning parsed JSON or small dataclasses):
|
||||
`server_version() -> tuple[int, int, int]`;
|
||||
`list_albums(owned_only=False) -> list[dict]` (with no filter it returns owned
|
||||
and shared albums; `owned_only=True` sends `isOwned=true`);
|
||||
`get_album(album_id)`; `suggestions(kind, **filters) -> list[str]`;
|
||||
`search_person(name) -> list[dict]`;
|
||||
`search_metadata(body) -> dict` (one page); `search_smart(body) -> dict`;
|
||||
`statistics(body) -> int` (returns `total`);
|
||||
`create_album(name, description, asset_ids) -> dict`;
|
||||
`add_assets(album_id, asset_ids) -> list[dict]` (raw `BulkIdResponseDto` list).
|
||||
- `iter_metadata(body, *, limit) -> AsyncIterator[dict]` pages until `limit`
|
||||
assets or there are no more pages. It must support **both** paging styles: legacy
|
||||
(`page` numbers, `assets.nextPage`) and filter (`cursor`, `assets.nextCursor`).
|
||||
Choose based on whether the body has a `"filter"` key.
|
||||
- Errors: any non-2xx becomes `ImmichError(status, message)`, with `message`
|
||||
taken from Immich's JSON `message` field when present (truncated to 200 chars).
|
||||
A connection error becomes `ImmichError(0, "Immich is unreachable")`. Never put
|
||||
the API key in an exception or log line.
|
||||
- There are no `delete_*`, `update_*`, `remove_*` or `share_*` methods, and no
|
||||
generic `request()` method is public.
|
||||
3. **`criteria.py`**
|
||||
- `PhotoCriteria` (pydantic v2 `BaseModel`, `extra="forbid"`), fields exactly as
|
||||
in §7.1: `taken_after: date | None`, `taken_before: date | None`, `state`,
|
||||
`city`, `country`, `person: str | None`, `media: Literal["photo","video","all"] = "all"`,
|
||||
`favorites_only: bool = False`.
|
||||
- Validators: `taken_before >= taken_after` when both are set. Strip blank strings to
|
||||
`None`. `is_empty()` is true when no filter field is set (media `"all"` and
|
||||
favorites `False` count as unset).
|
||||
- `local_day_bounds(c, tz) -> (gte_utc | None, lt_utc | None)`:
|
||||
`taken_after` → local midnight → UTC. `taken_before` → **next day's** local
|
||||
midnight → UTC (exclusive). Return ISO strings with `Z`. Test across the DST
|
||||
changes on 2026-03-08 and 2026-11-01 in `America/New_York`.
|
||||
- `to_search_body(c, *, style, tz, person_ids=None, album_ids=None,
|
||||
exclude_album_id=None, size=PAGE_SIZE, with_exif=True) -> dict`:
|
||||
- `legacy`: `takenAfter`, `takenBefore` (use the exclusive bound minus 1 ms, because
|
||||
legacy `takenBefore` is inclusive), `state`, `city`, `country`, `personIds`,
|
||||
`albumIds`, `type` (`IMAGE`/`VIDEO`, omitted for `all`), `isFavorite`, `size`,
|
||||
`withExif`. Set only what's given; never send `null`. `exclude_album_id`
|
||||
is **not** supported in legacy mode: raise `ValueError` and let callers diff locally.
|
||||
- `filter`: `{"filter": {"takenAt": {"gte", "lt"}, "state": {"eq"}, "city": {"eq"},
|
||||
"country": {"eq"}, "personIds": {"all": [...]}, "albumIds": {"any": [...]},
|
||||
..."type": {"eq"}, "isFavorite": {"eq": true}}, "size", "withExif"}`.
|
||||
With `exclude_album_id`, use `"albumIds": {"none": [id]}`, and combine it
|
||||
correctly with `album_ids` (raise if both are given).
|
||||
- `to_statistics_body(...)`: same mapping without `size` or `withExif`.
|
||||
- `async resolve_place(client, kind, value) -> str`: case-insensitive exact match
|
||||
against `client.suggestions(kind)` returns Immich's spelling. If nothing matches,
|
||||
raise `CriteriaError("No <kind> called 'X'. Closest: A, B, C")` using
|
||||
`difflib.get_close_matches(value, options, n=3, cutoff=0.5)`.
|
||||
4. **`server.py`**: only the `FastMCP("immich")` instance, `main()`, and a module
|
||||
docstring that points at the design. IMM-04 adds the tools.
|
||||
|
||||
**Tests (each must fail without the code it covers)**
|
||||
|
||||
- Config: missing URL; missing key, with the message not containing the key value;
|
||||
`public_url` falls back to `url`; time zone falls back from `IMMICH_TIMEZONE` to `TZ`
|
||||
to the default; writes flag accepts only `"true"`; bad `IMMICH_API_STYLE` raises.
|
||||
- Client: the `x-api-key` header is sent; `DELETE` and `PATCH` raise
|
||||
`PermissionError` and the transport is **never called**; `/trash/empty` and
|
||||
`/assets` raise; `iter_metadata` stops at `limit` mid-page; it follows `nextPage`
|
||||
(legacy) and `nextCursor` (filter); a 400 with `{"message": "x"}` becomes
|
||||
`ImmichError(400, "x")`; a connect error becomes status 0; the key never appears
|
||||
in `str(error)`.
|
||||
- Criteria: golden-body tests for **both** styles for (a) a state only, (b) a date
|
||||
range only, (c) everything set; inclusive `taken_before` round-trips; DST bounds;
|
||||
empty criteria → `is_empty()`; reversed dates raise; `exclude_album_id` in legacy
|
||||
raises; `resolve_place` returns the canonical spelling, and gives three closest
|
||||
matches on a miss.
|
||||
- Smoke: `python -c "import immich_mcp.server"` imports, and `mcp.name == "immich"`.
|
||||
|
||||
**Acceptance**
|
||||
|
||||
- `.venv/bin/pytest tests/unit -q` and `uvx ruff==0.8.2 check .` are green.
|
||||
- `IMMICH_URL=http://x IMMICH_API_KEY=y .venv/bin/python -m immich_mcp` starts and
|
||||
waits on stdin (Ctrl-C to exit). With either var missing it exits non-zero with a
|
||||
clear message.
|
||||
- `grep -rn "delete\|DELETE\|patch\|PATCH" immich_mcp/` matches only the allowlist
|
||||
test helpers or comments.
|
||||
|
||||
---
|
||||
|
||||
### [ ] IMM-03: Trip summaries (clustering, places, samples)
|
||||
|
||||
- **Owner:** worker · **Branch:** `feat/immich-mcp-summaries` · **Base:** `origin/main`
|
||||
- **Depends on:** nothing · **Runs alongside:** IMM-02 · **Merge:** after IMM-02, rebased
|
||||
- **PR title:** "Summarize Immich search results into trip date clusters"
|
||||
- **Design:** §7.2 (output fields), §7.4 (clustering), §7 (size cap)
|
||||
|
||||
**Goal.** Pure, synchronous functions that turn a list of Immich asset dicts
|
||||
(shaped like `AssetResponseDto` with `exifInfo`) into the compact summary that
|
||||
`immich_search_photos` and `immich_album_gaps` return. They do no I/O and don't import
|
||||
`client.py` or `criteria.py`.
|
||||
|
||||
**Files to create**
|
||||
|
||||
- `immich_mcp/summarize.py`
|
||||
- `tests/unit/test_immich_summarize.py`
|
||||
- `tests/fixtures/immich/assets_maine_trip.json`: about 60 hand-built assets (see the tests below).
|
||||
|
||||
**Do not create** `immich_mcp/__init__.py`; IMM-02 owns it. Without it the package
|
||||
imports as a namespace package, which is fine for tests.
|
||||
|
||||
**Functions**
|
||||
|
||||
1. `asset_day(asset, tz) -> date | None`: the date part of `asset["localDateTime"]`
|
||||
parsed **as-is**, with no time-zone conversion (it is wall-clock time; §7.4). If
|
||||
that's missing, convert `fileCreatedAt` to `tz` and take the date. `None` if both are missing.
|
||||
2. `asset_place(asset) -> tuple[str | None, str | None]`: `(city, state)` from
|
||||
`exifInfo`, with empty strings treated as `None`.
|
||||
3. `cluster_by_day(assets, tz, gap_days=2, keep=8) -> list[dict]`: sort the distinct
|
||||
days, and start a new cluster when `(day - prev_day).days > gap_days`. Each cluster is
|
||||
`{"start","end","count","top_place"}`, where `top_place` is the most common
|
||||
`"City, State"` (or just the state, or `null`). Sort by `count` descending, then
|
||||
`start` descending; keep `keep`.
|
||||
4. `top_places(assets, n=5) -> list[{"place","count"}]`, plus a `no_location` count
|
||||
(returned as a tuple).
|
||||
5. `media_counts(assets) -> {"photo": n, "video": m}` (`type` is `IMAGE`/`VIDEO`).
|
||||
6. `pick_samples(assets, n, public_url, tz) -> list[dict]`: `n` evenly spaced assets
|
||||
after sorting by day, each `{"date","place","file","url"}` with
|
||||
`url = f"{public_url}/photos/{id}"`. `n=0` → `[]`.
|
||||
7. `summarize(assets, *, total, tz, public_url, samples=5, truncated=False) -> dict`
|
||||
with the keys `total, scanned, truncated, first, last, clusters, top_places,
|
||||
no_location, by_media, samples`. `criteria` is **not** included here; the tool adds it.
|
||||
8. `fit(result: dict, max_chars=2500) -> dict`: if `json.dumps(result,
|
||||
separators=(",",":"), ensure_ascii=False)` is over `max_chars`, trim lists in this
|
||||
order until it fits: `samples` (one at a time down to 0), `top_places` (down to 1),
|
||||
`clusters` (down to 1). Then set `"trimmed": true`. Never trim scalars. Return a
|
||||
new dict and don't mutate the input.
|
||||
|
||||
**Tests**
|
||||
|
||||
- The fixture has a trip Aug 2–9, 2026 (with **no photos on Aug 5**, to show a quiet
|
||||
day doesn't split the trip), a single day in 2023, and one asset with neither
|
||||
`localDateTime` nor `exifInfo`. Then `cluster_by_day` returns exactly two clusters,
|
||||
with the trip first: `start=2026-08-02`, `end=2026-08-09`.
|
||||
- `gap_days=2` boundary: days 1, 4 → two clusters; days 1, 3 → one.
|
||||
- `localDateTime` `"2026-08-09T23:30:00.000Z"` stays Aug 9 under
|
||||
`tz=America/Los_Angeles` and `tz=Asia/Tokyo` (no conversion).
|
||||
- The `fileCreatedAt` fallback converts: `2026-08-10T02:00:00Z` with New York → Aug 9.
|
||||
- `pick_samples` spreads samples evenly (first and last included when `n >= 2`) and
|
||||
builds correct URLs.
|
||||
- `fit` on a large synthetic result is at most 2500 chars, sets `trimmed`, trims samples
|
||||
before places before clusters, and leaves its input unchanged.
|
||||
- Empty input → `total` passes through, `first`/`last` are `null`, and lists are empty.
|
||||
|
||||
**Acceptance:** tests and ruff green; `summarize.py` imports only the standard library.
|
||||
|
||||
---
|
||||
|
||||
### [ ] IMM-04: Phase 1 read tools in the MCP server
|
||||
|
||||
- **Owner:** worker · **Branch:** `feat/immich-mcp-read-tools` · **Base:** `origin/main`
|
||||
**after IMM-02 and IMM-03 are merged**
|
||||
- **Depends on:** IMM-02, IMM-03 (production is 3.3.1, so `filter` is the style exercised live; both styles must still be tested)
|
||||
- **Runs alongside:** IMM-05 · **Blocks:** IMM-06
|
||||
- **PR title:** "Add read-only Immich tools: find albums, search photos, smart search, album gaps"
|
||||
- **Design:** §3 (scenarios A, B-read, C-read, D), §7, §11
|
||||
|
||||
**Goal.** Register the four Phase 1 tools on the FastMCP server, with exactly the
|
||||
names, parameters and outputs in §7.2. The server must be stateless: every call
|
||||
builds its own `ImmichClient`, because raggr starts a new process per call (§5).
|
||||
|
||||
**Files to modify:** `immich_mcp/server.py`.
|
||||
**Files to create:** `immich_mcp/tools_read.py` (the tool bodies, kept out of `server.py`
|
||||
so IMM-06 can add `tools_write.py` without conflicts) and `tests/unit/test_immich_server.py`.
|
||||
**Do not change:** `client.py`, `criteria.py` and `summarize.py`, except for bug
|
||||
fixes, which must be listed in the report.
|
||||
|
||||
**Details**
|
||||
|
||||
1. **A shared call context** `async with call_context() as ctx:` loads config,
|
||||
opens the client, and works out `ctx.style`: `config.api_style` if set, otherwise
|
||||
`filter` when `server_version() >= (3, 2, 0)` and `legacy` below that. It catches
|
||||
`ImmichError`, `CriteriaError` and `ValueError` and turns them into
|
||||
`{"error": "<sentence>"}`. Tools never raise to the MCP layer.
|
||||
2. **Tool names are registered without a prefix**: `find_albums`,
|
||||
`search_photos`, `smart_search`, `album_gaps` (and in IMM-06 `create_album`,
|
||||
`add_to_album`). raggr's `select_tools` adds `immich_`, as it does for Homebox,
|
||||
so Simba sees the `immich_*` names from design §7. Claude Code shows them as
|
||||
`mcp__immich__find_albums`. Descriptions that mention another tool use the
|
||||
raggr name (`immich_add_to_album`).
|
||||
3. **The criteria parameters are flat** on every tool that takes them (§7.1): the
|
||||
tool function signatures declare `taken_after: str | None = None` (ISO date
|
||||
string), `taken_before`, `state`, `city`, `country`, `person`, `media`,
|
||||
`favorites_only` directly. Build `PhotoCriteria` inside. **Test that the
|
||||
generated input schema has no `$defs` or `$ref`.**
|
||||
4. **`normalize(ctx, criteria) -> (criteria, person_ids)`** resolves `state`, `city`
|
||||
and `country` through `resolve_place`, and `person` through `search_person`:
|
||||
exactly one case-insensitive name match is required, otherwise an error listing
|
||||
up to 5 candidate names.
|
||||
5. **`find_albums(query="", limit=10)`**: `list_albums()`, then rank: exact
|
||||
(case-insensitive) match first, then substring matches, then
|
||||
`difflib.SequenceMatcher` ratio ≥ 0.6, ties broken by `updatedAt` descending.
|
||||
An empty query returns the newest `limit` by `updatedAt`. Output is
|
||||
`{"albums": [{"id","name","asset_count","start","end","shared","owned","url"}]}`.
|
||||
`owned` comes from a second call, `GET /albums?isOwned=true` (add an
|
||||
`owned_only: bool = False` parameter to `client.list_albums`): an album is owned
|
||||
if its ID is in that set. This needs no extra API-key permission. `start`/`end`
|
||||
are converted to local dates in `config.timezone`. `limit` is clamped to 1–25.
|
||||
6. **`search_photos(<criteria>, samples=5)`**: reject empty criteria with
|
||||
`{"error": "Give at least a date range, place, or person."}`. Then
|
||||
`statistics()` for the total, `iter_metadata(limit=MAX_SCAN)`, and
|
||||
`fit(summarize(...) | {"criteria": <normalized flat criteria dict, dates as ISO strings>})`.
|
||||
7. **`smart_search(query, <criteria>, limit=10)`**: requires `query`; the
|
||||
criteria are optional. `search_smart` with the body from `to_search_body` plus
|
||||
`query`, and `size=limit` (clamped 1–25). Output is
|
||||
`{"results": [{"date","place","file","url"}]}` in rank order, passed through `fit`.
|
||||
8. **`album_gaps(album_id, pad_days=1)`**: `get_album`. With no
|
||||
`startDate` → error. The window is `[start - pad, end + pad]` in local dates.
|
||||
Then a **shared helper** `resolve_missing(ctx, album_id, criteria, limit)` that
|
||||
IMM-06 will reuse:
|
||||
- `filter` style: one search with `exclude_album_id=album_id`.
|
||||
- `legacy` style: collect the album's asset IDs (`iter_metadata` with
|
||||
`album_ids=[id]`, no limit up to `MAX_WRITE * 5`), then the window's assets,
|
||||
minus that set.
|
||||
- Returns `(missing_assets, truncated)`.
|
||||
Output: `summarize(missing)` plus `"album": {"id","name","url"}`,
|
||||
`"missing_count"`, and `"criteria"` (the window).
|
||||
9. **Tool descriptions** are one or two sentences each and at most 300 chars, written for the
|
||||
model, for example: *"Find photo albums by (partial) name. Call this first when the user
|
||||
mentions a trip or event."* The total schema size across all four tools, measured as
|
||||
`sum(len(json.dumps(t.inputSchema)) + len(t.description))`, must be **at most 4000
|
||||
chars** (about 1000 tokens), which leaves room for the two Phase 2 tools within the
|
||||
~1500-token budget in §2.6.
|
||||
|
||||
**Tests (`test_immich_server.py`)**
|
||||
|
||||
Use `mcp.shared.memory.create_connected_server_and_client_session(server._mcp_server)`
|
||||
and inject a fake Immich through a module-level hook such as
|
||||
`server.TRANSPORT_FACTORY = lambda: httpx.MockTransport(handler)`. Cover:
|
||||
|
||||
- `list_tools()` returns exactly the four names, with no `$ref` in any schema, within
|
||||
the schema budget.
|
||||
- Scenario A: albums "Maine 2026" and "Acadia / Maine" are both found for "maine",
|
||||
with the exact match ranked first. An empty query returns the newest first.
|
||||
- Scenario B (read): state "maine" resolves to "Maine"; the summary has two clusters;
|
||||
`criteria.state == "Maine"`; the total comes from statistics, not from the scan length.
|
||||
- A misspelled state ("Main") → an error with "Closest: Maine".
|
||||
- An ambiguous person → an error listing the candidates.
|
||||
- Empty criteria → the error, and **no HTTP call** is made beyond what's needed for setup.
|
||||
- Scenario C (read), in **both** styles: the missing photos are the window minus the
|
||||
album, and `missing_count` matches.
|
||||
- Scenario D: smart search passes `query`, clamps `limit` to 25, and keeps rank order.
|
||||
- Version detection: 3.1.9 → legacy bodies; 3.2.0 → filter bodies; the
|
||||
`IMMICH_API_STYLE` override skips the version call.
|
||||
- An Immich 500 or a connection error → `{"error": ...}`, not an exception.
|
||||
- Every tool result is at most 2500 chars, even with a 5000-asset fixture.
|
||||
|
||||
**Acceptance:** tests and ruff green. Manually, `npx @modelcontextprotocol/inspector
|
||||
.venv/bin/python -m immich_mcp` (with real env, optional) lists four tools. Report the
|
||||
measured per-call process start time: `time` around one `call_tool` through
|
||||
`langchain_mcp_adapters` with a fake URL is fine.
|
||||
|
||||
---
|
||||
|
||||
### [ ] IMM-05: Wire the server into raggr (admin agent, routing, UI, config)
|
||||
|
||||
- **Owner:** worker · **Branch:** `feat/immich-mcp-raggr` · **Base:** `origin/main`
|
||||
after IMM-02 merged
|
||||
- **Depends on:** IMM-02 · **Runs alongside:** IMM-04 · **Merge:** after IMM-04
|
||||
- **PR title:** "Load the Immich MCP tools into Simba's admin agent"
|
||||
- **Design:** §6 (raggr-side list), §8.2, §9, §10
|
||||
|
||||
**Goal.** raggr starts `python -m immich_mcp` the way it starts Homebox and
|
||||
BookStack: admin-only, prefixed `immich_`, allowlisted, routed, labelled and
|
||||
configured. Nothing user-visible happens unless `IMMICH_URL` and `IMMICH_API_KEY` are set.
|
||||
|
||||
**Files and exact changes**
|
||||
|
||||
1. `utils/mcp_tools.py`: `load_stdio_tools` (line 83) gains keyword-only
|
||||
`args: list[str] | None = None` and `cwd: str | None = None`. Pass
|
||||
`"args": args or []` (line 100) and, only when given, `"cwd": cwd`. Homebox and
|
||||
BookStack callers stay unchanged. Add a test that the connection dict gets `args`/`cwd`.
|
||||
2. **New** `utils/immich_mcp.py`, mirroring `utils/homebox_mcp.py`:
|
||||
- `IMMICH_READ_TOOLS = ("find_albums", "search_photos", "smart_search",
|
||||
"album_gaps")` and `IMMICH_WRITE_TOOLS = ("create_album", "add_to_album")`.
|
||||
The server registers these **unprefixed** names (IMM-04 detail 2) and
|
||||
`select_tools` adds the `immich_` prefix, like Homebox.
|
||||
- `immich_allowlist() -> tuple`: read tools, plus write tools when
|
||||
`IMMICH_ENABLE_WRITES == "true"`. This keeps `select_tools` from warning about
|
||||
"missing tools" when writes are off.
|
||||
- `load_immich_tools()`: returns `[]` with an info log if `IMMICH_URL` or
|
||||
`IMMICH_API_KEY` is unset. Otherwise
|
||||
`load_stdio_tools(server="immich", command=os.getenv("IMMICH_MCP_COMMAND", sys.executable),
|
||||
args=[] if os.getenv("IMMICH_MCP_COMMAND") else ["-m", "immich_mcp"],
|
||||
cwd=str(Path(__file__).resolve().parents[1]), env={IMMICH_URL, IMMICH_API_KEY,
|
||||
IMMICH_PUBLIC_URL, IMMICH_TIMEZONE, IMMICH_ENABLE_WRITES, IMMICH_API_STYLE — only
|
||||
the ones that are set}, allowlist=immich_allowlist(), prefix="immich", label="Immich")`.
|
||||
3. `app.py` line 134: `admin_tools = await load_homebox_tools() + await
|
||||
load_bookstack_tools() + await load_immich_tools()`, plus the import next to
|
||||
lines 45–46.
|
||||
4. `blueprints/conversation/tool_routing.py`:
|
||||
- `GROUP_TERMS["photos"] = "immich photo photos picture pictures pic pics album
|
||||
albums trip trips vacation"`.
|
||||
- `tool_group`: add `("immich_", "photos")` to the prefix list.
|
||||
- `ToolRouter.__init__` (line 87): change `startswith(("homebox", "bookstack"))` to
|
||||
`startswith(("homebox", "bookstack", "immich"))`.
|
||||
- Check that adding "album" or "trip" doesn't change any existing routing test.
|
||||
("page"/"pages" belong to `knowledge`; there's no overlap.)
|
||||
5. `blueprints/conversation/tool_labels.py` line 50: add
|
||||
`("immich_", "Looking through photos…")` to `TOOL_PREFIX_LABELS`.
|
||||
6. `raggr-frontend/src/components/ActivityTimeline.tsx`: import `Images` from
|
||||
`lucide-react` (line 23 import block), and add `["immich", Images]` to
|
||||
`TOOL_ICONS` (after `bookstack`).
|
||||
7. `blueprints/conversation/prompts.py` line 39: insert the §9 sentence right after
|
||||
the BookStack sentence, word for word. Keep `test_prompts.py` green; if it pins exact text,
|
||||
update the expectation in the same PR.
|
||||
8. `blueprints/integrations/__init__.py` line 92 admin list: add
|
||||
`("Immich", ("IMMICH_URL", "IMMICH_API_KEY"))` after BookStack.
|
||||
9. `config/validation.py`: add `"IMMICH_URL"` and `"IMMICH_PUBLIC_URL"` to the
|
||||
URL list (line 168), `"IMMICH_ENABLE_WRITES"` to the true/false list (line 159),
|
||||
and validate `IMMICH_API_STYLE ∈ {legacy, filter}` when set.
|
||||
10. `.env.example` (lines 66–70, the existing Immich block): keep `IMMICH_URL` and
|
||||
`IMMICH_API_KEY`, and add commented `IMMICH_PUBLIC_URL=`, `IMMICH_TIMEZONE=`,
|
||||
`IMMICH_ENABLE_WRITES=false`, `IMMICH_API_STYLE=`, with a one-line comment
|
||||
on the scoped-key permissions (§8.6). Keep `SEARCH_QUERY`/`DOWNLOAD_DIR` for the indexer.
|
||||
11. `docker-compose.yml` `x-raggr-common` environment block (near
|
||||
`HOMEBOX_URL` at line 91): `IMMICH_URL`, `IMMICH_API_KEY`, `IMMICH_PUBLIC_URL`,
|
||||
`IMMICH_TIMEZONE`, `IMMICH_ENABLE_WRITES`, `IMMICH_API_STYLE`, written as
|
||||
`- NAME=${NAME}` (or `${NAME:-}` to match neighbours).
|
||||
|
||||
**Tests**
|
||||
|
||||
- **New** `tests/unit/test_immich_mcp.py`, modelled on `test_homebox_mcp.py`: allowlist
|
||||
filtering and prefixing; write tools absent unless the flag is on; the admin gate
|
||||
refuses non-admins without calling the server; missing env → `[]` with no subprocess;
|
||||
the env passed to `load_stdio_tools` contains only `IMMICH_*` keys (patch it and inspect);
|
||||
default command is `sys.executable` with args `-m immich_mcp` and `cwd` is the repo root;
|
||||
`IMMICH_MCP_COMMAND` override → `args == []`.
|
||||
- `test_tool_routing.py`: "find my maine trip photos" routes to `photos` for admins;
|
||||
`immich_*` tools are hidden from non-admins in both static and dynamic modes.
|
||||
- `test_configuration.py`: a bad `IMMICH_URL` and a bad `IMMICH_ENABLE_WRITES` are
|
||||
rejected; an empty Immich config is fine (optional integration).
|
||||
- `tool_labels`: `tool_label("immich_find_albums") == "Looking through photos…"`.
|
||||
- Integration status: admins see an `Immich` row, non-admins don't.
|
||||
|
||||
**Acceptance:** backend tests, ruff, `yarn tsc --noEmit` and `yarn build` all green.
|
||||
With `IMMICH_*` unset the app boots exactly as before (log line "Immich MCP not
|
||||
configured").
|
||||
|
||||
---
|
||||
|
||||
### [ ] IMM-06: Phase 2 write tools behind `IMMICH_ENABLE_WRITES`
|
||||
|
||||
- **Owner:** worker · **Branch:** `feat/immich-mcp-write-tools` · **Base:** `origin/main`
|
||||
after IMM-04 merged (IMM-05 merged too, ideally)
|
||||
- **Depends on:** IMM-04 · **Blocks:** IMM-07
|
||||
- **PR title:** "Let Simba create Immich albums and add photos after confirmation"
|
||||
- **Design:** §3 (B and C end to end), §7.3, §8 (every layer)
|
||||
|
||||
**Goal.** Add `create_album` and `add_to_album` (prefixed to `immich_*` by raggr)
|
||||
with **every** write guard in §8. Nothing in this ticket can delete or remove anything.
|
||||
|
||||
**Files to create:** `immich_mcp/tools_write.py`, `tests/unit/test_immich_write_tools.py`.
|
||||
**Files to modify:** `immich_mcp/server.py` (register the write tools **only if**
|
||||
`load_config().writes_enabled` is true at startup), and `immich_mcp/tools_read.py` only if
|
||||
`resolve_missing` needs a small signature change (say so in the report).
|
||||
|
||||
**Details**
|
||||
|
||||
1. **`create_album(name, expected_count, <criteria>, description="")`**
|
||||
1. Strip `name`; if it's empty, error.
|
||||
2. `list_albums()`. If any album name matches case-insensitively → `{"error":
|
||||
"An album named 'X' already exists (N photos). Use immich_add_to_album to add to it.",
|
||||
"album": {id, name, url}}`.
|
||||
3. Normalize the criteria (reuse `normalize`). Empty criteria → the error from IMM-04.
|
||||
4. `total = statistics(...)`. If `total > MAX_WRITE` → error asking the user to narrow
|
||||
the range. Then collect **all** IDs with `iter_metadata(limit=MAX_WRITE)`. Request
|
||||
only IDs where the API allows; otherwise take `id` from each item.
|
||||
5. If `len(ids) != expected_count` → `{"error": "The search now matches N photos,
|
||||
not the M you confirmed. Show the user the new count and ask again.",
|
||||
"actual_count": N}`, and make **zero** write calls.
|
||||
6. `create_album(name, description, ids[:500])`, then `add_assets` for the rest in
|
||||
chunks of 500.
|
||||
7. Return `{"album": {"id","name","url"}, "added": n}`. If chunk N fails partway,
|
||||
return what was added so far plus `"error"`. Don't retry; the album exists and
|
||||
the user can run `add_to_album`.
|
||||
2. **`add_to_album(album_id, expected_new_count, <criteria>)`**
|
||||
1. Normalize the criteria, which can't be empty. `get_album` must succeed. If the
|
||||
album isn't in `list_albums(owned_only=True)`, error before resolving anything
|
||||
(Immich might reject it anyway, but this message is clearer).
|
||||
2. `missing, truncated = resolve_missing(ctx, album_id, criteria, limit=MAX_WRITE)`.
|
||||
If `truncated` → the too-many error.
|
||||
3. If `len(missing) != expected_new_count` → the count-mismatch error, with no writes.
|
||||
4. `add_assets` in chunks of 500. Tally `success`, `error == "duplicate"`
|
||||
(→ `already_present`), and other errors (→ `failed`, keeping at most 5
|
||||
`{"id","reason"}` items).
|
||||
5. Return `{"album": {...}, "added", "already_present", "failed"}`.
|
||||
3. **Descriptions** must say plainly: *"Only call after the user has said yes to the
|
||||
exact count you showed them. Pass that count as expected_count."* Keep both within
|
||||
the remaining schema budget (§2.6; all six tools together are at most about 6000 chars).
|
||||
4. **Registration.** With the flag off, `list_tools()` has exactly the four read tools.
|
||||
With it on, six.
|
||||
|
||||
**Tests (`test_immich_write_tools.py`, in-memory session + MockTransport)**
|
||||
|
||||
- Flag off → no write tools listed, and calling one → MCP "unknown tool" error.
|
||||
- Scenario B end to end: search → create with the matching count → one `POST /albums`
|
||||
with 455 IDs split 500/… (use 1,200 IDs to check chunking: one POST + two PUTs).
|
||||
- Count mismatch → no `POST`/`PUT` (assert on the recorded requests).
|
||||
- Duplicate name (case-insensitive) → error, and **no** search or write calls after `list_albums`.
|
||||
- `total > MAX_WRITE` → error, no writes.
|
||||
- Scenario C end to end in both API styles: gaps → add with `missing_count` →
|
||||
the PUT body has exactly the missing IDs.
|
||||
- Results with `duplicate` and `no_permission` are tallied correctly.
|
||||
- A partial failure on the second PUT → `added` reflects the first chunk, plus `error`.
|
||||
- The recorded requests across the **whole test module** contain no `DELETE` or `PATCH`
|
||||
(a module-scoped fixture asserts this at teardown).
|
||||
|
||||
**Acceptance:** tests and ruff green. The report shows a sample request log from the
|
||||
scenario B test. The conductor checks every §8 layer against the code during review and
|
||||
lists them in the PR body.
|
||||
|
||||
---
|
||||
|
||||
### [ ] IMM-07: Usage docs, Claude Code registration, design follow-ups
|
||||
|
||||
- **Owner:** worker · **Branch:** `docs/immich-mcp-usage` · **Base:** `origin/main`
|
||||
after IMM-05 and IMM-06 are merged
|
||||
- **Depends on:** IMM-05, IMM-06 (and the IMM-01 findings)
|
||||
- **PR title:** "Document the Immich MCP tools and how to use them from Claude Code"
|
||||
|
||||
**Files**
|
||||
|
||||
- **New** `docs/immich-mcp/usage.md`:
|
||||
1. What Simba can do with photos, using the four scenarios from design §3 as example
|
||||
prompts.
|
||||
2. Setup: create the scoped key (exact permission lists from IMM-01), the env vars
|
||||
table (copied from design §10, with the real defaults as merged), and how to turn
|
||||
writes on.
|
||||
3. **Claude Code:** the `claude mcp add immich …` command from design §10, using the
|
||||
real paths. Say that Claude Code keeps one long-lived process (unlike raggr's
|
||||
per-call processes); nothing in the server depends on either.
|
||||
4. Troubleshooting: "Immich MCP not configured" log; `{"error": "Immich is unreachable"}`;
|
||||
count-mismatch errors (expected: the library changed); a 403 on writes means the key
|
||||
lacks `album.create`/`albumAsset.create`; `IMMICH_API_STYLE` override.
|
||||
5. The safety model in five bullets (design §8, short form).
|
||||
- `docs/immich-mcp/design.md`: set the status to "implemented (Phase 1 + 2)"; in §13,
|
||||
fill in the answers from IMM-01 (version, partner photos); note any departures from
|
||||
the design that IMM-02–06 made (taken from their PR bodies). At minimum, add to §7
|
||||
that the server registers unprefixed names and raggr adds `immich_` (IMM-04 detail 2),
|
||||
and add to §7.2 that `owned` comes from `GET /albums?isOwned=true`.
|
||||
- `mkdocs.yml`: add `Immich MCP (usage): immich-mcp/usage.md` next to the design entry.
|
||||
- `CLAUDE.md`: under `utils/`, add `immich_mcp.py` (the loader); under the architecture
|
||||
list, add a short **`immich_mcp/`** entry (in-repo stdio MCP server for Immich albums
|
||||
and photo search; admin-only in raggr; also usable from Claude Code; see
|
||||
`docs/immich-mcp/`). Add `IMMICH_*` to the household-integrations env line.
|
||||
- Do **not** edit `docs/immich-mcp/tickets.md`. The conductor ticks it (sheepdog rule).
|
||||
|
||||
**Acceptance:** `mkdocs build --strict` passes, if mkdocs is available locally (otherwise
|
||||
check the links by hand). Every command in `usage.md` was actually run once by the worker
|
||||
or the conductor, except the ones that need production.
|
||||
|
||||
---
|
||||
|
||||
### [ ] IMM-08: Live verification and rollout
|
||||
|
||||
- **Owner:** conductor (needs production, the user's Immich, and judgment calls)
|
||||
- **Depends on:** IMM-02 to IMM-07 merged; `make redeploy` done
|
||||
- **Design:** §3, §14
|
||||
|
||||
**Phase 1 (read key, `IMMICH_ENABLE_WRITES` unset)**
|
||||
|
||||
1. Put `IMMICH_URL`, the read `IMMICH_API_KEY`, `IMMICH_PUBLIC_URL` and, if IMM-01
|
||||
decided it, `IMMICH_API_STYLE` in production `.env`. Run `make redeploy`. `make logs`
|
||||
should show "Immich MCP: loaded 4 tools".
|
||||
2. In Simba's web chat **as an admin**, run scenarios A, B (stop at "Want an album?"),
|
||||
C (stop at "Add them?") and D from design §3. For each one, record the tool calls
|
||||
from the activity timeline, the answer, the links (they must open in a browser), and
|
||||
rough latency.
|
||||
3. As a **non-admin** user, ask "find my maine photos". The answer must not use
|
||||
`immich_*` tools (check the timeline), and the integration status page has no Immich row.
|
||||
4. If a result is wrong (bad clusters, a missed album, a link to the wrong host), file a
|
||||
follow-up ticket here (IMM-09+) with the evidence. Don't patch production by hand.
|
||||
|
||||
**Phase 2 (write key)**
|
||||
|
||||
5. Swap in the write key and set `IMMICH_ENABLE_WRITES=true`, then `make redeploy`.
|
||||
Expect "loaded 6 tools".
|
||||
6. Scenario B end to end with the album name **"ZZ Simba test – delete me"** over a
|
||||
small, known date range (one day). Check the album in Immich has exactly the confirmed
|
||||
count.
|
||||
7. Count-mismatch drill: get a count, then upload one photo from that day on a phone,
|
||||
then confirm with the old count. Simba must refuse and re-show the new count.
|
||||
8. Scenario C end to end on the test album: remove a few photos from it in the Immich UI,
|
||||
ask what's missing, and confirm the add.
|
||||
9. Partner photos (design §13.2): if partner sharing exists, include a partner photo's
|
||||
day and record whether the add works or comes back in `failed`.
|
||||
10. Delete the test album **by hand in the Immich UI**. Simba can't.
|
||||
|
||||
**Claude Code**
|
||||
|
||||
11. Register the server on the laptop with the `usage.md` command. In a fresh Claude Code
|
||||
session, ask "do I have a Maine album?" and check it answers with the link.
|
||||
|
||||
**Done when:** every step has recorded evidence (paste a short table into the IMM-08 PR
|
||||
or a comment on the design PR); any follow-ups are filed as IMM-09+ in this file; the
|
||||
production `.env` matches the final intended state (the write key only if the user wants
|
||||
writes left on; ask them).
|
||||
@@ -0,0 +1,38 @@
|
||||
"""Create scheduled_messages, which no committed migration ever created.
|
||||
|
||||
Migrations 6–10 were generated outside the repository and never committed, and
|
||||
one of them created this table. Deployments that ran them already have it, so
|
||||
this is IF NOT EXISTS and a no-op there; fresh databases need it before
|
||||
migration 20 alters the table. Aerich cannot generate this file because its
|
||||
saved model state already includes the table: the DDL below is Tortoise's
|
||||
schema output for the model as of migration 19.
|
||||
"""
|
||||
|
||||
from tortoise import BaseDBAsyncClient
|
||||
|
||||
RUN_IN_TRANSACTION = True
|
||||
|
||||
|
||||
async def upgrade(db: BaseDBAsyncClient) -> str:
|
||||
return """
|
||||
CREATE TABLE IF NOT EXISTS "scheduled_messages" (
|
||||
"id" UUID NOT NULL PRIMARY KEY,
|
||||
"recipient" VARCHAR(255) NOT NULL,
|
||||
"channel" VARCHAR(20) NOT NULL,
|
||||
"content" TEXT NOT NULL,
|
||||
"subject" VARCHAR(255),
|
||||
"scheduled_at" TIMESTAMPTZ NOT NULL,
|
||||
"status" VARCHAR(20) NOT NULL DEFAULT 'pending',
|
||||
"recurrence" VARCHAR(20) NOT NULL DEFAULT 'none',
|
||||
"use_agent" BOOL NOT NULL DEFAULT False,
|
||||
"error_message" TEXT,
|
||||
"created_at" TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP,
|
||||
"updated_at" TIMESTAMPTZ NOT NULL DEFAULT CURRENT_TIMESTAMP,
|
||||
"created_by_id" UUID NOT NULL REFERENCES "users" ("id") ON DELETE CASCADE
|
||||
);"""
|
||||
|
||||
|
||||
async def downgrade(db: BaseDBAsyncClient) -> str:
|
||||
# Existing deployments created this table in an uncommitted migration;
|
||||
# never drop it from under them.
|
||||
return ""
|
||||
@@ -25,3 +25,4 @@ nav:
|
||||
- Authentication: authentication.md
|
||||
- Desktop Client (proposal): desktop-client.md
|
||||
- Cowork UI plan: cowork-ui-plan.md
|
||||
- Immich MCP (design): immich-mcp/design.md
|
||||
|
||||
@@ -7,8 +7,11 @@ import sys
|
||||
|
||||
from blueprints.rag.logic import (
|
||||
delete_all_documents,
|
||||
delete_documents_by_metadata,
|
||||
fetch_documents_from_paperless_ngx,
|
||||
get_vector_store_stats,
|
||||
index_documents,
|
||||
index_obsidian_documents,
|
||||
list_all_documents,
|
||||
)
|
||||
|
||||
@@ -31,11 +34,35 @@ async def index():
|
||||
|
||||
|
||||
async def reindex():
|
||||
"""Clear and reindex all documents."""
|
||||
"""Clear and reindex everything: Paperless documents and the Obsidian vault.
|
||||
|
||||
Paperless is fetched before anything is deleted, so an unreachable or
|
||||
empty Paperless leaves the existing index untouched.
|
||||
"""
|
||||
print("Fetching documents from Paperless-NGX...")
|
||||
documents = await fetch_documents_from_paperless_ngx()
|
||||
if not documents:
|
||||
raise RuntimeError(
|
||||
"Paperless returned no documents to index; leaving the index untouched"
|
||||
)
|
||||
print(f"Fetched {len(documents)} documents")
|
||||
print("Clearing existing documents...")
|
||||
delete_all_documents()
|
||||
print("Cleared")
|
||||
await index()
|
||||
await index_documents(documents)
|
||||
print("Paperless indexing complete!")
|
||||
await reindex_obsidian(clear=False)
|
||||
|
||||
|
||||
async def reindex_obsidian(clear: bool = True):
|
||||
"""Re-embed the Obsidian vault, leaving Paperless documents alone."""
|
||||
if clear:
|
||||
print("Clearing Obsidian notes...")
|
||||
delete_documents_by_metadata("source", "obsidian")
|
||||
print("Indexing Obsidian vault...")
|
||||
await index_obsidian_documents()
|
||||
print("Obsidian indexing complete!")
|
||||
stats()
|
||||
|
||||
|
||||
def list_docs(limit: int = 10, show_content: bool = False):
|
||||
@@ -60,7 +87,8 @@ def main():
|
||||
Examples:
|
||||
%(prog)s stats # Show vector store statistics
|
||||
%(prog)s index # Index new documents from Paperless-NGX
|
||||
%(prog)s reindex # Clear and reindex all documents
|
||||
%(prog)s reindex # Clear and reindex Paperless + Obsidian
|
||||
%(prog)s reindex-obsidian # Clear and reindex only Obsidian notes
|
||||
%(prog)s list 10 # List first 10 documents
|
||||
%(prog)s list 20 --show-content # List 20 documents with content preview
|
||||
""",
|
||||
@@ -75,7 +103,12 @@ Examples:
|
||||
subparsers.add_parser("index", help="Index documents from Paperless-NGX")
|
||||
|
||||
# Reindex command
|
||||
subparsers.add_parser("reindex", help="Clear and reindex all documents")
|
||||
subparsers.add_parser(
|
||||
"reindex", help="Clear and reindex Paperless documents and Obsidian notes"
|
||||
)
|
||||
subparsers.add_parser(
|
||||
"reindex-obsidian", help="Clear and reindex only Obsidian notes"
|
||||
)
|
||||
|
||||
# List command
|
||||
list_parser = subparsers.add_parser("list", help="List documents in vector store")
|
||||
@@ -99,6 +132,8 @@ Examples:
|
||||
asyncio.run(index())
|
||||
elif args.command == "reindex":
|
||||
asyncio.run(reindex())
|
||||
elif args.command == "reindex-obsidian":
|
||||
asyncio.run(reindex_obsidian())
|
||||
elif args.command == "list":
|
||||
list_docs(limit=args.limit, show_content=args.show_content)
|
||||
except KeyboardInterrupt:
|
||||
|
||||
+28
-19
@@ -19,27 +19,36 @@ if [ "${OBSIDIAN_CONTINUOUS_SYNC:-false}" = "true" ]; then
|
||||
if [ -z "${OBSIDIAN_EMAIL:-}" ] || [ -z "${OBSIDIAN_PASSWORD:-}" ] || [ -z "${OBSIDIAN_VAULT_ID:-}" ]; then
|
||||
echo "WARNING: OBSIDIAN_EMAIL, OBSIDIAN_PASSWORD, or OBSIDIAN_VAULT_ID not set. Skipping sync."
|
||||
else
|
||||
echo "Setting up Obsidian sync..."
|
||||
|
||||
VAULT_PATH="${OBSIDIAN_VAULT_PATH:-/app/data/obsidian}"
|
||||
SYNC_MODE="${OBSIDIAN_SYNC_MODE:-bidirectional}"
|
||||
mkdir -p "${VAULT_PATH}"
|
||||
|
||||
# Login and setup sync (foreground, must complete before sync starts)
|
||||
if ob login --email "${OBSIDIAN_EMAIL}" --password "${OBSIDIAN_PASSWORD}" && \
|
||||
ob sync-setup \
|
||||
--vault "${OBSIDIAN_VAULT_ID}" \
|
||||
--path "${VAULT_PATH}" \
|
||||
--password "${OBSIDIAN_E2E_PASSWORD:-}" \
|
||||
--device-name "${OBSIDIAN_DEVICE_NAME:-simbarag}"; then
|
||||
# Remove stale lock from previous container run
|
||||
rm -rf "${VAULT_PATH}/.obsidian/.sync.lock"
|
||||
# Set sync to pull-only (read-only) mode
|
||||
ob sync-config --mode pull-only --path "${VAULT_PATH}"
|
||||
# Start continuous sync in background
|
||||
echo "Starting Obsidian continuous sync (pull-only)..."
|
||||
ob sync --continuous --path "${VAULT_PATH}" &
|
||||
else
|
||||
echo "WARNING: Obsidian sync setup failed. Continuing without sync."
|
||||
fi
|
||||
# Blue and green share the vault volume. flock on a file in it works
|
||||
# across containers, so this container only sets up and runs sync once
|
||||
# the previous container's sync process has exited. Two clients writing
|
||||
# one vault and its sync state would otherwise race. Setup runs in the
|
||||
# background so the app starts while it waits.
|
||||
(
|
||||
flock 9
|
||||
echo "Setting up Obsidian sync (${SYNC_MODE})..."
|
||||
if ob login --email "${OBSIDIAN_EMAIL}" --password "${OBSIDIAN_PASSWORD}" && \
|
||||
ob sync-setup \
|
||||
--vault "${OBSIDIAN_VAULT_ID}" \
|
||||
--path "${VAULT_PATH}" \
|
||||
--password "${OBSIDIAN_E2E_PASSWORD:-}" \
|
||||
--device-name "${OBSIDIAN_DEVICE_NAME:-simbarag}"; then
|
||||
# Safe now: the lock means no other container's sync is running.
|
||||
rm -rf "${VAULT_PATH}/.obsidian/.sync.lock"
|
||||
if ob sync-config --mode "${SYNC_MODE}" --conflict-strategy merge \
|
||||
--path "${VAULT_PATH}"; then
|
||||
echo "Starting Obsidian continuous sync (${SYNC_MODE})..."
|
||||
exec ob sync --continuous --path "${VAULT_PATH}"
|
||||
fi
|
||||
echo "WARNING: Obsidian sync-config failed. Continuing without sync."
|
||||
else
|
||||
echo "WARNING: Obsidian sync setup failed. Continuing without sync."
|
||||
fi
|
||||
) 9>"${VAULT_PATH}/.raggr-sync.lock" &
|
||||
fi
|
||||
fi
|
||||
|
||||
|
||||
@@ -85,6 +85,29 @@ class TestCreateArtifact:
|
||||
assert result is None
|
||||
assert await Artifact.all().count() == 0
|
||||
|
||||
async def test_drops_model_context_before_persisting(self, owner):
|
||||
"""model_context carries the full chunk text for the model's own view;
|
||||
it must never reach the DB (or, from there, the UI)."""
|
||||
user, conversation, message = owner
|
||||
artifact = await create_artifact_from_output(
|
||||
output=_output(
|
||||
{
|
||||
"kind": "search.documents",
|
||||
"title": "Search: vet",
|
||||
"data": {"results": [{"snippet": "short"}]},
|
||||
"model_context": {"results": [{"content": "x" * 5000}]},
|
||||
}
|
||||
),
|
||||
tool="simba_search",
|
||||
user=user,
|
||||
conversation=conversation,
|
||||
message=message,
|
||||
)
|
||||
assert artifact is not None
|
||||
saved = await Artifact.get(id=artifact.id)
|
||||
assert saved.data == {"results": [{"snippet": "short"}]}
|
||||
assert "model_context" not in saved.data
|
||||
|
||||
async def test_plain_string_output(self, owner):
|
||||
user, conversation, message = owner
|
||||
result = await create_artifact_from_output(
|
||||
|
||||
@@ -0,0 +1,226 @@
|
||||
"""LangGraph checkpoint pruning/deletion: bounds the on-disk checkpoint DB."""
|
||||
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import AsyncMock
|
||||
|
||||
import aiosqlite
|
||||
import pytest
|
||||
from langchain_core.messages import AIMessage, HumanMessage
|
||||
from langgraph.checkpoint.base import empty_checkpoint
|
||||
from langgraph.checkpoint.sqlite.aio import AsyncSqliteSaver
|
||||
from langgraph.graph import END, START, MessagesState, StateGraph
|
||||
from quart import Quart
|
||||
from quart_jwt_extended import JWTManager, create_access_token
|
||||
from tortoise import Tortoise
|
||||
|
||||
from blueprints.conversation import conversation_blueprint
|
||||
from blueprints.conversation.checkpoints import (
|
||||
DEFAULT_KEEP_PER_THREAD,
|
||||
delete_thread_checkpoints,
|
||||
keep_per_thread,
|
||||
prune_thread_checkpoints,
|
||||
)
|
||||
from blueprints.conversation.models import Conversation
|
||||
from blueprints.users.models import User
|
||||
|
||||
|
||||
async def _checkpoint_ids(saver, thread_id):
|
||||
async with saver.conn.execute(
|
||||
"SELECT checkpoint_id FROM checkpoints WHERE thread_id = ? ORDER BY checkpoint_id",
|
||||
(str(thread_id),),
|
||||
) as cur:
|
||||
return [row[0] async for row in cur]
|
||||
|
||||
|
||||
async def _write_rows(saver, thread_id):
|
||||
async with saver.conn.execute(
|
||||
"SELECT checkpoint_id FROM writes WHERE thread_id = ?",
|
||||
(str(thread_id),),
|
||||
) as cur:
|
||||
return [row[0] async for row in cur]
|
||||
|
||||
|
||||
async def _add_checkpoint(saver, thread_id):
|
||||
checkpoint = empty_checkpoint()
|
||||
config = {"configurable": {"thread_id": str(thread_id), "checkpoint_ns": ""}}
|
||||
await saver.aput(config, checkpoint, {}, {})
|
||||
return checkpoint["id"]
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
async def saver(tmp_path):
|
||||
async with aiosqlite.connect(tmp_path / "checkpoints.db") as conn:
|
||||
s = AsyncSqliteSaver(conn)
|
||||
await s.setup()
|
||||
yield s
|
||||
|
||||
|
||||
async def test_prune_keeps_newest_n_and_drops_orphaned_writes_only_for_pruned_thread(
|
||||
saver,
|
||||
):
|
||||
kept_ids = []
|
||||
pruned_ids = []
|
||||
for i in range(5):
|
||||
cid = await _add_checkpoint(saver, "pruned-thread")
|
||||
(pruned_ids if i < 3 else kept_ids).append(cid)
|
||||
await saver.aput_writes(
|
||||
{
|
||||
"configurable": {
|
||||
"thread_id": "pruned-thread",
|
||||
"checkpoint_ns": "",
|
||||
"checkpoint_id": cid,
|
||||
}
|
||||
},
|
||||
[("some_channel", "value")],
|
||||
f"task-{i}",
|
||||
)
|
||||
other_id = await _add_checkpoint(saver, "other-thread")
|
||||
await saver.aput_writes(
|
||||
{
|
||||
"configurable": {
|
||||
"thread_id": "other-thread",
|
||||
"checkpoint_ns": "",
|
||||
"checkpoint_id": other_id,
|
||||
}
|
||||
},
|
||||
[("some_channel", "value")],
|
||||
"task-other",
|
||||
)
|
||||
|
||||
await prune_thread_checkpoints(saver, "pruned-thread", keep=2)
|
||||
|
||||
remaining = await _checkpoint_ids(saver, "pruned-thread")
|
||||
assert sorted(remaining) == sorted(kept_ids)
|
||||
remaining_writes = await _write_rows(saver, "pruned-thread")
|
||||
assert sorted(remaining_writes) == sorted(kept_ids)
|
||||
for cid in pruned_ids:
|
||||
assert cid not in remaining
|
||||
assert cid not in remaining_writes
|
||||
|
||||
# Untouched: pruning one thread must not affect another.
|
||||
assert await _checkpoint_ids(saver, "other-thread") == [other_id]
|
||||
assert await _write_rows(saver, "other-thread") == [other_id]
|
||||
|
||||
|
||||
async def test_prune_preserves_full_latest_messages(saver):
|
||||
def echo(state):
|
||||
last = state["messages"][-1].content
|
||||
return {"messages": [AIMessage(content=f"reply {last}")]}
|
||||
|
||||
graph = StateGraph(MessagesState)
|
||||
graph.add_node("answer", echo)
|
||||
graph.add_edge(START, "answer")
|
||||
graph.add_edge("answer", END)
|
||||
agent = graph.compile(checkpointer=saver)
|
||||
|
||||
thread_id = "many-turns"
|
||||
config = {"configurable": {"thread_id": thread_id}}
|
||||
turn_count = 25
|
||||
for i in range(turn_count):
|
||||
await agent.ainvoke(
|
||||
{"messages": [HumanMessage(content=f"msg {i}")]}, config=config
|
||||
)
|
||||
|
||||
assert len(await _checkpoint_ids(saver, thread_id)) > DEFAULT_KEEP_PER_THREAD
|
||||
|
||||
await prune_thread_checkpoints(saver, thread_id, keep=5)
|
||||
|
||||
assert len(await _checkpoint_ids(saver, thread_id)) == 5
|
||||
state = await agent.aget_state(config)
|
||||
# Every turn's human + AI message is still present: pruning dropped old
|
||||
# checkpoint rows, but the newest row's channel_values already holds the
|
||||
# full accumulated message list (not a delta).
|
||||
assert len(state.values["messages"]) == turn_count * 2
|
||||
assert state.values["messages"][0].content == "msg 0"
|
||||
assert state.values["messages"][-1].content == f"reply msg {turn_count - 1}"
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
async def conv_app(tmp_path):
|
||||
await Tortoise.init(
|
||||
db_url=f"sqlite://{tmp_path / 'conv.sqlite'}",
|
||||
modules={
|
||||
"models": ["blueprints.users.models", "blueprints.conversation.models"]
|
||||
},
|
||||
)
|
||||
await Tortoise.generate_schemas()
|
||||
app = Quart(__name__)
|
||||
app.config.update(
|
||||
TESTING=True,
|
||||
JWT_SECRET_KEY="checkpoint-test-secret-at-least-thirty-two-characters",
|
||||
)
|
||||
JWTManager(app)
|
||||
app.register_blueprint(conversation_blueprint)
|
||||
user = await User.create(username="owner", email="owner@test")
|
||||
async with app.app_context():
|
||||
headers = {
|
||||
"Authorization": f"Bearer {create_access_token(identity=str(user.id))}"
|
||||
}
|
||||
try:
|
||||
yield app, headers, user
|
||||
finally:
|
||||
await Tortoise.close_connections()
|
||||
|
||||
|
||||
async def test_delete_conversation_via_api_removes_its_thread_and_leaves_others(
|
||||
conv_app, tmp_path
|
||||
):
|
||||
app, headers, user = conv_app
|
||||
conv1 = await Conversation.create(user=user, name="one")
|
||||
conv2 = await Conversation.create(user=user, name="two")
|
||||
|
||||
async with aiosqlite.connect(tmp_path / "checkpoints.db") as conn:
|
||||
s = AsyncSqliteSaver(conn)
|
||||
await s.setup()
|
||||
await _add_checkpoint(s, str(conv1.id))
|
||||
await _add_checkpoint(s, str(conv2.id))
|
||||
app.main_agent = SimpleNamespace(checkpointer=s)
|
||||
|
||||
response = await app.test_client().delete(
|
||||
f"/api/conversation/{conv1.id}", headers=headers
|
||||
)
|
||||
assert response.status_code == 200
|
||||
assert (await response.get_json())["success"] is True
|
||||
assert await Conversation.get_or_none(id=conv1.id) is None
|
||||
|
||||
assert await _checkpoint_ids(s, str(conv1.id)) == []
|
||||
assert len(await _checkpoint_ids(s, str(conv2.id))) == 1
|
||||
|
||||
|
||||
async def test_delete_conversation_succeeds_even_when_checkpoint_delete_fails(
|
||||
conv_app,
|
||||
):
|
||||
app, headers, user = conv_app
|
||||
conv = await Conversation.create(user=user, name="doomed")
|
||||
app.main_agent = SimpleNamespace(
|
||||
checkpointer=SimpleNamespace(
|
||||
adelete_thread=AsyncMock(side_effect=RuntimeError("sqlite is gone"))
|
||||
)
|
||||
)
|
||||
|
||||
response = await app.test_client().delete(
|
||||
f"/api/conversation/{conv.id}", headers=headers
|
||||
)
|
||||
assert response.status_code == 200
|
||||
assert (await response.get_json())["success"] is True
|
||||
assert await Conversation.get_or_none(id=conv.id) is None
|
||||
|
||||
|
||||
async def test_delete_thread_checkpoints_swallows_errors(caplog):
|
||||
checkpointer = SimpleNamespace(
|
||||
adelete_thread=AsyncMock(side_effect=RuntimeError("boom"))
|
||||
)
|
||||
await delete_thread_checkpoints(checkpointer, "t")
|
||||
assert "Failed to delete checkpoint thread" in caplog.text
|
||||
|
||||
|
||||
@pytest.mark.parametrize("raw", ["0", "-5", "not-a-number"])
|
||||
def test_keep_per_thread_rejects_invalid_values(monkeypatch, caplog, raw):
|
||||
monkeypatch.setenv("CHECKPOINT_KEEP_PER_THREAD", raw)
|
||||
assert keep_per_thread() == DEFAULT_KEEP_PER_THREAD
|
||||
assert "invalid" in caplog.text
|
||||
|
||||
|
||||
def test_keep_per_thread_accepts_valid_override(monkeypatch):
|
||||
monkeypatch.setenv("CHECKPOINT_KEEP_PER_THREAD", "7")
|
||||
assert keep_per_thread() == 7
|
||||
@@ -63,6 +63,7 @@ def test_missing_required_settings(production_env, name):
|
||||
("CHECKPOINT_DB_PATH", "/data/invalid\x00.db"),
|
||||
("CONVERSATION_SUMMARY_DB_PATH", ""),
|
||||
("OBSIDIAN_CONTINUOUS_SYNC", "yes"),
|
||||
("OBSIDIAN_SYNC_MODE", "push-only"),
|
||||
],
|
||||
)
|
||||
def test_malformed_and_unsafe_settings(production_env, name, value):
|
||||
@@ -202,6 +203,7 @@ def test_compose_forwards_runtime_settings():
|
||||
"CHECKPOINT_DB_PATH",
|
||||
"CONVERSATION_SUMMARY_DB_PATH",
|
||||
"OBSIDIAN_SYNC_INTERVAL",
|
||||
"OBSIDIAN_SYNC_MODE",
|
||||
"OIDC_AUTHORIZATION_ENDPOINT",
|
||||
"OIDC_TOKEN_ENDPOINT",
|
||||
"OIDC_USERINFO_ENDPOINT",
|
||||
|
||||
@@ -464,6 +464,64 @@ def test_focused_note_heading_index_uses_absolute_content_lines():
|
||||
assert msg.artifact["data"]["markdown"] == "full original"
|
||||
|
||||
|
||||
def _search_artifact(chunk_text: str) -> dict:
|
||||
return {
|
||||
"kind": "search.documents",
|
||||
"title": "Search: vet",
|
||||
"data": {
|
||||
"query": "vet",
|
||||
"source": "paperless",
|
||||
"results": [
|
||||
{
|
||||
"snippet": chunk_text[:50],
|
||||
"title": "vet_invoice",
|
||||
"source_id": "chunk-1",
|
||||
"url": "/sources/chunk-1",
|
||||
}
|
||||
],
|
||||
},
|
||||
"model_context": {
|
||||
"results": [
|
||||
{
|
||||
"content": chunk_text,
|
||||
"title": "vet_invoice",
|
||||
"source_id": "chunk-1",
|
||||
"url": "/sources/chunk-1",
|
||||
}
|
||||
]
|
||||
},
|
||||
}
|
||||
|
||||
|
||||
def test_search_result_compaction_keeps_full_chunk_text_under_a_generous_limit():
|
||||
import json
|
||||
|
||||
chunk_text = "Rabies vaccine administered. " + "detail " * 200
|
||||
msg = ToolMessage(
|
||||
content="display", artifact=_search_artifact(chunk_text), tool_call_id="a"
|
||||
)
|
||||
projected = context.compact_tool_result(msg, 100_000)
|
||||
parsed = json.loads(projected.content)
|
||||
(result,) = parsed["data"]["results"]
|
||||
assert result["content"] == chunk_text
|
||||
assert "snippet" not in result
|
||||
|
||||
|
||||
def test_search_result_compaction_shortens_content_but_keeps_source_ids_under_pressure():
|
||||
import json
|
||||
|
||||
chunk_text = "Rabies vaccine administered. " + "detail " * 200
|
||||
msg = ToolMessage(
|
||||
content="display", artifact=_search_artifact(chunk_text), tool_call_id="a"
|
||||
)
|
||||
projected = context.compact_tool_result(msg, 700)
|
||||
parsed = json.loads(projected.content)
|
||||
(result,) = parsed["data"]["results"]
|
||||
assert len(result["content"]) < len(chunk_text)
|
||||
assert result["source_id"] == "chunk-1"
|
||||
assert result["url"] == "/sources/chunk-1"
|
||||
|
||||
|
||||
def _conversation(turns, size=1000):
|
||||
messages = []
|
||||
for i in range(turns):
|
||||
@@ -542,3 +600,18 @@ def test_trim_drops_to_low_watermark_and_prefix_stays_stable():
|
||||
# ...so the retained prefix is unchanged for several turns in between.
|
||||
assert all(b - a >= 3 for a, b in zip(cuts, cuts[1:]))
|
||||
assert len(cuts) <= 5
|
||||
|
||||
|
||||
def test_search_result_compaction_keeps_source_ids_when_no_rows_fit():
|
||||
import json
|
||||
|
||||
chunk_text = "Rabies vaccine administered. " + "detail " * 200
|
||||
msg = ToolMessage(
|
||||
content="display", artifact=_search_artifact(chunk_text), tool_call_id="a"
|
||||
)
|
||||
projected = json.loads(context.compact_tool_result(msg, 300).content)
|
||||
data = projected["data"]
|
||||
assert data["results"] == []
|
||||
assert data["results_compaction"]["omitted_source_refs"] == [
|
||||
{"source_id": "chunk-1", "url": "/sources/chunk-1"}
|
||||
]
|
||||
|
||||
@@ -301,3 +301,45 @@ class TestStreamQueryFileKey:
|
||||
f"conversations/{CONV_ID}/files/u.txt"
|
||||
)
|
||||
assert name == "u.txt"
|
||||
|
||||
|
||||
class TestImageOwnership:
|
||||
async def test_presigns_only_keys_in_owned_conversations(self, app, headers, s3):
|
||||
presign = AsyncMock(return_value="https://s3.example/signed")
|
||||
with patch.object(conv, "s3_presigned_url", presign):
|
||||
client = app.test_client()
|
||||
own = await client.get(
|
||||
f"/api/conversation/image/conversations/{CONV_ID}/a.jpg",
|
||||
headers=headers,
|
||||
)
|
||||
assert own.status_code == 200
|
||||
for key in (
|
||||
f"conversations/{OTHER_ID}/a.jpg",
|
||||
f"conversations/{CONV_ID}/../{OTHER_ID}/a.jpg",
|
||||
"conversations/not-a-uuid/a.jpg",
|
||||
"beli/profile.jpg",
|
||||
):
|
||||
resp = await client.get(
|
||||
f"/api/conversation/image/{key}", headers=headers
|
||||
)
|
||||
assert resp.status_code == 404, key
|
||||
presign.assert_awaited_once_with(f"conversations/{CONV_ID}/a.jpg")
|
||||
|
||||
async def test_image_upload_requires_owned_conversation(self, app, headers, s3):
|
||||
with patch.object(conv.blueprints.users.models.User, "get", AsyncMock()):
|
||||
resp = await self._upload_image(app, headers)
|
||||
assert resp.status_code == 404
|
||||
s3.upload_image.assert_not_awaited()
|
||||
|
||||
@staticmethod
|
||||
async def _upload_image(app, headers):
|
||||
return await app.test_client().post(
|
||||
"/api/conversation/upload-image",
|
||||
headers=headers,
|
||||
form={"conversation_id": OTHER_ID},
|
||||
files={
|
||||
"file": FileStorage(
|
||||
io.BytesIO(_png_bytes()), filename="a.png", content_type="image/png"
|
||||
)
|
||||
},
|
||||
)
|
||||
|
||||
@@ -0,0 +1,66 @@
|
||||
import sys
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
import pytest
|
||||
|
||||
from scripts import manage_vectorstore as cli
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def calls(monkeypatch):
|
||||
calls = []
|
||||
monkeypatch.setattr(cli, "delete_all_documents", lambda: calls.append("delete_all"))
|
||||
monkeypatch.setattr(
|
||||
cli,
|
||||
"delete_documents_by_metadata",
|
||||
lambda k, v: calls.append(f"delete {k}={v}"),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
cli,
|
||||
"fetch_documents_from_paperless_ngx",
|
||||
AsyncMock(side_effect=lambda: calls.append("fetch") or ["doc"]),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
cli,
|
||||
"index_documents",
|
||||
AsyncMock(side_effect=lambda *docs: calls.append(f"paperless {list(docs)}")),
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
cli,
|
||||
"index_obsidian_documents",
|
||||
AsyncMock(side_effect=lambda: calls.append("obsidian")),
|
||||
)
|
||||
monkeypatch.setattr(cli, "stats", MagicMock())
|
||||
return calls
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"command, expected",
|
||||
[
|
||||
# A full reindex must not drop Obsidian notes after wiping everything.
|
||||
# Fetch before deleting, then index the fetched documents (no refetch).
|
||||
("reindex", ["fetch", "delete_all", "paperless [['doc']]", "obsidian"]),
|
||||
("reindex-obsidian", ["delete source=obsidian", "obsidian"]),
|
||||
],
|
||||
)
|
||||
def test_reindex_commands(calls, monkeypatch, command, expected):
|
||||
monkeypatch.setattr(sys, "argv", ["manage_vectorstore", command])
|
||||
cli.main()
|
||||
assert calls == expected
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"failure",
|
||||
[
|
||||
AsyncMock(side_effect=ConnectionError("Name or service not known")),
|
||||
AsyncMock(return_value=[]),
|
||||
],
|
||||
)
|
||||
def test_reindex_leaves_index_untouched_when_paperless_fails(
|
||||
calls, monkeypatch, failure
|
||||
):
|
||||
monkeypatch.setattr(cli, "fetch_documents_from_paperless_ngx", failure)
|
||||
monkeypatch.setattr(sys, "argv", ["manage_vectorstore", "reindex"])
|
||||
with pytest.raises(SystemExit):
|
||||
cli.main()
|
||||
assert "delete_all" not in calls
|
||||
@@ -0,0 +1,188 @@
|
||||
"""Tests for the incremental Obsidian sync: off-loop execution and delete SQL."""
|
||||
|
||||
import json
|
||||
import os
|
||||
import threading
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
|
||||
from blueprints.rag import logic
|
||||
|
||||
|
||||
class FakeConnection:
|
||||
def __init__(self, store):
|
||||
self.store = store
|
||||
|
||||
def execute(self, stmt, params=None):
|
||||
self.store.append((str(stmt), params or {}))
|
||||
|
||||
def commit(self):
|
||||
pass
|
||||
|
||||
def __enter__(self):
|
||||
return self
|
||||
|
||||
def __exit__(self, *exc_info):
|
||||
return False
|
||||
|
||||
|
||||
class FakeEngine:
|
||||
def __init__(self):
|
||||
self.executed: list[tuple[str, dict]] = []
|
||||
|
||||
def connect(self):
|
||||
return FakeConnection(self.executed)
|
||||
|
||||
|
||||
class FakeVectorStore:
|
||||
def __init__(self):
|
||||
self.calls: list[list[str]] = []
|
||||
|
||||
async def aadd_documents(self, documents):
|
||||
self.calls.append([d.page_content for d in documents])
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def vault_dir(tmp_path):
|
||||
(tmp_path / "new.md").write_text("# New\nNew note body")
|
||||
(tmp_path / "updated.md").write_text("# Updated\nUpdated note body")
|
||||
with patch.dict(os.environ, {"OBSIDIAN_VAULT_PATH": str(tmp_path)}):
|
||||
yield tmp_path
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
async def synced(monkeypatch, vault_dir):
|
||||
"""Run a sync with a fake vault: one new file, one updated file, one
|
||||
file that's been deleted off disk since the last sync."""
|
||||
updated_path = str(vault_dir / "updated.md")
|
||||
missing_path = str(vault_dir / "gone.md")
|
||||
|
||||
monkeypatch.setattr(
|
||||
logic, "_obsidian_index_cache", {updated_path: 1.0, missing_path: 1.0}
|
||||
)
|
||||
|
||||
engine = FakeEngine()
|
||||
monkeypatch.setattr(logic, "_get_engine", lambda: engine)
|
||||
monkeypatch.setattr(logic, "_get_collection_id", lambda: "collection-uuid")
|
||||
|
||||
def _unexpected_cache_load():
|
||||
raise AssertionError("cache load should be skipped when already populated")
|
||||
|
||||
monkeypatch.setattr(logic, "_load_obsidian_index_cache", _unexpected_cache_load)
|
||||
|
||||
store = FakeVectorStore()
|
||||
monkeypatch.setattr(logic, "_get_vector_store", lambda: store)
|
||||
|
||||
threads: dict[str, threading.Thread] = {}
|
||||
|
||||
real_walk = logic._walk_vault_mtimes
|
||||
|
||||
def recording_walk(service):
|
||||
threads["walk"] = threading.current_thread()
|
||||
return real_walk(service)
|
||||
|
||||
monkeypatch.setattr(logic, "_walk_vault_mtimes", recording_walk)
|
||||
|
||||
real_read = logic._read_obsidian_documents
|
||||
|
||||
def recording_read(service, filepaths, now):
|
||||
threads["read"] = threading.current_thread()
|
||||
return real_read(service, filepaths, now)
|
||||
|
||||
monkeypatch.setattr(logic, "_read_obsidian_documents", recording_read)
|
||||
|
||||
real_delete = logic.delete_documents_by_filepaths
|
||||
|
||||
def recording_delete(filepaths):
|
||||
threads["delete"] = threading.current_thread()
|
||||
return real_delete(filepaths)
|
||||
|
||||
monkeypatch.setattr(logic, "delete_documents_by_filepaths", recording_delete)
|
||||
|
||||
result = await logic.sync_obsidian_documents()
|
||||
return result, threads, engine, store, updated_path, missing_path
|
||||
|
||||
|
||||
class TestSyncOffLoop:
|
||||
async def test_counts_match_vault_changes(self, synced):
|
||||
result, *_ = synced
|
||||
assert result == {"added": 1, "updated": 1, "deleted": 1}
|
||||
|
||||
async def test_walk_runs_off_the_main_thread(self, synced):
|
||||
_, threads, *_ = synced
|
||||
assert threads["walk"] is not threading.main_thread()
|
||||
|
||||
async def test_reads_run_off_the_main_thread(self, synced):
|
||||
_, threads, *_ = synced
|
||||
assert threads["read"] is not threading.main_thread()
|
||||
|
||||
async def test_deletes_run_off_the_main_thread(self, synced):
|
||||
_, threads, *_ = synced
|
||||
assert threads["delete"] is not threading.main_thread()
|
||||
|
||||
async def test_embeds_new_and_updated_content(self, synced):
|
||||
_, _, _, store, *_ = synced
|
||||
embedded = {text for call in store.calls for text in call}
|
||||
assert any("New note body" in t for t in embedded)
|
||||
assert any("Updated note body" in t for t in embedded)
|
||||
|
||||
async def test_cache_reflects_add_update_delete(self, synced):
|
||||
result, _, _, _, updated_path, missing_path = synced
|
||||
assert missing_path not in logic._obsidian_index_cache
|
||||
assert updated_path in logic._obsidian_index_cache
|
||||
|
||||
async def test_deletes_use_jsonb_containment_with_bound_params(self, synced):
|
||||
_, _, engine, *_ = synced
|
||||
deletes = [(sql, params) for sql, params in engine.executed if "DELETE" in sql]
|
||||
assert deletes, "expected at least one delete statement"
|
||||
for sql, params in deletes:
|
||||
assert "cmetadata @> CAST(:filter AS jsonb)" in sql
|
||||
assert "->>:" not in sql # not the old, unindexed text-extraction filter
|
||||
assert "filter" in params
|
||||
parsed_filter = json.loads(params["filter"])
|
||||
assert "filepath" in parsed_filter
|
||||
|
||||
|
||||
async def test_delete_failure_keeps_deleted_path_in_cache_and_propagates(
|
||||
monkeypatch, vault_dir
|
||||
):
|
||||
"""If the batched delete raises, the sync must not have already forgotten
|
||||
the deleted file: otherwise the next sync never retries the delete and
|
||||
its chunks stay searchable forever with no record that they're stale."""
|
||||
updated_path = str(vault_dir / "updated.md")
|
||||
missing_path = str(vault_dir / "gone.md")
|
||||
|
||||
monkeypatch.setattr(
|
||||
logic, "_obsidian_index_cache", {updated_path: 1.0, missing_path: 1.0}
|
||||
)
|
||||
|
||||
def _raise_on_delete(filepaths):
|
||||
raise RuntimeError("db unavailable")
|
||||
|
||||
monkeypatch.setattr(logic, "delete_documents_by_filepaths", _raise_on_delete)
|
||||
|
||||
store = FakeVectorStore()
|
||||
monkeypatch.setattr(logic, "_get_vector_store", lambda: store)
|
||||
|
||||
with pytest.raises(RuntimeError, match="db unavailable"):
|
||||
await logic.sync_obsidian_documents()
|
||||
|
||||
assert missing_path in logic._obsidian_index_cache
|
||||
# The updated file's old chunks were never actually deleted, so its cache
|
||||
# entry must not be bumped to look freshly indexed.
|
||||
assert logic._obsidian_index_cache[updated_path] == 1.0
|
||||
assert store.calls == []
|
||||
|
||||
|
||||
async def test_delete_documents_by_metadata_uses_containment(monkeypatch):
|
||||
engine = FakeEngine()
|
||||
monkeypatch.setattr(logic, "_get_engine", lambda: engine)
|
||||
monkeypatch.setattr(logic, "_get_collection_id", lambda: "collection-uuid")
|
||||
|
||||
logic.delete_documents_by_metadata("source", "obsidian")
|
||||
|
||||
assert len(engine.executed) == 1
|
||||
sql, params = engine.executed[0]
|
||||
assert "cmetadata @> CAST(:filter AS jsonb)" in sql
|
||||
assert json.loads(params["filter"]) == {"source": "obsidian"}
|
||||
@@ -0,0 +1,20 @@
|
||||
from blueprints.conversation.prompts import SIMBA_SYSTEM_PROMPT
|
||||
|
||||
|
||||
def test_prompt_defaults_to_concise_answers():
|
||||
assert "concisely by default" in SIMBA_SYSTEM_PROMPT
|
||||
assert "thorough, detailed answers" not in SIMBA_SYSTEM_PROMPT
|
||||
# Reasoning tokens cost as much decode latency as answer tokens on this
|
||||
# hardware, so the prompt must not nudge toward more deliberation.
|
||||
assert "think things through" not in SIMBA_SYSTEM_PROMPT
|
||||
|
||||
|
||||
def test_prompt_keeps_key_behavioral_rules():
|
||||
for phrase in [
|
||||
"you MUST use the simba_search tool",
|
||||
"cite the provided Citation URLs with Markdown links",
|
||||
"never invent source IDs or references",
|
||||
"search before creating to avoid duplicates",
|
||||
"Use save_user_memory when the user explicitly asks",
|
||||
]:
|
||||
assert phrase in SIMBA_SYSTEM_PROMPT
|
||||
@@ -94,6 +94,32 @@ def test_artifact_citations_identify_exact_retrieved_chunk(monkeypatch):
|
||||
assert result["headings"] == ["Budget", "Savings"]
|
||||
|
||||
|
||||
async def test_serialized_chunk_header_is_compact_not_raw_metadata(monkeypatch):
|
||||
identifier = str(uuid4())
|
||||
doc = Document(
|
||||
id=identifier,
|
||||
page_content="Rabies vaccine administered",
|
||||
metadata={
|
||||
"source": "paperless",
|
||||
"filename": "vet_invoice.pdf",
|
||||
"created_date": datetime(2026, 9, 1, tzinfo=timezone.utc).timestamp(),
|
||||
"document_type": "Invoice",
|
||||
},
|
||||
)
|
||||
store = SimpleNamespace(
|
||||
amax_marginal_relevance_search=AsyncMock(return_value=[doc])
|
||||
)
|
||||
monkeypatch.setattr(logic, "_get_vector_store", lambda: store)
|
||||
text, _ = await logic.query_vector_store("vet", source="paperless")
|
||||
assert "{'source'" not in text
|
||||
assert str(doc.metadata) not in text
|
||||
assert "Title: vet_invoice" in text
|
||||
assert "Date: 2026-09-01" in text
|
||||
assert "Source: paperless" in text
|
||||
assert f"Citation: /sources/{identifier}" in text
|
||||
assert "Content: Rabies vaccine administered" in text
|
||||
|
||||
|
||||
def test_document_date_and_unknown_citations():
|
||||
assert (
|
||||
document_date(
|
||||
|
||||
@@ -521,6 +521,13 @@ async def test_simba_search(monkeypatch):
|
||||
assert result["snippet"].startswith("Rabies vaccine administered. ")
|
||||
assert len(result["snippet"]) <= SNIPPET_MAX_CHARS
|
||||
|
||||
(model_result,) = art["model_context"]["results"]
|
||||
assert model_result["content"] == PAPERLESS_DOCS[0].page_content
|
||||
assert len(model_result["content"]) > SNIPPET_MAX_CHARS
|
||||
assert model_result["title"] == result["title"]
|
||||
assert model_result["filename"] == result["filename"]
|
||||
assert "snippet" not in model_result
|
||||
|
||||
|
||||
async def test_simba_search_empty(monkeypatch):
|
||||
monkeypatch.setattr(agents, "query_vector_store", AsyncMock(return_value=("", [])))
|
||||
@@ -560,6 +567,14 @@ async def test_obsidian_search_notes(obsidian, monkeypatch):
|
||||
"date": "2026-01-02",
|
||||
}
|
||||
]
|
||||
assert art["model_context"]["results"] == [
|
||||
{
|
||||
"content": "Simba's vet is Dr Smith.",
|
||||
"title": "simba",
|
||||
"path": "pets/simba.md",
|
||||
"date": "2026-01-02",
|
||||
}
|
||||
]
|
||||
|
||||
|
||||
async def test_obsidian_search_notes_error(obsidian, monkeypatch):
|
||||
|
||||
Reference in New Issue
Block a user