Merge pull request #1163 from oriontech-me/fix/issue-1059-max-tokens-error-attribution

test(llm-agents): make an errored agent run name its own cause (#1188)

Author: Victor-w-Madeira

docs/core-functionality/llm-agents/agent-max-tokens.md

@@ -88,7 +88,10 @@ Shared setup per test:
    signal: the turn mounts (`div-chat-message` count rises) and the generating
    indicator clears (`button-stop` hidden, `button-send` back). The
    `chat-message-token-usage` badge is **not** the completion gate — see the
-   note on #1059/#569 below.
+   note on #1059/#569 below. An **error card** at either point ends the run
+   immediately with the provider's message (#1188): upstream renders
+   `error-card-stack` *instead of* the bot bubble, so any wait keyed on the
+   bubble outlives an errored turn.
 6. Assert the badge exists (the `max_tokens` observable), then hover it and read
    the **Output** token count from its tooltip (`Input: 1.0K / Output: 46`
    format; values may be plain integers, `N.NK`, or empty ⇒ 0).
@@ -150,7 +153,9 @@ Shared setup per test:
   failures with three distinct messages instead of one blind
   `toHaveCount … Received: 0`. A false positive is impossible in the other
   direction too — the badge assertion is a hard `toHaveCount(1)`, never skipped
-  when absent.
+  when absent. The third of those states only became true in **#1188**: an
+  errored turn renders no bot bubble at all, so it used to die 20 s later on
+  `locator.innerText` with the provider's error nowhere in the message.
 - **Force-failure check** (CONTRIBUTING §2) run during VERIFY on each hard
   assertion before `@stable`.
 
@@ -240,6 +245,24 @@ Shared setup per test:
   prints the reply that did render. Timeouts were **not** loosened (the total
   budget went from ~248 s to ~205 s) and no assertion was weakened — only the
   attribution changed.
+- **An errored run is resolved explicitly, at both points it can appear**
+  (#1188). Gating on the bot bubble fixed two of the three states but not the
+  third: upstream `chat-message.tsx` renders `ErrorView` **instead of**
+  `BotMessage` when `chat.category === "error"`, so an errored turn carries no
+  `div-chat-message` — only `error-card-stack`. Measured on `main` at
+  1.12.0.dev10 against a drained Anthropic key (a deterministic reproducer: the
+  provider answers `400 … credit balance is too low` every time), both tests
+  failed with `locator.innerText: Timeout 20000ms exceeded — waiting for
+  getByTestId('div-chat-message').last()`, never reaching the badge assertion
+  where the informative message lives. The cause was on screen and in the run
+  stream the whole time — the fixture's flow-error advisory (#1162) printed the
+  provider's 400 in the same run. `runPrompt()` now checks for the error card
+  after the turn-start poll (the run can fail before any bubble mounts) **and**
+  after the completion signal (the measured case: the bubble mounts, then is
+  replaced), failing with the provider's message expanded out of the error
+  accordion. The reply read is also count-guarded, so a finished turn that
+  rendered nothing still reaches the badge assertion instead of a locator
+  timeout. This adds no pass path — an errored run still fails.
 - **Where the #1059 signature came from.** The filed hard failure (daily
   2026-07-29, `anthropic / claude-sonnet-5`) was **environmental collateral**, not
   a product regression: attempt 0 of that very test died on the day's dominant

tests/tests-automations/regression/core-functionality/llm-agents/agent-max-tokens.spec.ts

@@ -210,6 +210,10 @@ async function getSavedMaxTokens(page: Page): Promise<unknown> {
 // same pair that spec uses (the turn mounts, then the generating indicator
 // clears), then assert the badge separately so each failure names its own cause.
 // No timeout was loosened: the worst case went from ~248 s to ~205 s.
+//
+// The third state — "finished with an error" — is resolved explicitly at both
+// points it can appear, because upstream renders the error card INSTEAD of the
+// bot bubble, so every wait keyed on the bubble outlives it (#1188).
 async function runPrompt(page: Page): Promise<string> {
   const node = page.locator(
     '[data-testid^="rf__node-ChatInput"] [data-testid="textarea_str_input_value"]',
@@ -224,35 +228,94 @@ async function runPrompt(page: Page): Promise<string> {
   await expect(page.getByTestId("input-chat-playground").last()).toBeVisible({ timeout: 30000 });
 
   const messages = page.getByTestId("div-chat-message");
+  const errorCard = page.getByTestId("error-card-stack");
   const before = await messages.count();
   await page.getByTestId("button-send").last().click();
 
   // 1. The turn actually started — guards the "checked completion before
   //    generation started" race that an indicator-only wait returns early on
   //    (#354). `toBeGreaterThan` rather than an exact count, so it holds whether
-  //    or not the user bubble carries this testid.
+  //    or not the user bubble carries this testid. An errored turn is accepted
+  //    here as a start too: upstream renders `ErrorView` INSTEAD of the bot
+  //    bubble (`chat-message.tsx`: `chat.category === "error"`), so a run that
+  //    fails before any bubble mounts would otherwise wait the full 60 s for an
+  //    element that is never coming (#1188).
   await expect
-    .poll(() => messages.count(), { timeout: 60000 })
-    .toBeGreaterThan(before);
+    .poll(
+      async () => (await errorCard.count()) > 0 || (await messages.count()) > before,
+      {
+        timeout: 60000,
+        message: "the run neither started a reply nor rendered an error card",
+      },
+    )
+    .toBe(true);
+  await failIfRunErrored(page);
   // 2. Generation finished. This is the completion signal because it is emitted
   //    for every model and every response (#569).
   await expect(page.getByTestId("button-stop")).toBeHidden({ timeout: 120000 });
   await expect(page.getByTestId("button-send").last()).toBeVisible({ timeout: 10000 });
-
-  const reply = (await messages.last().innerText()).trim();
+  // The error can also arrive AFTER a bubble mounted, and that is the measured
+  // case on 1.12.0.dev10: the bubble is replaced by the error card, so
+  // `messages.last()` resolves to nothing and every later step waits on an
+  // element the error path does not render (#1188).
+  await failIfRunErrored(page);
+
+  // A finished turn that rendered no bubble at all must still reach the badge
+  // assertion below — reading `.last()` unguarded is what turned that state into
+  // a bare `locator.innerText` timeout with no cause in it.
+  const reply =
+    (await messages.count()) > 0
+      ? (await messages.last().innerText({ timeout: 5000 }).catch(() => "")).trim()
+      : "";
 
   // 3. Only now the observable itself. A finished turn that renders no badge is a
   //    MISSING OBSERVABLE, not a slow model — and the message says so, quoting the
-  //    reply that did render (an error bubble included) instead of timing out blind.
+  //    reply that did render instead of timing out blind.
   await expect(
     page.getByTestId("chat-message-token-usage"),
     `the finished response must expose a token-usage badge — it is the max_tokens ` +
-      `observable this spec reads. Rendered reply: ${reply.slice(0, 300)}`,
+      `observable this spec reads. Rendered reply: ${reply.slice(0, 300) || "(none rendered)"}`,
   ).toHaveCount(1, { timeout: 15000 });
 
   return reply;
 }
 
+// An errored run is a real outcome of this spec and must name itself. Without
+// this, the run fails several steps later on whatever element the error path
+// happens not to render — measured on `main` as `locator.innerText: Timeout
+// 20000ms exceeded` with the provider's 400 nowhere in the message (#1188).
+async function failIfRunErrored(page: Page): Promise<void> {
+  if ((await page.getByTestId("error-card-stack").count()) === 0) return;
+  throw new Error(
+    `the agent run errored instead of returning a response — ${await readRunError(page)}`,
+  );
+}
+
+// The provider's message sits in a collapsed accordion inside the error card
+// (upstream `error-message.tsx`), so expand it before reading — otherwise the
+// failure says "An error occurred" and nothing else, which is the same dead end
+// the timeout was.
+async function readRunError(page: Page): Promise<string> {
+  // `.last()`: the throw is gated on "any error card", so read the newest one.
+  const stack = page.getByTestId("error-card-stack").last();
+  // Best-effort: expanding is how we reach the provider text, never how we
+  // decide the run failed.
+  await stack
+    .getByText("An error occurred")
+    .last()
+    .click({ timeout: 5000 })
+    .catch(() => {});
+  const text = (await stack.innerText().catch(() => "")).replace(/\s+/g, " ").trim();
+  // Upstream renders the provider message only when the error carries a
+  // component (`error-message.tsx`), so a bare label is a real outcome — and it
+  // must not read as "here is the cause", or this helper reproduces the dead end
+  // it exists to remove.
+  return text && text.replace(/an error occurred/i, "").trim().length > 0
+    ? text
+    : `${text || "(empty error card)"} — the error card carried no provider message; ` +
+        `check the run's flow-error advisory in the test log or the flow's build log`;
+}
+
 // "1.9K" -> 1900, "46" -> 46, missing/empty -> 0 (a tight cap can be fully
 // consumed by reasoning before any visible token — observed with max_tokens=1).
 function parseTokenCount(raw: string | undefined): number {