fix(llm-agents): bound the multi-tool sequence run and report its real failure (#1378) - #1381
Open
Victor-w-Madeira wants to merge 2 commits into
Open
fix(llm-agents): bound the multi-tool sequence run and report its real failure (#1378)#1381Victor-w-Madeira wants to merge 2 commits into
Victor-w-Madeira wants to merge 2 commits into
Conversation
…l failure (#1378) Test 3 of agent-multi-tool-selection failed 3/3 on the 2026-08-08 PR run reporting `element(s) not found` on a Playground reply bubble. That message named the wrong thing: the run never produced a reply because the agent blew past the model's context window. Root cause is upstream and unbounded. `perform_web_search()` iterates every `div.result` DuckDuckGo returns and scrapes each linked page's entire text, with no result cap and no content truncation (langflow-ai/langflow#14469). Measured on 1.12.0.dev20: 10 results, 182,316 chars ~= 45.6k tokens in ONE call. Test 3 is the only test here whose instruction permits an open-ended sequence, so a non-converging run re-sends that payload every turn: 206,881 / 206,902 / 271,317 tokens in CI, 5,060,863 locally. Not a rate-limit tier problem -- the local org allows 4M TPM against CI's 200k and blew through it anyway, and no context window holds 5M tokens. Two changes: - Cap max_iterations at 8 for test 3, and assert the field holds that value (a fill that silently no-ops leaves the default 15 and re-opens this). - Drop the reply-bubble assert from test 3. The spec doc never specified one for it (tests 1-2 do specify it, at their step 6) -- it was orphan code, and it was the line that mis-reported every blow-up. A crashed run is still caught by the fixture, which owns that verdict. The cap bounds the blast radius; it does NOT make the test deterministic. Measured, --retries=0, 1.12.0.dev20 / gpt-4o-mini: default 15 -> 4/5, cap 8 -> 5/6 (same rate, 5M -> 129k worst case), cap 4 -> 0/2 (recursion_limit 13 is hit and no AI message persists at all). One search call is already unbounded -- 15,857 / 53,714 / 78,848 tokens across three queries the same day -- so no iteration cap can guarantee this test. Real stabilisation needs #14469. Test 3 stays out of @stable and its checklist bullet stays [-]. Force-fail executed: M5 (cap at a non-existent field id -> step fails), M4 (inverted sequence assert -> "tools out of order"). M1/M3 on the two @stable siblings were NOT executable locally: httpbin.org returns 503 and the describe is serial. Test 2 passes isolated; test 1 is unvalidated locally and is covered by the PR lane, where ECHO_BASE_URL points at go-httpbin.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses a flaky/deterministically-failing agent sequence test (agent-multi-tool-selection test 3) by bounding the agent run’s iteration budget and removing a misleading UI assertion, while documenting the rationale in the spec doc and checklist.
Changes:
- Add a
max_iterationscap (8) for the chained multi-tool sequence test and verify the value is actually applied. - Remove the “reply bubble renders” UI assertion from test 3 so failures are attributed to the underlying run/fixture verdict instead of “element(s) not found”.
- Update the spec doc and QA checklist entry to reflect the new bounded behavior and rationale.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/tests-automations/regression/core-functionality/llm-agents/agent-multi-tool-selection.spec.ts | Adds setMaxIterations() helper + applies an iteration cap for test 3; removes the orphan reply-bubble assertion. |
| QA-CHECKLIST.md | Updates the Tools & Integrations bullet for the multi-tool sequence test to note the new cap and why it remains [-]. |
| docs/core-functionality/llm-agents/agent-multi-tool-selection.md | Expands test 3 documentation with the cap rationale and updates the step-by-step contract. |
Suppressed comments (1)
docs/core-functionality/llm-agents/agent-multi-tool-selection.md:328
- This section contradicts the updated test contract above (Step 4 caps max_iterations at 8). It still claims a cap of 4 / 4 model calls / ~90k tokens, which is inconsistent with the spec (MAX_ITERATIONS_SEQUENCE = "8") and the new doc section.
tool round-trip for tests 1–2. **Test 3 is not one round-trip** — it runs a
multi-tool sequence bounded by the `max_iterations` cap of 4, so it costs up
to 4 model calls and sends up to ~90k tokens (see the context-budget note in
Step by step). It was unbounded before #1378, at up to 15 calls and millions
of tokens.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+182
to
+184
| // needs the payload bounded upstream (langflow-ai/langflow#14469); until then | ||
| // this test stays out of @stable and #1378 stays open. Re-measure before | ||
| // changing this number -- do not re-derive it on paper. |
Comment on lines
+231
to
+234
| > payload bounded upstream (`langflow-ai/langflow#14469`). Until then this | ||
| > test stays out of `@stable`, its checklist bullet stays `[-]`, and #1378 | ||
| > stays open. **Re-measure before changing the number — do not re-derive it | ||
| > on paper.** |
| #### 6.4 Tools and Integrations | ||
| - [ ] Agent with integrated external MCP tool executes action and returns result | ||
| - [-] Agent executes multiple tools in sequence → `llm-agents/agent-multi-tool-selection.spec.ts` (Test 3 — chained fetch→search, ordered `tool_use` assert; `@stable` gated on the clean baseline #818, per #827) | ||
| - [-] Agent executes multiple tools in sequence → `llm-agents/agent-multi-tool-selection.spec.ts` (Test 3 — chained fetch→search, ordered `tool_use` assert; `@stable` gated on the clean baseline #818, per #827. #1378 bounded the run with a measured `max_iterations` cap after the unbounded Web Search payload blew the context up — still `[-]`: the cap raises the pass rate, it does not make the agent's convergence deterministic) |
…ements (#1378) Copilot review on #1381, three findings, all correct: - "#1378 stays open" (spec comment + spec doc) contradicts the PR's own `Closes #1378`. After merge the sentence would be false. Both now point at the real dependency, langflow-ai/langflow#14469, and state the consequence that survives the merge: the test stays out of @stable and its bullet stays [-]. - The QA-CHECKLIST bullet claimed the cap "raises the pass rate". It does not, and the spec's own comment says so: 4/5 -> 5/6, within noise. The bullet was written before the cap=8 burst was measured and never revisited. It now states what the cap actually buys (bounded cost, correct attribution) and names the measurement. One more, not flagged by the review, in the region it pointed at: the External dependencies section still described the abandoned cap of 4 ("up to 4 model calls ... ~90k tokens"), a leftover from the arithmetic that was replaced by the measured 8. Corrected to 8 calls and the heaviest measured request (129,150 tokens). No behaviour change -- comments, spec doc and checklist bullet only.
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.
Closes #1378.
Problem
Test 3 of
agent-multi-tool-selection.spec.ts— agent runs the URL then Web Search tools in sequence — failed 3/3 on the 2026-08-08 PR run (31244373991) with:That message named the wrong thing. The bubble was absent because the run never produced a reply: the agent blew past the model's context window and the provider rejected the request. The spec was selected transitively on an unrelated PR, so the red looked like someone else's problem.
Root cause — upstream and unbounded
WebSearchComponent.perform_web_search()iterates everydiv.resultDuckDuckGo returns, fetches each linked page and extracts its entire text. No result cap, no content truncation:Measured in the container on
1.12.0.dev20, query"Sample Slide Show": 10 results, 182,316 chars ≈ 45.6k tokens in ONE call (largest single page: 40,755 chars).git log -S"max_results"across all refs: never existed — long-standing behavior, not a regression. Filed upstream as langflow-ai/langflow#14469.Test 3 is the only test in this file whose instruction permits an open-ended sequence, so when the agent does not converge it keeps calling
perform_searchand the conversation is re-sent every turn. Observed: 206,881 / 206,902 / 271,317 tokens in CI and 5,060,863 locally.Not a rate-limit tier problem, and that decides the fix. The CI org caps at 200k TPM; the local one at 4M — 20× larger — and the local run blew through it anyway. A bigger tier or a wider model buys nothing: no window on the market holds 5M tokens.
Changes
1. Cap
max_iterationsat 8 for test 3, and assert the field actually holds that value — a fill that silently no-ops leaves the default 15 in place and re-opens this on a run that still looks green.2. Drop the reply-bubble assert from test 3. The spec doc never specified one for it — tests 1–2 do specify it (their step 6), test 3 goes from step 4 straight to the sequence assert. It was orphan code, and it was the line that mis-reported every blow-up. A crashed run is still caught by the fixture, which owns that verdict (no
allowFlowErrors).What this does NOT do
It does not make the test deterministic. Measured,
--retries=0,1.12.0.dev20/gpt-4o-mini:max_iterationsRecursion limit of 13 reached without hitting a stop conditionThe cap buys cost and attribution, not reliability. A single
perform_searchcall is already unbounded — measured the same day across three queries: 15,857 / 53,714 / 78,848 tokens for one call, a 5× spread, with the query chosen by the agent. Two calls at the top of that range exceed 128k on their own, so no iteration cap can guarantee this test. Real stabilisation needs #14469.Test 3 therefore stays out of
@stableand its checklist bullet stays[-].Rejected — the arithmetic that produced 4. An earlier version derived the cap from a token budget (three calls needed, plus headroom, worst case inside 128k) and got 4. It is wrong in a way worth recording:
max_iterations=4sets a LangGraphrecursion_limitof 13, the agent hits it, and a run that stops that way persists no AI message at all — so the sequence assert fails on absent data rather than wrong data, and the cap meant to fix the test breaks it a second way. The spec doc records this so it is not re-derived.Covered tests
agent runs the URL then Web Search tools in sequence for a chained prompttool_uselist containsfetch_contentbeforeperform_search. Cap applied and verified on the field before the runTests 1–2 are untouched in logic; the file gained one import and one helper used only by test 3.
Validation (nightly
1.12.0.dev20,--workers=1 --retries=0,MODEL_TEST_ID=gpt-4o-mini)npm run typecheck✅ ·npm run lint✅ 0 errors ·npm run test:units✅ 552/552 ·npm run test:scripts✅ 797/797 · QA-CHECKLIST guard ✅ · coverage guard ✅ · spec-dep guard ✅ (exit 0)Force-fail — executed
TimeoutError: locator.click … inspector-add-max_iterations_FF_MUTATION. Proves the cap actually appliesperform_searchbeforefetch_content)tools out of order: ["fetch_content","perform_search"] … expected ["perform_search","fetch_content"]. Needed 2 attempts — the first failed on a blow-up, not the mutation. A third attempt showed["fetch_content","perform_search","perform_search"], the double search that drives the problemRevert proven:
grep FF_MUTATION→ 0 hits, file byte-identical to the pre-FF copy, gates re-run green.Gap — stated, not papered over
M1/M3on the two@stablesiblings were not executable locally.httpbin.orgreturns503(the #631 failure mode), test 1 fails on it, and the describe ismode: "serial"so the siblings are skipped. Test 2 passes isolated; test 1 is unvalidated locally. It is covered by this PR's lane, whereECHO_BASE_URLpoints at the in-network go-httpbin and the503does not occur — the PR runs all three tests, including both@stableones. Closing the gap locally would need the Langflow container restarted withLANGFLOW_SSRF_ALLOWED_HOSTS, discarding its credentials.CI expectation
The import graph selects exactly this spec (1 spec, 0 dropped, not suite-wide) and
needsModels=true, soCollect modelsruns and all three tests execute. Note the residual: the CI org caps at 200k TPM and retries land inside the same one-minute window, so failures there correlate rather than flake independently — that is why the original run was 3/3 rather than intermittent. The cap cuts consumption by ~40×, which makes it far less likely, but this was validated against a 4M-TPM org and cannot be asserted for CI without running there.Follow-ups
setMaxIterations()helper now duplicated withagent-max-iterations.spec.ts(left duplicated here on purpose: extracting would edit an@stablespec and widen this PR's blast radius).Dependencies
None beyond the usual provider key. No new external dependency;
max_iterationsis set through existing testids.