fix(journal): name-based entity resolution + tighter calibration + anti-repetition
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user