diff --git a/src/fabledassistant/routes/journal.py b/src/fabledassistant/routes/journal.py index 462e840..910a9b0 100644 --- a/src/fabledassistant/routes/journal.py +++ b/src/fabledassistant/routes/journal.py @@ -407,8 +407,14 @@ async def _day_payload(*, user_id: int, day_date: datetime.date): .order_by(Message.created_at) ) messages = (await session.execute(msgs_stmt)).scalars().all() + # conv.to_dict() recomputes message_count from the `messages` + # relationship, which isn't eager-loaded here, so it would report 0. + # We already have the real list — override with the known count, same + # convention the chat-list path uses (services/chat.py). + conv_dict = conv.to_dict() + conv_dict["message_count"] = len(messages) return jsonify({ "day_date": day_date.isoformat(), - "conversation": conv.to_dict(), + "conversation": conv_dict, "messages": [m.to_dict() for m in messages], }) diff --git a/src/fabledassistant/services/journal_prep.py b/src/fabledassistant/services/journal_prep.py index f1d9dcf..c661dcf 100644 --- a/src/fabledassistant/services/journal_prep.py +++ b/src/fabledassistant/services/journal_prep.py @@ -23,6 +23,7 @@ from __future__ import annotations import datetime import logging +import re from zoneinfo import ZoneInfo from sqlalchemy import select @@ -249,7 +250,9 @@ async def _open_threads(*, user_id: int, day_date: datetime.date) -> list[dict]: def _render_task_line(t: dict, *, include_due: bool, include_overdue: bool) -> str: line = f" - {t.get('title', '?')}" if include_overdue and t.get("days_overdue"): - line += f" (due {t['due_date']}, {t['days_overdue']} days ago)" + n = t["days_overdue"] + unit = "day" if n == 1 else "days" + line += f" (was due {t['due_date']}, {n} {unit} overdue)" elif include_due and t.get("due_date"): line += f" (due {t['due_date']})" if t.get("priority") and t["priority"] not in (None, "none"): @@ -277,7 +280,7 @@ def _render_sections_for_prompt(sections: dict) -> str: lines.append(_render_task_line(t, include_due=True, include_overdue=False)) lines.append("") if overdue: - lines.append("OVERDUE TASKS (still on the list, not currently due):") + lines.append("OVERDUE TASKS (past their due date, still open — backlog, not today's work):") for t in overdue[:8]: lines.append(_render_task_line(t, include_due=False, include_overdue=True)) lines.append("") @@ -312,6 +315,17 @@ def _render_sections_for_prompt(sections: dict) -> str: bits.append(f"{precip}% chance of precipitation") lines.append(" - " + ", ".join(bits)) lines.append("") + else: + # Explicit absent-marker. A silently-omitted weather block leaves a + # small model an unanchored void it tends to fill with plausible + # fabricated weather (observed: invented "68°F, 15% rain" with an + # empty weather section). A concrete "none" line + directive holds + # far better than relying on a negative system-prompt rule alone. + lines.append( + "WEATHER: none available — no weather, temperature, or precipitation " + "data exists for today. Do NOT mention weather in any form." + ) + lines.append("") projects = sections.get("projects") or [] if projects: @@ -342,8 +356,14 @@ def _render_sections_for_prompt(sections: dict) -> str: lines.append(f" - [{day}] {content}") lines.append("") - if not lines: - return "(No data for today — quiet morning.)" + # The weather-none marker is always emitted, so `lines` is never empty; + # a quiet day is one with no *substantive* sections beyond that marker. + substantive = [ln for ln in lines if ln and not ln.startswith("WEATHER: none")] + if not substantive: + return ( + "(No tasks, events, or notable data for today — a quiet day. " + "Do not invent weather or any other details.)" + ) return "\n".join(lines).rstrip() @@ -360,15 +380,19 @@ _PREP_SYSTEM_PROMPT = ( "greetings unless the actual content warrants two clauses' worth.\n" "- TASK BUCKETS — three sections may appear: TASKS DUE TODAY, UPCOMING TASKS, " "OVERDUE TASKS. Lead with TASKS DUE TODAY when present. Do NOT call overdue items " - "\"due today\" — they aren't. When OVERDUE TASKS appears, surface it with the " - "staleness duration (\"still on the list 68 days\") and frame it as something to " - "revisit, not as today's work. If the only data is overdue, lead with it but " - "frame it as a backlog reminder.\n" + "\"due today\" — they aren't. When OVERDUE TASKS appears, state the overdue " + "duration exactly as given (e.g. \"3 days overdue\") and frame it as backlog to " + "revisit, not as today's work. Do NOT echo the parenthetical section labels " + "verbatim. If the only data is overdue, lead with it but frame it as a backlog " + "reminder.\n" "- If RECENT JOURNAL MOMENTS or OPEN THREADS are present, mention one or two BRIEFLY " "at the end as context — not as the lead. Skip them if nothing notable.\n" "- Close with one short invitation to journal: \"What's on your mind?\", " "\"Anything to set down?\", \"How's the morning shaping up?\" — pick one, keep it under 8 words.\n" - "- Don't fabricate. Skip categories with no data; don't acknowledge their absence.\n" + "- Use ONLY the data below. A category marked 'none' (or absent) genuinely has no " + "data — do not invent it and do not mention it. In particular, NEVER state weather, " + "temperature, or precipitation unless an explicit WEATHER section with numbers " + "appears below.\n" "- Voice is competent assistant briefing the user. Not a friend writing a letter." ) @@ -379,6 +403,40 @@ def _fallback_prep_text(day_date: datetime.date) -> str: return f"{weekday}, {day_date.isoformat()}. What's on your mind?" +# Strong, low-false-positive weather signals. Deliberately NOT bare words like +# "rain"/"sunny"/"weather" (those legitimately appear in task/event titles — +# "buy rain boots"). Targets the concrete phrasings small models actually +# emit when fabricating ("partly cloudy with a high of 68°F and a 15% chance +# of rain"): temperature glyphs, "high/low of N", "chance of ", +# "(partly|mostly) (cloudy|sunny)", "overcast", "precipitation", "forecast". +_WEATHER_SIGNAL_RE = re.compile( + r""" + \d{1,3}\s?° + | \b\d{1,3}\s?°?\s?(?:degrees|fahrenheit|celsius)\b + | \b(?:high|low)\s+of\s+\d + | \bchance\s+of\s+(?:rain|showers?|precipitation|snow|sleet|storms?|thunder) + | \b(?:partly|mostly)\s+(?:cloudy|sunny)\b + | \bovercast\b + | \bprecipitation\b + | \bforecast\b + """, + re.IGNORECASE | re.VERBOSE, +) + + +def _prose_fabricated_weather(prose: str, sections: dict) -> bool: + """True when the prose talks weather but no weather data was gathered. + + The deterministic backstop for the system-prompt rule: an 8–14B model + still invents weather on quiet days even when told not to. If the + WEATHER section is genuinely empty and the prose trips a strong weather + signal, that text is fabricated. + """ + if sections.get("weather"): + return False + return bool(_WEATHER_SIGNAL_RE.search(prose or "")) + + async def _generate_prep_prose( *, sections: dict, @@ -400,25 +458,54 @@ async def _generate_prep_prose( f"Write the opener for today's journal." ) - messages = [ - {"role": "system", "content": _PREP_SYSTEM_PROMPT}, - {"role": "user", "content": user_trigger}, - ] + _WEATHER_CORRECTION = ( + "\n\nIMPORTANT: there is NO weather data for today. Do not mention " + "weather, temperature, sky conditions, or precipitation in any form." + ) - try: - prose = await generate_completion( - messages=messages, - model=model, - max_tokens=400, - ) - except Exception: - logger.exception("Daily prep prose generation failed for day %s", day_date) - return _fallback_prep_text(day_date) + # Up to 2 attempts: if the first trips the fabricated-weather guard, retry + # once with an explicit corrective appended to the user turn. A 14B model + # almost always complies on the corrected pass; if it still doesn't we log + # and accept (surgically excising a sentence risks breaking prose flow — + # better a rare stray clause than mangled output, and the log lets us + # measure whether a model bump is actually warranted). + prose = "" + for attempt in (1, 2): + trigger = user_trigger + if attempt == 2: + trigger = user_trigger + _WEATHER_CORRECTION + messages = [ + {"role": "system", "content": _PREP_SYSTEM_PROMPT}, + {"role": "user", "content": trigger}, + ] + try: + raw = await generate_completion( + messages=messages, + model=model, + max_tokens=400, + ) + except Exception: + logger.exception("Daily prep prose generation failed for day %s", day_date) + return _fallback_prep_text(day_date) - prose = (prose or "").strip() - if not prose: - logger.warning("LLM returned empty prep prose for day %s — using fallback", day_date) - return _fallback_prep_text(day_date) + prose = (raw or "").strip() + if not prose: + logger.warning("LLM returned empty prep prose for day %s — using fallback", day_date) + return _fallback_prep_text(day_date) + + if not _prose_fabricated_weather(prose, sections): + return prose + + if attempt == 1: + logger.warning( + "daily_prep: fabricated weather detected for day %s (no weather " + "data gathered) — regenerating with corrective", day_date, + ) + else: + logger.error( + "daily_prep: weather still fabricated after corrective retry " + "for day %s — accepting prose as-is", day_date, + ) return prose diff --git a/tests/test_journal_message_count.py b/tests/test_journal_message_count.py new file mode 100644 index 0000000..4975791 --- /dev/null +++ b/tests/test_journal_message_count.py @@ -0,0 +1,60 @@ +"""Regression coverage for the journal `message_count: 0` bug. + +`_day_payload` (routes/journal.py) loads messages in a separate query and +serializes the Conversation with `conv.to_dict()`. `Conversation.to_dict()` +derives `message_count` from the `messages` relationship, which is NOT +eager-loaded on that instance — so it silently fell back to 0 for every +journal day (observed via the fable MCP: conversation #291 reported +message_count 0 with 9 messages present). + +Full HTTP coverage of `_day_payload` needs a live DB (not available in the +unit-test env — see test_events_routes.py). These tests pin the model-level +contract that necessitates the route-side override. +""" +from datetime import date, datetime, timezone + +from fabledassistant.models.conversation import Conversation, Message + + +def _conv() -> Conversation: + now = datetime(2026, 5, 19, 9, 0, tzinfo=timezone.utc) + return Conversation( + user_id=1, + conversation_type="journal", + day_date=date(2026, 5, 19), + title="2026-05-19", + created_at=now, + updated_at=now, + ) + + +def test_to_dict_reports_zero_when_messages_relationship_not_loaded(): + """The trap: an untouched `messages` relationship is absent from the + instance state, so to_dict() reports 0 regardless of DB rows. This is + exactly the journal path's situation and why the route must override.""" + conv = _conv() + assert conv.to_dict()["message_count"] == 0 + + +def test_to_dict_counts_when_messages_loaded(): + """When the relationship IS populated the count is correct — confirming + the model isn't broken, the journal path just never loaded it.""" + conv = _conv() + conv.messages = [ + Message(conversation_id=1, role="assistant", content="prep"), + Message(conversation_id=1, role="user", content="hi"), + ] + assert conv.to_dict()["message_count"] == 2 + + +def test_day_payload_override_yields_true_count(): + """Pins the fix contract: _day_payload already holds the real message + list and overrides message_count with len(messages), the same way the + chat-list path (services/chat.py) supplies its own count.""" + conv = _conv() # messages relationship deliberately not loaded + messages = [object(), object(), object()] # stand-ins for the loaded rows + + conv_dict = conv.to_dict() + conv_dict["message_count"] = len(messages) + + assert conv_dict["message_count"] == 3 diff --git a/tests/test_journal_prep_filtering.py b/tests/test_journal_prep_filtering.py index 58f68fe..362dca3 100644 --- a/tests/test_journal_prep_filtering.py +++ b/tests/test_journal_prep_filtering.py @@ -145,7 +145,9 @@ def test_render_overdue_includes_staleness_duration(): } rendered = _render_sections_for_prompt(sections) assert "OVERDUE TASKS" in rendered - assert "68 days ago" in rendered + # Unambiguous overdue phrasing (replaced the old "68 days ago", which the + # model parroted alongside the header into a self-contradiction). + assert "68 days overdue" in rendered assert "2026-02-20" in rendered # Crucially, NOT framed as due today. assert "DUE TODAY" not in rendered diff --git a/tests/test_journal_prep_hardening.py b/tests/test_journal_prep_hardening.py new file mode 100644 index 0000000..5b50e21 --- /dev/null +++ b/tests/test_journal_prep_hardening.py @@ -0,0 +1,123 @@ +"""Coverage for the daily-prep hardening pass. + +Targets the two prep-quality defects observed via the fable MCP on +2026-05-19 (conversation #291): + + 1. Fabricated weather ("partly cloudy with a high of 68°F and a 15% + chance of rain") emitted with an empty weather section. + 2. Self-contradictory overdue phrasing ("still on the list, not + currently due, 88 days ago"; "1 days ago"). + +All targets are pure sync functions — no DB / LLM needed. +""" +import datetime + +from fabledassistant.services.journal_prep import ( + _prose_fabricated_weather, + _render_sections_for_prompt, + _render_task_line, +) + +TODAY = datetime.date(2026, 5, 19) + + +# ── Overdue phrasing ──────────────────────────────────────────────────────── + + +def test_overdue_line_singular_day(): + line = _render_task_line( + {"title": "Change my oil", "due_date": "2026-05-18", "days_overdue": 1}, + include_due=False, include_overdue=True, + ) + assert "1 day overdue" in line + assert "1 days" not in line # the old ungrammatical form is gone + + +def test_overdue_line_plural_days_no_contradiction(): + line = _render_task_line( + {"title": "Research X", "due_date": "2026-02-20", "days_overdue": 88}, + include_due=False, include_overdue=True, + ) + assert "88 days overdue" in line + assert "was due 2026-02-20" in line + assert "days ago" not in line # replaced by unambiguous "overdue" + + +def test_overdue_section_header_is_unambiguous(): + out = _render_sections_for_prompt({ + "tasks_overdue": [ + {"title": "Research X", "due_date": "2026-02-20", "days_overdue": 88}, + ], + }) + assert "past their due date, still open" in out + # The phrase that the model parroted into the self-contradiction is gone. + assert "not currently due" not in out + + +# ── Weather absent-marker ─────────────────────────────────────────────────── + + +def test_empty_weather_emits_explicit_none_marker(): + out = _render_sections_for_prompt({ + "tasks_overdue": [ + {"title": "Change my oil", "due_date": "2026-05-18", "days_overdue": 1}, + ], + "weather": [], + }) + assert "WEATHER: none available" in out + assert "Do NOT mention weather" in out + + +def test_present_weather_renders_data_not_marker(): + out = _render_sections_for_prompt({ + "tasks_due_today": [{"title": "Ship it"}], + "weather": [{ + "location_label": "Home", + "forecast_json": {"daily": { + "temperature_2m_max": [70], + "temperature_2m_min": [52], + "precipitation_probability_max": [10], + }}, + }], + }) + assert "WEATHER:" in out + assert "none available" not in out + assert "high 70°" in out + + +def test_quiet_day_returns_fabrication_forbidding_text(): + out = _render_sections_for_prompt({}) + assert "quiet day" in out + assert "Do not invent weather" in out + + +# ── Fabricated-weather guard ──────────────────────────────────────────────── + + +def test_guard_flags_the_real_observed_hallucination(): + prose = ( + "You have two overdue tasks. The weather today is partly cloudy " + "with a high of 68°F and a 15% chance of rain. What's on your mind?" + ) + assert _prose_fabricated_weather(prose, {"weather": []}) is True + + +def test_guard_flags_bare_temperature_glyph(): + assert _prose_fabricated_weather("Expect around 72° later.", {"weather": []}) is True + + +def test_guard_passes_clean_prose(): + prose = "Two tasks are overdue and nothing is due today. What's on your mind?" + assert _prose_fabricated_weather(prose, {"weather": []}) is False + + +def test_guard_no_false_positive_on_rain_in_task_title(): + # Bare "rain" must NOT trip the guard — only "chance of rain" etc. does. + prose = "Don't forget to buy rain boots and finish the training module." + assert _prose_fabricated_weather(prose, {"weather": []}) is False + + +def test_guard_allows_weather_when_data_present(): + prose = "Home is 70°F with a 10% chance of rain." + sections = {"weather": [{"location_label": "Home"}]} + assert _prose_fabricated_weather(prose, sections) is False