Improve hotel receptionist workflow reliability and simulation coverage - #6567
Open
u9g wants to merge 71 commits into
Open
Improve hotel receptionist workflow reliability and simulation coverage#6567u9g wants to merge 71 commits into
u9g wants to merge 71 commits into
Conversation
…nt simulate scenarios.yaml hardcodes every date literal against HOTEL_TODAY=2026-06-08, but nothing set that env var, so plain 'lk agent simulate' ran the agent on the real clock and all 14 expected-state scenarios failed on date diffs. Resolve TODAY as: HOTEL_TODAY override > SIM_TODAY when --simulation is in argv (lk always passes it; job subprocesses inherit argv) > date.today().
… audit New say_goodbye_and_close_call tool: the agent never says goodbye itself; the tool re-reads the standing policy against the transcript (one LLM call, verdict via forced may_end/missing tool calls) and, at most once per call, hands back the one thing the policy still requires - e.g. offering to book what was discussed - before delivering the farewell and closing the line. Fails open everywhere: errors, timeouts, or no verdict allow the hangup.
…n shutdown Shutting down the moment the farewell's speech handle completed raced the delivery of that final message to the caller - in text simulations there's no audio playout to absorb the gap, so the sim user never saw the goodbye and failed the turn on a 60s timeout (4/9 closes in a full-suite run). Delay the shutdown by a 2s grace; shutdown() is idempotent, so a caller hanging up during the grace is harmless.
…ession down Without audio output (text-only sessions) there is no playout time between the goodbye being committed and the session tearing down, so an immediate shutdown races the delivery of that final message to the remote participant - observed in text simulations as the last turn never reaching the user and timing out. Apply a short flush grace on both the standard and realtime shutdown paths; sessions with audio output are unaffected.
The 2s flush grace didn't fix the post-goodbye timeouts: simulated callers (like real ones) answer a farewell, and a session shut down under that reply leaves their turn hanging - the run then fails on a 60s turn timeout. Replace the fixed grace with a hang-up watchdog: after the goodbye, close only once the line stays quiet for 10s; anything the caller says cancels the pending close and the conversation continues.
…t-only session
The fixed flush grace was insufficient: users routinely answer a farewell
("okay, bye!"), and a session shut down under that reply leaves their turn
hanging against a dead session. In text-only sessions, close only once the
conversation stays quiet for 10s after the goodbye; any new user input
cancels the pending close and the conversation continues. Sessions with
audio output keep the existing immediate shutdown.
…dispute escalation The escalated_to_manager dispute outcome dropped policy_explanation entirely, so the agent said only "I've escalated this" - a brush-off that skips the position the caller is owed (e.g. a card-guaranteed no-show charge standing because no cancellation is on record). The outcome now leads with the policy explanation and instructs the agent to deliver it before the escalation.
… actually made The dispute_charge docstring told the model to pass accepts_offered_resolution=true only after the caller accepted an offered outcome, but nothing enforced it - the agent could waive a charge on its first call without ever explaining the policy or presenting the option. Track offered line items in userdata: the first waiver/credit attempt is blocked and returns the policy explanation plus the concrete offer to make; money moves only on the follow-up call, after the caller has responded. Nothing is filed on the blocked attempt, so dispute rows stay single.
…n and recipient deliver_to was one free-text field meaning either a room/suite or a recipient's name, so "to the Penthouse Suite, for Diane Okafor" got stored as a concatenation and failed the expected-state diff. The tool now takes deliver_to_location and recipient_name separately, and code decides what's persisted: the location when there is one, the recipient's name only for deliveries addressed to a person. No DB schema change.
The deliver_to_location docstring used "Penthouse Suite" as its example - the exact graded string from the VIP flowers scenario, canonical nowhere in the hotel's own data. Describe the field instead of demonstrating the benchmark's answer.
…epeat closes
Callers routinely answer a farewell ("you too!"), and the model then calls
say_goodbye_and_close_call again - producing a "Goodbye!" / "Take care!" /
"Goodbye!" loop while the watchdog kept getting cancelled. Track
goodbye_said in userdata: the first close delivers the farewell; repeat
closes skip the policy audit, answer only a real question, otherwise stay
quiet, and just re-arm the silent close watchdog.
…he pickup margin A concierge reconfirming a flight naturally captures when it departs; without it the airport-car pickup margin could only ever be a prose instruction the model routinely skipped. flight_reconfirmations gains a nullable departure_time, request_flight_reconfirmation captures it, and book_airport_car looks it up for the pickup's date and hands the agent the computed margin to say back - including TIGHT and not-before-departure warnings. The flight scenario's expected state now includes 17:40:00.
… offers The folio-dispute run showed the agent inventing "do you accept a waiver?" in speech right after reading the invoice, before dispute_charge ever ran - so the waiver gate's explain-first instruction arrived after the conversational damage. Carry the rule in lookup_invoice's own output: on a disputed line, call dispute_charge (accepts_offered_resolution=false) and let it hand back the policy position and the offer.
…ability check The agent stalled on price-match calls - announcing "let me check" for several turns without calling check_room_availability. A/B testing prompt wordings by replaying the failing transcript prefix against the LLM directly (n=10 per variant) showed a generic same-turn rule fixes 1/10 samples while a routing rule naming the situation and tool fixes 10/10; a "checking is an action, not an offer" section also improves recovery after a stall.
…rmation Make departure_time required-but-nullable in the tool schema so the model must confront the field: it asks the caller for the departure time before logging (10/10 on the failing replay prefix, vs 0/10 before), while a caller who doesn't know it can still pass null and never blocks the tool.
The rule fired only "once the required stay details are known", so a price-match opener with no dates got a bare refusal and no rate quote. Direct the agent to ask for the stay dates instead: 20/20 asks for dates on the failing prefix (vs 5/10 bare refusals before), 20/20 still calls check_room_availability when the dates are given upfront.
… window A caller asking for delivery before a guest's arrival got no answer on timing - the agent skipped straight to the next question. An abstract "answer from the florist policy" rule scored 0/10 on the failing replay prefix; naming the concrete window scored 20/20 (states 10 AM-6 PM, no guaranteed time, offers the before-noon preference) with the no-timing control unchanged at 20/20.
…t a phone When the reservation flow returned "phone number required", a caller pushing for an exception got an improvised transfer to the restaurant (3/10 on the failing replay prefix), writing a transfer row the scenario doesn't expect. Extend that tool result with the exception-ask handling: 20/20 now decline and invite a callback, while an explicit ask to be put through to the restaurant still gets the transfer offer 20/20. Tool-local instead of a global rule since only this result triggers the behavior.
- dispatch_emergency no longer blocks on a room number that isn't in inventory: staff is sent on the caller's word and the agent is told to re-confirm the room while help is moving (FK on emergency_dispatches dropped accordingly, with unit tests) - the airport-car persona gets a real room (402); room 408 never existed - resend_confirmation gains the same no-caller-turn idempotency guard as transfer_call/cancel, so a re-issued call after verification can't email the same document twice - start_booking_modification gains that guard too, so a completed modification is relayed instead of re-opened in a loop; the modify read-back now includes the room's view so a just-moved booking reads back as moved - the booking task states it holds exactly one room per flow, and max_tool_steps goes 5 -> 8 so a dense turn can't suppress confirm_booking mid-flow - confirmed guest-history preferences are wired to a concrete action (record_followup kind="other" before the booking flow), and the folio email-change note is an offer to take, not a step to drive
…ario rooms Rooms 412 (key-refusal test) and 408 (emergency dispatch on an unverified room) are deliberately absent from the seeded inventory - note that in the scenario file so a future data sweep doesn't "fix" them.
…ispatch room gate With every scenario persona now naming a room that exists in the seed (collapse: 402, locked-out buddy: 301), the unverified-room dispatch path has no caller, so dispatch_emergency goes back to requiring a real room (FK restored, unknown room raises for a calm re-confirm) and the tests assert that contract.
… both simulation runs Prompt and tool-surface fixes for the 17 failed scenarios in run SR_PpFDGoJuUkxy and the residual smoke-run problems (08-10, 12, 13, 18, 02): - quote line-item labels in lookup_invoice and route no-show disputes to the no_show category; escalation records a callback instead of a live transfer - add payment-timing quick fact and sync the lookup_policy index (deposit ask) - guard start_room_booking against parallel inline-task re-entry (deadlock) - give GetCardTask a no-usable-card exit and a held-booking/callback resolution - harden modify_restaurant_reservation new_party_size against guessed values - ground weekdays in set_stay returns; caller's stated date is authoritative - relay the full record on booking-confirmation calls; deliver all walk-plan pieces and record the manager callback unconditionally - quote spa price/duration before booking; speak florist/spa references - collect departure time in the flight-reconfirmation checklist - delivery_instruction only for constraints beyond the destination - require the caller's actual name on every record_followup kind - apologize with ownership on service-failure complaints - truthful safe alternatives only (no invented taxi/valet actions) - fix scenario 04's off-by-one relative date vs SIM_TODAY; tighten the smoke folio persona's required sequence Deposit and dispute fixes validated against bare gemma replays (baseline reproduced the failures; candidate 10/10 correct with clean controls).
…e it fires The 10 AM-6 PM window was stated three times - as a fact in the florist policy body, as a bullet in the system prompt, and nowhere that made the agent say it. A measured replay of the failing prefix (SR_3VLTdgAc5778 / SRJ_Qfpvh8xWnepU) scored 0/10 on the caller's timed-delivery request with the system-prompt bullet, 0/10 with it deleted, and 0/10 with it rewritten as a hard turn-route: that surface is inert here. Now the window is stated exactly once, on the lookup_policy florist output, as a situation-to-speech route. That scores 20/20 stating the window and the non-guarantee, with the negative control (caller makes no timing request) quiet at 0/20, and a plain 'what hours does the florist deliver' still answered 10/10. The 2 PM cutoff stays in the policy body only - ablation showed repeating it in the route bought nothing.
set_stay returned every available type on one pipe-delimited line
("queen 2beds (city or garden views) | ... | double queen (ocean view)").
The model read it as one blob and offered "an ocean view or a garden view"
for a double queen - garden belongs to queen_2beds, and a garden-view double
queen has never existed in inventory. The caller, given a false choice,
switched room type to keep the view, so the booking landed as queen_2beds
where the scenario expects double_queen.
Add describe_room_options() and use it for set_stay: one row per type, so
the view binding stays local, plus the verdict on that row - ask which one
when the type genuinely has two, state it as a fact when it has one.
The matrix stays availability-derived, never hardcoded: a type's view set
shrinks with what's free (queen_2beds shows garden-only, city-only, or both
depending on the dates), so a static list would over-promise.
Also adds the import set_stay's caller was missing.
Three surfaces all pushed the agent toward asking which view, with nothing saying the offer had to be filtered by the type the caller picked: - book_room's "(ask about any preference they've hinted at, like a view)" read as a standing instruction to ask. Replace it with the rule: ask only when the option's line lists two views, state the single view as a fact when it lists one. - persona.py's own exemplar asserted "Got it. Two-forty a night, ocean view." after the caller picks king - but king is city OR ocean, so the example demonstrated inventing one view for a two-view type. It now asks, which is the correct behavior for that type and no longer contradicts the rule above. - check_room_availability rendered the same single-line view blob set_stay did, on the browsing path that precedes many bookings. Route it through describe_room_options too, so a smeared type->view pair can't enter context before the booking flow starts. The full type->view matrix only became prescriptive in choose_room's error, i.e. after the caller had already been handed a choice that wasn't real.
confirm_booking() and confirm_reservation() raised ToolError(self._status()) when a required field was missing. _status() is a progress rendering - "stay and room captured - next: call open_name_dialog" - so read as a tool result it says only "here is the next step"; nothing in it says the booking did not happen. The is_error flag doesn't close the gap: the OpenAI provider format drops it and sends the message as the entire tool content, so the model saw a string indistinguishable from a successful choose_room return. On the second room of SRJ_sBxy9PqKCp49 the agent had already spoken a real HTL- code for the first room, and it filled the gap with an invented one - "You're all set for the second room with confirmation code HTL-9R3L5K1MZX2Y" over a booking that was never written. The caller hung up believing two rooms were confirmed; one was. Give the refusal path its own text stating the outcome before the next action, and keep _status() for the success and progress returns. No missing-field list: that leaks field names into spoken questions, which is why _status() is action-phrased in the first place. confirm_changes() has no missing-field guard and so does not share the pattern. test_confirm_refusal.py asserts the behavioral facts - raises, no row written, task not completed - plus the outcome statement and the absence of any code-shaped token, rather than the full prose.
…f re-asking GetNameTask and the three credit-card sub-tasks already scan the conversation on enter and confirm a value the user gave earlier rather than collecting it cold. GetEmailTask, GetPhoneNumberTask, GetAddressTask, and GetDOBTask did not - their on_enter said "Ask the user to provide X" unconditionally, so a second capture in one session made the user spell out an address or a phone number they had already given minutes before. Port the same on_enter pattern to the four that lacked it. No new API: the chat_ctx callers already pass is the whole mechanism, the update_* tool still records the value, and the existing confirmation read-back still gates completion - so a value lifted from a noisy transcript is checked by the user before the task finishes. require_explicit_ask still forces a cold ask for callers who want one (identity verification, deliberate re-collection). Then in the hotel example: a second room in the same call runs all four dialogs again, which is what its scenario grades. The multi-room instruction claimed the booking sub-task "auto-fills the guest's name, email, and phone from earlier in the conversation" - nothing did, and the model believed it, reading back a booking whose draft was empty. It now says what actually happens: each room collects again, the identity dialogs confirm rather than re-interrogate, and only the card is genuinely given a second time.
107 words restating what the tool return already says. After the second choose_room, _status() returns "stay and room captured - next: call open_name_dialog", and the instructions already require following the last tool return's directive immediately. The model overrode that directive because instructions.py claimed the flow auto-filled identity from the conversation. That claim is gone, so the directive stands on its own. Two clauses carry what's left: the draft, not recollection of the call, is the record of what a booking holds, and the scan-and-backfill rule covers the stay and room only. Test the minimum first - if the A/B shows the directive still loses, add one sentence back and measure that.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Comment on lines
+44
to
+51
| if "discriminator" in json_schema: | ||
| schema_path = ".".join(path) or "<root>" | ||
| raise ValueError( | ||
| "Discriminated unions cannot be represented faithfully in an OpenAI " | ||
| f"strict schema; refusing to drop the discriminator at path={schema_path}. " | ||
| "Use a non-discriminated representation or generate the tool schema with " | ||
| "strict=False." | ||
| ) |
Contributor
There was a problem hiding this comment.
🔴 Agents whose tools use tagged/branching parameter types now fail on every model request
Tool and output schemas that carry a type tag are now refused outright (raise ValueError at livekit-agents/livekit/agents/llm/_strict.py:46-51) instead of being converted into the shape the model provider accepts, so agents that worked before stop being able to talk to the model at all.
Impact: Any assistant using such a parameter or output shape breaks mid-conversation with a hard error rather than continuing to work.
Mechanism: strict schema generation is on by default for OpenAI/Anthropic and the discriminator was previously only cosmetic
to_strict_json_schema is reached from build_strict_openai_schema (livekit-agents/livekit/agents/llm/utils.py:236-242) and from to_openai_response_format (livekit-agents/livekit/agents/llm/utils.py:302). Both the OpenAI (livekit-agents/livekit/agents/llm/_provider_format/openai.py:227) and Anthropic (livekit-agents/livekit/agents/llm/_provider_format/anthropic.py:138) formatters default to strict=True, so the exception is raised on every chat() call for a ToolContext containing a tool with a Pydantic discriminated-union argument (e.g. Annotated[A | B, Field(discriminator="type")]) — or for any response_format model containing one.
Previously the code dropped the discriminator key and rewrote oneOf → anyOf, which the API accepts and which does not affect server-side validation: the Python model retains its discriminator, so arguments are still validated correctly after parsing. The discriminator key is purely informational for the model. Turning this into an unconditional ValueError removes a working capability, and the suggested escape hatch ("generate the tool schema with strict=False") is only reachable via the private _strict_tool_schema kwarg of the OpenAI plugin (livekit-plugins/livekit-plugins-openai/livekit/plugins/openai/llm.py:116), not through any documented API.
Prompt for agents
The new guard in `_ensure_strict_json_schema` raises ValueError whenever a JSON schema contains a `discriminator` key. This is reached from `build_strict_openai_schema` and `to_openai_response_format`, both used with strict=True by default in the OpenAI and Anthropic provider formatters, so any agent with a Pydantic discriminated-union tool argument (or a response_format model containing one) now fails on every LLM request instead of working as it did before. The previous behavior (drop `discriminator`, convert `oneOf` to `anyOf`) produced a schema the API accepts, and argument validation is unaffected because the Pydantic model itself keeps the discriminator. Consider reverting to the non-fatal degradation, or at minimum making the strictness failure opt-in / falling back to the legacy (non-strict) schema for that tool instead of raising, and expose a public way to select non-strict schemas.
Was this helpful? React with 👍 or 👎 to provide feedback.
| import os | ||
| import sys | ||
| from datetime import date, time | ||
| from enum import StrEnum |
Contributor
There was a problem hiding this comment.
🟡 Hotel receptionist example cannot start on Python 3.10
The example imports a class that only exists in newer Python versions (from enum import StrEnum at examples/hotel_receptionist/tools_services.py:8), even though the project promises support for Python 3.10, so the example fails to load there.
Impact: Users on Python 3.10 cannot run the hotel receptionist example at all.
Mechanism: enum.StrEnum is 3.11+, repo targets 3.10+
AGENTS.md (Code Style) states "Python 3.10+ compatibility required", and livekit-agents/pyproject.toml:11 declares requires-python = ">=3.10,<3.15". enum.StrEnum was introduced in Python 3.11, so from enum import StrEnum raises ImportError on 3.10, which breaks the import of tools_services.py (and therefore agent.py, which imports ServicesToolsMixin). class DeliveryPreference(str, Enum) is the 3.10-compatible equivalent and keeps .value / string behavior used by _delivery_instruction_value (examples/hotel_receptionist/tools_services.py:167-168).
Prompt for agents
`from enum import StrEnum` in examples/hotel_receptionist/tools_services.py requires Python 3.11, but the repository mandates Python 3.10+ compatibility (AGENTS.md code style; livekit-agents/pyproject.toml requires-python >=3.10). Replace the import with `from enum import Enum` and declare `class DeliveryPreference(str, Enum)` so behavior (including `.value` usage in `_delivery_instruction_value` and pydantic schema generation for the tool argument) stays the same on 3.10.
Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+580
to
+588
| if ( | ||
| ctx.userdata.caller_turns_at_last_resend >= 0 | ||
| and _count_caller_turns(self.session.history) | ||
| <= ctx.userdata.caller_turns_at_last_resend | ||
| ): | ||
| return ( | ||
| "that document already went out moments ago - do NOT send it again. " | ||
| f"Relay the outcome to the caller: {ctx.userdata.last_resend_message}" | ||
| ) |
Contributor
There was a problem hiding this comment.
🟡 Caller who asks for two documents in one breath only ever gets one emailed
The re-send tool refuses any second send within the same caller turn (caller_turns_at_last_resend check at examples/hotel_receptionist/tools_services.py:580-588) without looking at which document was asked for, so a caller who asks for both their confirmation and their bill at once silently never receives the second one.
Impact: The guest is told the document "already went out" and never receives one of the two documents they asked for.
Mechanism: the dedupe key is the caller-turn count only, not the document kind
After a successful send the tool stores the current caller-turn count (examples/hotel_receptionist/tools_services.py:593). A second call in the same turn — e.g. kind="folio" right after kind="booking_confirmation" for the request "email me my confirmation and a copy of my bill" — satisfies _count_caller_turns(...) <= caller_turns_at_last_resend, so it returns the "already went out" string and never calls db.send_email. The comparable guards for cancel (tools_rooms.py) and modification act on a single object, where turn-count-only dedupe is sound; here two distinct documents share the counter. Tracking the last sent kind (or a set of kinds sent per turn) would keep the duplicate-suppression intent while allowing a different document.
Prompt for agents
resend_confirmation's idempotency guard suppresses any second invocation within the same caller turn regardless of the `kind` argument, so a caller asking for both the booking confirmation and the folio in one turn only gets the first document emailed. Make the guard kind-aware: remember which kind(s) were sent and at which caller-turn count (e.g. store a mapping kind -> turn count, or a set of kinds sent at the current turn) in Userdata, and only short-circuit when the same kind is re-requested with no caller turn in between.
Was this helpful? React with 👍 or 👎 to provide feedback.
Comment on lines
+879
to
+890
| @ctx.session.once("close") | ||
| def _on_close(ev: CloseEvent) -> None: | ||
| try: | ||
| job_ctx = get_job_context() | ||
| except RuntimeError: | ||
| return # no job to shut down (console / tests) | ||
|
|
||
| async def _delete_room() -> None: | ||
| await job_ctx.delete_room() | ||
|
|
||
| job_ctx.add_shutdown_callback(_delete_room) | ||
| job_ctx.shutdown(reason=ev.reason.value) |
Contributor
There was a problem hiding this comment.
🟡 Repeated goodbye attempts make the example try to delete the call's room twice
A new end-of-call listener is attached every time the goodbye tool runs (ctx.session.once("close") at examples/hotel_receptionist/tools_services.py:879-880), so on a second goodbye the room-deletion and job-shutdown work is queued twice and runs twice when the line closes.
Impact: Duplicate teardown produces errors in the logs (the room is already gone on the second attempt) during a perfectly normal repeat goodbye.
Mechanism: one listener registration per tool invocation
The code explicitly anticipates repeat closes (repeat_close = ctx.userdata.goodbye_said, examples/hotel_receptionist/tools_services.py:869, and _farewell_instruction's "already said goodbye" branch). Each invocation registers a new closure with once("close"); once only guarantees a single firing per registration, so with two registrations both _on_close closures run at close, each calling job_ctx.add_shutdown_callback(_delete_room) and job_ctx.shutdown(...). delete_room is therefore awaited twice during shutdown, the second call against an already-deleted room. Registering the close hook once per session (e.g. guarded by a flag on Userdata) avoids it.
Prompt for agents
say_goodbye_and_close_call registers a new `session.once("close")` handler on every invocation, and the code explicitly supports repeat close attempts, so on a second goodbye two handlers fire at close and `delete_room` / `job_ctx.shutdown` run twice. Register the close hook only once per session/call - e.g. gate it behind a flag stored on Userdata (similar to `goodbye_said` / `end_call_nudged`) or move the registration to session setup in agent.py.
Was this helpful? React with 👍 or 👎 to provide feedback.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing
.venv/bin/python -m pytest examples/hotel_receptionist/test_*.py tests/test_tools.py tests/test_phone_number_workflow.py --allow-uncategorized(193 passed)