feat(assistant): let a failed turn name what the caller can do

.tegami/2026-08-06-assistant-failure-sentences.md

@@ -0,0 +1,18 @@
+---
+packages:
+  orgmemory: patch
+subject: Tell people what to do when an Assistant turn fails
+---
+
+## Fixes
+
+A failed Assistant turn now ends on a sentence naming what the person who hit it
+can do next, instead of one generic message for every cause. An expired gateway
+key, a rate limit, a model that is no longer offered, a gateway that did not
+answer in time, and a busy assistant are now distinguishable and separately
+actionable.
+
+Every message remains a fixed sentence chosen from the failure's category, so a
+misconfigured or unusually talkative AI gateway cannot surface its own text,
+credentials, or prompt content in the browser. A failure that matches no known
+category still ends on the previous generic message.

apps/api/src/main/java/com/orgmemory/api/assistant/AssistantStreamFailures.java

@@ -0,0 +1,84 @@
+package com.orgmemory.api.assistant;
+
+import com.orgmemory.core.assistant.AssistantUnavailableException;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+/**
+ * Turns a failed turn into one short sentence the person who hit it can act on.
+ *
+ * <p>A failed turn used to end as the fixed frame {@link #GENERIC}, so an expired gateway
+ * credential, a rate limit, a retired model and a broken deployment were indistinguishable to the
+ * only person in a position to do something about them. {@code failure_code} and the {@code WARN}
+ * line make a failure attributable to whoever operates the deployment; this makes it actionable to
+ * whoever is sitting in front of it.
+ *
+ * <p><strong>Every returned sentence is a fixed string.</strong> Nothing is interpolated from the
+ * failure, so a chatty or misconfigured gateway can never echo a key, a prompt fragment or provider
+ * internals into the browser. That constraint is the reason this reads a status rather than a
+ * message.
+ *
+ * <p>Saturation is read from the bounded {@code failureCode} carried on
+ * {@link AssistantUnavailableException} rather than from a status, because it never had one: the
+ * retrieval scheduler rejects the turn before any gateway is contacted. Everything else is keyed on
+ * the leading HTTP status that clients put on the message ({@code "400: ..."} from the OpenAI SDK,
+ * {@code "404 Not Found: ..."} from Spring) rather than on provider exception types, because the
+ * provider SDK belongs to the AI integration module and the delivery layer only needs the status.
+ */
+final class AssistantStreamFailures {
+
+    static final String GENERIC = "The assistant stream failed.";
+
+    static final String BUSY =
+            "The assistant is busy right now. Send the message again in a moment.";
+
+    /** A leading three-digit status, as HTTP client exceptions render it. */
+    private static final Pattern STATUS_PREFIX = Pattern.compile("^\\s*([45]\\d{2})(?::|\\s)");
+
+    private static final int MAX_CAUSE_DEPTH = 10;
+
+    private AssistantStreamFailures() {
+    }
+
+    static String describe(Throwable error) {
+        Throwable cause = error;
+        for (int depth = 0; cause != null && depth < MAX_CAUSE_DEPTH; depth++) {
+            if (cause instanceof AssistantUnavailableException unavailable
+                    && AssistantRetrievalScheduler.REJECTED.equals(unavailable.failureCode())) {
+                return BUSY;
+            }
+            int status = status(cause.getMessage());
+            if (status > 0) {
+                return forStatus(status);
+            }
+            cause = cause.getCause() == cause ? null : cause.getCause();
+        }
+        return GENERIC;
+    }
+
+    private static int status(String message) {
+        if (message == null) {
+            return 0;
+        }
+        Matcher matcher = STATUS_PREFIX.matcher(message);
+        return matcher.find() ? Integer.parseInt(matcher.group(1)) : 0;
+    }
+
+    static String forStatus(int status) {
+        return switch (status) {
+            case 400, 422 -> "The selected model rejected this request. "
+                    + "Pick a different chat model and send it again.";
+            case 401, 403 -> "The AI gateway rejected its credentials. "
+                    + "Ask an administrator to update its key.";
+            case 404 -> "The selected model is no longer available on this gateway. "
+                    + "Pick another model.";
+            case 408, 504 -> "The AI gateway did not answer in time. Send the message again.";
+            case 429 -> "The AI gateway is rate limiting requests. "
+                    + "Send the message again in a moment.";
+            default -> status >= 500
+                    ? "The AI gateway failed while answering. Send the message again."
+                    : "The AI gateway rejected this request. "
+                            + "Ask an administrator to check the model and gateway.";
+        };
+    }
+}

apps/api/src/main/java/com/orgmemory/api/assistant/UiMessageStream.java

@@ -32,7 +32,9 @@ static Flux<ServerSentEvent<String>> encode(
                             Flux.just(encoder.finish(), encoder.done()))
                     .onErrorResume(AssistantStreamAbortedException.class,
                             error -> Flux.just(encoder.abort(error.getMessage()), encoder.done()))
-                    .onErrorResume(ignored -> Flux.just(encoder.error(), encoder.done()));
+                    .onErrorResume(error -> Flux.just(
+                            encoder.error(AssistantStreamFailures.describe(error)),
+                            encoder.done()));
         });
     }
 
@@ -81,9 +83,9 @@ ServerSentEvent<String> finish() {
             return event(json.writeValueAsString(fields("type", "finish", "finishReason", "stop")));
         }
 
-        ServerSentEvent<String> error() {
+        ServerSentEvent<String> error(String errorText) {
             return event(json.writeValueAsString(
-                    fields("type", "error", "errorText", "The assistant stream failed.")));
+                    fields("type", "error", "errorText", errorText)));
         }
 
         ServerSentEvent<String> abort(String reason) {

apps/api/src/test/java/com/orgmemory/api/assistant/AssistantStreamFailuresTests.java

@@ -0,0 +1,83 @@
+package com.orgmemory.api.assistant;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import com.orgmemory.core.assistant.AssistantUnavailableException;
+import java.util.concurrent.RejectedExecutionException;
+import org.junit.jupiter.api.Test;
+
+/**
+ * The sentence a failed turn ends on is the only thing the person who hit it can act on, and it is
+ * also the last place a gateway's own words could reach a browser. These hold both ends: the
+ * failure has to be named, and naming it must not quote anything the failure said.
+ */
+class AssistantStreamFailuresTests {
+
+    @Test
+    void namesSaturationFromTheFailureCodeRatherThanAStatus() {
+        // The retrieval scheduler rejects before any gateway is contacted, so this failure never
+        // had a status to read. Without the code it would fall through to the generic sentence and
+        // tell a user to do nothing while the correct advice is to wait a moment.
+        AssistantUnavailableException rejected = new AssistantUnavailableException(
+                "The assistant is temporarily busy",
+                new RejectedExecutionException("queue full"),
+                AssistantRetrievalScheduler.REJECTED);
+
+        assertThat(AssistantStreamFailures.describe(rejected))
+                .isEqualTo(AssistantStreamFailures.BUSY);
+    }
+
+    @Test
+    void distinguishesCredentialFailureFromRateLimitFromRetiredModel() {
+        assertThat(AssistantStreamFailures.describe(new IllegalStateException("401: no key")))
+                .contains("credentials");
+        assertThat(AssistantStreamFailures.describe(new IllegalStateException("429 Too Many")))
+                .contains("rate limiting");
+        assertThat(AssistantStreamFailures.describe(new IllegalStateException("404 Not Found: x")))
+                .contains("no longer available");
+    }
+
+    @Test
+    void readsTheStatusThroughAWrappedCause() {
+        Throwable wrapped = new IllegalStateException(
+                "assistant failed",
+                new IllegalStateException("503: upstream down"));
+
+        assertThat(AssistantStreamFailures.describe(wrapped))
+                .isEqualTo(AssistantStreamFailures.forStatus(503));
+    }
+
+    @Test
+    void fallsBackToTheGenericSentenceWhenNothingIsRecognizable() {
+        assertThat(AssistantStreamFailures.describe(new IllegalStateException("provider secret")))
+                .isEqualTo(AssistantStreamFailures.GENERIC);
+    }
+
+    /**
+     * A self-referential cause is not hypothetical: exception plumbing that re-wraps its own cause
+     * produces one, and an unguarded walk would spin forever inside a streaming response.
+     */
+    @Test
+    void terminatesOnASelfReferentialCauseChain() {
+        Throwable looping = new IllegalStateException("no status here") {
+            @Override
+            public synchronized Throwable getCause() {
+                return this;
+            }
+        };
+
+        assertThat(AssistantStreamFailures.describe(looping))
+                .isEqualTo(AssistantStreamFailures.GENERIC);
+    }
+
+    @Test
+    void neverQuotesTheFailureItDescribes() {
+        String untrustedFailureDetail = "provider diagnostic containing a prompt fragment";
+
+        assertThat(AssistantStreamFailures.describe(
+                        new IllegalStateException(
+                                "500: " + untrustedFailureDetail, new RuntimeException(untrustedFailureDetail))))
+                .doesNotContain(untrustedFailureDetail)
+                .doesNotContain("prompt fragment");
+    }
+}

apps/api/src/test/java/com/orgmemory/api/assistant/UiMessageStreamTests.java

@@ -132,4 +132,25 @@ void providerFailureEmitsOpaqueErrorAndDone() {
         assertThat(data.getFirst()).contains("\"type\":\"start\"");
         assertThat(data).allMatch(frame -> !frame.contains("provider secret"));
     }
+
+    /**
+     * The opaque frame above is the floor, not the contract. A failure that carries a status has to
+     * reach the browser as the sentence for that status, or the encoder is silently discarding the
+     * only actionable thing the failure knew.
+     */
+    @Test
+    void aRecognizedFailureReachesTheBrowserAsItsOwnSentence() {
+        List<String> data = UiMessageStream.encode(
+                        Flux.error(new IllegalStateException("429: slow down", null)),
+                        MESSAGE_ID,
+                        json,
+                        Duration.ofHours(1),
+                        Duration.ofMinutes(1))
+                .map(ServerSentEvent::data)
+                .collectList()
+                .block();
+
+        assertThat(data).anyMatch(frame -> frame.contains("rate limiting"));
+        assertThat(data).allMatch(frame -> !frame.contains("slow down"));
+    }
 }

docs/increments/active/2026-08-06-assistant-conversation-memory-ssot/challenge-brief.md

@@ -0,0 +1,133 @@
+# Debate Brief — Chat Transcript SSOT
+
+You are one of two architects debating a material persistence-boundary decision
+in the OrgMemory repository. Read this file in full, inspect the repository
+evidence yourself, then write your response.
+
+## Hard constraints
+
+- **Read-only.** Do not edit, create, or delete any repository file. Do not run
+  migrations, mutate any database, or change git state.
+- Inspect the repo freely to check the claims below. Do not trust this brief;
+  verify it. If a claim here is wrong, say so with the file and line.
+- Your response goes into the debate record file you are told to append to.
+  Plain Markdown. No tools-only output, no truncation.
+
+## The question
+
+OrgMemory stores the same assistant conversation in two Postgres tables. Should
+it collapse to one store, or keep two and fix the weaker one?
+
+## Current state (verify these)
+
+**Table A — `assistant_conversation_messages`** (`core/src/main/resources/db/migration/V6__assistant_conversation_history.sql`)
+- Columns include `organization_id`, `actor_user_id`, `role`, `content`,
+  `sequence_id bigint GENERATED ALWAYS AS IDENTITY`, `version`.
+- `CHECK (role IN ('USER','ASSISTANT'))`, `CHECK (length(content) BETWEEN 1 AND 200000)`.
+- Foreign keys to `app_users` and to `assistant_conversations`, `ON DELETE CASCADE`.
+- Written by `AssistantConversationService` (`core/src/main/java/com/orgmemory/core/assistant/AssistantConversationService.java`):
+  the USER row when a turn starts, the ASSISTANT row in `completeTurn(...)`,
+  which returns early when the answer is blank.
+- Read by `AssistantConversationService.history(...)`, which serves
+  `GET /api/assistant/conversations/{conversationId}/messages`.
+- Also carries answer citations.
+
+**Table B — `spring_ai_chat_memory`** (same migration file, top)
+- Columns: `conversation_id varchar(36)`, `content text`, `type varchar(10)`,
+  `timestamp`, `sequence_id bigint`. **No `organization_id`, no `actor_user_id`,
+  no foreign key.**
+- `CHECK (type IN ('USER','ASSISTANT','SYSTEM','TOOL'))`.
+- Written by Spring AI's `MessageChatMemoryAdvisor` through the `ChatMemory`
+  bean built in `apps/api/src/main/java/com/orgmemory/api/assistant/AssistantConfiguration.java`
+  as `MessageWindowChatMemory.builder().maxMessages(20).build()`, wrapped in
+  `ObservedChatMemory`.
+- `MessageWindowChatMemory` trims to its window on write and `saveAll` replaces
+  the conversation's row set, so rows beyond the window are physically deleted.
+- Consumed only via the `ChatMemory` interface at
+  `integrations/ai-model-gateways/src/main/java/com/orgmemory/integrations/ai/gateway/SpringAiChatModelAdapter.java`
+  (`MessageChatMemoryAdvisor.builder(chatMemory).build()`, ~line 277 and again
+  in `assistantMemoryClient`).
+
+**Consistency today.** `apps/api/src/main/java/com/orgmemory/api/assistant/AssistantController.java`
+(~line 379) deletes a conversation by calling `conversations.delete(actor, conversationId)`
+and then `memory.clear(conversationId.toString())` — two stores, two calls, not
+one transaction, orchestrated in the delivery layer.
+
+## Measured evidence from the production deployment (2026-08-06)
+
+Production data is test-only; there are no real customers and no backfill
+obligation.
+
+- `assistant_conversation_messages`: 1148 rows / 539 conversations. Longest
+  conversation 56 messages.
+- `spring_ai_chat_memory`: 1085 rows / 519 conversations. Longest 20 — exactly
+  the window cap, confirming trimming is live.
+- **20 conversations have zero rows in `spring_ai_chat_memory`** while their
+  transcript exists in table A.
+- Message type distribution in `spring_ai_chat_memory`: `USER` 591,
+  `ASSISTANT` 494. **No `SYSTEM` and no `TOOL` rows exist**, despite the schema
+  allowing them.
+
+## Position A — collapse to one store
+
+Implement `ChatMemory` over `assistant_conversation_messages`. Drop
+`MessageWindowChatMemory`, `ChatMemoryRepository`, and the
+`spring_ai_chat_memory` table. Keep the `ChatMemory` *interface*, because
+`SpringAiChatModelAdapter` depends on it.
+
+Proposed shape:
+- `add()` → no-op, documented: `AssistantConversationService` is the sole writer
+  and is the only caller holding `organizationId`, `actorUserId` and citations,
+  which `ChatMemory.add(String, List<Message>)` does not receive.
+- `get()` → windowed read ordered by `sequence_id`, snapped forward to the
+  nearest `USER` message so the window never begins on an assistant reply whose
+  prompt was cut off.
+- `clear()` → delegate to the conversation service delete.
+
+Claimed benefits: drift becomes structurally impossible; model memory enters the
+tenancy model for the first time; deletion becomes one domain transaction; the
+destructive `saveAll` disappears.
+
+## Position B — keep two stores, fix the weaker one
+
+Keep the separation the migration comment declares intentional
+(`-- Spring AI ChatMemory is the bounded context sent back to the model. The
+complete product transcript is stored separately below.`). Bring
+`spring_ai_chat_memory` into the tenancy model instead: add `organization_id`,
+foreign keys and cascade, and move deletion out of the controller into a single
+domain transaction.
+
+Claimed benefits: the model window and the product transcript are genuinely
+different concerns with different lifecycles and retention; keeping Spring AI's
+own repository preserves upstream compatibility and avoids an interface
+implementation whose `add()` violates its contract by doing nothing.
+
+## Points each side must engage with
+
+1. Is a no-op `add()` an acceptable implementation of `ChatMemory`, or a
+   contract violation that will mislead the next maintainer? What happens if a
+   future Spring AI upgrade, or another advisor, calls `add()` and expects
+   persistence?
+2. Does the absence of `SYSTEM`/`TOOL` rows today prove table A can hold
+   everything the model window needs, or is it an artifact of the current
+   feature set that tool-calling will invalidate? Note the Skill tool loop in
+   `apps/api/.../AssistantConfiguration.java` and `AssistantSkillToolCallbacks`.
+3. `completeTurn(...)` returns early on a blank answer, so a failed turn leaves
+   a USER row with no ASSISTANT row. Under each position, what does the model
+   window see on the next turn, and is the snap-forward read sufficient?
+4. What explains the 20 conversations with zero rows in
+   `spring_ai_chat_memory`, and does either position prevent that class of
+   divergence or merely relabel it?
+5. Retention and privacy: `spring_ai_chat_memory` holds message content with no
+   tenant column. Under Position B, is adding `organization_id` sufficient, or
+   does the second writer remain the actual risk?
+6. Cost and reversibility: which position is cheaper to undo if wrong?
+
+## Required output shape
+
+1. **Position** — which architecture you defend, in one sentence.
+2. **Evidence** — concrete file paths and line references supporting it.
+3. **Attacks** — specific, evidence-backed attacks on the opposing position.
+   Attack the position, not a strawman of it.
+4. **Concessions** — what the other side is genuinely right about.
+5. **Falsifier** — what fact, if true, would change your mind.