fix: Bedrock debt remediation from PR #144 CR#146
Merged
Conversation
commit: |
d8d364b to
bae1bf1
Compare
Move createMockReq/createMockRes/createDefaults into helpers/mock-res.ts to eliminate duplication between bedrock and bedrock-converse test files. Convert reasoning-all-providers to per-test server lifecycle (beforeEach/afterEach) preventing cross-test state leakage.
Set completionReq.stream = true in streaming handler. Use deterministic
tool_use_${index} fallback IDs instead of random generation. Change
textContent || null to ?? null to preserve empty strings. Warn on
unsupported content block types and unexpected roles. Add webSearches
warning on tool-call-only responses. Update tests for new semantics.
unwrap inputSchema
Wrap contentBlockStop and messageStop payloads to match real AWS
Converse API shape. Remove duplicate top-level contentBlockIndex from
contentBlockStart/contentBlockDelta. Add trailing metadata events
(usage + latencyMs) to all three stream builders. Filter empty-string
text blocks in converseToCompletionRequest. Unwrap inputSchema from
Converse { json: {...} } wrapper. Set completionReq.stream = true in
streaming handler. Add content-loss warnings for non-text blocks.
Fix error type || to ??.
Update messageStop/contentBlockStop assertions for wrapped shapes. Add safeResolve guard in postPartialBinary. Add unit tests for contentBlockStop shape, contentWithToolCalls stream structure, and metadata event presence. Add content+toolCalls streaming integration coverage for both invoke and converse paths.
806d9c4 to
601f107
Compare
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.
Summary
Remediates all 15 debt items from the PR #144 code review backlog plus 8 pre-existing issues surfaced during CR. Includes the PR #144 native stream format changes that landed after v1.16.1.
Fixed
invoke-with-response-streamnow emits Anthropic-native snake_case payloads (content_block_delta,input_json_delta) wrapped in Bedrock EventStreamchunkframes, instead of Converse-style camelCase events. Converse-stream retains camelCase format. (PR fix: emit native Bedrock invoke stream chunks #144, sf-jin-ku)completionReq.stream = truein streaming handler; deterministictool_use_${index}fallback IDs;textContent ?? nullto preserve empty strings; unsupported content block and unexpected role warnings; webSearches warning on tool-call-only responsescontentBlockStopandmessageStoppayloads to match real AWS Converse API; remove duplicate top-levelcontentBlockIndex; add trailingmetadataevents (usage + latencyMs) to all stream buildersinputSchemafrom{ json: {...} }Converse API wrapper;completionReq.stream = truein streaming handler; content-loss warnings; error type??fixChanged
createMockReq/createMockRes/createDefaults) inhelpers/mock-res.tsTest plan
pnpm test)pnpm run format:check)pnpm run lint)