From fbece0ba9a34e1149b9b65fce79f79987bca585b Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Thu, 7 May 2026 14:04:07 -0400 Subject: [PATCH 01/19] insight-chat: design for tool catalog, system prompt, and SMS fixes Lays out the cycle: split generation system prompt into identity vs procedural blocks so personas drive voice/shape, add per-turn system_prompt override on chat (ephemeral in append mode, persisted on amend), gate optional tools on data presence, and fix the days_radius bug in get_sms_messages. Co-Authored-By: Claude Opus 4.7 (1M context) --- specs/002-insight-chat-improvements.md | 392 +++++++++++++++++++++++++ 1 file changed, 392 insertions(+) create mode 100644 specs/002-insight-chat-improvements.md diff --git a/specs/002-insight-chat-improvements.md b/specs/002-insight-chat-improvements.md new file mode 100644 index 0000000..4efa9e3 --- /dev/null +++ b/specs/002-insight-chat-improvements.md @@ -0,0 +1,392 @@ +# Insight Chat improvements — design + +**Date:** 2026-05-07 +**Branch:** `feature/insight-chat-improvements` (in both `ImageApi/` and `FileViewer-React/`) +**Scope:** ImageApi photo-anchored insight + chat surface, plus the +FileViewer-React client. Apollo's free/visit chat is **not** in this cycle. + +## Problem + +Three concrete gaps in today's insight + chat surface: + +1. **Tool drift.** ImageApi exposes 13 tools to the LLM. Some are gated on + `apollo_enabled` / `has_vision`, but several optional ones + (`search_rag`, `get_calendar_events`, `get_location_history`) are + registered unconditionally even when their backing tables are empty. + Descriptions vary in quality and a couple have outright bugs. +2. **Inconsistent / incomplete tool descriptions.** Tools like + `search_messages` describe their selection rules but omit useful + examples; `store_fact` doesn't show the `object_entity_id` vs + `object_value` choice; `get_sms_messages` accepts a `days_radius` + parameter that the backing client silently ignores. The LLM is being + instructed against a slightly wrong reality. +3. **System prompt fights the persona.** Today's generation prompt + prepends the user's `custom_system_prompt` and then immediately asserts + `"You are a personal photo memory assistant..."`. The user message + demands `"a detailed insight with a title and summary"`. Both + contradict whatever voice / shape / POV the persona just established. + On chat continuation the persona is baked into the stored transcript at + generation time and can't be changed without regenerating. + +## Goals + +- Tool catalog is **representative** — every tool registered for a turn is + backed by data the user actually has. +- Tool descriptions are **concise but complete**, with examples for any + tool whose param choice has multiple modes or non-obvious interactions. +- Persona / system prompt is **authoritative** for voice, length, and + shape — both at generation and during chat continuation. +- Per-turn system prompt overrides on chat work without surprising + side-effects on the stored transcript outside `amend` mode. + +## Non-goals + +- Apollo backend / frontend changes. Separate cycle. +- Refactoring the `generate_photo_title` post-hoc title flow. Already + takes `custom_system_prompt`. +- Tool consolidation (e.g. merging `search_messages` + `get_sms_messages`). + Considered and deferred — keeps blast radius small. +- Removing knowledge-memory tools (`recall_*` / `store_*`). Audit + confirmed they have a live read path via `knowledge.rs` HTTP routes. +- Persisting persona changes to the stored transcript outside `amend` + mode. Deliberate — re-opens use the persona currently active in the + client, not a sticky historical setting. + +--- + +## Design + +### A. System prompt — generation + +Today (`insight_generator.rs:3305–3326`): + +``` +[custom_system_prompt if any] + +"You are a personal photo memory assistant helping to reconstruct..." + +{owner_id_note} + +{fewshot_block} + +"IMPORTANT INSTRUCTIONS: +1. You MUST call multiple tools... +2. When calling get_sms_messages and search_rag... +3. Use recall_facts_for_photo... +... +8. You have a hard budget of {max_iterations} iterations..." +``` + +The first concatenation is the bug: `custom` claims one identity, the +next line asserts another. + +**New structure** — two named blocks, in order: + +``` +[Identity / voice / format block] ← persona-controlled (or neutral default) +[Procedural block] ← always identity-free +``` + +**Identity block:** +- When `custom_system_prompt` is supplied: use that string verbatim, no + pre/append. +- When not: a neutral default that doesn't fight a future persona. + Working text: `"You are reconstructing a memory from a photo. Use the + gathered context to write a thoughtful summary; you decide voice, + length, and shape."` + +**Procedural block** — identity-free, always emitted: + +``` +Tool-use guidance: +- You have a budget of {max_iterations} tool-calling iterations. +- Call tools to gather context BEFORE writing your final answer; don't + answer after one or two calls. +- When calling get_sms_messages or search_rag, make at least one call + WITHOUT a contact filter — surrounding events matter even when a + contact is known. +- Use recall_facts_for_photo + recall_entities to load any prior + knowledge about subjects in the photo. +- When you identify people / places / events / things, use store_entity + + store_fact to grow the persistent memory. +- A tool returning no results is informative; continue with the others. + +{owner_id_note if applicable} +{fewshot_block if applicable} +``` + +Differences from today's "IMPORTANT INSTRUCTIONS" block: removed the +"you are a personal photo memory assistant" framing and the explicit +"at least 5 tool calls" floor (replaced with the softer "don't answer +after one or two"). Few-shot stays — it's pattern-of-tool-use, not +identity. + +### B. User message — generation + +Today (line 3357): + +``` +{visual_block}Please analyze this photo and gather any relevant context +from the surrounding weeks. + +Photo file path: {file_path} +Date taken: {date} +{contact_info} +{gps_info} +{tags_info} + +Use the available tools to gather more context about this moment +(messages, calendar events, location history, etc.), then write a +detailed insight with a title and summary. +``` + +Problems: the trailing line bakes in output shape ("title and +summary"), and the title from the resulting response is **discarded +anyway** — `generate_photo_title` (line 3494) regenerates the title +post-hoc from the summary. So the prompt is constraining voice for no +data-model benefit. + +**New payload** — context-only, no output prescription: + +``` +{visual_block}Photo file path: {file_path} +Date taken: {date} +{contact_info} +{gps_info} +{tags_info} + +Gather context with the available tools, then respond. +``` + +The persona owns shape. If a user wants "title-then-paragraph" output, +their persona prompt says so. + +### C. System prompt — chat continuation + +Add `system_prompt: Option` to `ChatTurnRequest` (and to its +HTTP wrapper `ChatTurnHttpRequest`). It carries through both the +non-streaming `chat_turn` and the streaming `chat_turn_stream`. + +**Append mode (default, `amend=false`)** — ephemeral +swap-and-restore, mirroring the existing `annotate_system_with_budget` +pattern: + +1. Load stored transcript. +2. If `system_prompt` is `Some(s)`: + - If first message is a `system` role: stash original content, + replace with `s`. + - Else: prepend a synthetic ephemeral system message with `s` (note + it's synthetic so the restore step pops it rather than rewriting). +3. Run `annotate_system_with_budget` on top (existing per-turn budget + note appends to whatever's there now). +4. Run the agentic loop. +5. **Before persistence**, restore the original system content (or pop + the synthetic one). Run `restore_system_content` for the budget + annotation as today. +6. Save. + +Result: the model sees the override; the stored transcript is +unchanged outside the model's actual reply. + +**Amend mode (`amend=true`)**: + +- If `system_prompt` is supplied: the override stays in place during + the serialization for the new insight row. The new row's + `training_messages` system message is the override. `is_current=false` + flips on prior rows as today. +- If not supplied: behaves as today (stored transcript's system message + carries forward unchanged). + +### D. FileViewer-React — client wiring + +`hooks/useInsightChat.tsx`: +- `SendTurnOptions` gains `systemPromptOverride?: string | null`. +- Inside `sendTurn`, before issuing the streaming POST: + 1. Read the active persona's `systemPrompt` from AsyncStorage + (already loaded for generation flows — reuse the same accessor). + 2. If a one-shot `systemPromptOverride` is set, append as a suffix + (`${persona}\n\n${override}`) so persona voice survives + override + tweaks the turn. + 3. Include the resulting string as `system_prompt` on the request body. +- No history-load change. The history endpoint still returns the stored + transcript. + +`components/InsightChatModal.tsx`: +- Add a small "Style note" composer affordance — a one-shot text input + that, when filled, becomes the `systemPromptOverride` for the next + send. Cleared after send. +- The existing persona chip continues to open `PersonaManagerModal`. + +`hooks/usePersonas.tsx` and the bundled defaults: +- Built-in `assistant` and `journal` prompts get audited and rewritten + to **explicitly state voice / shape / length** — since the framework + no longer guarantees a default shape, the persona must. + +### E. Tool catalog — gating + +Widen `build_tool_definitions` from `(has_vision: bool, apollo_enabled: +bool)` to a single `ToolGateOpts` struct: + +```rust +pub struct ToolGateOpts { + pub has_vision: bool, + pub apollo_enabled: bool, + pub daily_summaries_present: bool, + pub calendar_present: bool, + pub location_history_present: bool, +} +``` + +The chat / generation services compute the three new fields lazily per +turn via `SELECT 1 FROM LIMIT 1` (cheap; cached for the turn's +duration). Lazy because operators import data after launch and we don't +want to require a restart for the LLM to discover its new capabilities. + +Per-tool gating: + +| Tool | Existing gate | New gate | +|---|---|---| +| `describe_photo` | `has_vision` | unchanged | +| `get_personal_place_at` | `apollo_enabled` | unchanged | +| `get_calendar_events` | none | `calendar_present` | +| `get_location_history` | none | `location_history_present` | +| `search_rag` | none | `daily_summaries_present` | + +All other tools always-on. (`get_sms_messages` and `search_messages` +fail informatively if SMS-API is unreachable; not worth a startup probe +since intermittent failures are the same shape.) + +### F. Tool descriptions — convention + +Every description follows: + +1. One sentence: **what** + **when to call**. +2. Param semantics worth knowing (units, ranges, mode behavior, + precedence). +3. **Example invocation** for tools with multiple modes, optional bands, + or non-obvious parameter interactions. +4. Cross-references when relevant: `prefer X when both apply`. + +Banned: all-caps section headers inside descriptions +(`"CONTENT search"`, `"TIME-BASED fetch"`); persona-prescriptive language +(`"you are a..."`); behavioral references to other tools by description +rather than name. + +Tools getting examples: `search_messages`, `search_rag`, `store_fact`, +`get_sms_messages`. Trivial tools (`get_current_datetime`, +`reverse_geocode`, `get_file_tags`) skip the example. + +Sample (`search_messages`): + +> Search SMS/MMS message bodies. Modes: `fts5` (keyword + phrase + prefix +> + AND/OR/NOT + NEAR proximity), `semantic` (embedding similarity, +> requires generated embeddings), `hybrid` (RRF merge, recommended; +> degrades to `fts5` when embeddings absent). Optional `start_ts` / +> `end_ts` (real-UTC unix seconds) and `contact_id` filters. For pure +> date / contact browsing without keywords, prefer `get_sms_messages`. +> +> Examples: +> - `{query: "trader joe's"}` — phrase across all time. +> - `{query: "dinner", contact_id: 42, start_ts: 1700000000, end_ts: 1700604800}` +> — keyword within a contact and a week. +> - `{query: "NEAR(meeting work, 5)"}` — proximity search. + +### G. SMS tool fixes + +#### `get_sms_messages` — honor `days_radius` + +Today: `sms_client::fetch_messages_for_contact(contact, center_ts)` +hardcodes `Duration::days(4)` (lines 31–37). The tool accepts +`days_radius` and silently ignores it. + +**Fix:** widen the signature to +`fetch_messages_for_contact(contact, center_ts, days_radius)`. Tool +plumbs through. Default 4 retained for back-compat. + +#### `search_messages` — add date and contact_id filters + +Today: ImageApi's `search_messages` only forwards `query`, `mode`, +`limit` to SMS-API. + +**Fix:** add `start_ts`, `end_ts`, `contact_id` parameters. +- `contact_id` forwards directly to SMS-API + (`/api/messages/search/?contact_id=`). +- `start_ts` / `end_ts` are not natively accepted by SMS-API's search + endpoint. Apply client-side post-filter on the response (Apollo's + pattern: `chat_tools.py:670–680`). Bump the SMS-API `limit` to a + larger fetch pool when a date filter is supplied so in-window matches + aren't lost to out-of-window FTS rank. + +--- + +## Implementation sequencing + +Each step is independently mergeable. + +### ImageApi PRs + +1. **Split system-prompt assembly + neutralize user message.** Two + named blocks; user message context-only. Default identity string + added. Tests: golden snapshots of the resulting `system_content` + with and without `custom_system_prompt`. +2. **`system_prompt` field on chat request + swap/restore + amend + persistence.** Mirrors `annotate_system_with_budget` pattern. Tests: + round-trip system content unchanged in append mode; persisted in + amend mode. +3. **`fetch_messages_for_contact` honors `days_radius`.** Tool wires + the param through. Tests: window math at the client level. +4. **`ToolGateOpts` + per-tool description rewrites.** Description + text changes are the bulk of the diff but no behavior change beyond + gating. + +### FileViewer-React PR + +5. **Chat hook sends `system_prompt`; modal gets style-note input; + built-in personas updated to specify shape.** The + `useInsightChat.sendTurn` call site picks up the persona and + includes it on every chat turn body. Style-note input is a one-shot + suffix. + +## Testing & verification + +**Automated:** +- Unit (Rust): swap-and-restore round-trip preserves stored transcript. +- Unit (Rust): amend mode persists override into new insight row. +- Unit (Rust): `fetch_messages_for_contact(days_radius=N)` produces a + window of `2N` days centered on `center_ts`. +- Unit (Rust): `build_tool_definitions(opts)` excludes gated tools when + the corresponding flag is false. + +**Manual:** +- Run a chat turn against an existing insight without `system_prompt` → + output unchanged from baseline. +- Same insight, with override → output reflects new voice. +- Re-open chat → original baked persona still authoritative (override + was ephemeral). +- Regenerate an insight with the journal persona → model's voice + matches journal style; no "memory assistant" framing leaks through. +- Toggle data presence (delete a row from `calendar_events`) → tool + drops from the catalog on the next turn. + +## Risks + +- **Default identity wording matters.** A too-neutral default ("Use the + gathered context to write a summary") might produce flatter output + than today's "personal photo memory assistant" framing for users + who never set a persona. Mitigation: tune the default with a small + set of test photos before merging. +- **Persona-suffix style notes can contradict persona voice.** A user + who picks `journal` (first person, warm) and adds the style note + "respond in bullet points" will get a tonal collision. Acceptable — + the user expressed a per-turn intent and we honor it. Document the + composition rule in the persona-manager UI. +- **Lazy data-presence probes add a per-turn `SELECT 1`.** Negligible + on SQLite (sub-millisecond) but adds up across many turns. Cache the + result for the turn's duration; don't re-probe per-tool. + +## Open questions + +None blocking. Items deferred to a possible follow-up cycle: + +- Apollo parity for the same per-turn override pattern (already + present; just needs RN client wiring on the photo path which is + already proxy). +- Tool consolidation (`search_messages` + `get_sms_messages` → + single `search_messages` with optional date filter, Apollo-style). + Considered and deferred — separate spec. From 204428b0c0373aa9087b8711e6cf55160577d894 Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Thu, 7 May 2026 14:15:09 -0400 Subject: [PATCH 02/19] insight-chat: implementation plan for the spec Five sequenced PRs: 1. Split generation system prompt + neutralize user message 2. system_prompt field on chat request (ephemeral / amend-persisted) 3. fetch_messages_for_contact honors days_radius 4. ToolGateOpts + per-tool description rewrites + search_messages gains start_ts/end_ts/contact_id 5. FileViewer-React: persona system_prompt on every turn + style note Each PR independently mergeable. Tests inline TDD per task. Co-Authored-By: Claude Opus 4.7 (1M context) --- specs/002-insight-chat-improvements-plan.md | 2147 +++++++++++++++++++ 1 file changed, 2147 insertions(+) create mode 100644 specs/002-insight-chat-improvements-plan.md diff --git a/specs/002-insight-chat-improvements-plan.md b/specs/002-insight-chat-improvements-plan.md new file mode 100644 index 0000000..857c464 --- /dev/null +++ b/specs/002-insight-chat-improvements-plan.md @@ -0,0 +1,2147 @@ +# Insight Chat Improvements Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Make the insight + chat surface honor user-provided system prompts (no baked POV/format), gate optional tools on data presence, fix the `days_radius` no-op in `get_sms_messages`, and rewrite tool descriptions to a consistent convention. + +**Architecture:** Backend changes in ImageApi (Rust/Actix) split the generation system prompt into identity-vs-procedural blocks, add a `system_prompt` field to chat-turn requests with ephemeral swap-and-restore in append mode and persistence in amend mode, and gate tool registration on a per-turn `ToolGateOpts` struct. Frontend changes in FileViewer-React wire the active persona's prompt into every chat turn and add a one-shot "style note" composer affordance. + +**Tech Stack:** Rust (actix-web, diesel, kamadak-exif, serde), TypeScript (React Native, expo-router, AsyncStorage, react-native-sse), SQLite. Tests: `cargo test` for ImageApi. + +**Spec:** `specs/002-insight-chat-improvements.md` + +**Branch:** `feature/insight-chat-improvements` (already created in both `ImageApi/` and `FileViewer-React/`). + +--- + +## File structure + +### ImageApi (Rust) — files modified + +| File | Responsibility | +|---|---| +| `src/ai/insight_generator.rs` | Split system-prompt assembly; neutralize user message; widen `build_tool_definitions(opts: ToolGateOpts)`; add `current_gate_opts()` method to `InsightGenerator` for the chat path; description rewrites; SMS `search_messages` tool gains date+contact_id. | +| `src/ai/insight_chat.rs` | Add `system_prompt: Option` to `ChatTurnRequest`; helper `apply_system_prompt_override` and its restore; pass the override through `chat_turn` + `chat_turn_stream`; persist on amend. | +| `src/ai/handlers.rs` | Add `system_prompt: Option` to `ChatTurnHttpRequest`; plumb through both `chat_turn_handler` and `chat_stream_handler`. | +| `src/ai/sms_client.rs` | Widen `fetch_messages_for_contact` to take `days_radius`. | +| (no new files) | All changes are in-place. | + +### FileViewer-React (TypeScript) — files modified + +| File | Responsibility | +|---|---| +| `hooks/usePersonas.tsx` | Update `DEFAULT_PERSONAS` system prompts to specify voice/shape explicitly. | +| `hooks/useInsightChat.tsx` | `SendTurnOptions` gains `systemPromptOverride?: string \| null`; `sendTurn` reads selected persona and includes `system_prompt` in the request body. | +| `components/InsightChatModal.tsx` | One-shot "Style note" input next to the composer; passes `systemPromptOverride` into `sendTurn`. | + +--- + +## PR 1 — ImageApi: Split system-prompt assembly + neutralize user message + +**Files:** +- Modify: `src/ai/insight_generator.rs:3291-3326` (system-prompt assembly) +- Modify: `src/ai/insight_generator.rs:3356-3371` (user message construction) +- Test: `src/ai/insight_generator.rs` (existing `#[cfg(test)] mod tests` at bottom) + +### Task 1.1: Add a unit test for the new system-prompt builder (failing) + +- [ ] **Step 1: Add a failing test for the new builder** + +Add to the existing `#[cfg(test)] mod tests` block at the bottom of `src/ai/insight_generator.rs`: + +```rust + #[test] + fn build_system_content_uses_custom_prompt_verbatim_for_identity() { + let out = InsightGenerator::build_system_content( + Some("You are a journal writer in first person, warm and reflective."), + None, // no owner_id_note + "", // empty fewshot block + 6, // max_iterations + ); + // The custom prompt is the *only* identity language — no + // "you are a personal photo memory assistant" anywhere. + assert!( + out.starts_with("You are a journal writer in first person, warm and reflective."), + "custom prompt must lead the system content; got: {}", + &out[..out.len().min(200)], + ); + assert!( + !out.contains("personal photo memory assistant"), + "framework identity must not leak when custom prompt is supplied" + ); + // Procedural block must still arrive. + assert!(out.contains("Tool-use guidance")); + assert!(out.contains("budget of 6")); + } + + #[test] + fn build_system_content_uses_neutral_default_when_no_custom() { + let out = InsightGenerator::build_system_content(None, None, "", 6); + // Neutral default — does not assert a strong identity. + assert!(out.contains("reconstructing a memory from a photo")); + assert!(!out.contains("personal photo memory assistant")); + // Procedural block still present. + assert!(out.contains("Tool-use guidance")); + } + + #[test] + fn build_system_content_includes_fewshot_and_owner_id() { + let owner = "\n\nYour identity in the knowledge store: Alice (entity ID: 7)."; + let fewshot = "## Examples\n\n### Example 1\n...\n\n---\n\n"; + let out = InsightGenerator::build_system_content(None, Some(owner), fewshot, 6); + assert!(out.contains("Alice (entity ID: 7)")); + assert!(out.contains("## Examples")); + } +``` + +- [ ] **Step 2: Run the test to verify it fails** + +```bash +cd /home/cameron/development/opencode/ImageApi +cargo test --lib build_system_content -- --nocapture +``` + +Expected: compile error — `build_system_content` is not yet a function on `InsightGenerator`. + +### Task 1.2: Implement `build_system_content` + +- [ ] **Step 1: Add the new helper as an associated function on `InsightGenerator`** + +In `src/ai/insight_generator.rs`, add this method inside the `impl InsightGenerator { ... }` block (above `generate_agentic_insight_for_photo`): + +```rust + /// Assemble the chat system prompt from two named blocks: + /// + /// 1. **Identity / voice / format** — `custom_system_prompt` verbatim + /// when supplied, or a neutral default that doesn't fight a future + /// persona. The framework never asserts an identity that could + /// contradict the persona. + /// 2. **Procedural scaffolding** — tool-use guidance, iteration budget, + /// contact-filter rule. Identity-free; never asserts voice or shape. + /// + /// `owner_id_note` and `fewshot_block` are pre-rendered strings (they + /// already encode their own headers / blank lines). Pass empty / None + /// to skip. + pub(crate) fn build_system_content( + custom_system_prompt: Option<&str>, + owner_id_note: Option<&str>, + fewshot_block: &str, + max_iterations: usize, + ) -> String { + let identity = match custom_system_prompt { + Some(s) if !s.trim().is_empty() => s.trim().to_string(), + _ => String::from( + "You are reconstructing a memory from a photo. Use the gathered \ + context to write a thoughtful summary; you decide voice, length, and shape." + ), + }; + + let owner = owner_id_note.unwrap_or(""); + + let procedural = format!( + "Tool-use guidance:\n\ + - You have a budget of {max_iterations} tool-calling iterations.\n\ + - Call tools to gather context BEFORE writing your final answer; don't answer after one or two calls.\n\ + - When calling get_sms_messages or search_rag, make at least one call WITHOUT a contact filter \ + — surrounding events matter even when a contact is known.\n\ + - Use recall_facts_for_photo + recall_entities to load any prior knowledge about subjects in the photo.\n\ + - When you identify people / places / events / things, use store_entity + store_fact to grow the persistent memory.\n\ + - A tool returning no results is informative; continue with the others.", + max_iterations = max_iterations + ); + + // Compose: identity, then owner note (if any), then fewshot block + // (already self-delimited), then procedural. + let mut out = identity; + if !owner.is_empty() { + out.push_str(owner); + } + out.push_str("\n\n"); + if !fewshot_block.is_empty() { + out.push_str(fewshot_block); + } + out.push_str(&procedural); + out + } +``` + +- [ ] **Step 2: Run the tests to verify they pass** + +```bash +cd /home/cameron/development/opencode/ImageApi +cargo test --lib build_system_content -- --nocapture +``` + +Expected: all three tests pass. + +### Task 1.3: Wire `build_system_content` into the agentic flow + +- [ ] **Step 1: Replace the inlined system-prompt construction** + +In `src/ai/insight_generator.rs`, find the block at lines 3291-3326 that begins with `// 8. Build system message` and ends with `let system_content = if let Some(ref custom) = custom_system_prompt { ... };`. + +Replace that whole block (everything from `let owner_id_note = match owner_entity_id { ... }` through the end of `let system_content = ...;`) with: + +```rust + // 8. Build system message via the two-block helper. Custom prompt + // (when supplied) is the authoritative identity — the framework + // never appends a competing "you are a personal photo memory + // assistant" line. The procedural block stays identity-free. + let owner_id_note = match owner_entity_id { + Some(id) => Some(format!( + "\n\nYour identity in the knowledge store: {name} (entity ID: {id}). \ + When storing facts where you ({name}) are the object — for example, someone is your friend, \ + sibling, or colleague — use subject_entity_id for the other person and set object_value to \ + \"{name}\" (or use store_fact with the other person as subject). When storing facts about \ + {name} directly, use {id} as the subject_entity_id.", + name = owner_name, + id = id + )), + None => None, + }; + let fewshot_block = Self::render_fewshot_examples(&fewshot_examples); + let system_content = Self::build_system_content( + custom_system_prompt.as_deref(), + owner_id_note.as_deref(), + &fewshot_block, + max_iterations, + ); +``` + +- [ ] **Step 2: Build to confirm the wire-up compiles** + +```bash +cd /home/cameron/development/opencode/ImageApi +cargo build 2>&1 | tail -20 +``` + +Expected: clean build (no errors). + +- [ ] **Step 3: Re-run the unit tests to confirm regression-free** + +```bash +cd /home/cameron/development/opencode/ImageApi +cargo test --lib insight_generator -- --nocapture 2>&1 | tail -30 +``` + +Expected: all tests in `insight_generator` pass. + +### Task 1.4: Neutralize the user message + +- [ ] **Step 1: Replace the user-content template** + +In `src/ai/insight_generator.rs` around line 3356, find: + +```rust + let user_content = format!( + "{visual_block}Please analyze this photo and gather any relevant context from the surrounding weeks.\n\n\ + Photo file path: {}\n\ + Date taken: {}\n\ + {}\n\ + {}\n\ + {}\n\n\ + Use the available tools to gather more context about this moment (messages, calendar events, location history, etc.), \ + then write a detailed insight with a title and summary.", + file_path, + date_taken.format("%B %d, %Y"), + contact_info, + gps_info, + tags_info, + visual_block = visual_block, + ); +``` + +Replace with: + +```rust + // Context-only payload — no output-shape prescription. The persona / + // custom_system_prompt owns voice, length, and structure. The "title + // and summary" claim that used to live here was unused (the title is + // regenerated post-hoc from the summary by generate_photo_title). + let user_content = format!( + "{visual_block}Photo file path: {}\n\ + Date taken: {}\n\ + {}\n\ + {}\n\ + {}\n\n\ + Gather context with the available tools, then respond.", + file_path, + date_taken.format("%B %d, %Y"), + contact_info, + gps_info, + tags_info, + visual_block = visual_block, + ); +``` + +- [ ] **Step 2: Build to confirm the change compiles** + +```bash +cd /home/cameron/development/opencode/ImageApi +cargo build 2>&1 | tail -10 +``` + +Expected: clean build. + +### Task 1.5: Commit PR 1 + +- [ ] **Step 1: Run the full test suite** + +```bash +cd /home/cameron/development/opencode/ImageApi +cargo test --lib 2>&1 | tail -20 +``` + +Expected: all tests pass. + +- [ ] **Step 2: Stage and commit** + +```bash +cd /home/cameron/development/opencode/ImageApi +git add src/ai/insight_generator.rs +git commit -m "$(cat <<'EOF' +insight-chat: split generation system prompt into identity + procedural blocks + +The framework no longer asserts "you are a personal photo memory +assistant" alongside a user-supplied custom_system_prompt — the +persona is the authoritative identity. The procedural block (tool-use +guidance, iteration budget) stays identity-free. + +The user message also stops asking for "a detailed insight with a +title and summary" since the title is regenerated post-hoc anyway and +the wording was constraining voice for no data-model benefit. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +## PR 2 — ImageApi: `system_prompt` field on chat request + swap/restore + amend persistence + +**Files:** +- Modify: `src/ai/insight_chat.rs:30-51` (`ChatTurnRequest` struct) +- Modify: `src/ai/insight_chat.rs:208-550` (`chat_turn`) +- Modify: `src/ai/insight_chat.rs:659-1016` (`run_streaming_turn`) +- Modify: `src/ai/insight_chat.rs:1122-1154` (annotate/restore helpers — companion to a new helper) +- Modify: `src/ai/handlers.rs:621-645` (`ChatTurnHttpRequest`) +- Modify: `src/ai/handlers.rs:686-699` and `:900-913` (request mapping in both handlers) +- Test: `src/ai/insight_chat.rs` (existing `#[cfg(test)] mod tests` at bottom) + +### Task 2.1: Failing tests for the override behavior + +- [ ] **Step 1: Add unit tests for the new helpers** + +Add to the `#[cfg(test)] mod tests` block at the bottom of `src/ai/insight_chat.rs`: + +```rust + #[test] + fn apply_override_replaces_existing_system_message() { + let mut msgs = vec![ + ChatMessage::system("original persona"), + ChatMessage::user("hi"), + ]; + let stash = apply_system_prompt_override(&mut msgs, Some("new persona")); + assert_eq!(msgs[0].content, "new persona"); + // Stash carries the data we'll need to restore. + match stash { + Some(SystemPromptStash::Replaced { original }) => { + assert_eq!(original, "original persona"); + } + other => panic!("expected Replaced, got {:?}", other), + } + } + + #[test] + fn apply_override_prepends_synthetic_when_missing() { + let mut msgs = vec![ChatMessage::user("hi")]; + let stash = apply_system_prompt_override(&mut msgs, Some("new persona")); + assert_eq!(msgs.len(), 2); + assert_eq!(msgs[0].role, "system"); + assert_eq!(msgs[0].content, "new persona"); + assert!(matches!(stash, Some(SystemPromptStash::Prepended))); + } + + #[test] + fn apply_override_no_op_when_none() { + let mut msgs = vec![ + ChatMessage::system("sys"), + ChatMessage::user("hi"), + ]; + let stash = apply_system_prompt_override(&mut msgs, None); + assert!(stash.is_none()); + assert_eq!(msgs[0].content, "sys"); + } + + #[test] + fn apply_override_no_op_for_empty_string() { + let mut msgs = vec![ChatMessage::system("sys")]; + let stash = apply_system_prompt_override(&mut msgs, Some("")); + assert!(stash.is_none()); + assert_eq!(msgs[0].content, "sys"); + } + + #[test] + fn restore_override_replaces_back() { + let mut msgs = vec![ + ChatMessage::system("new"), + ChatMessage::user("hi"), + ]; + restore_system_prompt_override( + &mut msgs, + Some(SystemPromptStash::Replaced { original: "original".to_string() }), + ); + assert_eq!(msgs[0].content, "original"); + assert_eq!(msgs.len(), 2); + } + + #[test] + fn restore_override_pops_synthetic() { + let mut msgs = vec![ + ChatMessage::system("new"), + ChatMessage::user("hi"), + ]; + restore_system_prompt_override(&mut msgs, Some(SystemPromptStash::Prepended)); + assert_eq!(msgs.len(), 1); + assert_eq!(msgs[0].role, "user"); + } +``` + +- [ ] **Step 2: Run the tests to verify they fail** + +```bash +cd /home/cameron/development/opencode/ImageApi +cargo test --lib insight_chat::tests::apply_override -- --nocapture +``` + +Expected: compile errors — `apply_system_prompt_override`, `restore_system_prompt_override`, and `SystemPromptStash` are not yet defined. + +### Task 2.2: Implement override apply/restore helpers + +- [ ] **Step 1: Add the stash type and the two helpers** + +In `src/ai/insight_chat.rs`, just below the `restore_system_content` function (around line 1154), add: + +```rust +/// Receipt produced by [`apply_system_prompt_override`] so the caller can +/// undo the override before persistence. Two variants because we either +/// replaced an existing system message (need its original content) or +/// prepended a synthetic one (need to pop it). +#[derive(Debug)] +pub(crate) enum SystemPromptStash { + Replaced { original: String }, + Prepended, +} + +/// Apply a per-turn `system_prompt` override to `messages` so the model +/// sees the requested persona for this turn. Returns a stash the caller +/// must pass to [`restore_system_prompt_override`] before persisting the +/// transcript — without that step, append-mode chat would silently +/// rewrite the stored persona. +/// +/// No-op (returns `None`) when `override_prompt` is `None` or empty. +pub(crate) fn apply_system_prompt_override( + messages: &mut Vec, + override_prompt: Option<&str>, +) -> Option { + let prompt = match override_prompt { + Some(s) if !s.trim().is_empty() => s.trim().to_string(), + _ => return None, + }; + if let Some(first) = messages.first_mut() + && first.role == "system" + { + let original = std::mem::replace(&mut first.content, prompt); + return Some(SystemPromptStash::Replaced { original }); + } + messages.insert(0, ChatMessage::system(prompt)); + Some(SystemPromptStash::Prepended) +} + +/// Undo an override previously applied by [`apply_system_prompt_override`]. +/// No-op when `stash` is `None`. +pub(crate) fn restore_system_prompt_override( + messages: &mut Vec, + stash: Option, +) { + let Some(stash) = stash else { return }; + match stash { + SystemPromptStash::Replaced { original } => { + if let Some(first) = messages.first_mut() + && first.role == "system" + { + first.content = original; + } + } + SystemPromptStash::Prepended => { + if !messages.is_empty() && messages[0].role == "system" { + messages.remove(0); + } + } + } +} +``` + +- [ ] **Step 2: Run the unit tests** + +```bash +cd /home/cameron/development/opencode/ImageApi +cargo test --lib insight_chat::tests -- --nocapture 2>&1 | tail -30 +``` + +Expected: all six new tests pass; existing tests still pass. + +### Task 2.3: Add `system_prompt` to `ChatTurnRequest` + +- [ ] **Step 1: Add the field** + +In `src/ai/insight_chat.rs`, find the `ChatTurnRequest` struct around line 31 and add `system_prompt` between `max_iterations` and `amend`: + +```rust +#[derive(Debug)] +pub struct ChatTurnRequest { + pub library_id: i32, + pub file_path: String, + pub user_message: String, + /// Override the model id. Local mode: an Ollama model name. Hybrid: + /// an OpenRouter id. None defers to the stored insight's `model_version`. + pub model: Option, + /// Override the backend used for this turn. None defers to the stored + /// insight's `backend`. Switching `local -> hybrid` is rejected in v1. + pub backend: Option, + pub num_ctx: Option, + pub temperature: Option, + pub top_p: Option, + pub top_k: Option, + pub min_p: Option, + pub max_iterations: Option, + /// Per-turn system-prompt override. In append mode (default), applied + /// ephemerally — original system message restored before persistence. + /// In amend mode, persisted into the new insight row's system message. + /// None / empty = no change. + pub system_prompt: Option, + /// When true, write a new insight row (regenerating title) instead of + /// updating training_messages on the existing row. + pub amend: bool, +} +``` + +- [ ] **Step 2: Build to confirm** + +```bash +cd /home/cameron/development/opencode/ImageApi +cargo build 2>&1 | tail -10 +``` + +Expected: clean build (the new field is unused so far; that's fine). + +### Task 2.4: Apply override in `chat_turn` (non-streaming) + +- [ ] **Step 1: Hook the override into the loop** + +In `src/ai/insight_chat.rs`, in the `chat_turn` function around line 386, find the line: + +```rust + // 7. Append the new user turn. + messages.push(ChatMessage::user(req.user_message.clone())); + + // Temporarily annotate the system message with this turn's iteration + // budget so the model knows how many tool-calling rounds it has. We + // restore the original content before persistence so the note doesn't + // accumulate across turns. + let original_system_content = annotate_system_with_budget(&mut messages, max_iterations); +``` + +Replace with: + +```rust + // 7. Append the new user turn. + messages.push(ChatMessage::user(req.user_message.clone())); + + // Apply per-turn system-prompt override BEFORE the budget annotation + // so the budget note attaches to the override, not the original. + // The stash is consumed below before persistence (append mode) or + // dropped (amend mode, where the override stays in place). + let override_stash = + apply_system_prompt_override(&mut messages, req.system_prompt.as_deref()); + + // Temporarily annotate the system message with this turn's iteration + // budget so the model knows how many tool-calling rounds it has. We + // restore the original content before persistence so the note doesn't + // accumulate across turns. + let original_system_content = annotate_system_with_budget(&mut messages, max_iterations); +``` + +- [ ] **Step 2: Restore appropriately before persistence** + +Still in `chat_turn`, find: + +```rust + // Drop the per-turn iteration-budget note from the system message + // before we persist so it doesn't snowball on each subsequent turn. + restore_system_content(&mut messages, original_system_content); + + // 9. Persist. Append mode rewrites the JSON blob in place; amend + // mode regenerates the title and inserts a new insight row, + // relying on store_insight to flip prior rows' is_current=false. + let json = serde_json::to_string(&messages) + .map_err(|e| anyhow!("failed to serialize chat history: {}", e))?; +``` + +Replace with: + +```rust + // Drop the per-turn iteration-budget note from the system message + // before we persist so it doesn't snowball on each subsequent turn. + restore_system_content(&mut messages, original_system_content); + + // Append mode: undo the per-turn system-prompt override so the + // stored transcript keeps the original baked persona. Amend mode: + // keep the override in place — it becomes the new insight row's + // system message. + if !req.amend { + restore_system_prompt_override(&mut messages, override_stash); + } + + // 9. Persist. Append mode rewrites the JSON blob in place; amend + // mode regenerates the title and inserts a new insight row, + // relying on store_insight to flip prior rows' is_current=false. + let json = serde_json::to_string(&messages) + .map_err(|e| anyhow!("failed to serialize chat history: {}", e))?; +``` + +- [ ] **Step 3: Build** + +```bash +cd /home/cameron/development/opencode/ImageApi +cargo build 2>&1 | tail -10 +``` + +Expected: clean build. (Note: with `req.amend = false`, the local `override_stash` binding may produce an "unused if amend mode" warning when `amend` is true — silenced by the binding existing in the conditional path. Should compile clean.) + +### Task 2.5: Apply override in `run_streaming_turn` + +- [ ] **Step 1: Hook into the streaming loop** + +In `src/ai/insight_chat.rs`, in `run_streaming_turn` around line 813, find: + +```rust + messages.push(ChatMessage::user(req.user_message.clone())); + + let original_system_content = annotate_system_with_budget(&mut messages, max_iterations); +``` + +Replace with: + +```rust + messages.push(ChatMessage::user(req.user_message.clone())); + + // Mirror chat_turn: per-turn override goes on first, budget note next. + let override_stash = + apply_system_prompt_override(&mut messages, req.system_prompt.as_deref()); + + let original_system_content = annotate_system_with_budget(&mut messages, max_iterations); +``` + +- [ ] **Step 2: Restore before streaming-mode persistence** + +Still in `run_streaming_turn`, find: + +```rust + // Drop the per-turn iteration-budget note from the system message + // before we persist so it doesn't snowball on each subsequent turn. + restore_system_content(&mut messages, original_system_content); + + // Persist. + let json = serde_json::to_string(&messages) + .map_err(|e| anyhow!("failed to serialize chat history: {}", e))?; +``` + +Replace with: + +```rust + // Drop the per-turn iteration-budget note from the system message + // before we persist so it doesn't snowball on each subsequent turn. + restore_system_content(&mut messages, original_system_content); + + // Append mode: undo the per-turn system-prompt override (mirrors + // chat_turn). Amend mode: keep the override — it becomes the new + // insight row's system message. + if !req.amend { + restore_system_prompt_override(&mut messages, override_stash); + } + + // Persist. + let json = serde_json::to_string(&messages) + .map_err(|e| anyhow!("failed to serialize chat history: {}", e))?; +``` + +- [ ] **Step 3: Build** + +```bash +cd /home/cameron/development/opencode/ImageApi +cargo build 2>&1 | tail -10 +``` + +Expected: clean build. + +### Task 2.6: Wire `system_prompt` through HTTP handlers + +- [ ] **Step 1: Add to `ChatTurnHttpRequest`** + +In `src/ai/handlers.rs` around line 622, find the `ChatTurnHttpRequest` struct and add the field: + +```rust +#[derive(Debug, Deserialize)] +pub struct ChatTurnHttpRequest { + pub file_path: String, + #[serde(default)] + pub library: Option, + pub user_message: String, + #[serde(default)] + pub model: Option, + #[serde(default)] + pub backend: Option, + #[serde(default)] + pub num_ctx: Option, + #[serde(default)] + pub temperature: Option, + #[serde(default)] + pub top_p: Option, + #[serde(default)] + pub top_k: Option, + #[serde(default)] + pub min_p: Option, + #[serde(default)] + pub max_iterations: Option, + /// Per-turn system-prompt override. Ephemeral in append mode, + /// persisted in amend mode. See ChatTurnRequest for semantics. + #[serde(default)] + pub system_prompt: Option, + #[serde(default)] + pub amend: bool, +} +``` + +- [ ] **Step 2: Plumb into the non-streaming handler** + +In `src/ai/handlers.rs` around line 686, find: + +```rust + let chat_req = ChatTurnRequest { + library_id: library.id, + file_path: request.file_path.clone(), + user_message: request.user_message.clone(), + model: request.model.clone(), + backend: request.backend.clone(), + num_ctx: request.num_ctx, + temperature: request.temperature, + top_p: request.top_p, + top_k: request.top_k, + min_p: request.min_p, + max_iterations: request.max_iterations, + amend: request.amend, + }; +``` + +Replace with: + +```rust + let chat_req = ChatTurnRequest { + library_id: library.id, + file_path: request.file_path.clone(), + user_message: request.user_message.clone(), + model: request.model.clone(), + backend: request.backend.clone(), + num_ctx: request.num_ctx, + temperature: request.temperature, + top_p: request.top_p, + top_k: request.top_k, + min_p: request.min_p, + max_iterations: request.max_iterations, + system_prompt: request.system_prompt.clone(), + amend: request.amend, + }; +``` + +- [ ] **Step 3: Plumb into the streaming handler** + +Same file, around line 900, find the identical block in `chat_stream_handler` and apply the same change — add `system_prompt: request.system_prompt.clone(),` between `max_iterations` and `amend`. + +- [ ] **Step 4: Build** + +```bash +cd /home/cameron/development/opencode/ImageApi +cargo build 2>&1 | tail -10 +``` + +Expected: clean build. + +### Task 2.7: Add a swap-restore round-trip test + +- [ ] **Step 1: Failing test for round-trip preservation** + +Add to the `#[cfg(test)] mod tests` block in `src/ai/insight_chat.rs`: + +```rust + #[test] + fn override_round_trip_preserves_original_system_message() { + let mut msgs = vec![ + ChatMessage::system("original persona"), + ChatMessage::user("first user"), + assistant_text("first reply"), + ]; + let stash = apply_system_prompt_override(&mut msgs, Some("ephemeral persona")); + assert_eq!(msgs[0].content, "ephemeral persona"); + restore_system_prompt_override(&mut msgs, stash); + assert_eq!(msgs[0].content, "original persona"); + assert_eq!(msgs.len(), 3); + assert_eq!(msgs[1].role, "user"); + assert_eq!(msgs[2].role, "assistant"); + } + + #[test] + fn override_with_synthetic_round_trip_drops_extra_message() { + let mut msgs = vec![ChatMessage::user("first user")]; + let stash = apply_system_prompt_override(&mut msgs, Some("ephemeral")); + assert_eq!(msgs.len(), 2); + assert_eq!(msgs[0].role, "system"); + restore_system_prompt_override(&mut msgs, stash); + assert_eq!(msgs.len(), 1); + assert_eq!(msgs[0].role, "user"); + } +``` + +- [ ] **Step 2: Run them** + +```bash +cd /home/cameron/development/opencode/ImageApi +cargo test --lib insight_chat::tests::override -- --nocapture 2>&1 | tail -20 +``` + +Expected: both pass. + +### Task 2.8: Commit PR 2 + +- [ ] **Step 1: Run the full ImageApi test suite** + +```bash +cd /home/cameron/development/opencode/ImageApi +cargo test --lib 2>&1 | tail -20 +``` + +Expected: all tests pass. + +- [ ] **Step 2: Stage and commit** + +```bash +cd /home/cameron/development/opencode/ImageApi +git add src/ai/insight_chat.rs src/ai/handlers.rs +git commit -m "$(cat <<'EOF' +insight-chat: per-turn system_prompt override on chat continuation + +Append mode: applied ephemerally — original system message restored +before persistence so re-opens see the baked persona. Amend mode: +override stays in place and becomes the new insight row's system +message. Pattern mirrors annotate_system_with_budget. + +Adds system_prompt field on both ChatTurnHttpRequest and ChatTurnRequest; +plumbs through chat_turn and chat_turn_stream identically. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +## PR 3 — ImageApi: `fetch_messages_for_contact` honors `days_radius` + +**Files:** +- Modify: `src/ai/sms_client.rs:23-76` (`fetch_messages_for_contact`) +- Modify: `src/ai/insight_generator.rs:1799-1869` (`tool_get_sms_messages`) +- Test: `src/ai/sms_client.rs` (new tests at bottom) + +### Task 3.1: Failing test for the days_radius window math + +- [ ] **Step 1: Add test for the helper that computes the window** + +To `src/ai/sms_client.rs`, add at the bottom: + +```rust +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn window_for_radius_produces_2n_day_span() { + // Helper function under test below. + let center: i64 = 1_700_000_000; // 2023-11-14 22:13:20 UTC + let (start, end) = SmsApiClient::window_for_radius(center, 7); + assert_eq!(end - start, 14 * 86400); + assert_eq!(start + 7 * 86400, center); + assert_eq!(end - 7 * 86400, center); + } + + #[test] + fn window_for_radius_clamps_negative_to_one() { + // Days_radius < 1 falls through to a 1-day radius (so the call + // doesn't return 0 messages from a degenerate window). + let (start, end) = SmsApiClient::window_for_radius(100_000, 0); + assert_eq!(end - start, 2 * 86400); + } +} +``` + +- [ ] **Step 2: Run to verify failure** + +```bash +cd /home/cameron/development/opencode/ImageApi +cargo test --lib sms_client::tests -- --nocapture +``` + +Expected: compile error — `window_for_radius` doesn't exist. + +### Task 3.2: Implement `window_for_radius` and widen `fetch_messages_for_contact` + +- [ ] **Step 1: Add the helper and widen the function** + +In `src/ai/sms_client.rs`, replace the existing `fetch_messages_for_contact` (lines 26-76) with: + +```rust + /// Compute a `[start, end]` unix-second window of `2 * radius_days` + /// centered on `center_ts`. `radius_days < 1` is clamped to 1 to avoid + /// degenerate zero-width windows. + pub(crate) fn window_for_radius(center_ts: i64, radius_days: i64) -> (i64, i64) { + let r = radius_days.max(1); + let span = r * 86400; + (center_ts - span, center_ts + span) + } + + /// Fetch messages for a specific contact within ±`radius_days` of the + /// given timestamp. Falls back to all contacts when no messages found + /// for the named contact. Sorted by proximity to the center timestamp. + pub async fn fetch_messages_for_contact( + &self, + contact: Option<&str>, + center_timestamp: i64, + radius_days: i64, + ) -> Result> { + let (start_ts, end_ts) = Self::window_for_radius(center_timestamp, radius_days); + + let center_dt = chrono::DateTime::from_timestamp(center_timestamp, 0) + .ok_or_else(|| anyhow::anyhow!("Invalid timestamp"))?; + + // If contact specified, try fetching for that contact first + if let Some(contact_name) = contact { + log::info!( + "Fetching SMS for contact: {} (±{} days from {})", + contact_name, + radius_days.max(1), + center_dt.format("%Y-%m-%d %H:%M:%S") + ); + let messages = self + .fetch_messages(start_ts, end_ts, Some(contact_name), Some(center_timestamp)) + .await?; + + if !messages.is_empty() { + log::info!( + "Found {} messages for contact {}", + messages.len(), + contact_name + ); + return Ok(messages); + } + + log::info!( + "No messages found for contact {}, falling back to all contacts", + contact_name + ); + } + + // Fallback to all contacts + log::info!( + "Fetching all SMS messages (±{} days from {})", + radius_days.max(1), + center_dt.format("%Y-%m-%d %H:%M:%S") + ); + self.fetch_messages(start_ts, end_ts, None, Some(center_timestamp)) + .await + } +``` + +- [ ] **Step 2: Run the new tests** + +```bash +cd /home/cameron/development/opencode/ImageApi +cargo test --lib sms_client::tests -- --nocapture 2>&1 | tail -10 +``` + +Expected: both pass. + +### Task 3.3: Update callers to pass `days_radius` + +- [ ] **Step 1: Update `tool_get_sms_messages` to pass `days_radius`** + +In `src/ai/insight_generator.rs` around line 1836-1840, find: + +```rust + match self + .sms_client + .fetch_messages_for_contact(contact.as_deref(), timestamp) + .await +``` + +Replace with: + +```rust + match self + .sms_client + .fetch_messages_for_contact(contact.as_deref(), timestamp, days_radius) + .await +``` + +- [ ] **Step 2: Find any other callers and update them** + +```bash +cd /home/cameron/development/opencode/ImageApi +grep -rn "fetch_messages_for_contact" src/ --include="*.rs" +``` + +Expected output: only the one caller in `insight_generator.rs` (now updated) and the definition in `sms_client.rs`. If any other call sites exist, update each by adding a third argument matching the call's intent — most likely `4` (the historical hardcoded value). + +- [ ] **Step 3: Build** + +```bash +cd /home/cameron/development/opencode/ImageApi +cargo build 2>&1 | tail -10 +``` + +Expected: clean build. + +### Task 3.4: Commit PR 3 + +- [ ] **Step 1: Run the full test suite** + +```bash +cd /home/cameron/development/opencode/ImageApi +cargo test --lib 2>&1 | tail -20 +``` + +Expected: all tests pass. + +- [ ] **Step 2: Stage and commit** + +```bash +cd /home/cameron/development/opencode/ImageApi +git add src/ai/sms_client.rs src/ai/insight_generator.rs +git commit -m "$(cat <<'EOF' +insight-chat: get_sms_messages tool now honors days_radius + +The agentic tool definition advertised a days_radius parameter but +sms_client::fetch_messages_for_contact was hardcoded to ±4 days, +silently ignoring whatever value the LLM chose. Plumb the parameter +through; default 4 retained at the tool level for back-compat. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +## PR 4 — ImageApi: `ToolGateOpts` + per-tool description rewrites + SMS tool param expansion + +**Files:** +- Modify: `src/ai/insight_generator.rs:2488-2772` (`build_tool_definitions` + tool definitions) +- Modify: `src/ai/insight_generator.rs:1718-1796` (`tool_search_messages` to accept new params) +- Modify: `src/ai/insight_chat.rs` — chat service must compute `ToolGateOpts` per turn +- Test: `src/ai/insight_generator.rs` (new tests in existing `mod tests`) + +### Task 4.1: Define `ToolGateOpts` + +- [ ] **Step 1: Add the struct** + +In `src/ai/insight_generator.rs`, just above the `impl InsightGenerator { ... }` block that contains `build_tool_definitions` (search for `pub(crate) fn build_tool_definitions`), add: + +```rust +/// Per-call gating flags for `build_tool_definitions`. Tools whose backing +/// data is empty (or whose env-var guard is unset) are dropped from the +/// catalog so the LLM doesn't reach for a tool that always returns "No +/// results found." — that wastes iteration budget. +#[derive(Debug, Clone, Copy, Default)] +pub struct ToolGateOpts { + pub has_vision: bool, + pub apollo_enabled: bool, + pub daily_summaries_present: bool, + pub calendar_present: bool, + pub location_history_present: bool, +} +``` + +- [ ] **Step 2: Build to confirm** + +```bash +cd /home/cameron/development/opencode/ImageApi +cargo build 2>&1 | tail -10 +``` + +Expected: clean build. (Unused-struct warning at this stage is fine.) + +### Task 4.2: Failing test for the new `build_tool_definitions(opts)` signature + +- [ ] **Step 1: Test that gates drop tools when flags are off** + +Add to the `#[cfg(test)] mod tests` block in `src/ai/insight_generator.rs` (near the existing `summarize_*` tests): + +```rust + #[test] + fn build_tool_definitions_drops_gated_tools() { + let opts = ToolGateOpts { + has_vision: false, + apollo_enabled: false, + daily_summaries_present: false, + calendar_present: false, + location_history_present: false, + }; + let tools = InsightGenerator::build_tool_definitions(opts); + let names: Vec<&str> = tools.iter().map(|t| t.function.name.as_str()).collect(); + + // Always-on tools survive. + assert!(names.contains(&"search_messages")); + assert!(names.contains(&"get_sms_messages")); + assert!(names.contains(&"get_file_tags")); + assert!(names.contains(&"reverse_geocode")); + assert!(names.contains(&"get_current_datetime")); + assert!(names.contains(&"recall_entities")); + assert!(names.contains(&"recall_facts_for_photo")); + assert!(names.contains(&"store_entity")); + assert!(names.contains(&"store_fact")); + + // Gated tools are absent. + assert!(!names.contains(&"describe_photo")); + assert!(!names.contains(&"get_personal_place_at")); + assert!(!names.contains(&"search_rag")); + assert!(!names.contains(&"get_calendar_events")); + assert!(!names.contains(&"get_location_history")); + } + + #[test] + fn build_tool_definitions_includes_gated_tools_when_present() { + let opts = ToolGateOpts { + has_vision: true, + apollo_enabled: true, + daily_summaries_present: true, + calendar_present: true, + location_history_present: true, + }; + let tools = InsightGenerator::build_tool_definitions(opts); + let names: Vec<&str> = tools.iter().map(|t| t.function.name.as_str()).collect(); + assert!(names.contains(&"describe_photo")); + assert!(names.contains(&"get_personal_place_at")); + assert!(names.contains(&"search_rag")); + assert!(names.contains(&"get_calendar_events")); + assert!(names.contains(&"get_location_history")); + } +``` + +- [ ] **Step 2: Run to verify failure** + +```bash +cd /home/cameron/development/opencode/ImageApi +cargo test --lib build_tool_definitions -- --nocapture +``` + +Expected: compile error — current signature is `(has_vision: bool, apollo_enabled: bool)`, tests pass `ToolGateOpts`. + +### Task 4.3: Widen `build_tool_definitions` and add gating + +- [ ] **Step 1: Replace the function signature and rewrite description bodies** + +In `src/ai/insight_generator.rs`, replace the entire `build_tool_definitions` function (around lines 2488-2772) with: + +```rust + /// Build the list of tool definitions for the agentic loop, gated by + /// `opts`. Always-on tools: `search_messages`, `get_sms_messages`, + /// `get_file_tags`, `reverse_geocode`, `get_current_datetime`, the + /// four knowledge-memory tools. Conditional: `describe_photo` (vision + /// model), `get_personal_place_at` (Apollo configured), `search_rag` + /// (daily_summaries populated), `get_calendar_events` (calendar + /// populated), `get_location_history` (location history populated). + pub(crate) fn build_tool_definitions(opts: ToolGateOpts) -> Vec { + let mut tools: Vec = Vec::new(); + + if opts.daily_summaries_present { + tools.push(Tool::function( + "search_rag", + "Date-anchored semantic search over the user's daily-summary corpus. \ + Returns up to `limit` summaries most semantically similar to `query`, \ + weighted toward summaries near `date`. For raw message text across all \ + time, prefer `search_messages`. \ + Examples: `{query: \"family dinner\", date: \"2018-12-24\"}` — what \ + daily summaries near Christmas Eve mention family / dinner / gathering. \ + `{query: \"work travel\", date: \"2019-06-15\", contact: \"Alice\"}` — \ + narrowed to summaries that involve Alice.", + serde_json::json!({ + "type": "object", + "required": ["query", "date"], + "properties": { + "query": { "type": "string", "description": "Free-text query, semantically matched." }, + "date": { "type": "string", "description": "Anchor date, YYYY-MM-DD. Summaries near this date rank higher." }, + "contact": { "type": "string", "description": "Optional contact name to bias toward conversations with that person." }, + "limit": { "type": "integer", "description": "Max summaries to return (default 10, max 25)." } + } + }), + )); + } + + tools.push(Tool::function( + "search_messages", + "Search SMS/MMS message bodies. Modes: `fts5` (keyword + phrase + prefix + AND/OR/NOT + NEAR proximity), \ + `semantic` (embedding similarity, requires generated embeddings), `hybrid` (RRF merge, recommended; \ + degrades to fts5 when embeddings absent). Optional `start_ts` / `end_ts` (real-UTC unix seconds) and \ + `contact_id` filters. For pure date / contact browsing without keywords, prefer `get_sms_messages`. \ + Examples: `{query: \"trader joe's\"}` — phrase across all time. \ + `{query: \"dinner\", contact_id: 42, start_ts: 1700000000, end_ts: 1700604800}` — keyword within a contact and a week. \ + `{query: \"NEAR(meeting work, 5)\"}` — proximity search.", + serde_json::json!({ + "type": "object", + "required": ["query"], + "properties": { + "query": { "type": "string", "description": "Search query. Min 3 chars. fts5 supports phrase (\"\"), prefix (*), AND/OR/NOT, and NEAR proximity." }, + "mode": { "type": "string", "enum": ["fts5", "semantic", "hybrid"], "description": "Search strategy. Default: hybrid." }, + "limit": { "type": "integer", "description": "Max results (default 20, max 50)." }, + "contact_id": { "type": "integer", "description": "Optional numeric contact id to scope the search." }, + "start_ts": { "type": "integer", "description": "Optional inclusive lower bound, real-UTC unix seconds." }, + "end_ts": { "type": "integer", "description": "Optional inclusive upper bound, real-UTC unix seconds." } + } + }), + )); + + tools.push(Tool::function( + "get_sms_messages", + "Fetch SMS/MMS messages near a date (and optionally from a specific contact). Use when you know the date \ + or want context around a photo's timestamp. For keyword search without a date, use `search_messages`. \ + Returns up to `limit` messages within `±days_radius` of `date`, sorted by proximity. \ + Example: `{date: \"2018-08-12\", contact: \"Mom\", days_radius: 2}` — messages from Mom within ±2 days of Aug 12 2018.", + serde_json::json!({ + "type": "object", + "required": ["date"], + "properties": { + "date": { "type": "string", "description": "Center date, YYYY-MM-DD." }, + "contact": { "type": "string", "description": "Optional contact name (case-insensitive). Falls back to all contacts on no match." }, + "days_radius": { "type": "integer", "description": "Days before and after to include (default 4)." }, + "limit": { "type": "integer", "description": "Max messages to return (default 60, max 150)." } + } + }), + )); + + if opts.calendar_present { + tools.push(Tool::function( + "get_calendar_events", + "Fetch calendar events near a date — meetings, scheduled activities, all-day events. \ + Returns events within `±days_radius` of `date`. \ + Example: `{date: \"2019-03-22\", days_radius: 3}` — events within a week of March 22 2019.", + serde_json::json!({ + "type": "object", + "required": ["date"], + "properties": { + "date": { "type": "string", "description": "Center date, YYYY-MM-DD." }, + "days_radius": { "type": "integer", "description": "Days before and after to include (default 7)." }, + "limit": { "type": "integer", "description": "Max events to return (default 20, max 50)." } + } + }), + )); + } + + if opts.location_history_present { + tools.push(Tool::function( + "get_location_history", + "Fetch raw location records (lat/lon/timestamp/activity) near a date. The default 14-day radius is \ + wide because location density varies; tighten to ±1 day for a single-trip query. For a coordinate's \ + named place, use `reverse_geocode` (or `get_personal_place_at` when Apollo is enabled).", + serde_json::json!({ + "type": "object", + "required": ["date"], + "properties": { + "date": { "type": "string", "description": "Center date, YYYY-MM-DD." }, + "days_radius": { "type": "integer", "description": "Days before and after to include (default 14)." } + } + }), + )); + } + + tools.push(Tool::function( + "get_file_tags", + "Get user-applied tags for a specific photo file path. Tags are user-curated, not auto-detected.", + serde_json::json!({ + "type": "object", + "required": ["file_path"], + "properties": { + "file_path": { "type": "string", "description": "File path of the photo." } + } + }), + )); + + tools.push(Tool::function( + "reverse_geocode", + "Convert GPS lat/lon to a human-readable place name (city, state). Use for any coordinate the LLM has \ + obtained from EXIF or `get_location_history`. When Apollo is configured, prefer `get_personal_place_at` \ + — it returns the user's named places (Home / Work / etc.) which are more specific.", + serde_json::json!({ + "type": "object", + "required": ["latitude", "longitude"], + "properties": { + "latitude": { "type": "number", "description": "Decimal degrees." }, + "longitude": { "type": "number", "description": "Decimal degrees." } + } + }), + )); + + if opts.apollo_enabled { + tools.push(Tool::function( + "get_personal_place_at", + "Return any of the user's named Places (e.g. Home, Work, Cabin) whose radius contains (latitude, longitude). \ + Smallest radius first — most specific match wins. More specific than `reverse_geocode`; prefer this when \ + both apply. Returns place name, category, free-text description, and radius.", + serde_json::json!({ + "type": "object", + "required": ["latitude", "longitude"], + "properties": { + "latitude": { "type": "number", "description": "Decimal degrees." }, + "longitude": { "type": "number", "description": "Decimal degrees." } + } + }), + )); + } + + tools.push(Tool::function( + "recall_entities", + "Search the persistent knowledge memory for previously learned people, places, events, or things. \ + Use BEFORE writing the insight to ground the model on what's already known.", + serde_json::json!({ + "type": "object", + "properties": { + "name": { "type": "string", "description": "Name or partial name (case-insensitive substring match)." }, + "entity_type": { "type": "string", "enum": ["person", "place", "event", "thing"] }, + "limit": { "type": "integer", "description": "Max results (default 20, max 50)." } + } + }), + )); + + tools.push(Tool::function( + "recall_facts_for_photo", + "Retrieve all stored facts linked to a specific photo. Call at the start of insight generation to load \ + prior knowledge about subjects in this photo without scanning the whole knowledge base.", + serde_json::json!({ + "type": "object", + "required": ["file_path"], + "properties": { + "file_path": { "type": "string", "description": "File path of the photo." } + } + }), + )); + + tools.push(Tool::function( + "store_entity", + "Upsert a person / place / event / thing into the knowledge memory. Returns the entity id (use it as \ + `subject_entity_id` or `object_entity_id` in `store_fact`). Idempotent on canonical name.", + serde_json::json!({ + "type": "object", + "required": ["name", "entity_type"], + "properties": { + "name": { "type": "string", "description": "Canonical name (e.g. \"John Smith\", \"Banff National Park\")." }, + "entity_type": { "type": "string", "enum": ["person", "place", "event", "thing"] }, + "description": { "type": "string", "description": "Brief description." } + } + }), + )); + + tools.push(Tool::function( + "store_fact", + "Record a fact about an entity in the knowledge memory. Always linked to the current photo. \ + You must provide EITHER `object_entity_id` (when the object is itself a stored entity — e.g. \ + person A is_friend_of person B) OR `object_value` (free-text attribute — e.g. role=\"software engineer\"). \ + `object_entity_id` takes precedence when both are present. \ + Examples: \ + `{subject_entity_id: 7, predicate: \"is_friend_of\", object_entity_id: 12}` — links two known entities. \ + `{subject_entity_id: 7, predicate: \"lives_in\", object_value: \"Portland, Oregon\"}` — free-text attribute.", + serde_json::json!({ + "type": "object", + "required": ["subject_entity_id", "predicate"], + "properties": { + "subject_entity_id": { "type": "integer", "description": "Entity id this fact is about." }, + "predicate": { "type": "string", "description": "Relationship or attribute (e.g. is_friend_of, located_in, attended_event)." }, + "object_entity_id": { "type": "integer", "description": "Use when the object is itself a stored entity. Takes precedence over object_value." }, + "object_value": { "type": "string", "description": "Use for free-text attributes where the object is not a stored entity." }, + "photo_role": { "type": "string", "description": "How this entity appears in the photo (default \"subject\")." } + } + }), + )); + + tools.push(Tool::function( + "get_current_datetime", + "Get the current date and time. Useful when reasoning about how long ago a photo was taken.", + serde_json::json!({ + "type": "object", + "properties": {} + }), + )); + + if opts.has_vision { + tools.push(Tool::function( + "describe_photo", + "Generate a visual description of the current photo — people, location, objects, activity visible \ + in the image. Only available with vision-capable models.", + serde_json::json!({ + "type": "object", + "properties": {} + }), + )); + } + + tools + } +``` + +- [ ] **Step 2: Build to find call sites that need updating** + +```bash +cd /home/cameron/development/opencode/ImageApi +cargo build 2>&1 | tail -25 +``` + +Expected: build errors at the call sites in `insight_chat.rs` (`build_tool_definitions(offer_describe_tool, ...)`) and in `insight_generator.rs` itself (the agentic generation site at line 3377). The struct test should now compile and pass after we update the call sites. + +### Task 4.4: Add `current_gate_opts` helper on `InsightGenerator` + +- [ ] **Step 1: Add a method that probes data presence** + +In `src/ai/insight_generator.rs`, inside `impl InsightGenerator { ... }`, add (a sensible location is near `apollo_enabled()` — search for `pub fn apollo_enabled`): + +```rust + /// Compute the per-call tool gate options by probing each backing + /// table. Cheap (`SELECT 1 FROM LIMIT 1` shape via the existing + /// count methods); meant to be called once per chat turn / generation. + /// `has_vision` is supplied by the caller because it depends on the + /// model selected for this turn, not on persistent state. + pub fn current_gate_opts(&self, has_vision: bool) -> ToolGateOpts { + let cx = opentelemetry::Context::new(); + let calendar_present = { + let mut dao = self + .calendar_dao + .lock() + .expect("Unable to lock CalendarEventDao"); + dao.get_event_count(&cx).map(|n| n > 0).unwrap_or(false) + }; + let location_history_present = { + let mut dao = self + .location_dao + .lock() + .expect("Unable to lock LocationHistoryDao"); + dao.get_location_count(&cx).map(|n| n > 0).unwrap_or(false) + }; + let daily_summaries_present = { + let mut dao = self + .daily_summary_dao + .lock() + .expect("Unable to lock DailySummaryDao"); + dao.get_summary_count(&cx).map(|n| n > 0).unwrap_or(false) + }; + ToolGateOpts { + has_vision, + apollo_enabled: self.apollo_enabled(), + daily_summaries_present, + calendar_present, + location_history_present, + } + } +``` + +- [ ] **Step 2: Build** + +```bash +cd /home/cameron/development/opencode/ImageApi +cargo build 2>&1 | tail -10 +``` + +Expected: errors only at the call sites of `build_tool_definitions` (now mismatched signature). New helper compiles cleanly. + +### Task 4.5: Update the agentic generation call site + +- [ ] **Step 1: Switch to the new signature** + +In `src/ai/insight_generator.rs` around line 3373-3377, find: + +```rust + // 10. Define tools. Hybrid mode omits `describe_photo` since the + // chat model receives the visual description inline. + let offer_describe_tool = has_vision && !is_hybrid; + let tools = + Self::build_tool_definitions(offer_describe_tool, self.apollo_client.is_enabled()); +``` + +Replace with: + +```rust + // 10. Define tools. Gate flags computed from current data presence; + // hybrid mode omits describe_photo since the chat model receives + // the visual description inline (so we pass `false` for has_vision + // in hybrid mode regardless of the model's actual capability). + let gate_opts = self.current_gate_opts(has_vision && !is_hybrid); + let tools = Self::build_tool_definitions(gate_opts); +``` + +- [ ] **Step 2: Build** + +```bash +cd /home/cameron/development/opencode/ImageApi +cargo build 2>&1 | tail -10 +``` + +Expected: this site compiles; remaining errors are in `insight_chat.rs`. + +### Task 4.6: Update the chat call sites + +- [ ] **Step 1: Replace both `build_tool_definitions` calls in `insight_chat.rs`** + +In `src/ai/insight_chat.rs`, find (around line 362): + +```rust + let offer_describe_tool = !is_hybrid && local_first_user_has_image; + let tools = InsightGenerator::build_tool_definitions( + offer_describe_tool, + self.generator.apollo_enabled(), + ); +``` + +Replace with: + +```rust + let offer_describe_tool = !is_hybrid && local_first_user_has_image; + // current_gate_opts(has_vision) sets gate_opts.has_vision = has_vision + // and probes the per-table presence flags. Pass `offer_describe_tool` + // directly — the `!is_hybrid && local_first_user_has_image` decision + // is the chat-path's vision predicate. + let gate_opts = self.generator.current_gate_opts(offer_describe_tool); + let tools = InsightGenerator::build_tool_definitions(gate_opts); +``` + +Then around line 793 (in `run_streaming_turn`), find the identical block and replace the same way. + +- [ ] **Step 2: Build** + +```bash +cd /home/cameron/development/opencode/ImageApi +cargo build 2>&1 | tail -10 +``` + +Expected: clean build. + +- [ ] **Step 3: Run the new gate tests** + +```bash +cd /home/cameron/development/opencode/ImageApi +cargo test --lib build_tool_definitions -- --nocapture +``` + +Expected: both gate tests pass. + +### Task 4.7: Expand `tool_search_messages` to honor the new params + +- [ ] **Step 1: Read existing `tool_search_messages`** + +The current implementation at `src/ai/insight_generator.rs:1718-1796` only forwards `query`, `mode`, and `limit` to SMS-API. Now it should also forward `start_ts`, `end_ts`, and `contact_id`. + +**Pattern:** SMS-API's `/api/messages/search/` accepts `contact_id` natively but not date range — we add the date filter as a client-side post-filter (mirrors Apollo's `chat_tools.py:670–680`). Bump the SMS-API `limit` to 100 when a date filter is present so in-window matches aren't lost to out-of-window FTS rank. + +- [ ] **Step 2: Add a new method on `SmsApiClient` for the broader search call** + +In `src/ai/sms_client.rs`, just below the existing `search_messages` method (around line 290), add: + +```rust + /// Same shape as `search_messages` but with optional `contact_id`. The + /// SMS-API endpoint accepts contact_id natively; date filtering is the + /// caller's responsibility (post-filter on the returned rows). + pub async fn search_messages_with_contact( + &self, + query: &str, + mode: &str, + limit: usize, + contact_id: Option, + ) -> Result> { + let mut url = format!( + "{}/api/messages/search/?q={}&mode={}&limit={}", + self.base_url, + urlencoding::encode(query), + urlencoding::encode(mode), + limit + ); + if let Some(cid) = contact_id { + url.push_str(&format!("&contact_id={}", cid)); + } + + let mut request = self.client.get(&url); + if let Some(token) = &self.token { + request = request.header("Authorization", format!("Bearer {}", token)); + } + + let response = request.send().await?; + if !response.status().is_success() { + let status = response.status(); + let body = response.text().await.unwrap_or_default(); + return Err(anyhow::anyhow!( + "SMS search request failed: {} - {}", + status, + body + )); + } + + let data: SmsSearchResponse = response.json().await?; + Ok(data.results) + } +``` + +- [ ] **Step 3: Replace `tool_search_messages` body** + +In `src/ai/insight_generator.rs`, replace the `tool_search_messages` function (lines 1718-1796) with: + +```rust + /// Tool: search_messages — keyword / semantic / hybrid search over all + /// SMS message bodies via the Django FTS5 + embeddings pipeline. Now + /// supports optional `contact_id`, `start_ts`, `end_ts` filters. + async fn tool_search_messages(&self, args: &serde_json::Value) -> String { + let query = match args.get("query").and_then(|v| v.as_str()) { + Some(q) if !q.trim().is_empty() => q.trim(), + _ => { + let has_date = args.get("date").is_some() + || args.get("start_ts").is_some() + || args.get("end_ts").is_some(); + let has_contact = args.get("contact").is_some() + || args.get("contact_id").is_some(); + if has_date || has_contact { + return "Error: search_messages needs a 'query' (keywords/phrase). \ + To fetch messages around a date or from a contact without keywords, \ + call get_sms_messages with { date, contact? } instead." + .to_string(); + } + return "Error: missing required parameter 'query'".to_string(); + } + }; + if query.len() < 3 { + return "Error: query must be at least 3 characters".to_string(); + } + let mode = args + .get("mode") + .and_then(|v| v.as_str()) + .map(|s| s.to_lowercase()) + .unwrap_or_else(|| "hybrid".to_string()); + if !matches!(mode.as_str(), "fts5" | "semantic" | "hybrid") { + return format!( + "Error: unknown mode '{}'; expected one of: fts5, semantic, hybrid", + mode + ); + } + let user_limit = args + .get("limit") + .and_then(|v| v.as_i64()) + .unwrap_or(20) + .clamp(1, 50) as usize; + let contact_id = args.get("contact_id").and_then(|v| v.as_i64()); + let start_ts = args.get("start_ts").and_then(|v| v.as_i64()); + let end_ts = args.get("end_ts").and_then(|v| v.as_i64()); + let has_date_filter = start_ts.is_some() || end_ts.is_some(); + + // When a date filter is supplied, fetch a larger pool from SMS-API + // so in-window matches that ranked lower than out-of-window ones + // aren't lost. + let fetch_limit = if has_date_filter { 100 } else { user_limit }; + + log::info!( + "tool_search_messages: query='{}', mode={}, contact_id={:?}, range=[{:?}, {:?}], user_limit={}, fetch_limit={}", + query, mode, contact_id, start_ts, end_ts, user_limit, fetch_limit + ); + + let hits = match self + .sms_client + .search_messages_with_contact(query, &mode, fetch_limit, contact_id) + .await + { + Ok(h) => h, + Err(e) => return format!("Error searching messages: {}", e), + }; + + // Date-range post-filter on the client side. SMS-API's /search/ + // doesn't accept date params; mirroring Apollo's pattern here. + let filtered: Vec<_> = hits + .into_iter() + .filter(|h| { + if let Some(s) = start_ts + && h.date < s + { + return false; + } + if let Some(e) = end_ts + && h.date > e + { + return false; + } + true + }) + .take(user_limit) + .collect(); + + if filtered.is_empty() { + return "No messages matched.".to_string(); + } + + let user_name = user_display_name(); + let mut out = String::new(); + out.push_str(&format!( + "Found {} messages (mode: {}{}):\n\n", + filtered.len(), + mode, + if has_date_filter { ", date-filtered" } else { "" } + )); + for h in filtered { + let date = chrono::DateTime::from_timestamp(h.date, 0) + .map(|dt| dt.format("%Y-%m-%d").to_string()) + .unwrap_or_else(|| h.date.to_string()); + let direction: &str = if h.type_ == 2 { + &user_name + } else { + &h.contact_name + }; + let score = h + .similarity_score + .map(|s| format!(" [score {:.2}]", s)) + .unwrap_or_default(); + out.push_str(&format!( + "[{}]{} {} — {}\n\n", + date, score, direction, h.body + )); + } + out + } +``` + +- [ ] **Step 4: Build** + +```bash +cd /home/cameron/development/opencode/ImageApi +cargo build 2>&1 | tail -10 +``` + +Expected: clean build. + +### Task 4.8: Commit PR 4 + +- [ ] **Step 1: Run the full test suite** + +```bash +cd /home/cameron/development/opencode/ImageApi +cargo test --lib 2>&1 | tail -20 +``` + +Expected: all tests pass. + +- [ ] **Step 2: Stage and commit** + +```bash +cd /home/cameron/development/opencode/ImageApi +git add src/ai/insight_generator.rs src/ai/sms_client.rs src/ai/insight_chat.rs +git commit -m "$(cat <<'EOF' +insight-chat: ToolGateOpts + per-tool description rewrites + +Tools whose backing tables are empty (calendar, location_history, +daily_summaries) drop out of the catalog so the LLM doesn't waste +iteration budget calling them only to receive "no results found". +Vision and apollo gates already existed; this generalizes the pattern. + +search_messages gains start_ts/end_ts/contact_id filters (date filter +is a client-side post-filter; SMS-API only accepts contact_id natively +on the search endpoint). + +Descriptions follow a consistent convention: one sentence (what + +when), param semantics, examples for tools with non-obvious param +choices. No more all-caps headers, no more identity-prescriptive +language inside descriptions. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +## PR 5 — FileViewer-React: Wire system_prompt; style note input; persona prompt audit + +**Files:** +- Modify: `hooks/usePersonas.tsx:15-40` (`DEFAULT_PERSONAS`) +- Modify: `hooks/useInsightChat.tsx:41-44` (`SendTurnOptions`); `:163-499` (`sendTurn`) +- Modify: `components/InsightChatModal.tsx` (composer composition; new state) + +### Task 5.1: Update built-in persona prompts to specify voice/shape + +- [ ] **Step 1: Replace `DEFAULT_PERSONAS`** + +In `hooks/usePersonas.tsx`, replace the `DEFAULT_PERSONAS` constant (lines 15-40): + +```typescript +export const DEFAULT_PERSONAS: Persona[] = [ + { + id: "default", + name: "Default Assistant", + systemPrompt: + "You are my long-term memory assistant. Use only the information " + + "provided. Do not invent details. Respond in 3–6 sentences in third " + + "person, leading with the most concrete moment from the photo and " + + "the surrounding context. Plain prose, no headings.", + isBuiltIn: true, + createdAt: Date.now(), + }, + { + id: "journal", + name: "Personal Journal", + systemPrompt: + "You are a personal journal writer. Write in first person, present " + + "tense, with warmth and reflection — focusing on emotions and " + + "meaningful moments. Use only the information provided; do not " + + "invent details. Aim for 4–8 sentences in a single flowing " + + "paragraph, no headings.", + isBuiltIn: true, + createdAt: Date.now(), + }, + { + id: "factual", + name: "Factual Reporter", + systemPrompt: + "You are a factual memory recorder. Be precise, objective, and " + + "concise. Lead with the date and place, then list what / when / who " + + "in 2–4 short sentences. Use only the information provided; if a " + + "detail is unknown, say so rather than guessing.", + isBuiltIn: true, + createdAt: Date.now(), + }, +]; +``` + +- [ ] **Step 2: Confirm typescript compiles** + +```bash +cd /home/cameron/development/opencode/FileViewer-React +npx tsc --noEmit 2>&1 | tail -10 +``` + +Expected: no errors related to the changed file. + +### Task 5.2: Wire persona into chat-turn body + +- [ ] **Step 1: Add `systemPromptOverride` to `SendTurnOptions`** + +In `hooks/useInsightChat.tsx` around line 41, replace: + +```typescript +export interface SendTurnOptions { + /** When true the server inserts a new insight row with a regenerated title. */ + amend?: boolean; +} +``` + +With: + +```typescript +export interface SendTurnOptions { + /** When true the server inserts a new insight row with a regenerated title. */ + amend?: boolean; + /** One-shot extra style note for this turn. Appended as a suffix to the + * active persona's system prompt so persona voice survives + the note + * tweaks this single turn. Cleared by the caller after send. */ + systemPromptOverride?: string | null; +} +``` + +- [ ] **Step 2: Import the persona resolver and DEFAULT_PERSONAS at the top** + +In `hooks/useInsightChat.tsx` around line 1-6, add an import (above `getLogger`): + +```typescript +import { DEFAULT_PERSONAS, Persona } from "@/hooks/usePersonas"; +``` + +- [ ] **Step 3: Add a small helper near the top of the file** + +After the existing imports and before `useInsightChat()` (around line 98), add: + +```typescript +async function resolveSystemPrompt( + override: string | null | undefined +): Promise { + // Read the active persona id + custom personas list, look up the + // matching prompt, and (optionally) suffix the per-turn override. + // Returns null when both pieces are absent — server treats null as + // "no override; keep the baked-in transcript system message." + let basePrompt: string | null = null; + try { + const [selectedId, customJson] = await AsyncStorage.multiGet([ + "@insights_selected_persona", + "@insights_personas", + ]).then(entries => entries.map(([, v]) => v)); + const customs: Persona[] = customJson ? JSON.parse(customJson) : []; + const all: Persona[] = [...DEFAULT_PERSONAS, ...customs]; + const persona = + all.find(p => p.id === selectedId) ?? DEFAULT_PERSONAS[0] ?? null; + basePrompt = persona?.systemPrompt ?? null; + } catch { + // ignore — fall through to override-only or null + } + const trimmedOverride = (override ?? "").trim(); + if (basePrompt && trimmedOverride) { + return `${basePrompt}\n\n${trimmedOverride}`; + } + if (basePrompt) return basePrompt; + if (trimmedOverride) return trimmedOverride; + return null; +} +``` + +- [ ] **Step 4: Use it inside `sendTurn`** + +In `hooks/useInsightChat.tsx` around line 267-271, find: + +```typescript + const requestBody: Record = { + file_path: filePath, + user_message: trimmed, + amend: !!opts?.amend, + }; +``` + +Replace with: + +```typescript + const systemPrompt = await resolveSystemPrompt(opts?.systemPromptOverride); + + const requestBody: Record = { + file_path: filePath, + user_message: trimmed, + amend: !!opts?.amend, + }; + if (systemPrompt) requestBody.system_prompt = systemPrompt; +``` + +- [ ] **Step 5: Confirm typescript compiles** + +```bash +cd /home/cameron/development/opencode/FileViewer-React +npx tsc --noEmit 2>&1 | tail -10 +``` + +Expected: no errors related to the changed file. + +### Task 5.3: Add a one-shot "Style note" input to the chat modal + +- [ ] **Step 1: Add state + plumbing for the style note** + +In `components/InsightChatModal.tsx` around line 67-79, find the existing `useState` block: + +```typescript + const [draft, setDraft] = useState(""); + const [amend, setAmend] = useState(false); + const scrollRef = useRef(null); +``` + +Replace with: + +```typescript + const [draft, setDraft] = useState(""); + const [amend, setAmend] = useState(false); + // One-shot style note for the next turn. Cleared after send. Suffixed + // onto the active persona's prompt server-side via system_prompt. + const [styleNote, setStyleNote] = useState(""); + const scrollRef = useRef(null); +``` + +- [ ] **Step 2: Pass `systemPromptOverride` through `onSend`** + +Same file around line 153-158, find: + +```typescript + const onSend = () => { + const text = draft.trim(); + if (!text || sending) return; + setDraft(""); + sendTurn(filePath, libraryParam, text, { amend }); + }; +``` + +Replace with: + +```typescript + const onSend = () => { + const text = draft.trim(); + if (!text || sending) return; + const note = styleNote.trim(); + setDraft(""); + setStyleNote(""); // one-shot + sendTurn(filePath, libraryParam, text, { + amend, + systemPromptOverride: note ? note : null, + }); + }; +``` + +- [ ] **Step 3: Render the style-note input above the composer** + +Same file, find the composer block (around line 387-417) starting with: + +```typescript + ` block, insert: + +```typescript + + + Style: + + + +``` + +- [ ] **Step 4: Add the matching styles** + +Same file, in the `StyleSheet.create({ ... })` block at the bottom (around line 717), add to the existing styles object (anywhere; near `composer` reads naturally): + +```typescript + styleNoteRow: { + flexDirection: "row", + alignItems: "center", + paddingHorizontal: 8, + paddingVertical: 4, + borderTopWidth: 1, + gap: 4, + }, + styleNoteInput: { + flex: 1, + minHeight: 28, + borderWidth: 1, + borderRadius: 6, + paddingHorizontal: 8, + paddingVertical: 4, + fontSize: 12, + }, +``` + +- [ ] **Step 5: Confirm typescript compiles** + +```bash +cd /home/cameron/development/opencode/FileViewer-React +npx tsc --noEmit 2>&1 | tail -10 +``` + +Expected: no errors. + +### Task 5.4: Manual smoke test + +- [ ] **Step 1: Start the dev server** + +```bash +cd /home/cameron/development/opencode/FileViewer-React +npm start +``` + +Expected: Expo dev server running. + +- [ ] **Step 2: Verify the UI loads** + +Open the app on a device or simulator. Navigate to a photo with an existing insight, open the chat modal. Confirm the new "Style:" row appears between the message list and the composer. + +- [ ] **Step 3: Send a turn with the journal persona + a style note** + +Switch persona to "Personal Journal" via the chip. In the Style input type "respond in two sentences only." Send a message. Verify the reply is short and in first person; verify the Style input clears after send. + +- [ ] **Step 4: Re-open and confirm the override was ephemeral** + +Close and re-open the chat modal. Send another turn (no style note this time, default persona). Verify the reply matches the default persona — the override didn't stick. + +### Task 5.5: Commit PR 5 + +- [ ] **Step 1: Stage and commit** + +```bash +cd /home/cameron/development/opencode/FileViewer-React +git add hooks/usePersonas.tsx hooks/useInsightChat.tsx components/InsightChatModal.tsx +git commit -m "$(cat <<'EOF' +insight-chat: send persona system_prompt on chat turns + one-shot style note + +The active persona's systemPrompt is read from AsyncStorage and sent +on every chat turn body so persona changes take effect on the next +reply (server applies it ephemerally in append mode, persists in +amend mode). + +Adds a one-shot Style input next to the composer for per-turn tweaks +("answer in bullet points") that suffixes the persona prompt and +clears after send. + +Built-in personas updated to specify voice/length/shape explicitly +since the framework no longer asserts a default shape. + +Co-Authored-By: Claude Opus 4.7 (1M context) +EOF +)" +``` + +--- + +## Self-review checklist (post-write, before handoff) + +- [ ] Spec section A (system prompt — generation): covered by PR 1 Tasks 1.1–1.5. +- [ ] Spec section B (user message — generation): covered by PR 1 Task 1.4. +- [ ] Spec section C (system_prompt on chat request): covered by PR 2 Tasks 2.3, 2.6. +- [ ] Spec section D (append-mode swap-and-restore): covered by PR 2 Tasks 2.2, 2.4, 2.5. +- [ ] Spec section E (amend-mode persistence): covered by PR 2 Tasks 2.4, 2.5 (the `if !req.amend` guard). +- [ ] Spec section F (FileViewer-React client wiring): covered by PR 5 Tasks 5.1–5.4. +- [ ] Spec section G (gating on data presence): covered by PR 4 Tasks 4.1, 4.3, 4.4, 4.5, 4.6. +- [ ] Spec section "tool description convention": covered by PR 4 Task 4.3 (each tool's new description body). +- [ ] Spec section G — `get_sms_messages` `days_radius` no-op fix: covered by PR 3. +- [ ] Spec section G — `search_messages` adds `start_ts`/`end_ts`/`contact_id`: covered by PR 4 Task 4.7. + +## Open notes + +- **OpenRouter sampling-param plumbing** is not in scope — `temperature` / `top_p` / `top_k` / `min_p` already flow through `ChatTurnRequest`. No change needed. +- **Tests for the streaming path's override** are deliberately omitted at the unit level — `run_streaming_turn` is integration-shaped (depends on Ollama). The non-streaming round-trip test in PR 2 Task 2.7 exercises the same helper logic; visual smoke in PR 5 Task 5.4 covers the streaming end-to-end. +- **No migration required** — no schema changes; `system_prompt` is request-only. From 8ae4099d46db2471df9a1a4b4122539143083b67 Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Thu, 7 May 2026 14:20:45 -0400 Subject: [PATCH 03/19] insight-chat: split generation system prompt into identity + procedural blocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The framework no longer asserts "you are a personal photo memory assistant" alongside a user-supplied custom_system_prompt — the persona is the authoritative identity. The procedural block (tool-use guidance, iteration budget) stays identity-free. The user message also stops asking for "a detailed insight with a title and summary" since the title is regenerated post-hoc anyway and the wording was constraining voice for no data-model benefit. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/ai/insight_generator.rs | 137 ++++++++++++++++++++++++++++-------- 1 file changed, 108 insertions(+), 29 deletions(-) diff --git a/src/ai/insight_generator.rs b/src/ai/insight_generator.rs index e03b1af..d05cd4d 100644 --- a/src/ai/insight_generator.rs +++ b/src/ai/insight_generator.rs @@ -2943,6 +2943,58 @@ Return ONLY the summary, nothing else."#, } } + /// Assemble the chat system prompt from two named blocks: + /// + /// 1. **Identity / voice / format** — `custom_system_prompt` verbatim + /// when supplied, or a neutral default that doesn't fight a future + /// persona. The framework never asserts an identity that could + /// contradict the persona. + /// 2. **Procedural scaffolding** — tool-use guidance, iteration budget, + /// contact-filter rule. Identity-free; never asserts voice or shape. + /// + /// `owner_id_note` and `fewshot_block` are pre-rendered strings (they + /// already encode their own headers / blank lines). Pass empty / None + /// to skip. + pub(crate) fn build_system_content( + custom_system_prompt: Option<&str>, + owner_id_note: Option<&str>, + fewshot_block: &str, + max_iterations: usize, + ) -> String { + let identity = match custom_system_prompt { + Some(s) if !s.trim().is_empty() => s.trim().to_string(), + _ => String::from( + "You are reconstructing a memory from a photo. Use the gathered \ + context to write a thoughtful summary; you decide voice, length, and shape." + ), + }; + + let owner = owner_id_note.unwrap_or(""); + + let procedural = format!( + "Tool-use guidance:\n\ + - You have a budget of {max_iterations} tool-calling iterations.\n\ + - Call tools to gather context BEFORE writing your final answer; don't answer after one or two calls.\n\ + - When calling get_sms_messages or search_rag, make at least one call WITHOUT a contact filter \ + — surrounding events matter even when a contact is known.\n\ + - Use recall_facts_for_photo + recall_entities to load any prior knowledge about subjects in the photo.\n\ + - When you identify people / places / events / things, use store_entity + store_fact to grow the persistent memory.\n\ + - A tool returning no results is informative; continue with the others.", + max_iterations = max_iterations + ); + + let mut out = identity; + if !owner.is_empty() { + out.push_str(owner); + } + out.push_str("\n\n"); + if !fewshot_block.is_empty() { + out.push_str(fewshot_block); + } + out.push_str(&procedural); + out + } + pub async fn generate_agentic_insight_for_photo( &self, file_path: &str, @@ -3288,9 +3340,12 @@ Return ONLY the summary, nothing else."#, None }; - // 8. Build system message + // 8. Build system message via the two-block helper. Custom prompt + // (when supplied) is the authoritative identity — the framework + // never appends a competing "you are a personal photo memory + // assistant" line. The procedural block stays identity-free. let owner_id_note = match owner_entity_id { - Some(id) => format!( + Some(id) => Some(format!( "\n\nYour identity in the knowledge store: {name} (entity ID: {id}). \ When storing facts where you ({name}) are the object — for example, someone is your friend, \ sibling, or colleague — use subject_entity_id for the other person and set object_value to \ @@ -3298,32 +3353,16 @@ Return ONLY the summary, nothing else."#, {name} directly, use {id} as the subject_entity_id.", name = owner_name, id = id - ), - None => String::new(), + )), + None => None, }; let fewshot_block = Self::render_fewshot_examples(&fewshot_examples); - let base_system = format!( - "You are a personal photo memory assistant helping to reconstruct a memory from a photo.{owner_id_note}\n\n\ - {fewshot_block}\ - IMPORTANT INSTRUCTIONS:\n\ - 1. You MUST call multiple tools to gather context BEFORE writing any final insight. Do not produce a final answer after only one or two tool calls.\n\ - 2. When calling get_sms_messages and search_rag, always make at least one call WITHOUT a contact filter to capture what else was happening in {owner_name}'s life around this date — other conversations, events, and activities provide important wider context even when a specific contact is known.\n\ - 3. Use recall_facts_for_photo to load any previously stored knowledge about subjects in this photo.\n\ - 4. Use recall_entities to look up known people, places, or things that appear in this photo.\n\ - 5. When you identify people, places, events, or notable things in this photo: use store_entity to record them and store_fact to record key facts (relationships, roles, attributes). This builds a persistent memory for future insights.\n\ - 6. Only produce your final insight AFTER you have gathered context from at least 5 tool calls.\n\ - 7. If a tool returns no results, that is useful information — continue calling the remaining tools anyway.\n\ - 8. You have a hard budget of {max_iterations} tool-calling iterations before the loop ends. Plan your context gathering so you can write a complete final insight within that budget.", - owner_id_note = owner_id_note, - fewshot_block = fewshot_block, - owner_name = owner_name, - max_iterations = max_iterations + let system_content = Self::build_system_content( + custom_system_prompt.as_deref(), + owner_id_note.as_deref(), + &fewshot_block, + max_iterations, ); - let system_content = if let Some(ref custom) = custom_system_prompt { - format!("{}\n\n{}", custom, base_system) - } else { - base_system.to_string() - }; // 9. Build user message let gps_info = exif @@ -3353,15 +3392,17 @@ Return ONLY the summary, nothing else."#, .map(|d| format!("Visual description (from local vision model):\n{}\n\n", d)) .unwrap_or_default(); + // Context-only payload — no output-shape prescription. The persona / + // custom_system_prompt owns voice, length, and structure. The "title + // and summary" claim that used to live here was unused (the title is + // regenerated post-hoc from the summary by generate_photo_title). let user_content = format!( - "{visual_block}Please analyze this photo and gather any relevant context from the surrounding weeks.\n\n\ - Photo file path: {}\n\ + "{visual_block}Photo file path: {}\n\ Date taken: {}\n\ {}\n\ {}\n\ {}\n\n\ - Use the available tools to gather more context about this moment (messages, calendar events, location history, etc.), \ - then write a detailed insight with a title and summary.", + Gather context with the available tools, then respond.", file_path, date_taken.format("%B %d, %Y"), contact_info, @@ -3923,6 +3964,44 @@ mod tests { assert_eq!(out, "11 chars"); } + #[test] + fn build_system_content_uses_custom_prompt_verbatim_for_identity() { + let out = InsightGenerator::build_system_content( + Some("You are a journal writer in first person, warm and reflective."), + None, + "", + 6, + ); + assert!( + out.starts_with("You are a journal writer in first person, warm and reflective."), + "custom prompt must lead the system content; got: {}", + &out[..out.len().min(200)], + ); + assert!( + !out.contains("personal photo memory assistant"), + "framework identity must not leak when custom prompt is supplied" + ); + assert!(out.contains("Tool-use guidance")); + assert!(out.contains("budget of 6")); + } + + #[test] + fn build_system_content_uses_neutral_default_when_no_custom() { + let out = InsightGenerator::build_system_content(None, None, "", 6); + assert!(out.contains("reconstructing a memory from a photo")); + assert!(!out.contains("personal photo memory assistant")); + assert!(out.contains("Tool-use guidance")); + } + + #[test] + fn build_system_content_includes_fewshot_and_owner_id() { + let owner = "\n\nYour identity in the knowledge store: Alice (entity ID: 7)."; + let fewshot = "## Examples\n\n### Example 1\n...\n\n---\n\n"; + let out = InsightGenerator::build_system_content(None, Some(owner), fewshot, 6); + assert!(out.contains("Alice (entity ID: 7)")); + assert!(out.contains("## Examples")); + } + #[test] fn render_fewshot_empty_returns_empty_string() { assert!(InsightGenerator::render_fewshot_examples(&[]).is_empty()); From 177187f6a2bcfd364cfbefbcd1422d779e7f56d4 Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Thu, 7 May 2026 14:27:59 -0400 Subject: [PATCH 04/19] insight-chat: code-review polish on the system-prompt split - Use Option::map instead of manual match-on-Option (drops clippy::manual_map). - Drop redundant `max_iterations = max_iterations` from the format! call. - Use captured identifiers consistently in the user_content format!. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/ai/insight_generator.rs | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/src/ai/insight_generator.rs b/src/ai/insight_generator.rs index d05cd4d..bd51b32 100644 --- a/src/ai/insight_generator.rs +++ b/src/ai/insight_generator.rs @@ -2980,7 +2980,6 @@ Return ONLY the summary, nothing else."#, - Use recall_facts_for_photo + recall_entities to load any prior knowledge about subjects in the photo.\n\ - When you identify people / places / events / things, use store_entity + store_fact to grow the persistent memory.\n\ - A tool returning no results is informative; continue with the others.", - max_iterations = max_iterations ); let mut out = identity; @@ -3344,8 +3343,8 @@ Return ONLY the summary, nothing else."#, // (when supplied) is the authoritative identity — the framework // never appends a competing "you are a personal photo memory // assistant" line. The procedural block stays identity-free. - let owner_id_note = match owner_entity_id { - Some(id) => Some(format!( + let owner_id_note = owner_entity_id.map(|id| { + format!( "\n\nYour identity in the knowledge store: {name} (entity ID: {id}). \ When storing facts where you ({name}) are the object — for example, someone is your friend, \ sibling, or colleague — use subject_entity_id for the other person and set object_value to \ @@ -3353,9 +3352,8 @@ Return ONLY the summary, nothing else."#, {name} directly, use {id} as the subject_entity_id.", name = owner_name, id = id - )), - None => None, - }; + ) + }); let fewshot_block = Self::render_fewshot_examples(&fewshot_examples); let system_content = Self::build_system_content( custom_system_prompt.as_deref(), @@ -3397,18 +3395,13 @@ Return ONLY the summary, nothing else."#, // and summary" claim that used to live here was unused (the title is // regenerated post-hoc from the summary by generate_photo_title). let user_content = format!( - "{visual_block}Photo file path: {}\n\ - Date taken: {}\n\ - {}\n\ - {}\n\ - {}\n\n\ + "{visual_block}Photo file path: {file_path}\n\ + Date taken: {date}\n\ + {contact_info}\n\ + {gps_info}\n\ + {tags_info}\n\n\ Gather context with the available tools, then respond.", - file_path, - date_taken.format("%B %d, %Y"), - contact_info, - gps_info, - tags_info, - visual_block = visual_block, + date = date_taken.format("%B %d, %Y"), ); // 10. Define tools. Hybrid mode omits `describe_photo` since the From faa289882f3a05d14af7887f2ef517441a8b80ea Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Thu, 7 May 2026 14:34:08 -0400 Subject: [PATCH 05/19] insight-chat: per-turn system_prompt override on chat continuation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Append mode: applied ephemerally — original system message restored before persistence so re-opens see the baked persona. Amend mode: override stays in place and becomes the new insight row's system message. Pattern mirrors annotate_system_with_budget. Adds system_prompt field on both ChatTurnHttpRequest and ChatTurnRequest; plumbs through chat_turn and chat_turn_stream identically. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/ai/handlers.rs | 6 ++ src/ai/insight_chat.rs | 186 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 192 insertions(+) diff --git a/src/ai/handlers.rs b/src/ai/handlers.rs index 7ec6ab7..f6a0a37 100644 --- a/src/ai/handlers.rs +++ b/src/ai/handlers.rs @@ -640,6 +640,10 @@ pub struct ChatTurnHttpRequest { pub min_p: Option, #[serde(default)] pub max_iterations: Option, + /// Per-turn system-prompt override. Ephemeral in append mode, + /// persisted in amend mode. See ChatTurnRequest for semantics. + #[serde(default)] + pub system_prompt: Option, #[serde(default)] pub amend: bool, } @@ -695,6 +699,7 @@ pub async fn chat_turn_handler( top_k: request.top_k, min_p: request.min_p, max_iterations: request.max_iterations, + system_prompt: request.system_prompt.clone(), amend: request.amend, }; @@ -909,6 +914,7 @@ pub async fn chat_stream_handler( top_k: request.top_k, min_p: request.min_p, max_iterations: request.max_iterations, + system_prompt: request.system_prompt.clone(), amend: request.amend, }; diff --git a/src/ai/insight_chat.rs b/src/ai/insight_chat.rs index 792f124..6724f8b 100644 --- a/src/ai/insight_chat.rs +++ b/src/ai/insight_chat.rs @@ -45,6 +45,11 @@ pub struct ChatTurnRequest { pub top_k: Option, pub min_p: Option, pub max_iterations: Option, + /// Per-turn system-prompt override. In append mode (default), applied + /// ephemerally — original system message restored before persistence. + /// In amend mode, persisted into the new insight row's system message. + /// None / empty = no change. + pub system_prompt: Option, /// When true, write a new insight row (regenerating title) instead of /// updating training_messages on the existing row. pub amend: bool, @@ -385,6 +390,13 @@ impl InsightChatService { // 7. Append the new user turn. messages.push(ChatMessage::user(req.user_message.clone())); + // Apply per-turn system-prompt override BEFORE the budget annotation + // so the budget note attaches to the override, not the original. + // The stash is consumed below before persistence (append mode) or + // dropped (amend mode, where the override stays in place). + let override_stash = + apply_system_prompt_override(&mut messages, req.system_prompt.as_deref()); + // Temporarily annotate the system message with this turn's iteration // budget so the model knows how many tool-calling rounds it has. We // restore the original content before persistence so the note doesn't @@ -481,6 +493,14 @@ impl InsightChatService { // before we persist so it doesn't snowball on each subsequent turn. restore_system_content(&mut messages, original_system_content); + // Append mode: undo the per-turn system-prompt override so the + // stored transcript keeps the original baked persona. Amend mode: + // keep the override in place — it becomes the new insight row's + // system message. + if !req.amend { + restore_system_prompt_override(&mut messages, override_stash); + } + // 9. Persist. Append mode rewrites the JSON blob in place; amend // mode regenerates the title and inserts a new insight row, // relying on store_insight to flip prior rows' is_current=false. @@ -812,6 +832,10 @@ impl InsightChatService { messages.push(ChatMessage::user(req.user_message.clone())); + // Mirror chat_turn: per-turn override goes on first, budget note next. + let override_stash = + apply_system_prompt_override(&mut messages, req.system_prompt.as_deref()); + let original_system_content = annotate_system_with_budget(&mut messages, max_iterations); let mut tool_calls_made = 0usize; @@ -946,6 +970,13 @@ impl InsightChatService { // before we persist so it doesn't snowball on each subsequent turn. restore_system_content(&mut messages, original_system_content); + // Append mode: undo the per-turn system-prompt override (mirrors + // chat_turn). Amend mode: keep the override — it becomes the new + // insight row's system message. + if !req.amend { + restore_system_prompt_override(&mut messages, override_stash); + } + // Persist. let json = serde_json::to_string(&messages) .map_err(|e| anyhow!("failed to serialize chat history: {}", e))?; @@ -1153,6 +1184,64 @@ fn restore_system_content(messages: &mut [ChatMessage], original: Option } } +/// Receipt produced by [`apply_system_prompt_override`] so the caller can +/// undo the override before persistence. Two variants because we either +/// replaced an existing system message (need its original content) or +/// prepended a synthetic one (need to pop it). +#[derive(Debug)] +pub(crate) enum SystemPromptStash { + Replaced { original: String }, + Prepended, +} + +/// Apply a per-turn `system_prompt` override to `messages` so the model +/// sees the requested persona for this turn. Returns a stash the caller +/// must pass to [`restore_system_prompt_override`] before persisting the +/// transcript — without that step, append-mode chat would silently +/// rewrite the stored persona. +/// +/// No-op (returns `None`) when `override_prompt` is `None` or empty. +pub(crate) fn apply_system_prompt_override( + messages: &mut Vec, + override_prompt: Option<&str>, +) -> Option { + let prompt = match override_prompt { + Some(s) if !s.trim().is_empty() => s.trim().to_string(), + _ => return None, + }; + if let Some(first) = messages.first_mut() + && first.role == "system" + { + let original = std::mem::replace(&mut first.content, prompt); + return Some(SystemPromptStash::Replaced { original }); + } + messages.insert(0, ChatMessage::system(prompt)); + Some(SystemPromptStash::Prepended) +} + +/// Undo an override previously applied by [`apply_system_prompt_override`]. +/// No-op when `stash` is `None`. +pub(crate) fn restore_system_prompt_override( + messages: &mut Vec, + stash: Option, +) { + let Some(stash) = stash else { return }; + match stash { + SystemPromptStash::Replaced { original } => { + if let Some(first) = messages.first_mut() + && first.role == "system" + { + first.content = original; + } + } + SystemPromptStash::Prepended => { + if !messages.is_empty() && messages[0].role == "system" { + messages.remove(0); + } + } + } +} + /// View returned to clients for chat-UI rendering. #[derive(Debug)] pub struct HistoryView { @@ -1386,4 +1475,101 @@ mod tests { let cut = find_raw_cut(&msgs, 2).expect("boundary cut should succeed"); assert_eq!(cut, msgs.len()); } + + #[test] + fn apply_override_replaces_existing_system_message() { + let mut msgs = vec![ + ChatMessage::system("original persona"), + ChatMessage::user("hi"), + ]; + let stash = apply_system_prompt_override(&mut msgs, Some("new persona")); + assert_eq!(msgs[0].content, "new persona"); + match stash { + Some(SystemPromptStash::Replaced { original }) => { + assert_eq!(original, "original persona"); + } + other => panic!("expected Replaced, got {:?}", other), + } + } + + #[test] + fn apply_override_prepends_synthetic_when_missing() { + let mut msgs = vec![ChatMessage::user("hi")]; + let stash = apply_system_prompt_override(&mut msgs, Some("new persona")); + assert_eq!(msgs.len(), 2); + assert_eq!(msgs[0].role, "system"); + assert_eq!(msgs[0].content, "new persona"); + assert!(matches!(stash, Some(SystemPromptStash::Prepended))); + } + + #[test] + fn apply_override_no_op_when_none() { + let mut msgs = vec![ + ChatMessage::system("sys"), + ChatMessage::user("hi"), + ]; + let stash = apply_system_prompt_override(&mut msgs, None); + assert!(stash.is_none()); + assert_eq!(msgs[0].content, "sys"); + } + + #[test] + fn apply_override_no_op_for_empty_string() { + let mut msgs = vec![ChatMessage::system("sys")]; + let stash = apply_system_prompt_override(&mut msgs, Some("")); + assert!(stash.is_none()); + assert_eq!(msgs[0].content, "sys"); + } + + #[test] + fn restore_override_replaces_back() { + let mut msgs = vec![ + ChatMessage::system("new"), + ChatMessage::user("hi"), + ]; + restore_system_prompt_override( + &mut msgs, + Some(SystemPromptStash::Replaced { original: "original".to_string() }), + ); + assert_eq!(msgs[0].content, "original"); + assert_eq!(msgs.len(), 2); + } + + #[test] + fn restore_override_pops_synthetic() { + let mut msgs = vec![ + ChatMessage::system("new"), + ChatMessage::user("hi"), + ]; + restore_system_prompt_override(&mut msgs, Some(SystemPromptStash::Prepended)); + assert_eq!(msgs.len(), 1); + assert_eq!(msgs[0].role, "user"); + } + + #[test] + fn override_round_trip_preserves_original_system_message() { + let mut msgs = vec![ + ChatMessage::system("original persona"), + ChatMessage::user("first user"), + assistant_text("first reply"), + ]; + let stash = apply_system_prompt_override(&mut msgs, Some("ephemeral persona")); + assert_eq!(msgs[0].content, "ephemeral persona"); + restore_system_prompt_override(&mut msgs, stash); + assert_eq!(msgs[0].content, "original persona"); + assert_eq!(msgs.len(), 3); + assert_eq!(msgs[1].role, "user"); + assert_eq!(msgs[2].role, "assistant"); + } + + #[test] + fn override_with_synthetic_round_trip_drops_extra_message() { + let mut msgs = vec![ChatMessage::user("first user")]; + let stash = apply_system_prompt_override(&mut msgs, Some("ephemeral")); + assert_eq!(msgs.len(), 2); + assert_eq!(msgs[0].role, "system"); + restore_system_prompt_override(&mut msgs, stash); + assert_eq!(msgs.len(), 1); + assert_eq!(msgs[0].role, "user"); + } } From 428f24b0f8282b59e8e5b5a7d8397d09a8084d56 Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Thu, 7 May 2026 14:40:04 -0400 Subject: [PATCH 06/19] insight-chat: code-review polish on the chat system_prompt override - Trim the override input once via Option::map(str::trim).filter(...). - Use matches!() in restore_system_prompt_override's Prepended arm so it reads consistently with the Replaced arm. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/ai/insight_chat.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/ai/insight_chat.rs b/src/ai/insight_chat.rs index 6724f8b..e782070 100644 --- a/src/ai/insight_chat.rs +++ b/src/ai/insight_chat.rs @@ -1205,10 +1205,7 @@ pub(crate) fn apply_system_prompt_override( messages: &mut Vec, override_prompt: Option<&str>, ) -> Option { - let prompt = match override_prompt { - Some(s) if !s.trim().is_empty() => s.trim().to_string(), - _ => return None, - }; + let prompt = override_prompt.map(str::trim).filter(|s| !s.is_empty())?.to_string(); if let Some(first) = messages.first_mut() && first.role == "system" { @@ -1235,7 +1232,7 @@ pub(crate) fn restore_system_prompt_override( } } SystemPromptStash::Prepended => { - if !messages.is_empty() && messages[0].role == "system" { + if matches!(messages.first(), Some(m) if m.role == "system") { messages.remove(0); } } From 659e7bd973e0721548643c01d84f1313cec8ce8f Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Thu, 7 May 2026 14:42:42 -0400 Subject: [PATCH 07/19] insight-chat: get_sms_messages tool now honors days_radius MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The agentic tool definition advertised a days_radius parameter but sms_client::fetch_messages_for_contact was hardcoded to ±4 days, silently ignoring whatever value the LLM chose. Plumb the parameter through; default 4 retained at the tool level for back-compat. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/ai/insight_generator.rs | 6 ++--- src/ai/sms_client.rs | 51 +++++++++++++++++++++++++++---------- 2 files changed, 41 insertions(+), 16 deletions(-) diff --git a/src/ai/insight_generator.rs b/src/ai/insight_generator.rs index bd51b32..ca28115 100644 --- a/src/ai/insight_generator.rs +++ b/src/ai/insight_generator.rs @@ -983,7 +983,7 @@ impl InsightGenerator { // Step 1: Get FULL immediate temporal context (±4 days, ALL messages) let immediate_messages = self .sms_client - .fetch_messages_for_contact(contact.as_deref(), timestamp) + .fetch_messages_for_contact(contact.as_deref(), timestamp, 4) .await .unwrap_or_else(|e| { log::error!("Failed to fetch immediate messages: {}", e); @@ -1129,7 +1129,7 @@ impl InsightGenerator { log::info!("Using traditional time-based message retrieval (±4 days)"); let sms_messages = self .sms_client - .fetch_messages_for_contact(contact.as_deref(), timestamp) + .fetch_messages_for_contact(contact.as_deref(), timestamp, 4) .await .unwrap_or_else(|e| { log::error!("Failed to fetch SMS messages: {}", e); @@ -1835,7 +1835,7 @@ Return ONLY the summary, nothing else."#, match self .sms_client - .fetch_messages_for_contact(contact.as_deref(), timestamp) + .fetch_messages_for_contact(contact.as_deref(), timestamp, days_radius) .await { Ok(messages) if !messages.is_empty() => { diff --git a/src/ai/sms_client.rs b/src/ai/sms_client.rs index ad6d28e..46db005 100644 --- a/src/ai/sms_client.rs +++ b/src/ai/sms_client.rs @@ -20,31 +20,35 @@ impl SmsApiClient { } } - /// Fetch messages for a specific contact within ±4 days of the given timestamp - /// Falls back to all contacts if no messages found for the specific contact - /// Messages are sorted by proximity to the center timestamp + /// Compute a `[start, end]` unix-second window of `2 * radius_days` + /// centered on `center_ts`. `radius_days < 1` is clamped to 1 to avoid + /// degenerate zero-width windows. + pub(crate) fn window_for_radius(center_ts: i64, radius_days: i64) -> (i64, i64) { + let r = radius_days.max(1); + let span = r * 86400; + (center_ts - span, center_ts + span) + } + + /// Fetch messages for a specific contact within ±`radius_days` of the + /// given timestamp. Falls back to all contacts when no messages found + /// for the named contact. Sorted by proximity to the center timestamp. pub async fn fetch_messages_for_contact( &self, contact: Option<&str>, center_timestamp: i64, + radius_days: i64, ) -> Result> { - use chrono::Duration; + let (start_ts, end_ts) = Self::window_for_radius(center_timestamp, radius_days); - // Calculate ±4 days range around the center timestamp let center_dt = chrono::DateTime::from_timestamp(center_timestamp, 0) .ok_or_else(|| anyhow::anyhow!("Invalid timestamp"))?; - let start_dt = center_dt - Duration::days(4); - let end_dt = center_dt + Duration::days(4); - - let start_ts = start_dt.timestamp(); - let end_ts = end_dt.timestamp(); - // If contact specified, try fetching for that contact first if let Some(contact_name) = contact { log::info!( - "Fetching SMS for contact: {} (±4 days from {})", + "Fetching SMS for contact: {} (±{} days from {})", contact_name, + radius_days.max(1), center_dt.format("%Y-%m-%d %H:%M:%S") ); let messages = self @@ -68,7 +72,8 @@ impl SmsApiClient { // Fallback to all contacts log::info!( - "Fetching all SMS messages (±4 days from {})", + "Fetching all SMS messages (±{} days from {})", + radius_days.max(1), center_dt.format("%Y-%m-%d %H:%M:%S") ); self.fetch_messages(start_ts, end_ts, None, Some(center_timestamp)) @@ -379,3 +384,23 @@ struct SmsSearchResponse { #[serde(default)] search_method: String, } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn window_for_radius_produces_2n_day_span() { + let center: i64 = 1_700_000_000; + let (start, end) = SmsApiClient::window_for_radius(center, 7); + assert_eq!(end - start, 14 * 86400); + assert_eq!(start + 7 * 86400, center); + assert_eq!(end - 7 * 86400, center); + } + + #[test] + fn window_for_radius_clamps_negative_to_one() { + let (start, end) = SmsApiClient::window_for_radius(100_000, 0); + assert_eq!(end - start, 2 * 86400); + } +} From b02da0d0cc38a82f75fb42cd4fca3060b052c514 Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Thu, 7 May 2026 14:47:46 -0400 Subject: [PATCH 08/19] insight-chat: code-review polish on the days_radius fix - Bind effective_radius once in fetch_messages_for_contact so the log output and window math share a single source of truth for the clamp. - Clamp tool-supplied days_radius to [1, 30] at the tool boundary so a runaway LLM value can't produce a thousand-day window. - Split the negative-input test into a real negative-input case alongside the zero-input case. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/ai/insight_generator.rs | 3 ++- src/ai/sms_client.rs | 13 ++++++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/ai/insight_generator.rs b/src/ai/insight_generator.rs index ca28115..911068b 100644 --- a/src/ai/insight_generator.rs +++ b/src/ai/insight_generator.rs @@ -1812,7 +1812,8 @@ Return ONLY the summary, nothing else."#, let days_radius = args .get("days_radius") .and_then(|v| v.as_i64()) - .unwrap_or(4); + .unwrap_or(4) + .clamp(1, 30); let limit = args .get("limit") .and_then(|v| v.as_i64()) diff --git a/src/ai/sms_client.rs b/src/ai/sms_client.rs index 46db005..a4843ee 100644 --- a/src/ai/sms_client.rs +++ b/src/ai/sms_client.rs @@ -38,6 +38,7 @@ impl SmsApiClient { center_timestamp: i64, radius_days: i64, ) -> Result> { + let effective_radius = radius_days.max(1); let (start_ts, end_ts) = Self::window_for_radius(center_timestamp, radius_days); let center_dt = chrono::DateTime::from_timestamp(center_timestamp, 0) @@ -48,7 +49,7 @@ impl SmsApiClient { log::info!( "Fetching SMS for contact: {} (±{} days from {})", contact_name, - radius_days.max(1), + effective_radius, center_dt.format("%Y-%m-%d %H:%M:%S") ); let messages = self @@ -73,7 +74,7 @@ impl SmsApiClient { // Fallback to all contacts log::info!( "Fetching all SMS messages (±{} days from {})", - radius_days.max(1), + effective_radius, center_dt.format("%Y-%m-%d %H:%M:%S") ); self.fetch_messages(start_ts, end_ts, None, Some(center_timestamp)) @@ -399,8 +400,14 @@ mod tests { } #[test] - fn window_for_radius_clamps_negative_to_one() { + fn window_for_radius_clamps_zero_to_one() { let (start, end) = SmsApiClient::window_for_radius(100_000, 0); assert_eq!(end - start, 2 * 86400); } + + #[test] + fn window_for_radius_clamps_negative_to_one() { + let (start, end) = SmsApiClient::window_for_radius(100_000, -7); + assert_eq!(end - start, 2 * 86400); + } } From f50d32667b0e9374f4d3175c0eadac7a6b2ecc46 Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Thu, 7 May 2026 14:56:58 -0400 Subject: [PATCH 09/19] insight-chat: ToolGateOpts + per-tool description rewrites Tools whose backing tables are empty (calendar, location_history, daily_summaries) drop out of the catalog so the LLM doesn't waste iteration budget calling them only to receive "no results found". Vision and apollo gates already existed; this generalizes the pattern. search_messages gains start_ts/end_ts/contact_id filters (date filter is a client-side post-filter; SMS-API only accepts contact_id natively on the search endpoint). Descriptions follow a consistent convention: one sentence (what + when), param semantics, examples for tools with non-obvious param choices. No more all-caps headers, no more identity-prescriptive language inside descriptions. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/ai/insight_chat.rs | 20 +- src/ai/insight_generator.rs | 629 +++++++++++++++++------------- src/ai/sms_client.rs | 41 ++ src/database/daily_summary_dao.rs | 25 ++ 4 files changed, 437 insertions(+), 278 deletions(-) diff --git a/src/ai/insight_chat.rs b/src/ai/insight_chat.rs index e782070..93c3e55 100644 --- a/src/ai/insight_chat.rs +++ b/src/ai/insight_chat.rs @@ -364,10 +364,12 @@ impl InsightChatService { .map(|imgs| !imgs.is_empty()) .unwrap_or(false); let offer_describe_tool = !is_hybrid && local_first_user_has_image; - let tools = InsightGenerator::build_tool_definitions( - offer_describe_tool, - self.generator.apollo_enabled(), - ); + // current_gate_opts(has_vision) sets gate_opts.has_vision = has_vision + // and probes the per-table presence flags. Pass `offer_describe_tool` + // directly — the `!is_hybrid && local_first_user_has_image` decision + // is the chat-path's vision predicate. + let gate_opts = self.generator.current_gate_opts(offer_describe_tool); + let tools = InsightGenerator::build_tool_definitions(gate_opts); // Image base64 only needed when describe_photo is on the menu. Load // lazily to avoid disk IO when the loop never invokes it. @@ -810,10 +812,12 @@ impl InsightChatService { .map(|imgs| !imgs.is_empty()) .unwrap_or(false); let offer_describe_tool = !is_hybrid && local_first_user_has_image; - let tools = InsightGenerator::build_tool_definitions( - offer_describe_tool, - self.generator.apollo_enabled(), - ); + // current_gate_opts(has_vision) sets gate_opts.has_vision = has_vision + // and probes the per-table presence flags. Pass `offer_describe_tool` + // directly — the `!is_hybrid && local_first_user_has_image` decision + // is the chat-path's vision predicate. + let gate_opts = self.generator.current_gate_opts(offer_describe_tool); + let tools = InsightGenerator::build_tool_definitions(gate_opts); let image_base64: Option = if offer_describe_tool { self.generator.load_image_as_base64(&normalized).ok() diff --git a/src/ai/insight_generator.rs b/src/ai/insight_generator.rs index 911068b..1cba095 100644 --- a/src/ai/insight_generator.rs +++ b/src/ai/insight_generator.rs @@ -89,6 +89,19 @@ pub struct InsightGenerator { libraries: Vec, } +/// Per-call gating flags for `build_tool_definitions`. Tools whose backing +/// data is empty (or whose env-var guard is unset) are dropped from the +/// catalog so the LLM doesn't reach for a tool that always returns "No +/// results found." — that wastes iteration budget. +#[derive(Debug, Clone, Copy, Default)] +pub struct ToolGateOpts { + pub has_vision: bool, + pub apollo_enabled: bool, + pub daily_summaries_present: bool, + pub calendar_present: bool, + pub location_history_present: bool, +} + impl InsightGenerator { pub fn new( ollama: OllamaClient, @@ -130,6 +143,45 @@ impl InsightGenerator { self.apollo_client.is_enabled() } + /// Compute the per-call tool gate options by probing each backing + /// table. Cheap (`SELECT 1 FROM LIMIT 1` shape via the existing + /// count methods); meant to be called once per chat turn / generation. + /// `has_vision` is supplied by the caller because it depends on the + /// model selected for this turn, not on persistent state. + pub fn current_gate_opts(&self, has_vision: bool) -> ToolGateOpts { + let cx = opentelemetry::Context::new(); + let calendar_present = { + let mut dao = self + .calendar_dao + .lock() + .expect("Unable to lock CalendarEventDao"); + dao.get_event_count(&cx).map(|n| n > 0).unwrap_or(false) + }; + let location_history_present = { + let mut dao = self + .location_dao + .lock() + .expect("Unable to lock LocationHistoryDao"); + dao.get_location_count(&cx).map(|n| n > 0).unwrap_or(false) + }; + let daily_summaries_present = { + let mut dao = self + .daily_summary_dao + .lock() + .expect("Unable to lock DailySummaryDao"); + dao.get_total_summary_count(&cx) + .map(|n| n > 0) + .unwrap_or(false) + }; + ToolGateOpts { + has_vision, + apollo_enabled: self.apollo_enabled(), + daily_summaries_present, + calendar_present, + location_history_present, + } + } + /// Resolve `rel_path` against the configured libraries, returning the /// first root under which the file exists. Insights may be generated /// for any library — the generator itself doesn't know which — so we @@ -1711,24 +1763,21 @@ Return ONLY the summary, nothing else."#, } /// Tool: search_messages — keyword / semantic / hybrid search over all - /// SMS message bodies via the Django FTS5 + embeddings pipeline. Unlike - /// `search_rag` (daily summaries, date-weighted) this hits raw message - /// text across time and is the right choice for exact phrases, proper - /// nouns, URLs, or anything where specific wording matters. + /// SMS message bodies via the Django FTS5 + embeddings pipeline. Now + /// supports optional `contact_id`, `start_ts`, `end_ts` filters. async fn tool_search_messages(&self, args: &serde_json::Value) -> String { let query = match args.get("query").and_then(|v| v.as_str()) { Some(q) if !q.trim().is_empty() => q.trim(), _ => { - // Redirect when the model reached for this tool with a - // date/contact-shaped intent — get_sms_messages is the right - // call. Without this hint, small models often just retry - // search_messages again with the same args. - let has_date = args.get("date").is_some(); - let has_contact = args.get("contact").is_some(); + let has_date = args.get("date").is_some() + || args.get("start_ts").is_some() + || args.get("end_ts").is_some(); + let has_contact = args.get("contact").is_some() + || args.get("contact_id").is_some(); if has_date || has_contact { return "Error: search_messages needs a 'query' (keywords/phrase). \ - To fetch messages around a date or from a contact, call \ - get_sms_messages with { date, contact? } instead." + To fetch messages around a date or from a contact without keywords, \ + call get_sms_messages with { date, contact? } instead." .to_string(); } return "Error: missing required parameter 'query'".to_string(); @@ -1748,51 +1797,86 @@ Return ONLY the summary, nothing else."#, mode ); } - let limit = args + let user_limit = args .get("limit") .and_then(|v| v.as_i64()) .unwrap_or(20) .clamp(1, 50) as usize; + let contact_id = args.get("contact_id").and_then(|v| v.as_i64()); + let start_ts = args.get("start_ts").and_then(|v| v.as_i64()); + let end_ts = args.get("end_ts").and_then(|v| v.as_i64()); + let has_date_filter = start_ts.is_some() || end_ts.is_some(); + + // When a date filter is supplied, fetch a larger pool from SMS-API + // so in-window matches that ranked lower than out-of-window ones + // aren't lost. + let fetch_limit = if has_date_filter { 100 } else { user_limit }; log::info!( - "tool_search_messages: query='{}', mode={}, limit={}", - query, - mode, - limit + "tool_search_messages: query='{}', mode={}, contact_id={:?}, range=[{:?}, {:?}], user_limit={}, fetch_limit={}", + query, mode, contact_id, start_ts, end_ts, user_limit, fetch_limit ); - match self.sms_client.search_messages(query, &mode, limit).await { - Ok(hits) if hits.is_empty() => "No messages matched.".to_string(), - Ok(hits) => { - let mut out = String::new(); - out.push_str(&format!( - "Found {} messages (mode: {}):\n\n", - hits.len(), - mode - )); - let user_name = user_display_name(); - for h in hits { - let date = chrono::DateTime::from_timestamp(h.date, 0) - .map(|dt| dt.format("%Y-%m-%d").to_string()) - .unwrap_or_else(|| h.date.to_string()); - let direction: &str = if h.type_ == 2 { - &user_name - } else { - &h.contact_name - }; - let score = h - .similarity_score - .map(|s| format!(" [score {:.2}]", s)) - .unwrap_or_default(); - out.push_str(&format!( - "[{}]{} {} — {}\n\n", - date, score, direction, h.body - )); + let hits = match self + .sms_client + .search_messages_with_contact(query, &mode, fetch_limit, contact_id) + .await + { + Ok(h) => h, + Err(e) => return format!("Error searching messages: {}", e), + }; + + // Date-range post-filter on the client side. SMS-API's /search/ + // doesn't accept date params; mirroring Apollo's pattern here. + let filtered: Vec<_> = hits + .into_iter() + .filter(|h| { + if let Some(s) = start_ts + && h.date < s + { + return false; } - out - } - Err(e) => format!("Error searching messages: {}", e), + if let Some(e) = end_ts + && h.date > e + { + return false; + } + true + }) + .take(user_limit) + .collect(); + + if filtered.is_empty() { + return "No messages matched.".to_string(); } + + let user_name = user_display_name(); + let mut out = String::new(); + out.push_str(&format!( + "Found {} messages (mode: {}{}):\n\n", + filtered.len(), + mode, + if has_date_filter { ", date-filtered" } else { "" } + )); + for h in filtered { + let date = chrono::DateTime::from_timestamp(h.date, 0) + .map(|dt| dt.format("%Y-%m-%d").to_string()) + .unwrap_or_else(|| h.date.to_string()); + let direction: &str = if h.type_ == 2 { + &user_name + } else { + &h.contact_name + }; + let score = h + .similarity_score + .map(|s| format!(" [score {:.2}]", s)) + .unwrap_or_default(); + out.push_str(&format!( + "[{}]{} {} — {}\n\n", + date, score, direction, h.body + )); + } + out } /// Tool: get_sms_messages — fetch SMS messages near a date for a contact @@ -2485,283 +2569,238 @@ Return ONLY the summary, nothing else."#, // ── Agentic insight generation ────────────────────────────────────── - /// Build the list of tool definitions for the agentic loop - pub(crate) fn build_tool_definitions(has_vision: bool, apollo_enabled: bool) -> Vec { - let mut tools = vec![ - Tool::function( + /// Build the list of tool definitions for the agentic loop, gated by + /// `opts`. Always-on tools: `search_messages`, `get_sms_messages`, + /// `get_file_tags`, `reverse_geocode`, `get_current_datetime`, the + /// four knowledge-memory tools. Conditional: `describe_photo` (vision + /// model), `get_personal_place_at` (Apollo configured), `search_rag` + /// (daily_summaries populated), `get_calendar_events` (calendar + /// populated), `get_location_history` (location history populated). + pub(crate) fn build_tool_definitions(opts: ToolGateOpts) -> Vec { + let mut tools: Vec = Vec::new(); + + if opts.daily_summaries_present { + tools.push(Tool::function( "search_rag", - "Search conversation history using semantic search. Use this to find relevant past conversations about specific topics, people, or events.", + "Date-anchored semantic search over the user's daily-summary corpus. \ + Returns up to `limit` summaries most semantically similar to `query`, \ + weighted toward summaries near `date`. For raw message text across all \ + time, prefer `search_messages`. \ + Examples: `{query: \"family dinner\", date: \"2018-12-24\"}` — what \ + daily summaries near Christmas Eve mention family / dinner / gathering. \ + `{query: \"work travel\", date: \"2019-06-15\", contact: \"Alice\"}` — \ + narrowed to summaries that involve Alice.", serde_json::json!({ "type": "object", "required": ["query", "date"], "properties": { - "query": { - "type": "string", - "description": "The search query to find relevant conversations" - }, - "date": { - "type": "string", - "description": "The reference date in YYYY-MM-DD format" - }, - "contact": { - "type": "string", - "description": "Optional contact name to filter results" - }, - "limit": { - "type": "integer", - "description": "Maximum number of results to return (default: 10, max: 25)" - } - } - }), - ), - Tool::function( - "search_messages", - "CONTENT search over SMS message bodies by keywords/phrases/topics across all time. Use when you're looking for specific wording (phrases, proper nouns, URLs, topics) and DON'T have a date in mind. NOT for time-based queries — if you know the date or want messages around a date, call get_sms_messages instead. Modes: 'fts5' (keyword, supports \"phrase\" / prefix* / AND / NEAR(w1 w2, 5)), 'semantic' (embedding similarity), 'hybrid' (recommended — merges both via reciprocal rank fusion).", - serde_json::json!({ - "type": "object", - "required": ["query"], - "properties": { - "query": { - "type": "string", - "description": "Search query. Min 3 chars. For fts5 mode, supports phrase (\"\"), prefix (*), AND/OR/NOT, and NEAR proximity." - }, - "mode": { - "type": "string", - "enum": ["fts5", "semantic", "hybrid"], - "description": "Search strategy. Default: hybrid." - }, - "limit": { - "type": "integer", - "description": "Maximum number of results (default: 20, max: 50)" - } - } - }), - ), - Tool::function( - "get_sms_messages", - "TIME-BASED fetch of SMS/text messages around a specific date (and optionally from a specific contact). Returns the actual message conversation for that window. Use this whenever you know the date or want the context around a photo's timestamp. Omit contact to search across all conversations. For keyword/topic search without a date, use search_messages instead.", - serde_json::json!({ - "type": "object", - "required": ["date"], - "properties": { - "date": { - "type": "string", - "description": "The center date in YYYY-MM-DD format" - }, - "contact": { - "type": "string", - "description": "Optional contact name to filter messages. If omitted, searches all conversations." - }, - "days_radius": { - "type": "integer", - "description": "Number of days before and after the date to search (default: 4)" - }, - "limit": { - "type": "integer", - "description": "Maximum number of messages to return (default: 60, max: 150)" - } - } - }), - ), - Tool::function( - "get_calendar_events", - "Fetch calendar events near a specific date. Shows scheduled events, meetings, and activities.", - serde_json::json!({ - "type": "object", - "required": ["date"], - "properties": { - "date": { - "type": "string", - "description": "The center date in YYYY-MM-DD format" - }, - "days_radius": { - "type": "integer", - "description": "Number of days before and after the date to search (default: 7)" - }, - "limit": { - "type": "integer", - "description": "Maximum number of events to return (default: 20, max: 50)" - } - } - }), - ), - Tool::function( - "get_location_history", - "Fetch location history records near a specific date. Shows places visited and activities.", - serde_json::json!({ - "type": "object", - "required": ["date"], - "properties": { - "date": { - "type": "string", - "description": "The center date in YYYY-MM-DD format" - }, - "days_radius": { - "type": "integer", - "description": "Number of days before and after the date to search (default: 14)" - } - } - }), - ), - Tool::function( - "get_file_tags", - "Get tags/labels that have been applied to a specific photo file.", - serde_json::json!({ - "type": "object", - "required": ["file_path"], - "properties": { - "file_path": { - "type": "string", - "description": "The file path of the photo to get tags for" - } - } - }), - ), - ]; - - tools.push(Tool::function( - "reverse_geocode", - "Convert GPS latitude/longitude coordinates to a human-readable place name (city, state). Use this when GPS coordinates are available in the photo metadata, or to resolve coordinates returned by get_location_history.", - serde_json::json!({ - "type": "object", - "required": ["latitude", "longitude"], - "properties": { - "latitude": { - "type": "number", - "description": "GPS latitude in decimal degrees" - }, - "longitude": { - "type": "number", - "description": "GPS longitude in decimal degrees" - } - } - }), - )); - - // Personal place lookup. Only registered when the integration is - // enabled — otherwise the LLM gets a tool that always errors. - if apollo_enabled { - tools.push(Tool::function( - "get_personal_place_at", - "Get the user's personal, named place (e.g. Home, Work, Cabin) at a GPS coordinate, if any. Returns the place name, category, free-text description (the user's own notes about the location), and radius. More specific than reverse_geocode — prefer this when both apply.", - serde_json::json!({ - "type": "object", - "required": ["latitude", "longitude"], - "properties": { - "latitude": { "type": "number", "description": "GPS latitude in decimal degrees" }, - "longitude": { "type": "number", "description": "GPS longitude in decimal degrees" } + "query": { "type": "string", "description": "Free-text query, semantically matched." }, + "date": { "type": "string", "description": "Anchor date, YYYY-MM-DD. Summaries near this date rank higher." }, + "contact": { "type": "string", "description": "Optional contact name to bias toward conversations with that person." }, + "limit": { "type": "integer", "description": "Max summaries to return (default 10, max 25)." } + } + }), + )); + } + + tools.push(Tool::function( + "search_messages", + "Search SMS/MMS message bodies. Modes: `fts5` (keyword + phrase + prefix + AND/OR/NOT + NEAR proximity), \ + `semantic` (embedding similarity, requires generated embeddings), `hybrid` (RRF merge, recommended; \ + degrades to fts5 when embeddings absent). Optional `start_ts` / `end_ts` (real-UTC unix seconds) and \ + `contact_id` filters. For pure date / contact browsing without keywords, prefer `get_sms_messages`. \ + Examples: `{query: \"trader joe's\"}` — phrase across all time. \ + `{query: \"dinner\", contact_id: 42, start_ts: 1700000000, end_ts: 1700604800}` — keyword within a contact and a week. \ + `{query: \"NEAR(meeting work, 5)\"}` — proximity search.", + serde_json::json!({ + "type": "object", + "required": ["query"], + "properties": { + "query": { "type": "string", "description": "Search query. Min 3 chars. fts5 supports phrase (\"\"), prefix (*), AND/OR/NOT, and NEAR proximity." }, + "mode": { "type": "string", "enum": ["fts5", "semantic", "hybrid"], "description": "Search strategy. Default: hybrid." }, + "limit": { "type": "integer", "description": "Max results (default 20, max 50)." }, + "contact_id": { "type": "integer", "description": "Optional numeric contact id to scope the search." }, + "start_ts": { "type": "integer", "description": "Optional inclusive lower bound, real-UTC unix seconds." }, + "end_ts": { "type": "integer", "description": "Optional inclusive upper bound, real-UTC unix seconds." } + } + }), + )); + + tools.push(Tool::function( + "get_sms_messages", + "Fetch SMS/MMS messages near a date (and optionally from a specific contact). Use when you know the date \ + or want context around a photo's timestamp. For keyword search without a date, use `search_messages`. \ + Returns up to `limit` messages within `±days_radius` of `date`, sorted by proximity. \ + Example: `{date: \"2018-08-12\", contact: \"Mom\", days_radius: 2}` — messages from Mom within ±2 days of Aug 12 2018.", + serde_json::json!({ + "type": "object", + "required": ["date"], + "properties": { + "date": { "type": "string", "description": "Center date, YYYY-MM-DD." }, + "contact": { "type": "string", "description": "Optional contact name (case-insensitive). Falls back to all contacts on no match." }, + "days_radius": { "type": "integer", "description": "Days before and after to include (default 4)." }, + "limit": { "type": "integer", "description": "Max messages to return (default 60, max 150)." } + } + }), + )); + + if opts.calendar_present { + tools.push(Tool::function( + "get_calendar_events", + "Fetch calendar events near a date — meetings, scheduled activities, all-day events. \ + Returns events within `±days_radius` of `date`. \ + Example: `{date: \"2019-03-22\", days_radius: 3}` — events within a week of March 22 2019.", + serde_json::json!({ + "type": "object", + "required": ["date"], + "properties": { + "date": { "type": "string", "description": "Center date, YYYY-MM-DD." }, + "days_radius": { "type": "integer", "description": "Days before and after to include (default 7)." }, + "limit": { "type": "integer", "description": "Max events to return (default 20, max 50)." } + } + }), + )); + } + + if opts.location_history_present { + tools.push(Tool::function( + "get_location_history", + "Fetch raw location records (lat/lon/timestamp/activity) near a date. The default 14-day radius is \ + wide because location density varies; tighten to ±1 day for a single-trip query. For a coordinate's \ + named place, use `reverse_geocode` (or `get_personal_place_at` when Apollo is enabled).", + serde_json::json!({ + "type": "object", + "required": ["date"], + "properties": { + "date": { "type": "string", "description": "Center date, YYYY-MM-DD." }, + "days_radius": { "type": "integer", "description": "Days before and after to include (default 14)." } + } + }), + )); + } + + tools.push(Tool::function( + "get_file_tags", + "Get user-applied tags for a specific photo file path. Tags are user-curated, not auto-detected.", + serde_json::json!({ + "type": "object", + "required": ["file_path"], + "properties": { + "file_path": { "type": "string", "description": "File path of the photo." } + } + }), + )); + + tools.push(Tool::function( + "reverse_geocode", + "Convert GPS lat/lon to a human-readable place name (city, state). Use for any coordinate the LLM has \ + obtained from EXIF or `get_location_history`. When Apollo is configured, prefer `get_personal_place_at` \ + — it returns the user's named places (Home / Work / etc.) which are more specific.", + serde_json::json!({ + "type": "object", + "required": ["latitude", "longitude"], + "properties": { + "latitude": { "type": "number", "description": "Decimal degrees." }, + "longitude": { "type": "number", "description": "Decimal degrees." } + } + }), + )); + + if opts.apollo_enabled { + tools.push(Tool::function( + "get_personal_place_at", + "Return any of the user's named Places (e.g. Home, Work, Cabin) whose radius contains (latitude, longitude). \ + Smallest radius first — most specific match wins. More specific than `reverse_geocode`; prefer this when \ + both apply. Returns place name, category, free-text description, and radius.", + serde_json::json!({ + "type": "object", + "required": ["latitude", "longitude"], + "properties": { + "latitude": { "type": "number", "description": "Decimal degrees." }, + "longitude": { "type": "number", "description": "Decimal degrees." } } }), )); } - // Knowledge memory tools tools.push(Tool::function( "recall_entities", - "Search the knowledge memory for people, places, events, or things previously learned from other photos. Use this to retrieve context about subjects appearing in this photo.", + "Search the persistent knowledge memory for previously learned people, places, events, or things. \ + Use BEFORE writing the insight to ground the model on what's already known.", serde_json::json!({ "type": "object", "properties": { - "name": { - "type": "string", - "description": "Name or partial name to search for (case-insensitive substring match)" - }, - "entity_type": { - "type": "string", - "enum": ["person", "place", "event", "thing"], - "description": "Filter by entity type (optional)" - }, - "limit": { - "type": "integer", - "description": "Maximum number of results to return (default: 20, max: 50)" - } + "name": { "type": "string", "description": "Name or partial name (case-insensitive substring match)." }, + "entity_type": { "type": "string", "enum": ["person", "place", "event", "thing"] }, + "limit": { "type": "integer", "description": "Max results (default 20, max 50)." } } }), )); tools.push(Tool::function( "recall_facts_for_photo", - "Retrieve all known facts linked to a specific photo from the knowledge memory. Use this at the start of insight generation to load any previously stored knowledge about subjects in this photo.", + "Retrieve all stored facts linked to a specific photo. Call at the start of insight generation to load \ + prior knowledge about subjects in this photo without scanning the whole knowledge base.", serde_json::json!({ "type": "object", "required": ["file_path"], "properties": { - "file_path": { - "type": "string", - "description": "The file path of the photo to retrieve facts for" - } + "file_path": { "type": "string", "description": "File path of the photo." } } }), )); tools.push(Tool::function( "store_entity", - "Store or update a person, place, event, or thing in the knowledge memory. Call this when you identify a subject in this photo that should be remembered for future insights.", + "Upsert a person / place / event / thing into the knowledge memory. Returns the entity id (use it as \ + `subject_entity_id` or `object_entity_id` in `store_fact`). Idempotent on canonical name.", serde_json::json!({ "type": "object", "required": ["name", "entity_type"], "properties": { - "name": { - "type": "string", - "description": "The canonical name of the entity (e.g. 'John Smith', 'Banff National Park')" - }, - "entity_type": { - "type": "string", - "enum": ["person", "place", "event", "thing"], - "description": "The type of entity" - }, - "description": { - "type": "string", - "description": "A brief description of the entity" - } + "name": { "type": "string", "description": "Canonical name (e.g. \"John Smith\", \"Banff National Park\")." }, + "entity_type": { "type": "string", "enum": ["person", "place", "event", "thing"] }, + "description": { "type": "string", "description": "Brief description." } } }), )); tools.push(Tool::function( "store_fact", - "Record a fact about an entity in the knowledge memory. Provide EITHER object_entity_id (when the object is a known entity whose ID you have) OR object_value (for free-text attributes). The fact will be linked to the current photo automatically.", + "Record a fact about an entity in the knowledge memory. Always linked to the current photo. \ + You must provide EITHER `object_entity_id` (when the object is itself a stored entity — e.g. \ + person A is_friend_of person B) OR `object_value` (free-text attribute — e.g. role=\"software engineer\"). \ + `object_entity_id` takes precedence when both are present. \ + Examples: \ + `{subject_entity_id: 7, predicate: \"is_friend_of\", object_entity_id: 12}` — links two known entities. \ + `{subject_entity_id: 7, predicate: \"lives_in\", object_value: \"Portland, Oregon\"}` — free-text attribute.", serde_json::json!({ "type": "object", "required": ["subject_entity_id", "predicate"], "properties": { - "subject_entity_id": { - "type": "integer", - "description": "The ID of the entity this fact is about (returned by store_entity or recall_entities)" - }, - "predicate": { - "type": "string", - "description": "The relationship or attribute (e.g. 'is_friend_of', 'located_in', 'attended_event', 'is_sibling_of')" - }, - "object_entity_id": { - "type": "integer", - "description": "Use when the object is a known entity (e.g. another person's entity ID for 'is_friend_of '). Takes precedence over object_value." - }, - "object_value": { - "type": "string", - "description": "Use for free-text attributes where the object is not a stored entity (e.g. 'Portland, Oregon', 'software engineer')" - }, - "photo_role": { - "type": "string", - "description": "How this entity appears in the photo (e.g. 'subject', 'background', 'location'). Defaults to 'subject'." - } + "subject_entity_id": { "type": "integer", "description": "Entity id this fact is about." }, + "predicate": { "type": "string", "description": "Relationship or attribute (e.g. is_friend_of, located_in, attended_event)." }, + "object_entity_id": { "type": "integer", "description": "Use when the object is itself a stored entity. Takes precedence over object_value." }, + "object_value": { "type": "string", "description": "Use for free-text attributes where the object is not a stored entity." }, + "photo_role": { "type": "string", "description": "How this entity appears in the photo (default \"subject\")." } } }), )); tools.push(Tool::function( "get_current_datetime", - "Get the current date and time. Useful for understanding how long ago the photo was taken.", + "Get the current date and time. Useful when reasoning about how long ago a photo was taken.", serde_json::json!({ "type": "object", "properties": {} }), )); - if has_vision { + if opts.has_vision { tools.push(Tool::function( "describe_photo", - "Generate a visual description of the photo. Describes people, location, and activity visible in the image.", + "Generate a visual description of the current photo — people, location, objects, activity visible \ + in the image. Only available with vision-capable models.", serde_json::json!({ "type": "object", "properties": {} @@ -3405,11 +3444,12 @@ Return ONLY the summary, nothing else."#, date = date_taken.format("%B %d, %Y"), ); - // 10. Define tools. Hybrid mode omits `describe_photo` since the - // chat model receives the visual description inline. - let offer_describe_tool = has_vision && !is_hybrid; - let tools = - Self::build_tool_definitions(offer_describe_tool, self.apollo_client.is_enabled()); + // 10. Define tools. Gate flags computed from current data presence; + // hybrid mode omits describe_photo since the chat model receives + // the visual description inline (so we pass `false` for has_vision + // in hybrid mode regardless of the model's actual capability). + let gate_opts = self.current_gate_opts(has_vision && !is_hybrid); + let tools = Self::build_tool_definitions(gate_opts); // 11. Build initial messages. In hybrid mode images are never // attached to the wire message — the description is part of @@ -3690,6 +3730,55 @@ mod tests { use super::*; use crate::ai::ollama::{ToolCall, ToolCallFunction}; + #[test] + fn build_tool_definitions_drops_gated_tools() { + let opts = ToolGateOpts { + has_vision: false, + apollo_enabled: false, + daily_summaries_present: false, + calendar_present: false, + location_history_present: false, + }; + let tools = InsightGenerator::build_tool_definitions(opts); + let names: Vec<&str> = tools.iter().map(|t| t.function.name.as_str()).collect(); + + // Always-on tools survive. + assert!(names.contains(&"search_messages")); + assert!(names.contains(&"get_sms_messages")); + assert!(names.contains(&"get_file_tags")); + assert!(names.contains(&"reverse_geocode")); + assert!(names.contains(&"get_current_datetime")); + assert!(names.contains(&"recall_entities")); + assert!(names.contains(&"recall_facts_for_photo")); + assert!(names.contains(&"store_entity")); + assert!(names.contains(&"store_fact")); + + // Gated tools are absent. + assert!(!names.contains(&"describe_photo")); + assert!(!names.contains(&"get_personal_place_at")); + assert!(!names.contains(&"search_rag")); + assert!(!names.contains(&"get_calendar_events")); + assert!(!names.contains(&"get_location_history")); + } + + #[test] + fn build_tool_definitions_includes_gated_tools_when_present() { + let opts = ToolGateOpts { + has_vision: true, + apollo_enabled: true, + daily_summaries_present: true, + calendar_present: true, + location_history_present: true, + }; + let tools = InsightGenerator::build_tool_definitions(opts); + let names: Vec<&str> = tools.iter().map(|t| t.function.name.as_str()).collect(); + assert!(names.contains(&"describe_photo")); + assert!(names.contains(&"get_personal_place_at")); + assert!(names.contains(&"search_rag")); + assert!(names.contains(&"get_calendar_events")); + assert!(names.contains(&"get_location_history")); + } + fn place(name: &str, description: &str) -> ApolloPlace { ApolloPlace { id: 1, diff --git a/src/ai/sms_client.rs b/src/ai/sms_client.rs index a4843ee..1ead9df 100644 --- a/src/ai/sms_client.rs +++ b/src/ai/sms_client.rs @@ -295,6 +295,47 @@ impl SmsApiClient { Ok(data.results) } + /// Same shape as `search_messages` but with optional `contact_id`. The + /// SMS-API endpoint accepts contact_id natively; date filtering is the + /// caller's responsibility (post-filter on the returned rows). + pub async fn search_messages_with_contact( + &self, + query: &str, + mode: &str, + limit: usize, + contact_id: Option, + ) -> Result> { + let mut url = format!( + "{}/api/messages/search/?q={}&mode={}&limit={}", + self.base_url, + urlencoding::encode(query), + urlencoding::encode(mode), + limit + ); + if let Some(cid) = contact_id { + url.push_str(&format!("&contact_id={}", cid)); + } + + let mut request = self.client.get(&url); + if let Some(token) = &self.token { + request = request.header("Authorization", format!("Bearer {}", token)); + } + + let response = request.send().await?; + if !response.status().is_success() { + let status = response.status(); + let body = response.text().await.unwrap_or_default(); + return Err(anyhow::anyhow!( + "SMS search request failed: {} - {}", + status, + body + )); + } + + let data: SmsSearchResponse = response.json().await?; + Ok(data.results) + } + pub async fn summarize_context( &self, messages: &[SmsMessage], diff --git a/src/database/daily_summary_dao.rs b/src/database/daily_summary_dao.rs index 276a5a2..5614a06 100644 --- a/src/database/daily_summary_dao.rs +++ b/src/database/daily_summary_dao.rs @@ -75,6 +75,13 @@ pub trait DailySummaryDao: Sync + Send { context: &opentelemetry::Context, contact: &str, ) -> Result; + + /// Get total count of all summaries (across all contacts). Used by + /// `current_gate_opts` to check whether daily_summaries are present. + fn get_total_summary_count( + &mut self, + context: &opentelemetry::Context, + ) -> Result; } pub struct SqliteDailySummaryDao { @@ -454,6 +461,24 @@ impl DailySummaryDao for SqliteDailySummaryDao { }) .map_err(|_| DbError::new(DbErrorKind::QueryError)) } + + fn get_total_summary_count( + &mut self, + context: &opentelemetry::Context, + ) -> Result { + trace_db_call(context, "query", "get_total_summary_count", |_span| { + let mut conn = self + .connection + .lock() + .expect("Unable to get DailySummaryDao"); + + diesel::sql_query("SELECT COUNT(*) as count FROM daily_conversation_summaries") + .get_result::(conn.deref_mut()) + .map(|r| r.count) + .map_err(|e| anyhow::anyhow!("Count query error: {:?}", e)) + }) + .map_err(|_| DbError::new(DbErrorKind::QueryError)) + } } // Helper structs for raw SQL queries From e539c083c93cff528095921f95f1794c194cfc1b Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Thu, 7 May 2026 15:07:57 -0400 Subject: [PATCH 10/19] insight-chat: code-review polish on the tool-gating PR - search_messages now delegates to search_messages_with_contact(.., None) so the two methods share a single HTTP path. Drops the dead-code warning and the ~30-line duplication. - DailySummaryDao gains has_any_summaries (LIMIT 1 existence probe) used by current_gate_opts; the SELECT COUNT(*) get_total_summary_count added in the prior commit is removed (it had no other caller). - current_gate_opts doc comment corrected to describe what the probes actually do. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/ai/insight_generator.rs | 15 +++++++------- src/ai/sms_client.rs | 30 ++++----------------------- src/database/daily_summary_dao.rs | 34 +++++++++++++++++++------------ 3 files changed, 33 insertions(+), 46 deletions(-) diff --git a/src/ai/insight_generator.rs b/src/ai/insight_generator.rs index 1cba095..98995e3 100644 --- a/src/ai/insight_generator.rs +++ b/src/ai/insight_generator.rs @@ -144,10 +144,13 @@ impl InsightGenerator { } /// Compute the per-call tool gate options by probing each backing - /// table. Cheap (`SELECT 1 FROM LIMIT 1` shape via the existing - /// count methods); meant to be called once per chat turn / generation. - /// `has_vision` is supplied by the caller because it depends on the - /// model selected for this turn, not on persistent state. + /// table for presence. `daily_summaries_present` uses a `LIMIT 1` + /// existence probe; `calendar_present` and `location_history_present` + /// use the existing `get_event_count` / `get_location_count` + /// methods (small enough that a full `COUNT(*)` is fine). Meant to + /// be called once per chat turn / generation. `has_vision` is + /// supplied by the caller because it depends on the model selected + /// for this turn, not on persistent state. pub fn current_gate_opts(&self, has_vision: bool) -> ToolGateOpts { let cx = opentelemetry::Context::new(); let calendar_present = { @@ -169,9 +172,7 @@ impl InsightGenerator { .daily_summary_dao .lock() .expect("Unable to lock DailySummaryDao"); - dao.get_total_summary_count(&cx) - .map(|n| n > 0) - .unwrap_or(false) + dao.has_any_summaries(&cx).unwrap_or(false) }; ToolGateOpts { has_vision, diff --git a/src/ai/sms_client.rs b/src/ai/sms_client.rs index 1ead9df..cddac00 100644 --- a/src/ai/sms_client.rs +++ b/src/ai/sms_client.rs @@ -261,38 +261,16 @@ impl SmsApiClient { /// - "fts5" keyword-only, supports phrase / prefix / boolean / NEAR /// - "semantic" embedding similarity /// - "hybrid" both merged via reciprocal rank fusion (recommended) + /// + /// Equivalent to `search_messages_with_contact(query, mode, limit, None)`; + /// kept as a convenience for callers that don't filter by contact. pub async fn search_messages( &self, query: &str, mode: &str, limit: usize, ) -> Result> { - let url = format!( - "{}/api/messages/search/?q={}&mode={}&limit={}", - self.base_url, - urlencoding::encode(query), - urlencoding::encode(mode), - limit - ); - - let mut request = self.client.get(&url); - if let Some(token) = &self.token { - request = request.header("Authorization", format!("Bearer {}", token)); - } - - let response = request.send().await?; - if !response.status().is_success() { - let status = response.status(); - let body = response.text().await.unwrap_or_default(); - return Err(anyhow::anyhow!( - "SMS search request failed: {} - {}", - status, - body - )); - } - - let data: SmsSearchResponse = response.json().await?; - Ok(data.results) + self.search_messages_with_contact(query, mode, limit, None).await } /// Same shape as `search_messages` but with optional `contact_id`. The diff --git a/src/database/daily_summary_dao.rs b/src/database/daily_summary_dao.rs index 5614a06..8f1c5a9 100644 --- a/src/database/daily_summary_dao.rs +++ b/src/database/daily_summary_dao.rs @@ -76,12 +76,13 @@ pub trait DailySummaryDao: Sync + Send { contact: &str, ) -> Result; - /// Get total count of all summaries (across all contacts). Used by - /// `current_gate_opts` to check whether daily_summaries are present. - fn get_total_summary_count( + /// Cheap presence check — returns true iff at least one daily summary row + /// exists. Used by gating logic that only needs "is the table empty?", + /// avoiding a `COUNT(*)` full scan on large corpora. + fn has_any_summaries( &mut self, context: &opentelemetry::Context, - ) -> Result; + ) -> Result; } pub struct SqliteDailySummaryDao { @@ -462,20 +463,27 @@ impl DailySummaryDao for SqliteDailySummaryDao { .map_err(|_| DbError::new(DbErrorKind::QueryError)) } - fn get_total_summary_count( - &mut self, - context: &opentelemetry::Context, - ) -> Result { - trace_db_call(context, "query", "get_total_summary_count", |_span| { + fn has_any_summaries(&mut self, context: &opentelemetry::Context) -> Result { + trace_db_call(context, "query", "has_any_summaries", |_span| { let mut conn = self .connection .lock() .expect("Unable to get DailySummaryDao"); - diesel::sql_query("SELECT COUNT(*) as count FROM daily_conversation_summaries") - .get_result::(conn.deref_mut()) - .map(|r| r.count) - .map_err(|e| anyhow::anyhow!("Count query error: {:?}", e)) + #[derive(QueryableByName)] + struct ProbeResult { + #[diesel(sql_type = diesel::sql_types::Integer)] + #[allow(dead_code)] + one: i32, + } + + let rows: Vec = diesel::sql_query( + "SELECT 1 as one FROM daily_conversation_summaries LIMIT 1", + ) + .load(conn.deref_mut()) + .map_err(|e| anyhow::anyhow!("Failed to probe daily summaries: {}", e))?; + + Ok(!rows.is_empty()) }) .map_err(|_| DbError::new(DbErrorKind::QueryError)) } From 1cdc0f6eb98cff3a96d910b9cbc2cee536877c29 Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Thu, 7 May 2026 15:10:31 -0400 Subject: [PATCH 11/19] insight-chat: drop the dead SmsApiClient::search_messages wrapper The post-PR-4 delegation kept it as a convenience for callers that don't filter by contact, but nothing actually uses it. Delete to clear the dead_code warning. search_messages_with_contact remains as the single entry point. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/ai/sms_client.rs | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/src/ai/sms_client.rs b/src/ai/sms_client.rs index cddac00..b59c266 100644 --- a/src/ai/sms_client.rs +++ b/src/ai/sms_client.rs @@ -262,20 +262,8 @@ impl SmsApiClient { /// - "semantic" embedding similarity /// - "hybrid" both merged via reciprocal rank fusion (recommended) /// - /// Equivalent to `search_messages_with_contact(query, mode, limit, None)`; - /// kept as a convenience for callers that don't filter by contact. - pub async fn search_messages( - &self, - query: &str, - mode: &str, - limit: usize, - ) -> Result> { - self.search_messages_with_contact(query, mode, limit, None).await - } - - /// Same shape as `search_messages` but with optional `contact_id`. The - /// SMS-API endpoint accepts contact_id natively; date filtering is the - /// caller's responsibility (post-filter on the returned rows). + /// The SMS-API endpoint accepts `contact_id` natively; date filtering is + /// the caller's responsibility (post-filter on the returned rows). pub async fn search_messages_with_contact( &self, query: &str, From a8433c2e016da5f891f86132a48a28a5b78a2170 Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Thu, 7 May 2026 15:26:32 -0400 Subject: [PATCH 12/19] insight-chat: document the new system_prompt field in CLAUDE.md Add system_prompt to the /insights/chat body schema with a one-paragraph note on the append-vs-amend semantics so future readers find the contract alongside the rest of the chat-continuation docs. Co-Authored-By: Claude Opus 4.7 (1M context) --- CLAUDE.md | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 3567915..1e34b44 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -658,7 +658,12 @@ clients whether chat is available for a given insight. - `POST /insights/chat` runs one turn of the agentic loop against the replayed history. Body: `{ file_path, library?, user_message, model?, backend?, num_ctx?, - temperature?, top_p?, top_k?, min_p?, max_iterations?, amend? }`. + temperature?, top_p?, top_k?, min_p?, max_iterations?, system_prompt?, amend? }`. + `system_prompt` is a per-turn override: in append mode (default) it's applied + ephemerally — the original system message is restored before persistence so + the stored transcript keeps its baked persona. In amend mode the override + stays in place and becomes the new insight row's system message. Mirrors the + internal `annotate_system_with_budget` swap-and-restore pattern. - `POST /insights/chat/stream` is the SSE variant — same request body, response is `text/event-stream` with events: `iteration_start`, `text` (delta), `tool_call`, `tool_result`, `truncated`, `done`, plus a server-emitted `error_message` on From 2a273a3ed95223080b7e41386490176220216677 Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Thu, 7 May 2026 16:41:24 -0400 Subject: [PATCH 13/19] thumbnails: stop video failures from re-logging every watcher tick MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit generate_video_thumbnail used .output().expect(...), which only catches spawn failure — non-zero ffmpeg exits were silently discarded. With no thumbnail and no .unsupported sentinel left behind, the watcher re-detected the file as missing every quick-scan tick and re-logged "New file detected (missing thumbnail)" forever. Mirror the image branch: return io::Result, check status.success(), and write the sentinel from create_thumbnails on failure. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/main.rs | 11 ++++++++++- src/video/actors.rs | 18 +++++++++++++----- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/src/main.rs b/src/main.rs index 68f8f61..a3070b7 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1687,7 +1687,16 @@ fn create_thumbnails(libs: &[libraries::Library], excluded_dirs: &[String]) { ]); debug!("Generating video thumbnail: {:?}", thumb_path); - generate_video_thumbnail(src, &thumb_path); + if let Err(e) = generate_video_thumbnail(src, &thumb_path) { + let sentinel = unsupported_thumbnail_sentinel(&thumb_path); + error!( + "Unable to thumbnail video {:?}: {}. Writing sentinel {:?}", + src, e, sentinel + ); + if let Err(se) = std::fs::write(&sentinel, b"") { + warn!("Failed to write sentinel {:?}: {}", sentinel, se); + } + } video_span.end(); } else if is_image(&entry) { match generate_image_thumbnail(src, &thumb_path) { diff --git a/src/video/actors.rs b/src/video/actors.rs index c7300ef..7722a5d 100644 --- a/src/video/actors.rs +++ b/src/video/actors.rs @@ -107,19 +107,27 @@ pub async fn create_playlist(video_path: &str, playlist_file: &str) -> Result std::io::Result<()> { + let output = Command::new("ffmpeg") .arg("-ss") .arg("3") .arg("-i") - .arg(path.to_str().unwrap()) + .arg(path) .arg("-vframes") .arg("1") .arg("-f") .arg("image2") .arg(destination) - .output() - .expect("Failure to create video frame"); + .output()?; + + if !output.status.success() { + return Err(std::io::Error::other(format!( + "ffmpeg failed ({}): {}", + output.status, + String::from_utf8_lossy(&output.stderr).trim() + ))); + } + Ok(()) } /// Use ffmpeg to extract a 200px-wide thumbnail from formats the `image` crate From b42acbb3f3f72ca9263674f1868528ce98d5e8c1 Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Thu, 7 May 2026 16:42:41 -0400 Subject: [PATCH 14/19] fmt: cargo fmt sweep across drifted files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit No behavior change — purely whitespace/line-break cleanup that had accumulated since the last format run. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/ai/insight_chat.rs | 24 ++++++++++-------------- src/ai/insight_generator.rs | 19 ++++++++++++++----- src/database/daily_summary_dao.rs | 14 +++++--------- src/files.rs | 7 ++++++- src/main.rs | 9 +++------ src/memories.rs | 5 +---- 6 files changed, 39 insertions(+), 39 deletions(-) diff --git a/src/ai/insight_chat.rs b/src/ai/insight_chat.rs index 93c3e55..bbb03dd 100644 --- a/src/ai/insight_chat.rs +++ b/src/ai/insight_chat.rs @@ -1209,7 +1209,10 @@ pub(crate) fn apply_system_prompt_override( messages: &mut Vec, override_prompt: Option<&str>, ) -> Option { - let prompt = override_prompt.map(str::trim).filter(|s| !s.is_empty())?.to_string(); + let prompt = override_prompt + .map(str::trim) + .filter(|s| !s.is_empty())? + .to_string(); if let Some(first) = messages.first_mut() && first.role == "system" { @@ -1505,10 +1508,7 @@ mod tests { #[test] fn apply_override_no_op_when_none() { - let mut msgs = vec![ - ChatMessage::system("sys"), - ChatMessage::user("hi"), - ]; + let mut msgs = vec![ChatMessage::system("sys"), ChatMessage::user("hi")]; let stash = apply_system_prompt_override(&mut msgs, None); assert!(stash.is_none()); assert_eq!(msgs[0].content, "sys"); @@ -1524,13 +1524,12 @@ mod tests { #[test] fn restore_override_replaces_back() { - let mut msgs = vec![ - ChatMessage::system("new"), - ChatMessage::user("hi"), - ]; + let mut msgs = vec![ChatMessage::system("new"), ChatMessage::user("hi")]; restore_system_prompt_override( &mut msgs, - Some(SystemPromptStash::Replaced { original: "original".to_string() }), + Some(SystemPromptStash::Replaced { + original: "original".to_string(), + }), ); assert_eq!(msgs[0].content, "original"); assert_eq!(msgs.len(), 2); @@ -1538,10 +1537,7 @@ mod tests { #[test] fn restore_override_pops_synthetic() { - let mut msgs = vec![ - ChatMessage::system("new"), - ChatMessage::user("hi"), - ]; + let mut msgs = vec![ChatMessage::system("new"), ChatMessage::user("hi")]; restore_system_prompt_override(&mut msgs, Some(SystemPromptStash::Prepended)); assert_eq!(msgs.len(), 1); assert_eq!(msgs[0].role, "user"); diff --git a/src/ai/insight_generator.rs b/src/ai/insight_generator.rs index 98995e3..ebba8be 100644 --- a/src/ai/insight_generator.rs +++ b/src/ai/insight_generator.rs @@ -1773,8 +1773,7 @@ Return ONLY the summary, nothing else."#, let has_date = args.get("date").is_some() || args.get("start_ts").is_some() || args.get("end_ts").is_some(); - let has_contact = args.get("contact").is_some() - || args.get("contact_id").is_some(); + let has_contact = args.get("contact").is_some() || args.get("contact_id").is_some(); if has_date || has_contact { return "Error: search_messages needs a 'query' (keywords/phrase). \ To fetch messages around a date or from a contact without keywords, \ @@ -1815,7 +1814,13 @@ Return ONLY the summary, nothing else."#, log::info!( "tool_search_messages: query='{}', mode={}, contact_id={:?}, range=[{:?}, {:?}], user_limit={}, fetch_limit={}", - query, mode, contact_id, start_ts, end_ts, user_limit, fetch_limit + query, + mode, + contact_id, + start_ts, + end_ts, + user_limit, + fetch_limit ); let hits = match self @@ -1857,7 +1862,11 @@ Return ONLY the summary, nothing else."#, "Found {} messages (mode: {}{}):\n\n", filtered.len(), mode, - if has_date_filter { ", date-filtered" } else { "" } + if has_date_filter { + ", date-filtered" + } else { + "" + } )); for h in filtered { let date = chrono::DateTime::from_timestamp(h.date, 0) @@ -3006,7 +3015,7 @@ Return ONLY the summary, nothing else."#, Some(s) if !s.trim().is_empty() => s.trim().to_string(), _ => String::from( "You are reconstructing a memory from a photo. Use the gathered \ - context to write a thoughtful summary; you decide voice, length, and shape." + context to write a thoughtful summary; you decide voice, length, and shape.", ), }; diff --git a/src/database/daily_summary_dao.rs b/src/database/daily_summary_dao.rs index 8f1c5a9..ec2a161 100644 --- a/src/database/daily_summary_dao.rs +++ b/src/database/daily_summary_dao.rs @@ -79,10 +79,7 @@ pub trait DailySummaryDao: Sync + Send { /// Cheap presence check — returns true iff at least one daily summary row /// exists. Used by gating logic that only needs "is the table empty?", /// avoiding a `COUNT(*)` full scan on large corpora. - fn has_any_summaries( - &mut self, - context: &opentelemetry::Context, - ) -> Result; + fn has_any_summaries(&mut self, context: &opentelemetry::Context) -> Result; } pub struct SqliteDailySummaryDao { @@ -477,11 +474,10 @@ impl DailySummaryDao for SqliteDailySummaryDao { one: i32, } - let rows: Vec = diesel::sql_query( - "SELECT 1 as one FROM daily_conversation_summaries LIMIT 1", - ) - .load(conn.deref_mut()) - .map_err(|e| anyhow::anyhow!("Failed to probe daily summaries: {}", e))?; + let rows: Vec = + diesel::sql_query("SELECT 1 as one FROM daily_conversation_summaries LIMIT 1") + .load(conn.deref_mut()) + .map_err(|e| anyhow::anyhow!("Failed to probe daily summaries: {}", e))?; Ok(!rows.is_empty()) }) diff --git a/src/files.rs b/src/files.rs index 7db246f..90d6eca 100644 --- a/src/files.rs +++ b/src/files.rs @@ -1718,7 +1718,12 @@ mod tests { // Mock — files.rs tests don't exercise the date-override endpoints. // Returning a synthetic row keeps the trait satisfied without // depending on private DbError constructors. - Ok(mock_exif_row(library_id, rel_path, Some(date_taken), Some("manual".to_string()))) + Ok(mock_exif_row( + library_id, + rel_path, + Some(date_taken), + Some("manual".to_string()), + )) } fn clear_manual_date_taken( diff --git a/src/main.rs b/src/main.rs index a3070b7..f998160 100644 --- a/src/main.rs +++ b/src/main.rs @@ -995,10 +995,8 @@ async fn upload_image( } }; let perceptual = perceptual_hash::compute(&uploaded_path); - let resolved_date = date_resolver::resolve_date_taken( - &uploaded_path, - exif_data.date_taken, - ); + let resolved_date = + date_resolver::resolve_date_taken(&uploaded_path, exif_data.date_taken); let insert_exif = InsertImageExif { library_id: target_library.id, file_path: relative_path.clone(), @@ -1022,8 +1020,7 @@ async fn upload_image( size_bytes, phash_64: perceptual.map(|h| h.phash_64), dhash_64: perceptual.map(|h| h.dhash_64), - date_taken_source: resolved_date - .map(|r| r.source.as_str().to_string()), + date_taken_source: resolved_date.map(|r| r.source.as_str().to_string()), }; if let Ok(mut dao) = exif_dao.lock() { diff --git a/src/memories.rs b/src/memories.rs index 0153d2b..d31cbb8 100644 --- a/src/memories.rs +++ b/src/memories.rs @@ -213,10 +213,7 @@ pub fn extract_date_from_filename(filename: &str) -> Option Date: Thu, 7 May 2026 17:20:05 -0400 Subject: [PATCH 15/19] thumbnails: align video ffmpeg args with the image path so non-yuvj420p sources work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bare 'ffmpeg -ss 3 -i in -vframes 1 -f image2 out' command failed on sources whose decoded pix_fmt isn't yuvj420p (e.g. older Samsung phone videos in yuv420p). With no -vf filter chain, the decoded frame goes straight to the mjpeg encoder, which rejects it with 'Non full-range YUV is non-standard' and exits non-zero. generate_image_thumbnail_ffmpeg already handles the same class of source for HEIC/RAW by adding -vf scale=200:-1 -c:v mjpeg — the filter chain lets ffmpeg auto-insert the pix_fmt converter the encoder needs. Adopt the same args here. Side benefit: video thumbnails are now 200px wide on disk, matching image thumbnails (previously full-resolution). Pre-existing .unsupported sentinels for videos that hit this failure will need to be deleted manually to retry — they're under $THUMBNAILS//.../*.unsupported. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/video/actors.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/video/actors.rs b/src/video/actors.rs index 7722a5d..cfe2aa7 100644 --- a/src/video/actors.rs +++ b/src/video/actors.rs @@ -108,15 +108,27 @@ pub async fn create_playlist(video_path: &str, playlist_file: &str) -> Result std::io::Result<()> { + // -vf scale + -c:v mjpeg mirrors `generate_image_thumbnail_ffmpeg`. The + // filter chain matters as much as the scale does: without it, ffmpeg + // hands the decoded frame straight to the mjpeg encoder, which rejects + // any non-yuvj420p source ("Non full-range YUV is non-standard"). The + // filter chain lets ffmpeg auto-insert the pix_fmt converter the + // encoder needs, which is how the image-thumbnail path already handles + // the same class of source. let output = Command::new("ffmpeg") + .arg("-y") .arg("-ss") .arg("3") .arg("-i") .arg(path) .arg("-vframes") .arg("1") + .arg("-vf") + .arg("scale=200:-1") .arg("-f") .arg("image2") + .arg("-c:v") + .arg("mjpeg") .arg(destination) .output()?; From 388eb22cd28710fd1e57f2042c53b3bef739bf3c Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Thu, 7 May 2026 17:29:04 -0400 Subject: [PATCH 16/19] Remove full plan file, just keep spec --- specs/002-insight-chat-improvements-plan.md | 2147 ------------------- 1 file changed, 2147 deletions(-) delete mode 100644 specs/002-insight-chat-improvements-plan.md diff --git a/specs/002-insight-chat-improvements-plan.md b/specs/002-insight-chat-improvements-plan.md deleted file mode 100644 index 857c464..0000000 --- a/specs/002-insight-chat-improvements-plan.md +++ /dev/null @@ -1,2147 +0,0 @@ -# Insight Chat Improvements Implementation Plan - -> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. - -**Goal:** Make the insight + chat surface honor user-provided system prompts (no baked POV/format), gate optional tools on data presence, fix the `days_radius` no-op in `get_sms_messages`, and rewrite tool descriptions to a consistent convention. - -**Architecture:** Backend changes in ImageApi (Rust/Actix) split the generation system prompt into identity-vs-procedural blocks, add a `system_prompt` field to chat-turn requests with ephemeral swap-and-restore in append mode and persistence in amend mode, and gate tool registration on a per-turn `ToolGateOpts` struct. Frontend changes in FileViewer-React wire the active persona's prompt into every chat turn and add a one-shot "style note" composer affordance. - -**Tech Stack:** Rust (actix-web, diesel, kamadak-exif, serde), TypeScript (React Native, expo-router, AsyncStorage, react-native-sse), SQLite. Tests: `cargo test` for ImageApi. - -**Spec:** `specs/002-insight-chat-improvements.md` - -**Branch:** `feature/insight-chat-improvements` (already created in both `ImageApi/` and `FileViewer-React/`). - ---- - -## File structure - -### ImageApi (Rust) — files modified - -| File | Responsibility | -|---|---| -| `src/ai/insight_generator.rs` | Split system-prompt assembly; neutralize user message; widen `build_tool_definitions(opts: ToolGateOpts)`; add `current_gate_opts()` method to `InsightGenerator` for the chat path; description rewrites; SMS `search_messages` tool gains date+contact_id. | -| `src/ai/insight_chat.rs` | Add `system_prompt: Option` to `ChatTurnRequest`; helper `apply_system_prompt_override` and its restore; pass the override through `chat_turn` + `chat_turn_stream`; persist on amend. | -| `src/ai/handlers.rs` | Add `system_prompt: Option` to `ChatTurnHttpRequest`; plumb through both `chat_turn_handler` and `chat_stream_handler`. | -| `src/ai/sms_client.rs` | Widen `fetch_messages_for_contact` to take `days_radius`. | -| (no new files) | All changes are in-place. | - -### FileViewer-React (TypeScript) — files modified - -| File | Responsibility | -|---|---| -| `hooks/usePersonas.tsx` | Update `DEFAULT_PERSONAS` system prompts to specify voice/shape explicitly. | -| `hooks/useInsightChat.tsx` | `SendTurnOptions` gains `systemPromptOverride?: string \| null`; `sendTurn` reads selected persona and includes `system_prompt` in the request body. | -| `components/InsightChatModal.tsx` | One-shot "Style note" input next to the composer; passes `systemPromptOverride` into `sendTurn`. | - ---- - -## PR 1 — ImageApi: Split system-prompt assembly + neutralize user message - -**Files:** -- Modify: `src/ai/insight_generator.rs:3291-3326` (system-prompt assembly) -- Modify: `src/ai/insight_generator.rs:3356-3371` (user message construction) -- Test: `src/ai/insight_generator.rs` (existing `#[cfg(test)] mod tests` at bottom) - -### Task 1.1: Add a unit test for the new system-prompt builder (failing) - -- [ ] **Step 1: Add a failing test for the new builder** - -Add to the existing `#[cfg(test)] mod tests` block at the bottom of `src/ai/insight_generator.rs`: - -```rust - #[test] - fn build_system_content_uses_custom_prompt_verbatim_for_identity() { - let out = InsightGenerator::build_system_content( - Some("You are a journal writer in first person, warm and reflective."), - None, // no owner_id_note - "", // empty fewshot block - 6, // max_iterations - ); - // The custom prompt is the *only* identity language — no - // "you are a personal photo memory assistant" anywhere. - assert!( - out.starts_with("You are a journal writer in first person, warm and reflective."), - "custom prompt must lead the system content; got: {}", - &out[..out.len().min(200)], - ); - assert!( - !out.contains("personal photo memory assistant"), - "framework identity must not leak when custom prompt is supplied" - ); - // Procedural block must still arrive. - assert!(out.contains("Tool-use guidance")); - assert!(out.contains("budget of 6")); - } - - #[test] - fn build_system_content_uses_neutral_default_when_no_custom() { - let out = InsightGenerator::build_system_content(None, None, "", 6); - // Neutral default — does not assert a strong identity. - assert!(out.contains("reconstructing a memory from a photo")); - assert!(!out.contains("personal photo memory assistant")); - // Procedural block still present. - assert!(out.contains("Tool-use guidance")); - } - - #[test] - fn build_system_content_includes_fewshot_and_owner_id() { - let owner = "\n\nYour identity in the knowledge store: Alice (entity ID: 7)."; - let fewshot = "## Examples\n\n### Example 1\n...\n\n---\n\n"; - let out = InsightGenerator::build_system_content(None, Some(owner), fewshot, 6); - assert!(out.contains("Alice (entity ID: 7)")); - assert!(out.contains("## Examples")); - } -``` - -- [ ] **Step 2: Run the test to verify it fails** - -```bash -cd /home/cameron/development/opencode/ImageApi -cargo test --lib build_system_content -- --nocapture -``` - -Expected: compile error — `build_system_content` is not yet a function on `InsightGenerator`. - -### Task 1.2: Implement `build_system_content` - -- [ ] **Step 1: Add the new helper as an associated function on `InsightGenerator`** - -In `src/ai/insight_generator.rs`, add this method inside the `impl InsightGenerator { ... }` block (above `generate_agentic_insight_for_photo`): - -```rust - /// Assemble the chat system prompt from two named blocks: - /// - /// 1. **Identity / voice / format** — `custom_system_prompt` verbatim - /// when supplied, or a neutral default that doesn't fight a future - /// persona. The framework never asserts an identity that could - /// contradict the persona. - /// 2. **Procedural scaffolding** — tool-use guidance, iteration budget, - /// contact-filter rule. Identity-free; never asserts voice or shape. - /// - /// `owner_id_note` and `fewshot_block` are pre-rendered strings (they - /// already encode their own headers / blank lines). Pass empty / None - /// to skip. - pub(crate) fn build_system_content( - custom_system_prompt: Option<&str>, - owner_id_note: Option<&str>, - fewshot_block: &str, - max_iterations: usize, - ) -> String { - let identity = match custom_system_prompt { - Some(s) if !s.trim().is_empty() => s.trim().to_string(), - _ => String::from( - "You are reconstructing a memory from a photo. Use the gathered \ - context to write a thoughtful summary; you decide voice, length, and shape." - ), - }; - - let owner = owner_id_note.unwrap_or(""); - - let procedural = format!( - "Tool-use guidance:\n\ - - You have a budget of {max_iterations} tool-calling iterations.\n\ - - Call tools to gather context BEFORE writing your final answer; don't answer after one or two calls.\n\ - - When calling get_sms_messages or search_rag, make at least one call WITHOUT a contact filter \ - — surrounding events matter even when a contact is known.\n\ - - Use recall_facts_for_photo + recall_entities to load any prior knowledge about subjects in the photo.\n\ - - When you identify people / places / events / things, use store_entity + store_fact to grow the persistent memory.\n\ - - A tool returning no results is informative; continue with the others.", - max_iterations = max_iterations - ); - - // Compose: identity, then owner note (if any), then fewshot block - // (already self-delimited), then procedural. - let mut out = identity; - if !owner.is_empty() { - out.push_str(owner); - } - out.push_str("\n\n"); - if !fewshot_block.is_empty() { - out.push_str(fewshot_block); - } - out.push_str(&procedural); - out - } -``` - -- [ ] **Step 2: Run the tests to verify they pass** - -```bash -cd /home/cameron/development/opencode/ImageApi -cargo test --lib build_system_content -- --nocapture -``` - -Expected: all three tests pass. - -### Task 1.3: Wire `build_system_content` into the agentic flow - -- [ ] **Step 1: Replace the inlined system-prompt construction** - -In `src/ai/insight_generator.rs`, find the block at lines 3291-3326 that begins with `// 8. Build system message` and ends with `let system_content = if let Some(ref custom) = custom_system_prompt { ... };`. - -Replace that whole block (everything from `let owner_id_note = match owner_entity_id { ... }` through the end of `let system_content = ...;`) with: - -```rust - // 8. Build system message via the two-block helper. Custom prompt - // (when supplied) is the authoritative identity — the framework - // never appends a competing "you are a personal photo memory - // assistant" line. The procedural block stays identity-free. - let owner_id_note = match owner_entity_id { - Some(id) => Some(format!( - "\n\nYour identity in the knowledge store: {name} (entity ID: {id}). \ - When storing facts where you ({name}) are the object — for example, someone is your friend, \ - sibling, or colleague — use subject_entity_id for the other person and set object_value to \ - \"{name}\" (or use store_fact with the other person as subject). When storing facts about \ - {name} directly, use {id} as the subject_entity_id.", - name = owner_name, - id = id - )), - None => None, - }; - let fewshot_block = Self::render_fewshot_examples(&fewshot_examples); - let system_content = Self::build_system_content( - custom_system_prompt.as_deref(), - owner_id_note.as_deref(), - &fewshot_block, - max_iterations, - ); -``` - -- [ ] **Step 2: Build to confirm the wire-up compiles** - -```bash -cd /home/cameron/development/opencode/ImageApi -cargo build 2>&1 | tail -20 -``` - -Expected: clean build (no errors). - -- [ ] **Step 3: Re-run the unit tests to confirm regression-free** - -```bash -cd /home/cameron/development/opencode/ImageApi -cargo test --lib insight_generator -- --nocapture 2>&1 | tail -30 -``` - -Expected: all tests in `insight_generator` pass. - -### Task 1.4: Neutralize the user message - -- [ ] **Step 1: Replace the user-content template** - -In `src/ai/insight_generator.rs` around line 3356, find: - -```rust - let user_content = format!( - "{visual_block}Please analyze this photo and gather any relevant context from the surrounding weeks.\n\n\ - Photo file path: {}\n\ - Date taken: {}\n\ - {}\n\ - {}\n\ - {}\n\n\ - Use the available tools to gather more context about this moment (messages, calendar events, location history, etc.), \ - then write a detailed insight with a title and summary.", - file_path, - date_taken.format("%B %d, %Y"), - contact_info, - gps_info, - tags_info, - visual_block = visual_block, - ); -``` - -Replace with: - -```rust - // Context-only payload — no output-shape prescription. The persona / - // custom_system_prompt owns voice, length, and structure. The "title - // and summary" claim that used to live here was unused (the title is - // regenerated post-hoc from the summary by generate_photo_title). - let user_content = format!( - "{visual_block}Photo file path: {}\n\ - Date taken: {}\n\ - {}\n\ - {}\n\ - {}\n\n\ - Gather context with the available tools, then respond.", - file_path, - date_taken.format("%B %d, %Y"), - contact_info, - gps_info, - tags_info, - visual_block = visual_block, - ); -``` - -- [ ] **Step 2: Build to confirm the change compiles** - -```bash -cd /home/cameron/development/opencode/ImageApi -cargo build 2>&1 | tail -10 -``` - -Expected: clean build. - -### Task 1.5: Commit PR 1 - -- [ ] **Step 1: Run the full test suite** - -```bash -cd /home/cameron/development/opencode/ImageApi -cargo test --lib 2>&1 | tail -20 -``` - -Expected: all tests pass. - -- [ ] **Step 2: Stage and commit** - -```bash -cd /home/cameron/development/opencode/ImageApi -git add src/ai/insight_generator.rs -git commit -m "$(cat <<'EOF' -insight-chat: split generation system prompt into identity + procedural blocks - -The framework no longer asserts "you are a personal photo memory -assistant" alongside a user-supplied custom_system_prompt — the -persona is the authoritative identity. The procedural block (tool-use -guidance, iteration budget) stays identity-free. - -The user message also stops asking for "a detailed insight with a -title and summary" since the title is regenerated post-hoc anyway and -the wording was constraining voice for no data-model benefit. - -Co-Authored-By: Claude Opus 4.7 (1M context) -EOF -)" -``` - ---- - -## PR 2 — ImageApi: `system_prompt` field on chat request + swap/restore + amend persistence - -**Files:** -- Modify: `src/ai/insight_chat.rs:30-51` (`ChatTurnRequest` struct) -- Modify: `src/ai/insight_chat.rs:208-550` (`chat_turn`) -- Modify: `src/ai/insight_chat.rs:659-1016` (`run_streaming_turn`) -- Modify: `src/ai/insight_chat.rs:1122-1154` (annotate/restore helpers — companion to a new helper) -- Modify: `src/ai/handlers.rs:621-645` (`ChatTurnHttpRequest`) -- Modify: `src/ai/handlers.rs:686-699` and `:900-913` (request mapping in both handlers) -- Test: `src/ai/insight_chat.rs` (existing `#[cfg(test)] mod tests` at bottom) - -### Task 2.1: Failing tests for the override behavior - -- [ ] **Step 1: Add unit tests for the new helpers** - -Add to the `#[cfg(test)] mod tests` block at the bottom of `src/ai/insight_chat.rs`: - -```rust - #[test] - fn apply_override_replaces_existing_system_message() { - let mut msgs = vec![ - ChatMessage::system("original persona"), - ChatMessage::user("hi"), - ]; - let stash = apply_system_prompt_override(&mut msgs, Some("new persona")); - assert_eq!(msgs[0].content, "new persona"); - // Stash carries the data we'll need to restore. - match stash { - Some(SystemPromptStash::Replaced { original }) => { - assert_eq!(original, "original persona"); - } - other => panic!("expected Replaced, got {:?}", other), - } - } - - #[test] - fn apply_override_prepends_synthetic_when_missing() { - let mut msgs = vec![ChatMessage::user("hi")]; - let stash = apply_system_prompt_override(&mut msgs, Some("new persona")); - assert_eq!(msgs.len(), 2); - assert_eq!(msgs[0].role, "system"); - assert_eq!(msgs[0].content, "new persona"); - assert!(matches!(stash, Some(SystemPromptStash::Prepended))); - } - - #[test] - fn apply_override_no_op_when_none() { - let mut msgs = vec![ - ChatMessage::system("sys"), - ChatMessage::user("hi"), - ]; - let stash = apply_system_prompt_override(&mut msgs, None); - assert!(stash.is_none()); - assert_eq!(msgs[0].content, "sys"); - } - - #[test] - fn apply_override_no_op_for_empty_string() { - let mut msgs = vec![ChatMessage::system("sys")]; - let stash = apply_system_prompt_override(&mut msgs, Some("")); - assert!(stash.is_none()); - assert_eq!(msgs[0].content, "sys"); - } - - #[test] - fn restore_override_replaces_back() { - let mut msgs = vec![ - ChatMessage::system("new"), - ChatMessage::user("hi"), - ]; - restore_system_prompt_override( - &mut msgs, - Some(SystemPromptStash::Replaced { original: "original".to_string() }), - ); - assert_eq!(msgs[0].content, "original"); - assert_eq!(msgs.len(), 2); - } - - #[test] - fn restore_override_pops_synthetic() { - let mut msgs = vec![ - ChatMessage::system("new"), - ChatMessage::user("hi"), - ]; - restore_system_prompt_override(&mut msgs, Some(SystemPromptStash::Prepended)); - assert_eq!(msgs.len(), 1); - assert_eq!(msgs[0].role, "user"); - } -``` - -- [ ] **Step 2: Run the tests to verify they fail** - -```bash -cd /home/cameron/development/opencode/ImageApi -cargo test --lib insight_chat::tests::apply_override -- --nocapture -``` - -Expected: compile errors — `apply_system_prompt_override`, `restore_system_prompt_override`, and `SystemPromptStash` are not yet defined. - -### Task 2.2: Implement override apply/restore helpers - -- [ ] **Step 1: Add the stash type and the two helpers** - -In `src/ai/insight_chat.rs`, just below the `restore_system_content` function (around line 1154), add: - -```rust -/// Receipt produced by [`apply_system_prompt_override`] so the caller can -/// undo the override before persistence. Two variants because we either -/// replaced an existing system message (need its original content) or -/// prepended a synthetic one (need to pop it). -#[derive(Debug)] -pub(crate) enum SystemPromptStash { - Replaced { original: String }, - Prepended, -} - -/// Apply a per-turn `system_prompt` override to `messages` so the model -/// sees the requested persona for this turn. Returns a stash the caller -/// must pass to [`restore_system_prompt_override`] before persisting the -/// transcript — without that step, append-mode chat would silently -/// rewrite the stored persona. -/// -/// No-op (returns `None`) when `override_prompt` is `None` or empty. -pub(crate) fn apply_system_prompt_override( - messages: &mut Vec, - override_prompt: Option<&str>, -) -> Option { - let prompt = match override_prompt { - Some(s) if !s.trim().is_empty() => s.trim().to_string(), - _ => return None, - }; - if let Some(first) = messages.first_mut() - && first.role == "system" - { - let original = std::mem::replace(&mut first.content, prompt); - return Some(SystemPromptStash::Replaced { original }); - } - messages.insert(0, ChatMessage::system(prompt)); - Some(SystemPromptStash::Prepended) -} - -/// Undo an override previously applied by [`apply_system_prompt_override`]. -/// No-op when `stash` is `None`. -pub(crate) fn restore_system_prompt_override( - messages: &mut Vec, - stash: Option, -) { - let Some(stash) = stash else { return }; - match stash { - SystemPromptStash::Replaced { original } => { - if let Some(first) = messages.first_mut() - && first.role == "system" - { - first.content = original; - } - } - SystemPromptStash::Prepended => { - if !messages.is_empty() && messages[0].role == "system" { - messages.remove(0); - } - } - } -} -``` - -- [ ] **Step 2: Run the unit tests** - -```bash -cd /home/cameron/development/opencode/ImageApi -cargo test --lib insight_chat::tests -- --nocapture 2>&1 | tail -30 -``` - -Expected: all six new tests pass; existing tests still pass. - -### Task 2.3: Add `system_prompt` to `ChatTurnRequest` - -- [ ] **Step 1: Add the field** - -In `src/ai/insight_chat.rs`, find the `ChatTurnRequest` struct around line 31 and add `system_prompt` between `max_iterations` and `amend`: - -```rust -#[derive(Debug)] -pub struct ChatTurnRequest { - pub library_id: i32, - pub file_path: String, - pub user_message: String, - /// Override the model id. Local mode: an Ollama model name. Hybrid: - /// an OpenRouter id. None defers to the stored insight's `model_version`. - pub model: Option, - /// Override the backend used for this turn. None defers to the stored - /// insight's `backend`. Switching `local -> hybrid` is rejected in v1. - pub backend: Option, - pub num_ctx: Option, - pub temperature: Option, - pub top_p: Option, - pub top_k: Option, - pub min_p: Option, - pub max_iterations: Option, - /// Per-turn system-prompt override. In append mode (default), applied - /// ephemerally — original system message restored before persistence. - /// In amend mode, persisted into the new insight row's system message. - /// None / empty = no change. - pub system_prompt: Option, - /// When true, write a new insight row (regenerating title) instead of - /// updating training_messages on the existing row. - pub amend: bool, -} -``` - -- [ ] **Step 2: Build to confirm** - -```bash -cd /home/cameron/development/opencode/ImageApi -cargo build 2>&1 | tail -10 -``` - -Expected: clean build (the new field is unused so far; that's fine). - -### Task 2.4: Apply override in `chat_turn` (non-streaming) - -- [ ] **Step 1: Hook the override into the loop** - -In `src/ai/insight_chat.rs`, in the `chat_turn` function around line 386, find the line: - -```rust - // 7. Append the new user turn. - messages.push(ChatMessage::user(req.user_message.clone())); - - // Temporarily annotate the system message with this turn's iteration - // budget so the model knows how many tool-calling rounds it has. We - // restore the original content before persistence so the note doesn't - // accumulate across turns. - let original_system_content = annotate_system_with_budget(&mut messages, max_iterations); -``` - -Replace with: - -```rust - // 7. Append the new user turn. - messages.push(ChatMessage::user(req.user_message.clone())); - - // Apply per-turn system-prompt override BEFORE the budget annotation - // so the budget note attaches to the override, not the original. - // The stash is consumed below before persistence (append mode) or - // dropped (amend mode, where the override stays in place). - let override_stash = - apply_system_prompt_override(&mut messages, req.system_prompt.as_deref()); - - // Temporarily annotate the system message with this turn's iteration - // budget so the model knows how many tool-calling rounds it has. We - // restore the original content before persistence so the note doesn't - // accumulate across turns. - let original_system_content = annotate_system_with_budget(&mut messages, max_iterations); -``` - -- [ ] **Step 2: Restore appropriately before persistence** - -Still in `chat_turn`, find: - -```rust - // Drop the per-turn iteration-budget note from the system message - // before we persist so it doesn't snowball on each subsequent turn. - restore_system_content(&mut messages, original_system_content); - - // 9. Persist. Append mode rewrites the JSON blob in place; amend - // mode regenerates the title and inserts a new insight row, - // relying on store_insight to flip prior rows' is_current=false. - let json = serde_json::to_string(&messages) - .map_err(|e| anyhow!("failed to serialize chat history: {}", e))?; -``` - -Replace with: - -```rust - // Drop the per-turn iteration-budget note from the system message - // before we persist so it doesn't snowball on each subsequent turn. - restore_system_content(&mut messages, original_system_content); - - // Append mode: undo the per-turn system-prompt override so the - // stored transcript keeps the original baked persona. Amend mode: - // keep the override in place — it becomes the new insight row's - // system message. - if !req.amend { - restore_system_prompt_override(&mut messages, override_stash); - } - - // 9. Persist. Append mode rewrites the JSON blob in place; amend - // mode regenerates the title and inserts a new insight row, - // relying on store_insight to flip prior rows' is_current=false. - let json = serde_json::to_string(&messages) - .map_err(|e| anyhow!("failed to serialize chat history: {}", e))?; -``` - -- [ ] **Step 3: Build** - -```bash -cd /home/cameron/development/opencode/ImageApi -cargo build 2>&1 | tail -10 -``` - -Expected: clean build. (Note: with `req.amend = false`, the local `override_stash` binding may produce an "unused if amend mode" warning when `amend` is true — silenced by the binding existing in the conditional path. Should compile clean.) - -### Task 2.5: Apply override in `run_streaming_turn` - -- [ ] **Step 1: Hook into the streaming loop** - -In `src/ai/insight_chat.rs`, in `run_streaming_turn` around line 813, find: - -```rust - messages.push(ChatMessage::user(req.user_message.clone())); - - let original_system_content = annotate_system_with_budget(&mut messages, max_iterations); -``` - -Replace with: - -```rust - messages.push(ChatMessage::user(req.user_message.clone())); - - // Mirror chat_turn: per-turn override goes on first, budget note next. - let override_stash = - apply_system_prompt_override(&mut messages, req.system_prompt.as_deref()); - - let original_system_content = annotate_system_with_budget(&mut messages, max_iterations); -``` - -- [ ] **Step 2: Restore before streaming-mode persistence** - -Still in `run_streaming_turn`, find: - -```rust - // Drop the per-turn iteration-budget note from the system message - // before we persist so it doesn't snowball on each subsequent turn. - restore_system_content(&mut messages, original_system_content); - - // Persist. - let json = serde_json::to_string(&messages) - .map_err(|e| anyhow!("failed to serialize chat history: {}", e))?; -``` - -Replace with: - -```rust - // Drop the per-turn iteration-budget note from the system message - // before we persist so it doesn't snowball on each subsequent turn. - restore_system_content(&mut messages, original_system_content); - - // Append mode: undo the per-turn system-prompt override (mirrors - // chat_turn). Amend mode: keep the override — it becomes the new - // insight row's system message. - if !req.amend { - restore_system_prompt_override(&mut messages, override_stash); - } - - // Persist. - let json = serde_json::to_string(&messages) - .map_err(|e| anyhow!("failed to serialize chat history: {}", e))?; -``` - -- [ ] **Step 3: Build** - -```bash -cd /home/cameron/development/opencode/ImageApi -cargo build 2>&1 | tail -10 -``` - -Expected: clean build. - -### Task 2.6: Wire `system_prompt` through HTTP handlers - -- [ ] **Step 1: Add to `ChatTurnHttpRequest`** - -In `src/ai/handlers.rs` around line 622, find the `ChatTurnHttpRequest` struct and add the field: - -```rust -#[derive(Debug, Deserialize)] -pub struct ChatTurnHttpRequest { - pub file_path: String, - #[serde(default)] - pub library: Option, - pub user_message: String, - #[serde(default)] - pub model: Option, - #[serde(default)] - pub backend: Option, - #[serde(default)] - pub num_ctx: Option, - #[serde(default)] - pub temperature: Option, - #[serde(default)] - pub top_p: Option, - #[serde(default)] - pub top_k: Option, - #[serde(default)] - pub min_p: Option, - #[serde(default)] - pub max_iterations: Option, - /// Per-turn system-prompt override. Ephemeral in append mode, - /// persisted in amend mode. See ChatTurnRequest for semantics. - #[serde(default)] - pub system_prompt: Option, - #[serde(default)] - pub amend: bool, -} -``` - -- [ ] **Step 2: Plumb into the non-streaming handler** - -In `src/ai/handlers.rs` around line 686, find: - -```rust - let chat_req = ChatTurnRequest { - library_id: library.id, - file_path: request.file_path.clone(), - user_message: request.user_message.clone(), - model: request.model.clone(), - backend: request.backend.clone(), - num_ctx: request.num_ctx, - temperature: request.temperature, - top_p: request.top_p, - top_k: request.top_k, - min_p: request.min_p, - max_iterations: request.max_iterations, - amend: request.amend, - }; -``` - -Replace with: - -```rust - let chat_req = ChatTurnRequest { - library_id: library.id, - file_path: request.file_path.clone(), - user_message: request.user_message.clone(), - model: request.model.clone(), - backend: request.backend.clone(), - num_ctx: request.num_ctx, - temperature: request.temperature, - top_p: request.top_p, - top_k: request.top_k, - min_p: request.min_p, - max_iterations: request.max_iterations, - system_prompt: request.system_prompt.clone(), - amend: request.amend, - }; -``` - -- [ ] **Step 3: Plumb into the streaming handler** - -Same file, around line 900, find the identical block in `chat_stream_handler` and apply the same change — add `system_prompt: request.system_prompt.clone(),` between `max_iterations` and `amend`. - -- [ ] **Step 4: Build** - -```bash -cd /home/cameron/development/opencode/ImageApi -cargo build 2>&1 | tail -10 -``` - -Expected: clean build. - -### Task 2.7: Add a swap-restore round-trip test - -- [ ] **Step 1: Failing test for round-trip preservation** - -Add to the `#[cfg(test)] mod tests` block in `src/ai/insight_chat.rs`: - -```rust - #[test] - fn override_round_trip_preserves_original_system_message() { - let mut msgs = vec![ - ChatMessage::system("original persona"), - ChatMessage::user("first user"), - assistant_text("first reply"), - ]; - let stash = apply_system_prompt_override(&mut msgs, Some("ephemeral persona")); - assert_eq!(msgs[0].content, "ephemeral persona"); - restore_system_prompt_override(&mut msgs, stash); - assert_eq!(msgs[0].content, "original persona"); - assert_eq!(msgs.len(), 3); - assert_eq!(msgs[1].role, "user"); - assert_eq!(msgs[2].role, "assistant"); - } - - #[test] - fn override_with_synthetic_round_trip_drops_extra_message() { - let mut msgs = vec![ChatMessage::user("first user")]; - let stash = apply_system_prompt_override(&mut msgs, Some("ephemeral")); - assert_eq!(msgs.len(), 2); - assert_eq!(msgs[0].role, "system"); - restore_system_prompt_override(&mut msgs, stash); - assert_eq!(msgs.len(), 1); - assert_eq!(msgs[0].role, "user"); - } -``` - -- [ ] **Step 2: Run them** - -```bash -cd /home/cameron/development/opencode/ImageApi -cargo test --lib insight_chat::tests::override -- --nocapture 2>&1 | tail -20 -``` - -Expected: both pass. - -### Task 2.8: Commit PR 2 - -- [ ] **Step 1: Run the full ImageApi test suite** - -```bash -cd /home/cameron/development/opencode/ImageApi -cargo test --lib 2>&1 | tail -20 -``` - -Expected: all tests pass. - -- [ ] **Step 2: Stage and commit** - -```bash -cd /home/cameron/development/opencode/ImageApi -git add src/ai/insight_chat.rs src/ai/handlers.rs -git commit -m "$(cat <<'EOF' -insight-chat: per-turn system_prompt override on chat continuation - -Append mode: applied ephemerally — original system message restored -before persistence so re-opens see the baked persona. Amend mode: -override stays in place and becomes the new insight row's system -message. Pattern mirrors annotate_system_with_budget. - -Adds system_prompt field on both ChatTurnHttpRequest and ChatTurnRequest; -plumbs through chat_turn and chat_turn_stream identically. - -Co-Authored-By: Claude Opus 4.7 (1M context) -EOF -)" -``` - ---- - -## PR 3 — ImageApi: `fetch_messages_for_contact` honors `days_radius` - -**Files:** -- Modify: `src/ai/sms_client.rs:23-76` (`fetch_messages_for_contact`) -- Modify: `src/ai/insight_generator.rs:1799-1869` (`tool_get_sms_messages`) -- Test: `src/ai/sms_client.rs` (new tests at bottom) - -### Task 3.1: Failing test for the days_radius window math - -- [ ] **Step 1: Add test for the helper that computes the window** - -To `src/ai/sms_client.rs`, add at the bottom: - -```rust -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn window_for_radius_produces_2n_day_span() { - // Helper function under test below. - let center: i64 = 1_700_000_000; // 2023-11-14 22:13:20 UTC - let (start, end) = SmsApiClient::window_for_radius(center, 7); - assert_eq!(end - start, 14 * 86400); - assert_eq!(start + 7 * 86400, center); - assert_eq!(end - 7 * 86400, center); - } - - #[test] - fn window_for_radius_clamps_negative_to_one() { - // Days_radius < 1 falls through to a 1-day radius (so the call - // doesn't return 0 messages from a degenerate window). - let (start, end) = SmsApiClient::window_for_radius(100_000, 0); - assert_eq!(end - start, 2 * 86400); - } -} -``` - -- [ ] **Step 2: Run to verify failure** - -```bash -cd /home/cameron/development/opencode/ImageApi -cargo test --lib sms_client::tests -- --nocapture -``` - -Expected: compile error — `window_for_radius` doesn't exist. - -### Task 3.2: Implement `window_for_radius` and widen `fetch_messages_for_contact` - -- [ ] **Step 1: Add the helper and widen the function** - -In `src/ai/sms_client.rs`, replace the existing `fetch_messages_for_contact` (lines 26-76) with: - -```rust - /// Compute a `[start, end]` unix-second window of `2 * radius_days` - /// centered on `center_ts`. `radius_days < 1` is clamped to 1 to avoid - /// degenerate zero-width windows. - pub(crate) fn window_for_radius(center_ts: i64, radius_days: i64) -> (i64, i64) { - let r = radius_days.max(1); - let span = r * 86400; - (center_ts - span, center_ts + span) - } - - /// Fetch messages for a specific contact within ±`radius_days` of the - /// given timestamp. Falls back to all contacts when no messages found - /// for the named contact. Sorted by proximity to the center timestamp. - pub async fn fetch_messages_for_contact( - &self, - contact: Option<&str>, - center_timestamp: i64, - radius_days: i64, - ) -> Result> { - let (start_ts, end_ts) = Self::window_for_radius(center_timestamp, radius_days); - - let center_dt = chrono::DateTime::from_timestamp(center_timestamp, 0) - .ok_or_else(|| anyhow::anyhow!("Invalid timestamp"))?; - - // If contact specified, try fetching for that contact first - if let Some(contact_name) = contact { - log::info!( - "Fetching SMS for contact: {} (±{} days from {})", - contact_name, - radius_days.max(1), - center_dt.format("%Y-%m-%d %H:%M:%S") - ); - let messages = self - .fetch_messages(start_ts, end_ts, Some(contact_name), Some(center_timestamp)) - .await?; - - if !messages.is_empty() { - log::info!( - "Found {} messages for contact {}", - messages.len(), - contact_name - ); - return Ok(messages); - } - - log::info!( - "No messages found for contact {}, falling back to all contacts", - contact_name - ); - } - - // Fallback to all contacts - log::info!( - "Fetching all SMS messages (±{} days from {})", - radius_days.max(1), - center_dt.format("%Y-%m-%d %H:%M:%S") - ); - self.fetch_messages(start_ts, end_ts, None, Some(center_timestamp)) - .await - } -``` - -- [ ] **Step 2: Run the new tests** - -```bash -cd /home/cameron/development/opencode/ImageApi -cargo test --lib sms_client::tests -- --nocapture 2>&1 | tail -10 -``` - -Expected: both pass. - -### Task 3.3: Update callers to pass `days_radius` - -- [ ] **Step 1: Update `tool_get_sms_messages` to pass `days_radius`** - -In `src/ai/insight_generator.rs` around line 1836-1840, find: - -```rust - match self - .sms_client - .fetch_messages_for_contact(contact.as_deref(), timestamp) - .await -``` - -Replace with: - -```rust - match self - .sms_client - .fetch_messages_for_contact(contact.as_deref(), timestamp, days_radius) - .await -``` - -- [ ] **Step 2: Find any other callers and update them** - -```bash -cd /home/cameron/development/opencode/ImageApi -grep -rn "fetch_messages_for_contact" src/ --include="*.rs" -``` - -Expected output: only the one caller in `insight_generator.rs` (now updated) and the definition in `sms_client.rs`. If any other call sites exist, update each by adding a third argument matching the call's intent — most likely `4` (the historical hardcoded value). - -- [ ] **Step 3: Build** - -```bash -cd /home/cameron/development/opencode/ImageApi -cargo build 2>&1 | tail -10 -``` - -Expected: clean build. - -### Task 3.4: Commit PR 3 - -- [ ] **Step 1: Run the full test suite** - -```bash -cd /home/cameron/development/opencode/ImageApi -cargo test --lib 2>&1 | tail -20 -``` - -Expected: all tests pass. - -- [ ] **Step 2: Stage and commit** - -```bash -cd /home/cameron/development/opencode/ImageApi -git add src/ai/sms_client.rs src/ai/insight_generator.rs -git commit -m "$(cat <<'EOF' -insight-chat: get_sms_messages tool now honors days_radius - -The agentic tool definition advertised a days_radius parameter but -sms_client::fetch_messages_for_contact was hardcoded to ±4 days, -silently ignoring whatever value the LLM chose. Plumb the parameter -through; default 4 retained at the tool level for back-compat. - -Co-Authored-By: Claude Opus 4.7 (1M context) -EOF -)" -``` - ---- - -## PR 4 — ImageApi: `ToolGateOpts` + per-tool description rewrites + SMS tool param expansion - -**Files:** -- Modify: `src/ai/insight_generator.rs:2488-2772` (`build_tool_definitions` + tool definitions) -- Modify: `src/ai/insight_generator.rs:1718-1796` (`tool_search_messages` to accept new params) -- Modify: `src/ai/insight_chat.rs` — chat service must compute `ToolGateOpts` per turn -- Test: `src/ai/insight_generator.rs` (new tests in existing `mod tests`) - -### Task 4.1: Define `ToolGateOpts` - -- [ ] **Step 1: Add the struct** - -In `src/ai/insight_generator.rs`, just above the `impl InsightGenerator { ... }` block that contains `build_tool_definitions` (search for `pub(crate) fn build_tool_definitions`), add: - -```rust -/// Per-call gating flags for `build_tool_definitions`. Tools whose backing -/// data is empty (or whose env-var guard is unset) are dropped from the -/// catalog so the LLM doesn't reach for a tool that always returns "No -/// results found." — that wastes iteration budget. -#[derive(Debug, Clone, Copy, Default)] -pub struct ToolGateOpts { - pub has_vision: bool, - pub apollo_enabled: bool, - pub daily_summaries_present: bool, - pub calendar_present: bool, - pub location_history_present: bool, -} -``` - -- [ ] **Step 2: Build to confirm** - -```bash -cd /home/cameron/development/opencode/ImageApi -cargo build 2>&1 | tail -10 -``` - -Expected: clean build. (Unused-struct warning at this stage is fine.) - -### Task 4.2: Failing test for the new `build_tool_definitions(opts)` signature - -- [ ] **Step 1: Test that gates drop tools when flags are off** - -Add to the `#[cfg(test)] mod tests` block in `src/ai/insight_generator.rs` (near the existing `summarize_*` tests): - -```rust - #[test] - fn build_tool_definitions_drops_gated_tools() { - let opts = ToolGateOpts { - has_vision: false, - apollo_enabled: false, - daily_summaries_present: false, - calendar_present: false, - location_history_present: false, - }; - let tools = InsightGenerator::build_tool_definitions(opts); - let names: Vec<&str> = tools.iter().map(|t| t.function.name.as_str()).collect(); - - // Always-on tools survive. - assert!(names.contains(&"search_messages")); - assert!(names.contains(&"get_sms_messages")); - assert!(names.contains(&"get_file_tags")); - assert!(names.contains(&"reverse_geocode")); - assert!(names.contains(&"get_current_datetime")); - assert!(names.contains(&"recall_entities")); - assert!(names.contains(&"recall_facts_for_photo")); - assert!(names.contains(&"store_entity")); - assert!(names.contains(&"store_fact")); - - // Gated tools are absent. - assert!(!names.contains(&"describe_photo")); - assert!(!names.contains(&"get_personal_place_at")); - assert!(!names.contains(&"search_rag")); - assert!(!names.contains(&"get_calendar_events")); - assert!(!names.contains(&"get_location_history")); - } - - #[test] - fn build_tool_definitions_includes_gated_tools_when_present() { - let opts = ToolGateOpts { - has_vision: true, - apollo_enabled: true, - daily_summaries_present: true, - calendar_present: true, - location_history_present: true, - }; - let tools = InsightGenerator::build_tool_definitions(opts); - let names: Vec<&str> = tools.iter().map(|t| t.function.name.as_str()).collect(); - assert!(names.contains(&"describe_photo")); - assert!(names.contains(&"get_personal_place_at")); - assert!(names.contains(&"search_rag")); - assert!(names.contains(&"get_calendar_events")); - assert!(names.contains(&"get_location_history")); - } -``` - -- [ ] **Step 2: Run to verify failure** - -```bash -cd /home/cameron/development/opencode/ImageApi -cargo test --lib build_tool_definitions -- --nocapture -``` - -Expected: compile error — current signature is `(has_vision: bool, apollo_enabled: bool)`, tests pass `ToolGateOpts`. - -### Task 4.3: Widen `build_tool_definitions` and add gating - -- [ ] **Step 1: Replace the function signature and rewrite description bodies** - -In `src/ai/insight_generator.rs`, replace the entire `build_tool_definitions` function (around lines 2488-2772) with: - -```rust - /// Build the list of tool definitions for the agentic loop, gated by - /// `opts`. Always-on tools: `search_messages`, `get_sms_messages`, - /// `get_file_tags`, `reverse_geocode`, `get_current_datetime`, the - /// four knowledge-memory tools. Conditional: `describe_photo` (vision - /// model), `get_personal_place_at` (Apollo configured), `search_rag` - /// (daily_summaries populated), `get_calendar_events` (calendar - /// populated), `get_location_history` (location history populated). - pub(crate) fn build_tool_definitions(opts: ToolGateOpts) -> Vec { - let mut tools: Vec = Vec::new(); - - if opts.daily_summaries_present { - tools.push(Tool::function( - "search_rag", - "Date-anchored semantic search over the user's daily-summary corpus. \ - Returns up to `limit` summaries most semantically similar to `query`, \ - weighted toward summaries near `date`. For raw message text across all \ - time, prefer `search_messages`. \ - Examples: `{query: \"family dinner\", date: \"2018-12-24\"}` — what \ - daily summaries near Christmas Eve mention family / dinner / gathering. \ - `{query: \"work travel\", date: \"2019-06-15\", contact: \"Alice\"}` — \ - narrowed to summaries that involve Alice.", - serde_json::json!({ - "type": "object", - "required": ["query", "date"], - "properties": { - "query": { "type": "string", "description": "Free-text query, semantically matched." }, - "date": { "type": "string", "description": "Anchor date, YYYY-MM-DD. Summaries near this date rank higher." }, - "contact": { "type": "string", "description": "Optional contact name to bias toward conversations with that person." }, - "limit": { "type": "integer", "description": "Max summaries to return (default 10, max 25)." } - } - }), - )); - } - - tools.push(Tool::function( - "search_messages", - "Search SMS/MMS message bodies. Modes: `fts5` (keyword + phrase + prefix + AND/OR/NOT + NEAR proximity), \ - `semantic` (embedding similarity, requires generated embeddings), `hybrid` (RRF merge, recommended; \ - degrades to fts5 when embeddings absent). Optional `start_ts` / `end_ts` (real-UTC unix seconds) and \ - `contact_id` filters. For pure date / contact browsing without keywords, prefer `get_sms_messages`. \ - Examples: `{query: \"trader joe's\"}` — phrase across all time. \ - `{query: \"dinner\", contact_id: 42, start_ts: 1700000000, end_ts: 1700604800}` — keyword within a contact and a week. \ - `{query: \"NEAR(meeting work, 5)\"}` — proximity search.", - serde_json::json!({ - "type": "object", - "required": ["query"], - "properties": { - "query": { "type": "string", "description": "Search query. Min 3 chars. fts5 supports phrase (\"\"), prefix (*), AND/OR/NOT, and NEAR proximity." }, - "mode": { "type": "string", "enum": ["fts5", "semantic", "hybrid"], "description": "Search strategy. Default: hybrid." }, - "limit": { "type": "integer", "description": "Max results (default 20, max 50)." }, - "contact_id": { "type": "integer", "description": "Optional numeric contact id to scope the search." }, - "start_ts": { "type": "integer", "description": "Optional inclusive lower bound, real-UTC unix seconds." }, - "end_ts": { "type": "integer", "description": "Optional inclusive upper bound, real-UTC unix seconds." } - } - }), - )); - - tools.push(Tool::function( - "get_sms_messages", - "Fetch SMS/MMS messages near a date (and optionally from a specific contact). Use when you know the date \ - or want context around a photo's timestamp. For keyword search without a date, use `search_messages`. \ - Returns up to `limit` messages within `±days_radius` of `date`, sorted by proximity. \ - Example: `{date: \"2018-08-12\", contact: \"Mom\", days_radius: 2}` — messages from Mom within ±2 days of Aug 12 2018.", - serde_json::json!({ - "type": "object", - "required": ["date"], - "properties": { - "date": { "type": "string", "description": "Center date, YYYY-MM-DD." }, - "contact": { "type": "string", "description": "Optional contact name (case-insensitive). Falls back to all contacts on no match." }, - "days_radius": { "type": "integer", "description": "Days before and after to include (default 4)." }, - "limit": { "type": "integer", "description": "Max messages to return (default 60, max 150)." } - } - }), - )); - - if opts.calendar_present { - tools.push(Tool::function( - "get_calendar_events", - "Fetch calendar events near a date — meetings, scheduled activities, all-day events. \ - Returns events within `±days_radius` of `date`. \ - Example: `{date: \"2019-03-22\", days_radius: 3}` — events within a week of March 22 2019.", - serde_json::json!({ - "type": "object", - "required": ["date"], - "properties": { - "date": { "type": "string", "description": "Center date, YYYY-MM-DD." }, - "days_radius": { "type": "integer", "description": "Days before and after to include (default 7)." }, - "limit": { "type": "integer", "description": "Max events to return (default 20, max 50)." } - } - }), - )); - } - - if opts.location_history_present { - tools.push(Tool::function( - "get_location_history", - "Fetch raw location records (lat/lon/timestamp/activity) near a date. The default 14-day radius is \ - wide because location density varies; tighten to ±1 day for a single-trip query. For a coordinate's \ - named place, use `reverse_geocode` (or `get_personal_place_at` when Apollo is enabled).", - serde_json::json!({ - "type": "object", - "required": ["date"], - "properties": { - "date": { "type": "string", "description": "Center date, YYYY-MM-DD." }, - "days_radius": { "type": "integer", "description": "Days before and after to include (default 14)." } - } - }), - )); - } - - tools.push(Tool::function( - "get_file_tags", - "Get user-applied tags for a specific photo file path. Tags are user-curated, not auto-detected.", - serde_json::json!({ - "type": "object", - "required": ["file_path"], - "properties": { - "file_path": { "type": "string", "description": "File path of the photo." } - } - }), - )); - - tools.push(Tool::function( - "reverse_geocode", - "Convert GPS lat/lon to a human-readable place name (city, state). Use for any coordinate the LLM has \ - obtained from EXIF or `get_location_history`. When Apollo is configured, prefer `get_personal_place_at` \ - — it returns the user's named places (Home / Work / etc.) which are more specific.", - serde_json::json!({ - "type": "object", - "required": ["latitude", "longitude"], - "properties": { - "latitude": { "type": "number", "description": "Decimal degrees." }, - "longitude": { "type": "number", "description": "Decimal degrees." } - } - }), - )); - - if opts.apollo_enabled { - tools.push(Tool::function( - "get_personal_place_at", - "Return any of the user's named Places (e.g. Home, Work, Cabin) whose radius contains (latitude, longitude). \ - Smallest radius first — most specific match wins. More specific than `reverse_geocode`; prefer this when \ - both apply. Returns place name, category, free-text description, and radius.", - serde_json::json!({ - "type": "object", - "required": ["latitude", "longitude"], - "properties": { - "latitude": { "type": "number", "description": "Decimal degrees." }, - "longitude": { "type": "number", "description": "Decimal degrees." } - } - }), - )); - } - - tools.push(Tool::function( - "recall_entities", - "Search the persistent knowledge memory for previously learned people, places, events, or things. \ - Use BEFORE writing the insight to ground the model on what's already known.", - serde_json::json!({ - "type": "object", - "properties": { - "name": { "type": "string", "description": "Name or partial name (case-insensitive substring match)." }, - "entity_type": { "type": "string", "enum": ["person", "place", "event", "thing"] }, - "limit": { "type": "integer", "description": "Max results (default 20, max 50)." } - } - }), - )); - - tools.push(Tool::function( - "recall_facts_for_photo", - "Retrieve all stored facts linked to a specific photo. Call at the start of insight generation to load \ - prior knowledge about subjects in this photo without scanning the whole knowledge base.", - serde_json::json!({ - "type": "object", - "required": ["file_path"], - "properties": { - "file_path": { "type": "string", "description": "File path of the photo." } - } - }), - )); - - tools.push(Tool::function( - "store_entity", - "Upsert a person / place / event / thing into the knowledge memory. Returns the entity id (use it as \ - `subject_entity_id` or `object_entity_id` in `store_fact`). Idempotent on canonical name.", - serde_json::json!({ - "type": "object", - "required": ["name", "entity_type"], - "properties": { - "name": { "type": "string", "description": "Canonical name (e.g. \"John Smith\", \"Banff National Park\")." }, - "entity_type": { "type": "string", "enum": ["person", "place", "event", "thing"] }, - "description": { "type": "string", "description": "Brief description." } - } - }), - )); - - tools.push(Tool::function( - "store_fact", - "Record a fact about an entity in the knowledge memory. Always linked to the current photo. \ - You must provide EITHER `object_entity_id` (when the object is itself a stored entity — e.g. \ - person A is_friend_of person B) OR `object_value` (free-text attribute — e.g. role=\"software engineer\"). \ - `object_entity_id` takes precedence when both are present. \ - Examples: \ - `{subject_entity_id: 7, predicate: \"is_friend_of\", object_entity_id: 12}` — links two known entities. \ - `{subject_entity_id: 7, predicate: \"lives_in\", object_value: \"Portland, Oregon\"}` — free-text attribute.", - serde_json::json!({ - "type": "object", - "required": ["subject_entity_id", "predicate"], - "properties": { - "subject_entity_id": { "type": "integer", "description": "Entity id this fact is about." }, - "predicate": { "type": "string", "description": "Relationship or attribute (e.g. is_friend_of, located_in, attended_event)." }, - "object_entity_id": { "type": "integer", "description": "Use when the object is itself a stored entity. Takes precedence over object_value." }, - "object_value": { "type": "string", "description": "Use for free-text attributes where the object is not a stored entity." }, - "photo_role": { "type": "string", "description": "How this entity appears in the photo (default \"subject\")." } - } - }), - )); - - tools.push(Tool::function( - "get_current_datetime", - "Get the current date and time. Useful when reasoning about how long ago a photo was taken.", - serde_json::json!({ - "type": "object", - "properties": {} - }), - )); - - if opts.has_vision { - tools.push(Tool::function( - "describe_photo", - "Generate a visual description of the current photo — people, location, objects, activity visible \ - in the image. Only available with vision-capable models.", - serde_json::json!({ - "type": "object", - "properties": {} - }), - )); - } - - tools - } -``` - -- [ ] **Step 2: Build to find call sites that need updating** - -```bash -cd /home/cameron/development/opencode/ImageApi -cargo build 2>&1 | tail -25 -``` - -Expected: build errors at the call sites in `insight_chat.rs` (`build_tool_definitions(offer_describe_tool, ...)`) and in `insight_generator.rs` itself (the agentic generation site at line 3377). The struct test should now compile and pass after we update the call sites. - -### Task 4.4: Add `current_gate_opts` helper on `InsightGenerator` - -- [ ] **Step 1: Add a method that probes data presence** - -In `src/ai/insight_generator.rs`, inside `impl InsightGenerator { ... }`, add (a sensible location is near `apollo_enabled()` — search for `pub fn apollo_enabled`): - -```rust - /// Compute the per-call tool gate options by probing each backing - /// table. Cheap (`SELECT 1 FROM LIMIT 1` shape via the existing - /// count methods); meant to be called once per chat turn / generation. - /// `has_vision` is supplied by the caller because it depends on the - /// model selected for this turn, not on persistent state. - pub fn current_gate_opts(&self, has_vision: bool) -> ToolGateOpts { - let cx = opentelemetry::Context::new(); - let calendar_present = { - let mut dao = self - .calendar_dao - .lock() - .expect("Unable to lock CalendarEventDao"); - dao.get_event_count(&cx).map(|n| n > 0).unwrap_or(false) - }; - let location_history_present = { - let mut dao = self - .location_dao - .lock() - .expect("Unable to lock LocationHistoryDao"); - dao.get_location_count(&cx).map(|n| n > 0).unwrap_or(false) - }; - let daily_summaries_present = { - let mut dao = self - .daily_summary_dao - .lock() - .expect("Unable to lock DailySummaryDao"); - dao.get_summary_count(&cx).map(|n| n > 0).unwrap_or(false) - }; - ToolGateOpts { - has_vision, - apollo_enabled: self.apollo_enabled(), - daily_summaries_present, - calendar_present, - location_history_present, - } - } -``` - -- [ ] **Step 2: Build** - -```bash -cd /home/cameron/development/opencode/ImageApi -cargo build 2>&1 | tail -10 -``` - -Expected: errors only at the call sites of `build_tool_definitions` (now mismatched signature). New helper compiles cleanly. - -### Task 4.5: Update the agentic generation call site - -- [ ] **Step 1: Switch to the new signature** - -In `src/ai/insight_generator.rs` around line 3373-3377, find: - -```rust - // 10. Define tools. Hybrid mode omits `describe_photo` since the - // chat model receives the visual description inline. - let offer_describe_tool = has_vision && !is_hybrid; - let tools = - Self::build_tool_definitions(offer_describe_tool, self.apollo_client.is_enabled()); -``` - -Replace with: - -```rust - // 10. Define tools. Gate flags computed from current data presence; - // hybrid mode omits describe_photo since the chat model receives - // the visual description inline (so we pass `false` for has_vision - // in hybrid mode regardless of the model's actual capability). - let gate_opts = self.current_gate_opts(has_vision && !is_hybrid); - let tools = Self::build_tool_definitions(gate_opts); -``` - -- [ ] **Step 2: Build** - -```bash -cd /home/cameron/development/opencode/ImageApi -cargo build 2>&1 | tail -10 -``` - -Expected: this site compiles; remaining errors are in `insight_chat.rs`. - -### Task 4.6: Update the chat call sites - -- [ ] **Step 1: Replace both `build_tool_definitions` calls in `insight_chat.rs`** - -In `src/ai/insight_chat.rs`, find (around line 362): - -```rust - let offer_describe_tool = !is_hybrid && local_first_user_has_image; - let tools = InsightGenerator::build_tool_definitions( - offer_describe_tool, - self.generator.apollo_enabled(), - ); -``` - -Replace with: - -```rust - let offer_describe_tool = !is_hybrid && local_first_user_has_image; - // current_gate_opts(has_vision) sets gate_opts.has_vision = has_vision - // and probes the per-table presence flags. Pass `offer_describe_tool` - // directly — the `!is_hybrid && local_first_user_has_image` decision - // is the chat-path's vision predicate. - let gate_opts = self.generator.current_gate_opts(offer_describe_tool); - let tools = InsightGenerator::build_tool_definitions(gate_opts); -``` - -Then around line 793 (in `run_streaming_turn`), find the identical block and replace the same way. - -- [ ] **Step 2: Build** - -```bash -cd /home/cameron/development/opencode/ImageApi -cargo build 2>&1 | tail -10 -``` - -Expected: clean build. - -- [ ] **Step 3: Run the new gate tests** - -```bash -cd /home/cameron/development/opencode/ImageApi -cargo test --lib build_tool_definitions -- --nocapture -``` - -Expected: both gate tests pass. - -### Task 4.7: Expand `tool_search_messages` to honor the new params - -- [ ] **Step 1: Read existing `tool_search_messages`** - -The current implementation at `src/ai/insight_generator.rs:1718-1796` only forwards `query`, `mode`, and `limit` to SMS-API. Now it should also forward `start_ts`, `end_ts`, and `contact_id`. - -**Pattern:** SMS-API's `/api/messages/search/` accepts `contact_id` natively but not date range — we add the date filter as a client-side post-filter (mirrors Apollo's `chat_tools.py:670–680`). Bump the SMS-API `limit` to 100 when a date filter is present so in-window matches aren't lost to out-of-window FTS rank. - -- [ ] **Step 2: Add a new method on `SmsApiClient` for the broader search call** - -In `src/ai/sms_client.rs`, just below the existing `search_messages` method (around line 290), add: - -```rust - /// Same shape as `search_messages` but with optional `contact_id`. The - /// SMS-API endpoint accepts contact_id natively; date filtering is the - /// caller's responsibility (post-filter on the returned rows). - pub async fn search_messages_with_contact( - &self, - query: &str, - mode: &str, - limit: usize, - contact_id: Option, - ) -> Result> { - let mut url = format!( - "{}/api/messages/search/?q={}&mode={}&limit={}", - self.base_url, - urlencoding::encode(query), - urlencoding::encode(mode), - limit - ); - if let Some(cid) = contact_id { - url.push_str(&format!("&contact_id={}", cid)); - } - - let mut request = self.client.get(&url); - if let Some(token) = &self.token { - request = request.header("Authorization", format!("Bearer {}", token)); - } - - let response = request.send().await?; - if !response.status().is_success() { - let status = response.status(); - let body = response.text().await.unwrap_or_default(); - return Err(anyhow::anyhow!( - "SMS search request failed: {} - {}", - status, - body - )); - } - - let data: SmsSearchResponse = response.json().await?; - Ok(data.results) - } -``` - -- [ ] **Step 3: Replace `tool_search_messages` body** - -In `src/ai/insight_generator.rs`, replace the `tool_search_messages` function (lines 1718-1796) with: - -```rust - /// Tool: search_messages — keyword / semantic / hybrid search over all - /// SMS message bodies via the Django FTS5 + embeddings pipeline. Now - /// supports optional `contact_id`, `start_ts`, `end_ts` filters. - async fn tool_search_messages(&self, args: &serde_json::Value) -> String { - let query = match args.get("query").and_then(|v| v.as_str()) { - Some(q) if !q.trim().is_empty() => q.trim(), - _ => { - let has_date = args.get("date").is_some() - || args.get("start_ts").is_some() - || args.get("end_ts").is_some(); - let has_contact = args.get("contact").is_some() - || args.get("contact_id").is_some(); - if has_date || has_contact { - return "Error: search_messages needs a 'query' (keywords/phrase). \ - To fetch messages around a date or from a contact without keywords, \ - call get_sms_messages with { date, contact? } instead." - .to_string(); - } - return "Error: missing required parameter 'query'".to_string(); - } - }; - if query.len() < 3 { - return "Error: query must be at least 3 characters".to_string(); - } - let mode = args - .get("mode") - .and_then(|v| v.as_str()) - .map(|s| s.to_lowercase()) - .unwrap_or_else(|| "hybrid".to_string()); - if !matches!(mode.as_str(), "fts5" | "semantic" | "hybrid") { - return format!( - "Error: unknown mode '{}'; expected one of: fts5, semantic, hybrid", - mode - ); - } - let user_limit = args - .get("limit") - .and_then(|v| v.as_i64()) - .unwrap_or(20) - .clamp(1, 50) as usize; - let contact_id = args.get("contact_id").and_then(|v| v.as_i64()); - let start_ts = args.get("start_ts").and_then(|v| v.as_i64()); - let end_ts = args.get("end_ts").and_then(|v| v.as_i64()); - let has_date_filter = start_ts.is_some() || end_ts.is_some(); - - // When a date filter is supplied, fetch a larger pool from SMS-API - // so in-window matches that ranked lower than out-of-window ones - // aren't lost. - let fetch_limit = if has_date_filter { 100 } else { user_limit }; - - log::info!( - "tool_search_messages: query='{}', mode={}, contact_id={:?}, range=[{:?}, {:?}], user_limit={}, fetch_limit={}", - query, mode, contact_id, start_ts, end_ts, user_limit, fetch_limit - ); - - let hits = match self - .sms_client - .search_messages_with_contact(query, &mode, fetch_limit, contact_id) - .await - { - Ok(h) => h, - Err(e) => return format!("Error searching messages: {}", e), - }; - - // Date-range post-filter on the client side. SMS-API's /search/ - // doesn't accept date params; mirroring Apollo's pattern here. - let filtered: Vec<_> = hits - .into_iter() - .filter(|h| { - if let Some(s) = start_ts - && h.date < s - { - return false; - } - if let Some(e) = end_ts - && h.date > e - { - return false; - } - true - }) - .take(user_limit) - .collect(); - - if filtered.is_empty() { - return "No messages matched.".to_string(); - } - - let user_name = user_display_name(); - let mut out = String::new(); - out.push_str(&format!( - "Found {} messages (mode: {}{}):\n\n", - filtered.len(), - mode, - if has_date_filter { ", date-filtered" } else { "" } - )); - for h in filtered { - let date = chrono::DateTime::from_timestamp(h.date, 0) - .map(|dt| dt.format("%Y-%m-%d").to_string()) - .unwrap_or_else(|| h.date.to_string()); - let direction: &str = if h.type_ == 2 { - &user_name - } else { - &h.contact_name - }; - let score = h - .similarity_score - .map(|s| format!(" [score {:.2}]", s)) - .unwrap_or_default(); - out.push_str(&format!( - "[{}]{} {} — {}\n\n", - date, score, direction, h.body - )); - } - out - } -``` - -- [ ] **Step 4: Build** - -```bash -cd /home/cameron/development/opencode/ImageApi -cargo build 2>&1 | tail -10 -``` - -Expected: clean build. - -### Task 4.8: Commit PR 4 - -- [ ] **Step 1: Run the full test suite** - -```bash -cd /home/cameron/development/opencode/ImageApi -cargo test --lib 2>&1 | tail -20 -``` - -Expected: all tests pass. - -- [ ] **Step 2: Stage and commit** - -```bash -cd /home/cameron/development/opencode/ImageApi -git add src/ai/insight_generator.rs src/ai/sms_client.rs src/ai/insight_chat.rs -git commit -m "$(cat <<'EOF' -insight-chat: ToolGateOpts + per-tool description rewrites - -Tools whose backing tables are empty (calendar, location_history, -daily_summaries) drop out of the catalog so the LLM doesn't waste -iteration budget calling them only to receive "no results found". -Vision and apollo gates already existed; this generalizes the pattern. - -search_messages gains start_ts/end_ts/contact_id filters (date filter -is a client-side post-filter; SMS-API only accepts contact_id natively -on the search endpoint). - -Descriptions follow a consistent convention: one sentence (what + -when), param semantics, examples for tools with non-obvious param -choices. No more all-caps headers, no more identity-prescriptive -language inside descriptions. - -Co-Authored-By: Claude Opus 4.7 (1M context) -EOF -)" -``` - ---- - -## PR 5 — FileViewer-React: Wire system_prompt; style note input; persona prompt audit - -**Files:** -- Modify: `hooks/usePersonas.tsx:15-40` (`DEFAULT_PERSONAS`) -- Modify: `hooks/useInsightChat.tsx:41-44` (`SendTurnOptions`); `:163-499` (`sendTurn`) -- Modify: `components/InsightChatModal.tsx` (composer composition; new state) - -### Task 5.1: Update built-in persona prompts to specify voice/shape - -- [ ] **Step 1: Replace `DEFAULT_PERSONAS`** - -In `hooks/usePersonas.tsx`, replace the `DEFAULT_PERSONAS` constant (lines 15-40): - -```typescript -export const DEFAULT_PERSONAS: Persona[] = [ - { - id: "default", - name: "Default Assistant", - systemPrompt: - "You are my long-term memory assistant. Use only the information " + - "provided. Do not invent details. Respond in 3–6 sentences in third " + - "person, leading with the most concrete moment from the photo and " + - "the surrounding context. Plain prose, no headings.", - isBuiltIn: true, - createdAt: Date.now(), - }, - { - id: "journal", - name: "Personal Journal", - systemPrompt: - "You are a personal journal writer. Write in first person, present " + - "tense, with warmth and reflection — focusing on emotions and " + - "meaningful moments. Use only the information provided; do not " + - "invent details. Aim for 4–8 sentences in a single flowing " + - "paragraph, no headings.", - isBuiltIn: true, - createdAt: Date.now(), - }, - { - id: "factual", - name: "Factual Reporter", - systemPrompt: - "You are a factual memory recorder. Be precise, objective, and " + - "concise. Lead with the date and place, then list what / when / who " + - "in 2–4 short sentences. Use only the information provided; if a " + - "detail is unknown, say so rather than guessing.", - isBuiltIn: true, - createdAt: Date.now(), - }, -]; -``` - -- [ ] **Step 2: Confirm typescript compiles** - -```bash -cd /home/cameron/development/opencode/FileViewer-React -npx tsc --noEmit 2>&1 | tail -10 -``` - -Expected: no errors related to the changed file. - -### Task 5.2: Wire persona into chat-turn body - -- [ ] **Step 1: Add `systemPromptOverride` to `SendTurnOptions`** - -In `hooks/useInsightChat.tsx` around line 41, replace: - -```typescript -export interface SendTurnOptions { - /** When true the server inserts a new insight row with a regenerated title. */ - amend?: boolean; -} -``` - -With: - -```typescript -export interface SendTurnOptions { - /** When true the server inserts a new insight row with a regenerated title. */ - amend?: boolean; - /** One-shot extra style note for this turn. Appended as a suffix to the - * active persona's system prompt so persona voice survives + the note - * tweaks this single turn. Cleared by the caller after send. */ - systemPromptOverride?: string | null; -} -``` - -- [ ] **Step 2: Import the persona resolver and DEFAULT_PERSONAS at the top** - -In `hooks/useInsightChat.tsx` around line 1-6, add an import (above `getLogger`): - -```typescript -import { DEFAULT_PERSONAS, Persona } from "@/hooks/usePersonas"; -``` - -- [ ] **Step 3: Add a small helper near the top of the file** - -After the existing imports and before `useInsightChat()` (around line 98), add: - -```typescript -async function resolveSystemPrompt( - override: string | null | undefined -): Promise { - // Read the active persona id + custom personas list, look up the - // matching prompt, and (optionally) suffix the per-turn override. - // Returns null when both pieces are absent — server treats null as - // "no override; keep the baked-in transcript system message." - let basePrompt: string | null = null; - try { - const [selectedId, customJson] = await AsyncStorage.multiGet([ - "@insights_selected_persona", - "@insights_personas", - ]).then(entries => entries.map(([, v]) => v)); - const customs: Persona[] = customJson ? JSON.parse(customJson) : []; - const all: Persona[] = [...DEFAULT_PERSONAS, ...customs]; - const persona = - all.find(p => p.id === selectedId) ?? DEFAULT_PERSONAS[0] ?? null; - basePrompt = persona?.systemPrompt ?? null; - } catch { - // ignore — fall through to override-only or null - } - const trimmedOverride = (override ?? "").trim(); - if (basePrompt && trimmedOverride) { - return `${basePrompt}\n\n${trimmedOverride}`; - } - if (basePrompt) return basePrompt; - if (trimmedOverride) return trimmedOverride; - return null; -} -``` - -- [ ] **Step 4: Use it inside `sendTurn`** - -In `hooks/useInsightChat.tsx` around line 267-271, find: - -```typescript - const requestBody: Record = { - file_path: filePath, - user_message: trimmed, - amend: !!opts?.amend, - }; -``` - -Replace with: - -```typescript - const systemPrompt = await resolveSystemPrompt(opts?.systemPromptOverride); - - const requestBody: Record = { - file_path: filePath, - user_message: trimmed, - amend: !!opts?.amend, - }; - if (systemPrompt) requestBody.system_prompt = systemPrompt; -``` - -- [ ] **Step 5: Confirm typescript compiles** - -```bash -cd /home/cameron/development/opencode/FileViewer-React -npx tsc --noEmit 2>&1 | tail -10 -``` - -Expected: no errors related to the changed file. - -### Task 5.3: Add a one-shot "Style note" input to the chat modal - -- [ ] **Step 1: Add state + plumbing for the style note** - -In `components/InsightChatModal.tsx` around line 67-79, find the existing `useState` block: - -```typescript - const [draft, setDraft] = useState(""); - const [amend, setAmend] = useState(false); - const scrollRef = useRef(null); -``` - -Replace with: - -```typescript - const [draft, setDraft] = useState(""); - const [amend, setAmend] = useState(false); - // One-shot style note for the next turn. Cleared after send. Suffixed - // onto the active persona's prompt server-side via system_prompt. - const [styleNote, setStyleNote] = useState(""); - const scrollRef = useRef(null); -``` - -- [ ] **Step 2: Pass `systemPromptOverride` through `onSend`** - -Same file around line 153-158, find: - -```typescript - const onSend = () => { - const text = draft.trim(); - if (!text || sending) return; - setDraft(""); - sendTurn(filePath, libraryParam, text, { amend }); - }; -``` - -Replace with: - -```typescript - const onSend = () => { - const text = draft.trim(); - if (!text || sending) return; - const note = styleNote.trim(); - setDraft(""); - setStyleNote(""); // one-shot - sendTurn(filePath, libraryParam, text, { - amend, - systemPromptOverride: note ? note : null, - }); - }; -``` - -- [ ] **Step 3: Render the style-note input above the composer** - -Same file, find the composer block (around line 387-417) starting with: - -```typescript - ` block, insert: - -```typescript - - - Style: - - - -``` - -- [ ] **Step 4: Add the matching styles** - -Same file, in the `StyleSheet.create({ ... })` block at the bottom (around line 717), add to the existing styles object (anywhere; near `composer` reads naturally): - -```typescript - styleNoteRow: { - flexDirection: "row", - alignItems: "center", - paddingHorizontal: 8, - paddingVertical: 4, - borderTopWidth: 1, - gap: 4, - }, - styleNoteInput: { - flex: 1, - minHeight: 28, - borderWidth: 1, - borderRadius: 6, - paddingHorizontal: 8, - paddingVertical: 4, - fontSize: 12, - }, -``` - -- [ ] **Step 5: Confirm typescript compiles** - -```bash -cd /home/cameron/development/opencode/FileViewer-React -npx tsc --noEmit 2>&1 | tail -10 -``` - -Expected: no errors. - -### Task 5.4: Manual smoke test - -- [ ] **Step 1: Start the dev server** - -```bash -cd /home/cameron/development/opencode/FileViewer-React -npm start -``` - -Expected: Expo dev server running. - -- [ ] **Step 2: Verify the UI loads** - -Open the app on a device or simulator. Navigate to a photo with an existing insight, open the chat modal. Confirm the new "Style:" row appears between the message list and the composer. - -- [ ] **Step 3: Send a turn with the journal persona + a style note** - -Switch persona to "Personal Journal" via the chip. In the Style input type "respond in two sentences only." Send a message. Verify the reply is short and in first person; verify the Style input clears after send. - -- [ ] **Step 4: Re-open and confirm the override was ephemeral** - -Close and re-open the chat modal. Send another turn (no style note this time, default persona). Verify the reply matches the default persona — the override didn't stick. - -### Task 5.5: Commit PR 5 - -- [ ] **Step 1: Stage and commit** - -```bash -cd /home/cameron/development/opencode/FileViewer-React -git add hooks/usePersonas.tsx hooks/useInsightChat.tsx components/InsightChatModal.tsx -git commit -m "$(cat <<'EOF' -insight-chat: send persona system_prompt on chat turns + one-shot style note - -The active persona's systemPrompt is read from AsyncStorage and sent -on every chat turn body so persona changes take effect on the next -reply (server applies it ephemerally in append mode, persists in -amend mode). - -Adds a one-shot Style input next to the composer for per-turn tweaks -("answer in bullet points") that suffixes the persona prompt and -clears after send. - -Built-in personas updated to specify voice/length/shape explicitly -since the framework no longer asserts a default shape. - -Co-Authored-By: Claude Opus 4.7 (1M context) -EOF -)" -``` - ---- - -## Self-review checklist (post-write, before handoff) - -- [ ] Spec section A (system prompt — generation): covered by PR 1 Tasks 1.1–1.5. -- [ ] Spec section B (user message — generation): covered by PR 1 Task 1.4. -- [ ] Spec section C (system_prompt on chat request): covered by PR 2 Tasks 2.3, 2.6. -- [ ] Spec section D (append-mode swap-and-restore): covered by PR 2 Tasks 2.2, 2.4, 2.5. -- [ ] Spec section E (amend-mode persistence): covered by PR 2 Tasks 2.4, 2.5 (the `if !req.amend` guard). -- [ ] Spec section F (FileViewer-React client wiring): covered by PR 5 Tasks 5.1–5.4. -- [ ] Spec section G (gating on data presence): covered by PR 4 Tasks 4.1, 4.3, 4.4, 4.5, 4.6. -- [ ] Spec section "tool description convention": covered by PR 4 Task 4.3 (each tool's new description body). -- [ ] Spec section G — `get_sms_messages` `days_radius` no-op fix: covered by PR 3. -- [ ] Spec section G — `search_messages` adds `start_ts`/`end_ts`/`contact_id`: covered by PR 4 Task 4.7. - -## Open notes - -- **OpenRouter sampling-param plumbing** is not in scope — `temperature` / `top_p` / `top_k` / `min_p` already flow through `ChatTurnRequest`. No change needed. -- **Tests for the streaming path's override** are deliberately omitted at the unit level — `run_streaming_turn` is integration-shaped (depends on Ollama). The non-streaming round-trip test in PR 2 Task 2.7 exercises the same helper logic; visual smoke in PR 5 Task 5.4 covers the streaming end-to-end. -- **No migration required** — no schema changes; `system_prompt` is request-only. From b64a5bec2872f5d46ee2d2b0761e5a7d5de9f0ea Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Thu, 7 May 2026 17:43:16 -0400 Subject: [PATCH 17/19] insight-chat: add get_faces_in_photo agentic tool MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The LLM had no path to see face_detections data — get_file_tags returns user-applied tags, but a face that's been detected and bound to a person via the embedding-cluster auto-bind path doesn't always have a matching tag. The new tool joins face_detections with persons by content_hash and returns bound names + bboxes, plus unidentified faces (so smaller models can count people in the photo without inferring from a visual description). Gated on face_detections being non-empty via the same has_any_* pattern as daily_summaries. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/ai/insight_generator.rs | 114 ++++++++++++++++++++++++++++++++++ src/bin/populate_knowledge.rs | 4 ++ src/faces.rs | 18 ++++++ src/state.rs | 7 +++ 4 files changed, 143 insertions(+) diff --git a/src/ai/insight_generator.rs b/src/ai/insight_generator.rs index ebba8be..632b504 100644 --- a/src/ai/insight_generator.rs +++ b/src/ai/insight_generator.rs @@ -83,6 +83,9 @@ pub struct InsightGenerator { search_dao: Arc>>, tag_dao: Arc>>, + // Face detections (used by the get_faces_in_photo agentic tool) + face_dao: Arc>>, + // Knowledge memory knowledge_dao: Arc>>, @@ -100,6 +103,7 @@ pub struct ToolGateOpts { pub daily_summaries_present: bool, pub calendar_present: bool, pub location_history_present: bool, + pub faces_present: bool, } impl InsightGenerator { @@ -116,6 +120,7 @@ impl InsightGenerator { search_dao: Arc>>, tag_dao: Arc>>, knowledge_dao: Arc>>, + face_dao: Arc>>, libraries: Vec, ) -> Self { Self { @@ -131,6 +136,7 @@ impl InsightGenerator { search_dao, tag_dao, knowledge_dao, + face_dao, libraries, } } @@ -174,12 +180,20 @@ impl InsightGenerator { .expect("Unable to lock DailySummaryDao"); dao.has_any_summaries(&cx).unwrap_or(false) }; + let faces_present = { + let mut dao = self + .face_dao + .lock() + .expect("Unable to lock FaceDao"); + dao.has_any_faces(&cx).unwrap_or(false) + }; ToolGateOpts { has_vision, apollo_enabled: self.apollo_enabled(), daily_summaries_present, calendar_present, location_history_present, + faces_present, } } @@ -1529,6 +1543,7 @@ Return ONLY the summary, nothing else."#, "get_calendar_events" => self.tool_get_calendar_events(arguments, cx).await, "get_location_history" => self.tool_get_location_history(arguments, cx).await, "get_file_tags" => self.tool_get_file_tags(arguments, cx).await, + "get_faces_in_photo" => self.tool_get_faces_in_photo(arguments, cx).await, "describe_photo" => self.tool_describe_photo(ollama, image_base64).await, "reverse_geocode" => self.tool_reverse_geocode(arguments).await, "get_personal_place_at" => self.tool_get_personal_place_at(arguments).await, @@ -2149,6 +2164,82 @@ Return ONLY the summary, nothing else."#, } } + /// Tool: get_faces_in_photo — list face detections + person names for + /// the given file path. Resolves rel_path → content_hash via FaceDao, + /// then queries face_detections joined with persons (status='detected' + /// only). Returns a compact bullet list keyed for human-LLM readability. + async fn tool_get_faces_in_photo( + &self, + args: &serde_json::Value, + cx: &opentelemetry::Context, + ) -> String { + let file_path = match args.get("file_path").and_then(|v| v.as_str()) { + Some(p) if !p.trim().is_empty() => p.trim().to_string(), + _ => return "Error: missing required parameter 'file_path'".to_string(), + }; + log::info!("tool_get_faces_in_photo: file_path='{}'", file_path); + + // Resolve content_hash from any library that has this rel_path. + // Walk libraries in their declared order and take the first hit. + let mut content_hash: Option = None; + for lib in &self.libraries { + let mut dao = self.face_dao.lock().expect("Unable to lock FaceDao"); + if let Ok(Some(h)) = dao.resolve_content_hash(cx, lib.id, &file_path) { + content_hash = Some(h); + break; + } + } + let Some(content_hash) = content_hash else { + return "No content_hash found for that file path (the photo may not be indexed yet, \ + or the path doesn't match any library)." + .to_string(); + }; + + let faces = { + let mut dao = self.face_dao.lock().expect("Unable to lock FaceDao"); + match dao.list_for_content_hash(cx, &content_hash) { + Ok(rows) => rows, + Err(e) => return format!("Error querying faces: {}", e), + } + }; + + if faces.is_empty() { + return "No faces detected in this photo.".to_string(); + } + + // Render: bound faces grouped by person first, then unbound. The + // model uses the bound names directly; the unbound count + bbox + // helps it count people without naming them. + let bound: Vec<&_> = faces.iter().filter(|f| f.person_name.is_some()).collect(); + let unbound: Vec<&_> = faces.iter().filter(|f| f.person_name.is_none()).collect(); + + let mut out = format!("Found {} face(s) in this photo:\n", faces.len()); + for f in &bound { + out.push_str(&format!( + "- {} (confidence {:.2}, bbox x={:.2} y={:.2} w={:.2} h={:.2}, source: {})\n", + f.person_name.as_deref().unwrap_or("?"), + f.confidence, + f.bbox_x, + f.bbox_y, + f.bbox_w, + f.bbox_h, + f.source, + )); + } + for f in &unbound { + out.push_str(&format!( + "- (unidentified) confidence {:.2}, bbox x={:.2} y={:.2} w={:.2} h={:.2}, source: {}\n", + f.confidence, + f.bbox_x, + f.bbox_y, + f.bbox_w, + f.bbox_h, + f.source, + )); + } + out + } + /// Tool: describe_photo — generate a visual description of the photo async fn tool_describe_photo( &self, @@ -2733,6 +2824,25 @@ Return ONLY the summary, nothing else."#, )); } + if opts.faces_present { + tools.push(Tool::function( + "get_faces_in_photo", + "Return the faces detected in this photo with their bounding boxes and assigned person names \ + (when bound). Each face carries `person_name` (string or null), `bbox` ({x, y, w, h} normalized 0–1), \ + `confidence` (0–1), and `source` ('auto' from detector or 'manual' from a user-drawn bbox). \ + More authoritative than `get_file_tags` for counting people in a photo or naming who is present, \ + since it returns detected-but-unbound faces too. \ + Example: `{file_path: \"2019/06/IMG_4242.jpg\"}`.", + serde_json::json!({ + "type": "object", + "required": ["file_path"], + "properties": { + "file_path": { "type": "string", "description": "File path of the photo." } + } + }), + )); + } + tools.push(Tool::function( "recall_entities", "Search the persistent knowledge memory for previously learned people, places, events, or things. \ @@ -3748,6 +3858,7 @@ mod tests { daily_summaries_present: false, calendar_present: false, location_history_present: false, + faces_present: false, }; let tools = InsightGenerator::build_tool_definitions(opts); let names: Vec<&str> = tools.iter().map(|t| t.function.name.as_str()).collect(); @@ -3769,6 +3880,7 @@ mod tests { assert!(!names.contains(&"search_rag")); assert!(!names.contains(&"get_calendar_events")); assert!(!names.contains(&"get_location_history")); + assert!(!names.contains(&"get_faces_in_photo")); } #[test] @@ -3779,6 +3891,7 @@ mod tests { daily_summaries_present: true, calendar_present: true, location_history_present: true, + faces_present: true, }; let tools = InsightGenerator::build_tool_definitions(opts); let names: Vec<&str> = tools.iter().map(|t| t.function.name.as_str()).collect(); @@ -3787,6 +3900,7 @@ mod tests { assert!(names.contains(&"search_rag")); assert!(names.contains(&"get_calendar_events")); assert!(names.contains(&"get_location_history")); + assert!(names.contains(&"get_faces_in_photo")); } fn place(name: &str, description: &str) -> ApolloPlace { diff --git a/src/bin/populate_knowledge.rs b/src/bin/populate_knowledge.rs index 9c55e60..e72a27b 100644 --- a/src/bin/populate_knowledge.rs +++ b/src/bin/populate_knowledge.rs @@ -14,6 +14,7 @@ use image_api::database::{ SqliteInsightDao, SqliteKnowledgeDao, SqliteLocationHistoryDao, SqliteSearchHistoryDao, connect, }; +use image_api::faces::{FaceDao, SqliteFaceDao}; use image_api::file_types::{IMAGE_EXTENSIONS, VIDEO_EXTENSIONS}; use image_api::libraries::{self, Library}; use image_api::tags::{SqliteTagDao, TagDao}; @@ -182,6 +183,8 @@ async fn main() -> anyhow::Result<()> { Arc::new(Mutex::new(Box::new(SqliteTagDao::default()))); let knowledge_dao: Arc>> = Arc::new(Mutex::new(Box::new(SqliteKnowledgeDao::new()))); + let face_dao: Arc>> = + Arc::new(Mutex::new(Box::new(SqliteFaceDao::new()))); // Pass the full library set so `resolve_full_path` probes every root, // even when --library restricts the walk. A rel_path shared across @@ -199,6 +202,7 @@ async fn main() -> anyhow::Result<()> { search_dao, tag_dao, knowledge_dao, + face_dao, all_libs.clone(), ); diff --git a/src/faces.rs b/src/faces.rs index fb2fb87..d92b7ae 100644 --- a/src/faces.rs +++ b/src/faces.rs @@ -503,6 +503,10 @@ pub trait FaceDao: Send + Sync { into: i32, ) -> anyhow::Result; + /// Cheap presence probe — returns true iff at least one face has been + /// detected (excluding marker rows). Used by chat-tool gating. + fn has_any_faces(&mut self, ctx: &opentelemetry::Context) -> anyhow::Result; + /// Resolve `(library_id, rel_path)` → `content_hash` via image_exif. /// Returns None when the photo hasn't been EXIF-indexed yet (no row /// in image_exif) or when the row exists but content_hash is NULL. @@ -1432,6 +1436,20 @@ impl FaceDao for SqliteFaceDao { }) } + fn has_any_faces(&mut self, ctx: &opentelemetry::Context) -> anyhow::Result { + use anyhow::Context; + let mut conn = self.connection.lock().expect("face dao lock"); + trace_db_call(ctx, "query", "has_any_faces", |_span| { + face_detections::table + .filter(face_detections::status.eq("detected")) + .select(face_detections::id) + .first::(conn.deref_mut()) + .optional() + .map(|x| x.is_some()) + .with_context(|| "has_any_faces query") + }) + } + fn resolve_content_hash( &mut self, ctx: &opentelemetry::Context, diff --git a/src/state.rs b/src/state.rs index abbcc56..49d0b4c 100644 --- a/src/state.rs +++ b/src/state.rs @@ -1,5 +1,6 @@ use crate::ai::apollo_client::ApolloClient; use crate::ai::face_client::FaceClient; +use crate::faces; use crate::ai::insight_chat::{ChatLockMap, InsightChatService}; use crate::ai::openrouter::OpenRouterClient; use crate::ai::{InsightGenerator, OllamaClient, SmsApiClient}; @@ -206,6 +207,8 @@ impl Default for AppState { Arc::new(Mutex::new(Box::new(SqliteTagDao::default()))); let knowledge_dao: Arc>> = Arc::new(Mutex::new(Box::new(SqliteKnowledgeDao::new()))); + let face_dao: Arc>> = + Arc::new(Mutex::new(Box::new(faces::SqliteFaceDao::new()))); // Load base path and ensure the primary library row reflects it. let base_path = env::var("BASE_PATH").expect("BASE_PATH was not set in the env"); @@ -232,6 +235,7 @@ impl Default for AppState { search_dao.clone(), tag_dao.clone(), knowledge_dao, + face_dao.clone(), libraries_vec.clone(), ); @@ -348,6 +352,8 @@ impl AppState { Arc::new(Mutex::new(Box::new(SqliteTagDao::default()))); let knowledge_dao: Arc>> = Arc::new(Mutex::new(Box::new(SqliteKnowledgeDao::new()))); + let face_dao: Arc>> = + Arc::new(Mutex::new(Box::new(faces::SqliteFaceDao::new()))); // Initialize test InsightGenerator with all data sources let base_path_str = base_path.to_string_lossy().to_string(); @@ -371,6 +377,7 @@ impl AppState { search_dao.clone(), tag_dao.clone(), knowledge_dao, + face_dao.clone(), vec![test_lib], ); From 6f0c15d0c5603727b1add557d69fb25bed288a2d Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Thu, 7 May 2026 17:48:22 -0400 Subject: [PATCH 18/19] insight-chat: code-review polish on get_faces_in_photo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Drop redundant `use anyhow::Context` inside has_any_faces (already imported at the module level). - Drop dead `.unwrap_or("?")` on bound faces — the vec is filtered to is_some() so the fallback can never fire. - Reorder the face_dao constructor param + initializer to match the struct declaration (between tag_dao and knowledge_dao). Update both state.rs call sites and populate_knowledge.rs to match. - Hold face_dao lock once across the library-resolver loop instead of reacquiring per iteration. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/ai/insight_generator.rs | 25 ++++++++++++++----------- src/bin/populate_knowledge.rs | 2 +- src/faces.rs | 1 - src/state.rs | 4 ++-- 4 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/ai/insight_generator.rs b/src/ai/insight_generator.rs index 632b504..1267628 100644 --- a/src/ai/insight_generator.rs +++ b/src/ai/insight_generator.rs @@ -119,8 +119,8 @@ impl InsightGenerator { location_dao: Arc>>, search_dao: Arc>>, tag_dao: Arc>>, - knowledge_dao: Arc>>, face_dao: Arc>>, + knowledge_dao: Arc>>, libraries: Vec, ) -> Self { Self { @@ -135,8 +135,8 @@ impl InsightGenerator { location_dao, search_dao, tag_dao, - knowledge_dao, face_dao, + knowledge_dao, libraries, } } @@ -2180,15 +2180,16 @@ Return ONLY the summary, nothing else."#, log::info!("tool_get_faces_in_photo: file_path='{}'", file_path); // Resolve content_hash from any library that has this rel_path. - // Walk libraries in their declared order and take the first hit. - let mut content_hash: Option = None; - for lib in &self.libraries { + // Hold the FaceDao lock once across all libraries — resolve_content_hash + // is synchronous and there's no await in the loop body. + let content_hash: Option = { let mut dao = self.face_dao.lock().expect("Unable to lock FaceDao"); - if let Ok(Some(h)) = dao.resolve_content_hash(cx, lib.id, &file_path) { - content_hash = Some(h); - break; - } - } + self.libraries.iter().find_map(|lib| { + dao.resolve_content_hash(cx, lib.id, &file_path) + .ok() + .flatten() + }) + }; let Some(content_hash) = content_hash else { return "No content_hash found for that file path (the photo may not be indexed yet, \ or the path doesn't match any library)." @@ -2215,9 +2216,11 @@ Return ONLY the summary, nothing else."#, let mut out = format!("Found {} face(s) in this photo:\n", faces.len()); for f in &bound { + // Invariant: `bound` is filtered on `person_name.is_some()` above. + let name = f.person_name.as_deref().expect("bound face must have a name"); out.push_str(&format!( "- {} (confidence {:.2}, bbox x={:.2} y={:.2} w={:.2} h={:.2}, source: {})\n", - f.person_name.as_deref().unwrap_or("?"), + name, f.confidence, f.bbox_x, f.bbox_y, diff --git a/src/bin/populate_knowledge.rs b/src/bin/populate_knowledge.rs index e72a27b..c2d1ade 100644 --- a/src/bin/populate_knowledge.rs +++ b/src/bin/populate_knowledge.rs @@ -201,8 +201,8 @@ async fn main() -> anyhow::Result<()> { location_dao, search_dao, tag_dao, - knowledge_dao, face_dao, + knowledge_dao, all_libs.clone(), ); diff --git a/src/faces.rs b/src/faces.rs index d92b7ae..5b7a232 100644 --- a/src/faces.rs +++ b/src/faces.rs @@ -1437,7 +1437,6 @@ impl FaceDao for SqliteFaceDao { } fn has_any_faces(&mut self, ctx: &opentelemetry::Context) -> anyhow::Result { - use anyhow::Context; let mut conn = self.connection.lock().expect("face dao lock"); trace_db_call(ctx, "query", "has_any_faces", |_span| { face_detections::table diff --git a/src/state.rs b/src/state.rs index 49d0b4c..adbf527 100644 --- a/src/state.rs +++ b/src/state.rs @@ -234,8 +234,8 @@ impl Default for AppState { location_dao.clone(), search_dao.clone(), tag_dao.clone(), - knowledge_dao, face_dao.clone(), + knowledge_dao, libraries_vec.clone(), ); @@ -376,8 +376,8 @@ impl AppState { location_dao.clone(), search_dao.clone(), tag_dao.clone(), - knowledge_dao, face_dao.clone(), + knowledge_dao, vec![test_lib], ); From 8bd1a8507080ee4929fb16f7889cb75f94497d74 Mon Sep 17 00:00:00 2001 From: Cameron Cordes Date: Thu, 7 May 2026 17:53:31 -0400 Subject: [PATCH 19/19] insight-chat: cargo fmt sweep on the get_faces_in_photo additions Single-line dao lock + reordered faces import. No logic changes. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/ai/insight_generator.rs | 18 ++++++------------ src/state.rs | 2 +- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/src/ai/insight_generator.rs b/src/ai/insight_generator.rs index 1267628..3e0d7fb 100644 --- a/src/ai/insight_generator.rs +++ b/src/ai/insight_generator.rs @@ -181,10 +181,7 @@ impl InsightGenerator { dao.has_any_summaries(&cx).unwrap_or(false) }; let faces_present = { - let mut dao = self - .face_dao - .lock() - .expect("Unable to lock FaceDao"); + let mut dao = self.face_dao.lock().expect("Unable to lock FaceDao"); dao.has_any_faces(&cx).unwrap_or(false) }; ToolGateOpts { @@ -2217,16 +2214,13 @@ Return ONLY the summary, nothing else."#, let mut out = format!("Found {} face(s) in this photo:\n", faces.len()); for f in &bound { // Invariant: `bound` is filtered on `person_name.is_some()` above. - let name = f.person_name.as_deref().expect("bound face must have a name"); + let name = f + .person_name + .as_deref() + .expect("bound face must have a name"); out.push_str(&format!( "- {} (confidence {:.2}, bbox x={:.2} y={:.2} w={:.2} h={:.2}, source: {})\n", - name, - f.confidence, - f.bbox_x, - f.bbox_y, - f.bbox_w, - f.bbox_h, - f.source, + name, f.confidence, f.bbox_x, f.bbox_y, f.bbox_w, f.bbox_h, f.source, )); } for f in &unbound { diff --git a/src/state.rs b/src/state.rs index adbf527..e0234a2 100644 --- a/src/state.rs +++ b/src/state.rs @@ -1,6 +1,5 @@ use crate::ai::apollo_client::ApolloClient; use crate::ai::face_client::FaceClient; -use crate::faces; use crate::ai::insight_chat::{ChatLockMap, InsightChatService}; use crate::ai::openrouter::OpenRouterClient; use crate::ai::{InsightGenerator, OllamaClient, SmsApiClient}; @@ -11,6 +10,7 @@ use crate::database::{ connect, }; use crate::database::{PreviewDao, SqlitePreviewDao}; +use crate::faces; use crate::libraries::{self, Library, LibraryHealthMap}; use crate::tags::{SqliteTagDao, TagDao}; use crate::video::actors::{