From b728acd8411d6419843278c05d5a36bf911695e2 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 26 Apr 2026 17:16:33 -0400 Subject: [PATCH] fix(journal): name-based entity resolution + tighter calibration + anti-repetition MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three real bugs surfaced from inspecting today's journal turns: 1. record_moment was getting fed hallucinated person_ids (the LLM passed [1, 2] instead of the IDs save_person had just returned). Result: the moment was linked to two random old test-data notes ("test task 2", "Tell a joke"), not the people the user actually mentioned. 2. The calibration rule "ask before save_person" was being silently ignored — model just called save_person on first mention of Victoria and Mother without asking the user. 3. The model produced a verbatim-identical reply to its previous turn when the user mentioned "overwhelmed" twice — same numbered-list of 4 options, same closing line. The "warm listener / ask gentle questions" persona was pushing toward stock therapy-template patterns. Fixes: services/tools/journal.py — record_moment now accepts *_names parameters (person_names, place_names, task_titles, note_titles). Server resolves each name to a note ID via case-insensitive title match, scoped by note_type or task-status. *_ids parameters still exist but are now documented as DISCOURAGED. The LLM physically cannot invent the wrong ID when using names — names with no match are silently dropped. Resolution happens via _resolve_entity_ids_by_name helper. services/journal_pipeline.py — JOURNAL_PERSONA tightened (no more "warm/curious listener" framing that pushed toward stock comfort patterns). JOURNAL_CALIBRATION rewritten as scannable sections with imperative language: PEOPLE/PLACES require asking before save_person; TASK/NOTE state changes use the confirmation flow; MOMENTS are silent but MUST use *_names not *_ids; OTHER notes the no-set_rag_scope and no-auto-notes invariants. Added a RESPONSE STYLE section that explicitly forbids verbatim repetition and stock multi-option menus. After deploy, force-regenerate today's prep via fable_trigger_journal_prep to also pick up the tighter prep prompt from 590a07b. Co-Authored-By: Claude Opus 4.7 --- .../services/journal_pipeline.py | 62 ++++++-- src/fabledassistant/services/tools/journal.py | 136 ++++++++++++++++-- 2 files changed, 178 insertions(+), 20 deletions(-) diff --git a/src/fabledassistant/services/journal_pipeline.py b/src/fabledassistant/services/journal_pipeline.py index b65e4e2..e5db7f7 100644 --- a/src/fabledassistant/services/journal_pipeline.py +++ b/src/fabledassistant/services/journal_pipeline.py @@ -17,21 +17,61 @@ from fabledassistant.services.journal_search import search_journal from fabledassistant.services.user_profile import build_profile_context JOURNAL_PERSONA = ( - "You are a warm, curious listener helping the user record their day. " - "You are NOT a task manager. Your role is to listen, ask gentle " - "follow-up questions when something seems significant or underspecified, " - "and quietly maintain structure as a byproduct of conversation." + "You are a thoughtful journaling companion. The user is talking to you about " + "their day — listen, engage with what they actually said, and help them set down " + "what matters. You are not a customer-service bot. You are not a therapist. You " + "are not a task manager." ) JOURNAL_CALIBRATION = """\ -CALIBRATION (very important): -- When the user mentions a person you don't know, ask: "Is someone I should remember?" Confirm before calling update_person. -- When a person reference is ambiguous (multiple matches), ask which one. Never guess. -- When the user says something action-implying (e.g., "I finished X"), ASK before calling update_task. Silent updates feel derailing. -- Use the existing tool's `confirmed=false` -> user-confirms -> `confirmed=true` pattern. The frontend renders the inline confirm UI automatically. -- record_moment is the EXCEPTION: call it freely and silently when the user mentions a meaningful beat. Moments are cheap and user-correctable in the timeline view; gating them would kill the flow. +CALIBRATION (read carefully — these rules are not optional): + +PEOPLE AND PLACES — DO NOT silently create them. +- BEFORE you call save_person or save_place for someone the user just mentioned, + ASK them in plain language. Example: user says "I had coffee with Sarah." You + reply "Is Sarah someone I should add to your contacts? If so, who is she to you?" + WAIT for the user's reply, THEN call save_person. +- If the user later confirms, only then call the save_person/save_place tool. +- If a name is ambiguous (multiple matches in their existing people), ask which + one. Never guess. +- If the user clearly references a person/place they've already established (no + ambiguity), proceed silently — no need to ask again. + +TASK / NOTE STATE CHANGES — confirmation pattern. +- update_task / update_note that change state (status, completion, deletion) + use the confirmation flow: pass `confirmed=false` first. The frontend renders + an inline confirm UI. After the user clicks confirm, call again with + `confirmed=true`. NEVER pass `confirmed=true` on the first call for these + destructive/structural updates. +- Pure-read tools (list_tasks, search_notes, search_journal, get_weather, etc.) + are free — no confirmation needed. + +MOMENTS — record them silently. +- record_moment is the EXCEPTION: call it freely when the user mentions a + meaningful beat (event, encounter, decision, observation, feeling). No + confirmation. Moments are cheap and user-correctable later. +- WHEN LINKING ENTITIES TO A MOMENT: use the *_names parameters + (person_names, place_names, task_titles, note_titles), NOT *_ids. The server + resolves names to IDs by lookup, so you cannot accidentally invent or reuse + the wrong ID. Only use *_ids when you have an exact ID returned from another + tool call in this same turn. NEVER invent IDs. + +OTHER: - Do NOT call set_rag_scope. The journal scope is implicit. -- Notes are not auto-retrieved here. If you need to reference notes, call search_notes explicitly. +- Notes are not auto-retrieved. If you need to reference a note, call + search_notes explicitly. + +RESPONSE STYLE: +- Vary your replies. NEVER repeat a previous reply verbatim. If the user gives + similar input twice (e.g., mentions feeling overwhelmed twice), respond + differently — pick one specific thread from the new input to dig into. +- Avoid canned multi-option menus like "1. Show your calendar 2. List your + tasks 3. ...". They sound like a help-desk bot. Instead, pick a single + specific follow-up question or observation tied to what the user just said. +- Acknowledge what is NEW in the user's latest message — don't restart from + scratch each turn. +- Match the user's energy. Short replies for short messages; deeper engagement + for longer ones. Don't pad short replies into paragraphs. """ PHASE_GREETINGS = { diff --git a/src/fabledassistant/services/tools/journal.py b/src/fabledassistant/services/tools/journal.py index d675ec8..38cf416 100644 --- a/src/fabledassistant/services/tools/journal.py +++ b/src/fabledassistant/services/tools/journal.py @@ -5,6 +5,9 @@ import datetime import logging from typing import Any +from sqlalchemy import func, select + +from fabledassistant.models import Note, async_session from fabledassistant.services.journal_search import search_journal as svc_search_journal from fabledassistant.services.moments import create_moment from fabledassistant.services.tools._registry import tool @@ -12,6 +15,54 @@ from fabledassistant.services.tools._registry import tool logger = logging.getLogger(__name__) +async def _resolve_entity_ids_by_name( + *, + user_id: int, + names: list[str], + note_type: str | None = None, + is_task_only: bool | None = None, +) -> list[int]: + """Case-insensitive title lookup → note IDs. + + Names with no match are silently dropped (logged at debug). Used by + record_moment to resolve user-mentioned names without forcing the + LLM to track tool-return IDs across calls. + """ + if not names: + return [] + cleaned = [n.strip() for n in names if n and n.strip()] + if not cleaned: + return [] + lowered = [n.lower() for n in cleaned] + async with async_session() as session: + stmt = select(Note.id, func.lower(Note.title).label("ltitle")).where( + Note.user_id == user_id, + func.lower(Note.title).in_(lowered), + ) + if note_type is not None: + stmt = stmt.where(Note.note_type == note_type) + if is_task_only is True: + stmt = stmt.where(Note.status.isnot(None)) + elif is_task_only is False: + stmt = stmt.where(Note.status.is_(None)) + rows = (await session.execute(stmt)).all() + # Pick first match per name; preserve input order. + by_lower: dict[str, int] = {} + for note_id, ltitle in rows: + if ltitle not in by_lower: + by_lower[ltitle] = note_id + resolved: list[int] = [] + for low, original in zip(lowered, cleaned): + if low in by_lower: + resolved.append(by_lower[low]) + else: + logger.debug( + "record_moment: no entity match for %r (note_type=%s, task_only=%s)", + original, note_type, is_task_only, + ) + return resolved + + @tool( name="record_moment", description=( @@ -19,7 +70,10 @@ logger = logging.getLogger(__name__) "Use freely (no confirmation) for anything significant the user mentions: " "events, encounters, decisions, observations, feelings worth remembering. " "Each Moment is one or two sentences distilling the beat — not a full transcript. " - "Link people/places/tasks/notes by ID when the user has mentioned them." + "STRONGLY PREFER the *_names parameters when linking entities — the server " + "resolves names to IDs by lookup, so you cannot accidentally invent or " + "re-use the wrong ID. Use *_ids only when you have an exact ID returned " + "from another tool call in this same turn." ), parameters={ "content": { @@ -39,25 +93,45 @@ logger = logging.getLogger(__name__) "items": {"type": "string"}, "description": "Optional tags (no # prefix).", }, + "person_names": { + "type": "array", + "items": {"type": "string"}, + "description": "PREFERRED. Names of people mentioned (e.g. ['Victoria', 'Mom']). Server resolves to existing person notes by case-insensitive title match. Names with no match are silently skipped.", + }, + "place_names": { + "type": "array", + "items": {"type": "string"}, + "description": "PREFERRED. Names of places mentioned (e.g. ['the new ramen place']). Server resolves to existing place notes by title.", + }, + "task_titles": { + "type": "array", + "items": {"type": "string"}, + "description": "Titles of tasks this moment references. Server resolves to task IDs by title match.", + }, + "note_titles": { + "type": "array", + "items": {"type": "string"}, + "description": "Titles of notes this moment references. Server resolves to note IDs by title match.", + }, "person_ids": { "type": "array", "items": {"type": "integer"}, - "description": "Person IDs (note IDs with note_type='person') mentioned in this moment.", + "description": "DISCOURAGED — use person_names instead. Only valid if you obtained the ID from a tool result in this same turn. Never invent IDs.", }, "place_ids": { "type": "array", "items": {"type": "integer"}, - "description": "Place IDs (note IDs with note_type='place') mentioned in this moment.", + "description": "DISCOURAGED — use place_names instead. Same caveat as person_ids.", }, "task_ids": { "type": "array", "items": {"type": "integer"}, - "description": "Task IDs this moment references.", + "description": "DISCOURAGED — use task_titles instead. Same caveat as person_ids.", }, "note_ids": { "type": "array", "items": {"type": "integer"}, - "description": "Note IDs this moment references.", + "description": "DISCOURAGED — use note_titles instead. Same caveat as person_ids.", }, }, required=["content"], @@ -81,6 +155,50 @@ async def record_moment_tool(*, user_id, arguments, conv_id=None, **_ctx): else: occurred_dt = now + # Start with any explicit IDs the LLM provided. + person_ids = list(arguments.get("person_ids") or []) + place_ids = list(arguments.get("place_ids") or []) + task_ids = list(arguments.get("task_ids") or []) + note_ids = list(arguments.get("note_ids") or []) + + # Resolve names → IDs and merge. Names are the preferred path because the + # LLM is unreliable at tracking IDs across tool calls (see prior bug: + # hallucinated person_ids=[1,2] referencing test-data notes). + person_ids.extend( + await _resolve_entity_ids_by_name( + user_id=user_id, + names=arguments.get("person_names") or [], + note_type="person", + ) + ) + place_ids.extend( + await _resolve_entity_ids_by_name( + user_id=user_id, + names=arguments.get("place_names") or [], + note_type="place", + ) + ) + task_ids.extend( + await _resolve_entity_ids_by_name( + user_id=user_id, + names=arguments.get("task_titles") or [], + is_task_only=True, + ) + ) + note_ids.extend( + await _resolve_entity_ids_by_name( + user_id=user_id, + names=arguments.get("note_titles") or [], + is_task_only=False, + ) + ) + + # Dedupe while preserving order. + person_ids = list(dict.fromkeys(person_ids)) + place_ids = list(dict.fromkeys(place_ids)) + task_ids = list(dict.fromkeys(task_ids)) + note_ids = list(dict.fromkeys(note_ids)) + moment = await create_moment( user_id=user_id, content=content, @@ -89,10 +207,10 @@ async def record_moment_tool(*, user_id, arguments, conv_id=None, **_ctx): conversation_id=conv_id, raw_excerpt=arguments.get("raw_excerpt"), tags=arguments.get("tags") or [], - person_ids=arguments.get("person_ids") or [], - place_ids=arguments.get("place_ids") or [], - task_ids=arguments.get("task_ids") or [], - note_ids=arguments.get("note_ids") or [], + person_ids=person_ids, + place_ids=place_ids, + task_ids=task_ids, + note_ids=note_ids, ) return { "success": True,