Wait for idle instead of polling traced queries in LangSmith tests - #1830
Open
DABH wants to merge 5 commits into
Open
Wait for idle instead of polling traced queries in LangSmith tests#1830DABH wants to merge 5 commits into
DABH wants to merge 5 commits into
Conversation
TestComprehensiveTracing and the plugin end-to-end test waited for the workflow to reach its signal wait by polling is_waiting_for_signal every second through the traced client, then asserted that every QueryWorkflow run had exactly one HandleQuery child. Temporal does not guarantee that: a query issued while a workflow task is pending or started is buffered by the server and attached to every workflow task start until it completes, including the retry after the task times out or its completion is rejected, so the worker evaluates it twice (two HandleQuery runs). If the client's 30s RPC timeout fires first, the server drops the query before it is dispatched at all (no HandleQuery run). CI logs of the failing runs show exactly this: a QueryWorkflow call lasting 29.998s followed by "Error reporting WFT to server" for the test workflow. The same stalls made the unguarded raw query fail with "Timeout expired" and, once a task reached attempt 3, made the server reject updates with "Workflow Task in failed state". Readiness is now observed through the raw client's describe(), which is not traced and involves no workflow task, and the traced query is issued exactly once after the workflow is idle, so it is dispatched directly as a query task and cannot be re-attached to a retried workflow task. Updates are only sent once the workflow is idle again after the signal. The query assertion becomes an exact hierarchy check instead of a loop over a variable number of polls.
There was a problem hiding this comment.
🟢 Approval recommended
The focused test changes consistently avoid traced polling while preserving and strengthening trace assertions.
Pull request overview
Reworks LangSmith integration tests to wait for idle workflows before issuing single traced queries, reducing flaky duplicate or missing traces.
Changes:
- Adds untraced workflow-idle polling via
describe(). - Replaces traced query polling with one traced query and exact hierarchy assertions.
- Asserts raw query results before updates.
File summaries
| File | Description |
|---|---|
tests/contrib/langsmith/test_plugin.py |
Uses idle waits and single-query trace assertions. |
tests/contrib/langsmith/test_integration.py |
Adds idle/query helpers and updates integration scenarios. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The worker's RunWorkflow run reaches the collector asynchronously and can arrive after describe() already reports the first task complete, so clearing the collector after the idle wait raced it and left a stray root on fast runners. Nothing needs clearing now that readiness no longer polls a traced query, so assert the full hierarchy instead.
The traced activities finish instantly; their 10s start-to-close bound only satisfies Temporal. On a stalled runner an attempt timed out, the retry ran the activity again, and the exact hierarchy assertion saw one extra run. Give the bound a minute and disable retries so a failed attempt fails loudly.
DABH
marked this pull request as ready for review
September 11, 2026 04:44
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
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.
What was changed
The LangSmith tests wait for the workflow to go idle via untraced
describe()polling, issue the traced query exactly once, assert its exact trace, and only send updates once the workflow is idle again. The raw query result is now asserted too. The built-in query filtering test no longer clears the collector and asserts the full hierarchy, including theRunWorkflowroot.Why
The tests polled a traced query while the workflow was still running and asserted exactly one
HandleQueryrun per client query. The server attaches a buffered query to every workflow task attempt until it completes and drops it when the client RPC ends, so a stalled task yields duplicate handler runs or none, plusTimeout expiredandWorkflow Task in failed stateon the update. Each failing CI run shows a 30sQueryWorkflowfollowed by a worker eviction.Testing
All four failure shapes reproduced by holding a workflow task completion for 15s and 31s; the reworked tests pass under both. 90/90 flake-finder runs under load. Lint clean.