🤖 perf: optimize sendMessage integration tests with matrix expansion #467
Conversation
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
ammar-agent
force-pushed
the
ci-fast
branch
2 times, most recently
from
e927f61 to
567b4ef
Compare
Restructured tests to reduce API calls and execution time while maintaining high confidence in the code. Changes: - Moved 12 provider-agnostic tests from describe.each to single-provider block - Removed redundant provider parity test (smoke tests already verify both) - Optimized token limit test: reduced from 40-80 messages to 10, single provider - Added DEFAULT_PROVIDER constant (Anthropic - faster and cheaper) Impact: - API calls: 45 → 28 (38% reduction) - Expected time savings: ~100 seconds (30-40% faster) - Expected runtime: 4-5 minutes (down from 6-7 minutes) Test coverage maintained: - Both providers: smoke test, API key errors, model errors, tool policy, system instructions, images - Single provider: IPC/streaming logic, reconnection, editing, tool calls, continuity, token limits _Generated with `cmux`_
Matrix expansion (7 tests → 14 API calls): - Tool calls across providers - Conversation continuity - Mode-specific instructions - Token limit errors (both providers have different limits) - Additional system instructions - Image support (2 tests, vision models differ) Additional optimizations: - Token limit test: 15 → 10 messages (saves ~10-20s) - Tool policy timeouts: 30s → 20s - Simplified non-critical prompts Impact: - Before: 97s, 20 API calls, 19 tests - After: ~110-125s, 27 API calls, 26 tests - Net: +13-28s for significantly better provider coverage Philosophy: "Err on side of matrix" - test critical features across both providers while keeping pure application logic (IPC, validation, our business logic) as single-provider tests. Generated with `cmux`
ammar-agent
changed the title
Generated with `cmux`
10 messages wasn't enough to trigger context exceeded errors on updated models. Reverting to 15 messages which reliably triggers the error on both providers. Generated with `cmux`
OpenAI has different context limits and requires special options to disable auto-truncation. Restored original logic: - Anthropic: 15 messages (reduced from original 40 for speed) - OpenAI: 30 messages + disable auto-truncation Generated with `cmux`
Generated with `cmux`
…code - Add VISION_MODEL_CONFIGS with gpt-4o and claude-sonnet-4-5 (both support vision) - Extract sendImageMessage() helper to eliminate duplication between tests - Remove stale comment about single-provider image tests - Tests now use vision-capable models that properly handle image inputs
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.
Problem
sendMessage.test.tsintegration tests were taking 274 seconds to run, slowing down CI feedback loops.Solution
Applied two rounds of optimization:
Round 1: Initial Optimization (274s → 97s)
Round 2: Matrix Expansion (97s → ~110-125s)
Following the directive to "err on the side of more tests in the matrix", expanded coverage for critical features that may have provider-specific behavior:
Moved to matrix (7 tests → 14 API calls):
Additional optimizations:
Impact
After Round 2 (Current)
Trade-off: Modest runtime increase for comprehensive provider coverage on critical features.
Overall (Both Rounds)
Test Distribution Logic
Matrix Tests (20 tests)
Single Provider (12 tests)
Verification
Generated with
cmux