fix(amazonq): narrow tool-use truncation detection and classify retries in telemetry - #2847
Merged
laileni-aws merged 4 commits intoAug 20, 2026
Conversation
Treating every response stream that ends without a terminating tool-use `stop` event as a truncated tool input is too broad. A stream also ends without `stop` when the request is aborted (response-processing timeout or cancellation), and when the model announced a tool use but streamed no input at all. Reporting those as failures and re-prompting the model produces failed intermediate stream events for turns that were not broken, and can re-run the agent loop without making progress. - finalize() now only reports an incomplete tool input when partial input was actually received and the request was not aborted; other unterminated tool uses are left unstopped and filtered out downstream, as before. - The abort state is passed into finalize() from the response processor. - Consecutive incomplete tool-use retries are now bounded (MAX_INCOMPLETE_TOOL_USE_RETRIES). On exceeding the limit the agent loop stops and surfaces an actionable error instead of retrying indefinitely. Genuine truncation (partial input present, request not aborted) still routes into the existing recovery path and is retried.
laileni-aws
marked this pull request as ready for review
August 19, 2026 19:20
…metry A response stream whose tool-use input is cut off is retried inside the agent loop and usually recovers within the same user turn. Every one of those iterations emits amazonq_invokeLLM with result='Failed', so a per-call success rate built on that metric drops even though the user was unaffected. Report a `reason` alongside the existing result so the two cases can be told apart downstream: INCOMPLETE_TOOL_USE_RETRYING retry budget remains; transient, recovers INCOMPLETE_TOOL_USE_EXHAUSTED retries used up; the user sees an error The classification is computed before the emit. Because the retry budget is bounded, whether this iteration will be retried is already known at that point, so no post-hoc correlation is needed. result stays 'Failed' in both cases, so raw failure counts are unchanged and remain available for diagnostics. Consumers can now exclude the transient class from success-rate calculations while still counting the terminal give-up. The reason values are consumed by ToolkitTelemetryLambda to emit a separate EMF counter; renaming them requires updating that transform first. Retry behaviour is unchanged: incrementing the counter before the emit and testing `count <= MAX_INCOMPLETE_TOOL_USE_RETRIES` preserves the existing off-by-one, so 3 retries still follow the initial failure.
laileni-aws
marked this pull request as draft
August 19, 2026 22:33
The do-not-rename note on the reason constants pointed at a specific internal consumer by name. Describe it as a downstream metrics pipeline instead: the warning is what matters to anyone editing these values, and the constants are part of a contract rather than a link to one particular implementation. No functional change.
laileni-aws
marked this pull request as ready for review
August 19, 2026 23:12
kmmcclai
approved these changes
Aug 19, 2026
xinyiww1
approved these changes
Aug 20, 2026
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.
Issue
Three related problems in how the agentic chat loop handles a response stream that ends without a terminating tool-use
stopevent.1. Truncation detection was too broad. Every such stream was treated as a truncated tool input. But a stream also ends without
stopwhen:stopis expected and the abort is already reported by its own path; andReporting those as failures and re-prompting the model produces failed intermediate stream events for turns that were never broken, and can re-run the agent loop without making progress.
2. Retries were unbounded. The incomplete-tool-use retry path had no iteration limit, so repeated truncation could keep the loop running indefinitely.
3. The telemetry could not distinguish the cases. Every retried iteration emits
amazonq_invokeLLMwithresult='Failed', with nothing to separate a transient iteration that recovers inside the same user turn from a terminal failure the user actually sees. Any per-call success rate built on that metric therefore degrades precisely when error handling becomes more honest, even though users are unaffected.Changes
finalize()only reports genuine truncation. It now requires that partial input was actually received and that the request was not aborted. Other unterminated tool uses are left unstopped and filtered out downstream, exactly as they were before this behaviour was introduced. The abort state is passed intofinalize()from the response processor.Retries are bounded.
MAX_INCOMPLETE_TOOL_USE_RETRIES = 3, so at most 4 attempts in total. On exceeding the limit the loop stops and surfaces an actionable error to the user instead of retrying indefinitely.Failures are classified for telemetry.
amazonq_invokeLLMnow carries areasonalongside the existingresult:reasonINCOMPLETE_TOOL_USE_RETRYINGINCOMPLETE_TOOL_USE_EXHAUSTEDresultstays'Failed'in both cases, so raw failure counts are unchanged and remain available for diagnostics. Downstream consumers can exclude the transient class from per-call success rates while still counting the terminal give-up.The classification is computed before the emit. Because the retry budget is bounded, whether an iteration will be retried is already known at that point, so no post-hoc correlation is needed.
Genuine truncation still routes into the existing recovery path and is retried.
Why two reason values rather than one truncation flag
Excluding all truncation from a success rate would also hide the case where the agent retries, exhausts its budget and gives up — which the user does experience as an error. Keeping the terminal case distinct avoids trading one blind spot for another, which is the mistake that motivated this change in the first place.
Compatibility
reasonis a new optional field andresultsemantics are unchanged, so existing consumers are unaffected. The two string values are read by the downstream telemetry transform to emit a separate counter, so renaming either requires updating that transform first — noted in a comment on the constants.Retry behaviour is unchanged by the telemetry commit: incrementing the counter before the emit and testing
count <= MAX_INCOMPLETE_TOOL_USE_RETRIESpreserves the existing off-by-one.Testing
agenticChatEventParser.test.ts— 10/10 pass, including new cases for the aborted path and the zero-input path.chatTelemetryController.test.ts— 20/20 pass, including 3 new cases coveringreasonforwarding, the retrying/exhausted distinction, and thatresultstays'Failed'.tsc --noEmitclean for all changed files.Not covered: no end-to-end agent-loop test asserts the
RETRYINGtoEXHAUSTEDtransition at the retry-limit boundary. The existing controller test harness stubs the streaming client and could support one — happy to add it if reviewers want that boundary locked down.