fix(chat): make the FAQ cache reachable + stop double-emitting pending ops#247
Conversation
…g ops The FAQ response cache was write-dead: it only stored answers from zero-tool turns, but the system prompt directs the model to call describe_feature for exactly the cached topics, so the store condition was never satisfied and the cache never populated. - Cache a FAQ answer when the turn raised no pending confirmation and every tool it ran (if any) was a successful describe_feature — a static, user-data-free lookup, so the answer is safe to share across users. Called tool names are now tracked on the execution accumulator; the allow-list is describe_feature specifically (never 'any read-only tool', since user-specific reads must not be cached). - Key the cache by the language the question was asked in (derived from which EN/PT phrase matched) rather than the user's profile language, since the model answers in the message's language — stops serving a wrong-language answer across users. Calendar dedup: a confirmation-required operation was emitted into BOTH Operations and PendingOperations. Drop the redundant Operations copy for PendingConfirmation; the pending operation is the canonical representation of a not-yet-run op. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Two precise bug fixes backed by solid tests. The FAQ cache fix is well-designed: deriving locale from which language's phrase matched is strictly more correct than using the user's profile language (the model responds in the message language per system-prompt rule #2, so the old profile-language key could store an English answer under a Portuguese key for bilingual users). The IsShareableFaqTurn guard is correctly layered — PendingOperations.Count == 0 catches confirmation-required mutations, CalledToolNames.All(name == describe_feature) catches any non-static read by name (unconditionally recorded before any conditional branching in Add), and OperationResults.All(Succeeded) provides a tertiary backstop for failed describe_feature calls. The pending-confirmation double-emit fix is safe: PendingOperations is the complete, canonical field for the confirmation flow and Operations carries only audit data that clients don't render. Test coverage is thorough and correctly exercises all five shareability cases.
1 participant
Two backend Astra fixes found while validating the #301 epic against prod logs. Paired client fix (calendar text-leak) ships in orbit-ui-mobile.
1. FAQ response cache was write-dead
Prod logs showed asking "what is a streak freeze?" twice still made two full ~45-tool model calls — the cache never hit. Root cause: the store condition required a zero-tool turn (
iterations == 0), but system-prompt rule #19 directs the model to calldescribe_featurefor exactly the cached topics, so the condition was structurally unreachable.describe_feature— a static, user-data-free lookup, so the answer is safe to share across users. Called tool names are now tracked on the execution accumulator (IsShareableFaqTurn).describe_featurespecifically — not "any read-only tool", since user-specific reads (get_streak_info, …) must never be cached cross-user.2. Calendar confirmation double-emitted the operation
A confirmation-required op (e.g.
manage_calendar_sync) was emitted into bothOperationsandPendingOperations. Drop the redundantOperationscopy forPendingConfirmation— the pending operation is the canonical representation of a not-yet-run op. (Clients don't renderOperations, so this is a latent-contract cleanup; the user-visible leaked text bubble is fixed client-side in the paired PR.)Tests
IsShareableFaqTurn: describe_feature-only ✓, no-tools ✓, user-data tool ✗, failed describe_feature ✗, pending confirmation ✗ (unit-tested viaInternalsVisibleTo— avoids the static-cache parallelization hazard a handler test would introduce).Operations.🤖 Generated with Claude Code