From 1ad9d200376544169128a58ee93e77c6dd88a5bb Mon Sep 17 00:00:00 2001 From: alex Date: Tue, 10 Mar 2026 15:59:07 +0000 Subject: [PATCH] fix(chat): stabilize assistant add flow and location routing --- .cursorrules | 3 +- AGENTS.md | 3 +- CLAUDE.md | 3 +- backend/server/adventures/models.py | 14 ++++++- backend/server/chat/llm_client.py | 1 + .../src/lib/components/AITravelChat.svelte | 8 ++-- .../src/routes/collections/[id]/+page.svelte | 37 ++++++++++++++++++- 7 files changed, 60 insertions(+), 9 deletions(-) diff --git a/.cursorrules b/.cursorrules index 7901b148..b39f74b7 100644 --- a/.cursorrules +++ b/.cursorrules @@ -51,10 +51,11 @@ Run in order: format → lint → check → build. - Use DaisyUI semantic colors/classes (`bg-primary`, `text-base-content`). - Chat providers: dynamic catalog from `GET /api/chat/providers/`; configured in `CHAT_PROVIDER_CONFIG`. - Chat model override: dropdown selector fed by `GET /api/chat/providers/{provider}/models/`; per-provider persistence via `localStorage` key `voyage_chat_model_prefs`; backend `send_message` accepts optional `model`. -- Chat context: collection chats inject collection UUID + multi-stop itinerary context; system prompt guides `get_trip_details`-first reasoning and confirms only before first `add_to_itinerary`. +- Chat context: collection chats inject collection UUID + multi-stop itinerary context; system prompt guides `get_trip_details`-first reasoning and confirms only before first `add_to_itinerary`; `search_places` prompt guard requires the LLM to have a concrete location string before calling the tool (asks clarifying question otherwise). - Chat tool output: `role=tool` messages hidden from display; tool outputs render as concise summaries; persisted tool rows reconstructed on reload via `rebuildConversationMessages()`. - Chat errors: `_safe_error_payload()` maps LiteLLM exceptions to sanitized user-safe categories (never raw `exc.message`). - Invalid tool calls (missing required args) are detected and short-circuited with a user-visible error — not replayed into history. +- Geocoding: `background_geocode_and_assign()` runs in a thread after Location save; populates `region`, `city`, `country`, and also fills `Location.location` from reverse geocode `display_name` (truncated to field max_length) if blank or different. - Chat agent tools (`get_trip_details`, `add_to_itinerary`) respect collection sharing — both owners and `shared_with` members can use them; `list_trips` remains owner-only. - Do **not** attempt to fix known test/config issues during feature work. - Commit and merge completed feature branches promptly once validation passes (avoid leaving finished work unmerged). diff --git a/AGENTS.md b/AGENTS.md index 95b040e2..4d81ce66 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -67,10 +67,11 @@ Run in this order: - Security: handle CSRF tokens via `/auth/csrf/` and `X-CSRFToken` - Chat providers: dynamic catalog from `GET /api/chat/providers/`; configured in `CHAT_PROVIDER_CONFIG` - Chat model override: dropdown selector fed by `GET /api/chat/providers/{provider}/models/`; persisted in `localStorage` key `voyage_chat_model_prefs`; backend accepts optional `model` param in `send_message` -- Chat context: collection chats inject collection UUID + multi-stop itinerary context; system prompt guides `get_trip_details`-first reasoning and confirms only before first `add_to_itinerary` +- Chat context: collection chats inject collection UUID + multi-stop itinerary context; system prompt guides `get_trip_details`-first reasoning and confirms only before first `add_to_itinerary`; `search_places` prompt guard requires the LLM to have a concrete location string before calling the tool (asks clarifying question otherwise) - Chat tool output: `role=tool` messages hidden from display; tool outputs render as concise summaries; persisted tool rows reconstructed on reload via `rebuildConversationMessages()` - Chat error surfacing: `_safe_error_payload()` maps LiteLLM exceptions to sanitized user-safe categories (never forwards raw `exc.message`) - Invalid tool calls (missing required args) are detected and short-circuited with a user-visible error — not replayed into history +- Geocoding: `background_geocode_and_assign()` runs in a thread after Location save; populates `region`, `city`, `country`, and also fills `Location.location` from reverse geocode `display_name` (truncated to field max_length) if blank or different ## Conventions - Do **not** attempt to fix known test/configuration issues as part of feature work. diff --git a/CLAUDE.md b/CLAUDE.md index 5f0e4d42..768c63d1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -75,10 +75,11 @@ Run in this exact order: - CSRF handling: use `/auth/csrf/` + `X-CSRFToken` - Chat providers: dynamic catalog from `GET /api/chat/providers/`; configured in `CHAT_PROVIDER_CONFIG` - Chat model override: dropdown selector fed by `GET /api/chat/providers/{provider}/models/`; persisted in `localStorage` key `voyage_chat_model_prefs`; backend accepts optional `model` param in `send_message` -- Chat context: collection chats inject collection UUID + multi-stop itinerary context; system prompt guides `get_trip_details`-first reasoning and confirms only before first `add_to_itinerary` +- Chat context: collection chats inject collection UUID + multi-stop itinerary context; system prompt guides `get_trip_details`-first reasoning and confirms only before first `add_to_itinerary`; `search_places` prompt guard requires the LLM to have a concrete location string before calling the tool (asks clarifying question otherwise) - Chat tool output: `role=tool` messages hidden from display; tool outputs render as concise summaries; persisted tool rows reconstructed on reload via `rebuildConversationMessages()` - Chat error surfacing: `_safe_error_payload()` maps LiteLLM exceptions to sanitized user-safe categories (never forwards raw `exc.message`) - Invalid tool calls (missing required args) are detected and short-circuited with a user-visible error — not replayed into history +- Geocoding: `background_geocode_and_assign()` runs in a thread after Location save; populates `region`, `city`, `country`, and also fills `Location.location` from reverse geocode `display_name` (truncated to field max_length) if blank or different ## Conventions - Do **not** attempt to fix known test/configuration issues as part of feature work. diff --git a/backend/server/adventures/models.py b/backend/server/adventures/models.py index bdeaeac9..85a9d748 100644 --- a/backend/server/adventures/models.py +++ b/backend/server/adventures/models.py @@ -49,9 +49,19 @@ def background_geocode_and_assign(location_id: str): if country: location.country = country - # Save updated location info + update_fields = ["region", "city", "country"] + display_name = (result.get("display_name") or "").strip() + if display_name: + max_length = Location._meta.get_field("location").max_length + if max_length: + display_name = display_name[:max_length] + + if location.location != display_name: + location.location = display_name + update_fields.append("location") + # Save updated location info, skip geocode threading - location.save(update_fields=["region", "city", "country"], _skip_geocode=True) + location.save(update_fields=update_fields, _skip_geocode=True) except Exception as e: # Optional: log or print the error diff --git a/backend/server/chat/llm_client.py b/backend/server/chat/llm_client.py index 3ebb32f4..ec848385 100644 --- a/backend/server/chat/llm_client.py +++ b/backend/server/chat/llm_client.py @@ -346,6 +346,7 @@ When chat context includes a trip collection: - Treat context as itinerary-wide (potentially multiple stops), not a single destination - Use get_trip_details first when you need complete collection context before searching for places - Ground place searches in trip stops and dates from the provided trip context +- Only call search_places when you have a concrete, non-empty location string; if location is missing or unclear, ask a clarifying question to obtain it first Be conversational, helpful, and enthusiastic about travel. Keep responses concise but informative.""" diff --git a/frontend/src/lib/components/AITravelChat.svelte b/frontend/src/lib/components/AITravelChat.svelte index 8714b51c..da95be44 100644 --- a/frontend/src/lib/components/AITravelChat.svelte +++ b/frontend/src/lib/components/AITravelChat.svelte @@ -2,7 +2,7 @@ import { createEventDispatcher, onMount } from 'svelte'; import { t } from 'svelte-i18n'; import { mdiSend, mdiPlus, mdiDelete, mdiMenu, mdiClose } from '@mdi/js'; - import type { ChatProviderCatalogEntry } from '$lib/types.js'; + import type { ChatProviderCatalogEntry, CollectionItineraryItem, Location } from '$lib/types.js'; import { addToast } from '$lib/toasts'; type ToolResultEntry = { @@ -82,7 +82,7 @@ const dispatch = createEventDispatcher<{ close: void; - itemAdded: { locationId: string; date: string }; + itemAdded: { location: Location; itineraryItem: CollectionItineraryItem; date: string }; }>(); const MODEL_PREFS_STORAGE_KEY = 'voyage_chat_model_prefs'; @@ -620,7 +620,9 @@ throw new Error('Failed to add to itinerary'); } - dispatch('itemAdded', { locationId: location.id, date }); + const itineraryItem = await itineraryResponse.json(); + + dispatch('itemAdded', { location, itineraryItem, date }); addToast('success', $t('added_successfully')); closeDateSelector(); } catch (error) { diff --git a/frontend/src/routes/collections/[id]/+page.svelte b/frontend/src/routes/collections/[id]/+page.svelte index cd8df088..d59a1e7f 100644 --- a/frontend/src/routes/collections/[id]/+page.svelte +++ b/frontend/src/routes/collections/[id]/+page.svelte @@ -1,5 +1,12 @@