test(e2e): fix flaky multimodal + approval-flow specs (#818) - #819
Merged
Conversation
The multimodal and approval-flow E2E specs intermittently failed with empty assistant responses (surfaced as `chatStream fatal`). Root cause was a test-harness race, not aimock or the library — every request the harness actually sent succeeded. Multimodal: `sendMessageWithImage` typed into a controlled React input and then attached the image, which auto-sends using that input's value. Under CPU load `pressSequentially` dropped leading characters, so the prompt reached aimock truncated (e.g. "cribe this image") and 404'd as "No fixture matched"; and React state could lag the committed DOM value so the auto-send fired with empty text and dispatched no request at all. - helpers: type until the full prompt is committed, then retry the typing + attach until the send actually fires (user bubble renders). - ChatUI: the image auto-send reads the live input DOM value instead of possibly-stale React state. Approval-flow: `runTest` treated the optimistic user-message bump as "run started", returning before any real stream activity — a stalled run then timed out waiting for an approval that never appeared. - runTest: require real stream activity (loading on, a tool call, completion, or an assistant message) before returning, and retry the click otherwise. Verified: multimodal 200/200 with 0 flaky at 20x (4 workers, retries=2) and at 6 workers/retries=0; approval-flow 450/450 at 25x/8 workers/retries=0; full E2E suite green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughThree E2E test stability fixes: ChangesE2E Flakiness Fixes: Multimodal and Approval-Flow Tests
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
testing/e2e/tests/helpers.tsParsing error: "parserOptions.project" has been provided for testing/e2e/tests/tools-test/helpers.tsParsing error: "parserOptions.project" has been provided for Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
🚀 Changeset Version Preview1 package(s) bumped directly, 0 bumped as dependents. 🟩 Patch bumps
|
|
View your CI Pipeline Execution ↗ for commit 5b82dea
☁️ Nx Cloud last updated this comment at |
@tanstack/ai
@tanstack/ai-angular
@tanstack/ai-anthropic
@tanstack/ai-client
@tanstack/ai-code-mode
@tanstack/ai-code-mode-skills
@tanstack/ai-devtools-core
@tanstack/ai-elevenlabs
@tanstack/ai-event-client
@tanstack/ai-fal
@tanstack/ai-gemini
@tanstack/ai-grok
@tanstack/ai-groq
@tanstack/ai-isolate-cloudflare
@tanstack/ai-isolate-node
@tanstack/ai-isolate-quickjs
@tanstack/ai-mcp
@tanstack/ai-ollama
@tanstack/ai-openai
@tanstack/ai-openrouter
@tanstack/ai-preact
@tanstack/ai-react
@tanstack/ai-react-ui
@tanstack/ai-solid
@tanstack/ai-solid-ui
@tanstack/ai-svelte
@tanstack/ai-utils
@tanstack/ai-vue
@tanstack/ai-vue-ui
@tanstack/openai-base
@tanstack/preact-ai-devtools
@tanstack/react-ai-devtools
@tanstack/solid-ai-devtools
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@testing/e2e/tests/helpers.ts`:
- Around line 41-64: The test assertion using await
expect(userMessages.first()).toBeVisible() does not confirm that a new message
was actually sent because the first user message may already be visible from
earlier in the conversation. Instead, capture the initial count of user messages
before attaching the image, then after the attach, verify that the count has
increased by one. This ensures the assertion confirms a new message was added
rather than just checking visibility of an existing message. Store the initial
length before the fileInput.setInputFiles calls and add a subsequent assertion
that the user message count increased.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 237b63ee-3c3e-4702-a17f-fe530be6a8bc
📒 Files selected for processing (3)
testing/e2e/src/components/ChatUI.tsxtesting/e2e/tests/helpers.tstesting/e2e/tests/tools-test/helpers.ts
Comment on lines
+41
to
+64
| const userMessages = page.getByTestId('user-message') | ||
|
|
||
| // Attaching the image auto-sends, using the prompt currently in the chat | ||
| // input, and the matched aimock fixture keys on the exact user text. A | ||
| // *controlled* React input is fragile here under CPU load (CI, parallel | ||
| // workers) in two ways: typing char-by-char can drop characters, leaving a | ||
| // truncated value like "cribe this image" (which 404s as "No fixture | ||
| // matched" → empty `chatStream fatal`); and the attach's onChange can land | ||
| // before the typed value is committed, dispatching nothing at all. So drive | ||
| // the interaction to its observable outcome — the user bubble rendering — | ||
| // retrying both the typing and the attach until the send actually fires with | ||
| // the full prompt. A redundant re-attach is harmless: the client ignores a | ||
| // second send while the first is still streaming. | ||
| await expect(async () => { | ||
| await input.click() | ||
| await input.fill('') | ||
| await input.pressSequentially(text, { delay: 15 }) | ||
| // Confirm the full prompt is committed before attaching. | ||
| expect(await input.inputValue()).toBe(text) | ||
| // Reset the selection so re-attaching the same path re-fires onChange. | ||
| await fileInput.setInputFiles([]) | ||
| await fileInput.setInputFiles(imagePath) | ||
| await expect(userMessages.first()).toBeVisible({ timeout: 2_000 }) | ||
| }).toPass({ timeout: 15_000, intervals: [250, 500, 1000] }) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use message-count delta instead of .first() visibility to confirm the send actually fired.
At Line 63, userMessages.first() may already be visible from earlier messages, so this check can pass even when the new image send didn’t occur. Track a baseline count and assert it increases after attach.
Suggested fix
export async function sendMessageWithImage(
page: Page,
text: string,
imagePath: string,
) {
const input = page.getByTestId('chat-input')
const fileInput = page.getByTestId('image-attachment-input')
const userMessages = page.getByTestId('user-message')
+ const baselineUserMessageCount = await userMessages.count()
await expect(async () => {
await input.click()
await input.fill('')
await input.pressSequentially(text, { delay: 15 })
// Confirm the full prompt is committed before attaching.
expect(await input.inputValue()).toBe(text)
// Reset the selection so re-attaching the same path re-fires onChange.
await fileInput.setInputFiles([])
await fileInput.setInputFiles(imagePath)
- await expect(userMessages.first()).toBeVisible({ timeout: 2_000 })
+ await expect
+ .poll(async () => await userMessages.count(), { timeout: 2_000 })
+ .toBeGreaterThan(baselineUserMessageCount)
}).toPass({ timeout: 15_000, intervals: [250, 500, 1000] })
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@testing/e2e/tests/helpers.ts` around lines 41 - 64, The test assertion using
await expect(userMessages.first()).toBeVisible() does not confirm that a new
message was actually sent because the first user message may already be visible
from earlier in the conversation. Instead, capture the initial count of user
messages before attaching the image, then after the attach, verify that the
count has increased by one. This ensures the assertion confirms a new message
was added rather than just checking visibility of an existing message. Store the
initial length before the fileInput.setInputFiles calls and add a subsequent
assertion that the user message count increased.
Contributor
Author
|
I'm just going to merge this. Fixing flaky tests only |
tombeckenham
deleted the
818-flaky-e2e-multimodal-+-approval-flow-tests-intermittently-get-empty-responses-chatstream-fatal-from-aimock
branch
June 24, 2026 04:54
4 tasks
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.
🎯 Changes
Fixes the intermittent E2E failures in the multimodal and approval-flow specs (closes #818). The same commit both passed and failed across re-runs, surfacing as empty assistant responses /
chatStream fatal.Root cause was a test-harness race — not aimock and not the library. End-to-end instrumentation showed every request the harness actually sent succeeded (direct load: 360/360 raw, 480/480 via SDK, 0 fatals). The corruption was in how the test drove the controlled React input.
Multimodal (
multimodal-image,multimodal-structured):sendMessageWithImagetyped into a controlled input, then attached the image — which auto-sends using that input's value. Under CPU load:pressSequentiallydropped leading characters, so the prompt reached aimock truncated (e.g."cribe this image") →404 No fixture matched→ the emptychatStream fatalin the report.onChangeread empty text and dispatched no request at all.Approval-flow (rarer):
runTesttreated the optimistic user-message bump as "run started" and returned before any real stream activity; a stalled run then timed out waiting for an approval that never appeared (eventCount=0, no error).Fixes (test harness only)
tests/helpers.ts— type until the full prompt is committed, then retry the typing + attach until the send actually fires (user bubble renders).src/components/ChatUI.tsx— the image auto-send reads the live input DOM value instead of possibly-stale React state.tests/tools-test/helpers.ts—runTestrequires real stream activity (loading on, a tool call, completion, or an assistant message) before returning, and retries the click otherwise.Verification
✅ Checklist
pnpm run test:pr.🚀 Release Impact
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes