feature/streaming-insights #85

Merged
cameron merged 5 commits from feature/streaming-insights into master 2026-05-09 20:57:19 +00:00
Owner

Rework insights to better adhere to system prompts, improved tool examples, etc.

Rework insights to better adhere to system prompts, improved tool examples, etc.
cameron added 5 commits 2026-05-09 20:57:01 +00:00
Tap-Discuss-on-no-insight previously failed silently: ImageApi's
/insights/chat/stream required an existing agentic insight, errored
when missing, and emitted the failure as `event: error` — which the
frontend SSE consumer ignored (it listens for `error_message`).

This commit closes both gaps with a server-side state machine:

- /insights/chat/stream now branches on insight presence. Missing
  insight (or `regenerate: true` in the body) → bootstrap path:
  builds [System(req.system_prompt), User(req.user_message + image)],
  runs the agentic loop, generates a title, persists a new row via
  store_insight (which auto-flips priors). Existing insight →
  continuation path (unchanged behaviour).
- New `regenerate: bool` request field forces bootstrap even when an
  insight exists. Takes precedence over `amend`.
- `done` SSE payload field-name alignment with Apollo's frontend
  convention: prompt_eval_count → prompt_tokens, eval_count →
  eval_tokens, num_ctx echo added.
- `amended_insight_id` semantics broaden — now populated whenever the
  turn produced a new row (bootstrap, regenerate, or amend). Existing
  amend clients keep working unchanged; new clients get the new row's
  id for free.
- `event: error` → `event: error_message` so frontend errors stop
  silently dropping.

Refactor: extracted run_streaming_agentic_loop, build_chat_clients,
and generate_title as shared helpers between bootstrap and
continuation. Continuation path's outer logic moves to
run_continuation_streaming with no behaviour change.

Mobile-ready: any client (Apollo backend, mobile, future) sends one
request to /insights/chat/stream and gets the right path. Apollo's
proxy stays a dumb pipe.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
resolve_bootstrap_system_prompt and resolve_bootstrap_backend run on
every bootstrap turn — they pick the persisted system prompt and the
chosen backend label. They were inline conditionals before; pulling
them out makes the rules testable without spinning up the full
streaming stack.

9 new tests cover:
- system prompt fallback to BOOTSTRAP_DEFAULT_SYSTEM_PROMPT for None,
  empty string, whitespace-only
- supplied non-empty prompts pass through verbatim, with interior
  newlines / spacing preserved (Apollo personas use multi-line tool
  listings)
- backend defaults to "local" for None / empty
- "local" / "hybrid" accepted case-insensitively with edge-trim
- unknown labels return a descriptive error

Total insight_chat tests: 24 (up from 15). No behaviour change.
Bug: bootstrap user_content was just the user's typed message (plus
the hybrid visual description). Tools that take a file_path arg —
recall_facts_for_photo, get_file_tags, get_faces_in_photo — had no
way to learn the canonical path. Small models would invent
placeholders like "input_file_0.png" or call the tool with a name
guessed from a hidden multimodal input handle, neither of which
matched any real photo.

Fix: prepend a single-line "Photo file path: <normalized>\n\n" block
to user_content. Same shape generate_agentic_insight_for_photo
already uses for non-chat callers — kept the bootstrap minimal
(no date / GPS / tags pre-stuffing; the agentic loop can fetch
those via tools when needed).

Hybrid still injects the visual description block between the path
block and the user message; local mode just gets path + user text.
After refresh, the rendered transcript was showing two unwanted
artifacts in the initial user bubble:

  Photo file path: pics/DSC_5171.jpg
  please tell me about this photo and what was going on around it

  Please write your final answer now without calling any more tools.

Two distinct bugs:

1. Bootstrap was prepending `Photo file path: <path>` (and, in
   hybrid mode, the visual description block) into the user-turn
   content. The model needed it to call file_path-keyed tools, but
   the user could see it in their own bubble on replay.

2. The no-tools fallback ("Please write your final answer now…")
   was a synthetic user message we never stripped from history,
   so it persisted into training_messages, rendered as a second
   user bubble, AND wiped the prior tool-call accumulator inside
   load_history (user-turn handler clears pending_tools), which
   is why the tool invocations disappeared from the assistant
   bubble after refresh.

Fixes:

- New `build_bootstrap_system_message` helper composes the persona
  with a `--- PHOTO CONTEXT ---` block (path + optional visual
  description). Lives in the system message, not the user turn.
  The user's bubble shows only what they typed.
- Streaming agentic loop's no-tools fallback now records its
  insertion index and removes the synthetic user prompt from
  `messages` after the model responds. Final assistant content
  stays — it reads coherently on replay without the synthetic
  prompt above it. Applies to both bootstrap and continuation.

3 new tests cover the system-message composer (path-only, with
visual block, persona-trim). Total insight_chat unit tests: 27.
The bootstrap system message gave the model a file path and (in
hybrid mode) a visual description, but no temporal anchor. Models
defaulted to today's date when calling get_sms_messages — Nov 2014
photos were getting "2024-03-11" passed as `date`, missing every
historical message and leading the model to confidently misreport
context.

This commit folds two more EXIF-sourced facts into the
--- PHOTO CONTEXT --- block:

  Date taken: <YYYY-MM-DD or "unknown">
  GPS: <lat, lon to 4dp>           (omitted when no GPS)

Resolution waterfall for date_taken matches the documented canonical
date pipeline at the EXIF / filename steps, but intentionally stops
short of the fs-time fallback `generate_agentic_insight_for_photo`
uses — for chat we'd rather show "unknown" than mislead the model
with an inode mtime. GPS is taken straight from EXIF when both
lat/lon are populated; absent GPS suppresses the line entirely so
the model doesn't hallucinate coordinates.

InsightGenerator gains a `fetch_exif(file_path)` accessor (crate-
visible) so the chat service doesn't need its own ExifDao plumbing.

build_bootstrap_system_message picks up two new params (date,
gps); existing tests updated and 5 new tests cover:
- date present / absent / waterfall (EXIF wins, filename fallback,
  None when neither source has it)
- GPS present / absent
- ordering (path → date → visual)

Total insight_chat unit tests: 33 (up from 27).
cameron merged commit 55a986c249 into master 2026-05-09 20:57:19 +00:00
cameron deleted branch feature/streaming-insights 2026-05-09 20:57:20 +00:00
Sign in to join this conversation.
No Reviewers
No Label
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: Apps/ImageApi#85