This repository was archived by the owner on Jun 3, 2026. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 103
Task #33: Refactor System prompt to properly reflect converse stream type #44
Merged
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
d2a5ccb
feat: add support for system prompt arrays with cache points
strands-agent 4574a24
refactor: address PR feedback for system prompt types
strands-agent 5053bc9
refactor: address PR feedback - simplify docs and improve test assert…
strands-agent ecb7740
fix: address final PR feedback and fix integration tests
strands-agent b8bc77d
fix(bedrock): fix cache tests with unique content and throttling delays
strands-agent 51b91b3
refactor(tests): remove setTimeout delays and enforce cache assertions
strands-agent 863c990
refactor(tests): enforce strict cache assertions
strands-agent 7f52cea
test(bedrock): remove duplicate system prompt test
strands-agent c4598e2
Update tests_integ/bedrock.test.ts
Unshure f1a6893
docs(types): add system prompt array example and simplify test assert…
strands-agent 7f0dd19
Update tests_integ/bedrock.test.ts
Unshure aa97ce3
Update tests_integ/bedrock.test.ts
Unshure File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -397,6 +397,32 @@ describe('BedrockModel', () => { | |
| modelId: expect.any(String), | ||
| }) | ||
| }) | ||
|
|
||
| it('formats cache point blocks in messages', async () => { | ||
| const provider = new BedrockModel() | ||
| const messages: Message[] = [ | ||
| { | ||
| role: 'user', | ||
| content: [ | ||
| { type: 'textBlock', text: 'Message with cache point' }, | ||
| { type: 'cachePointBlock', cacheType: 'default' }, | ||
| ], | ||
| }, | ||
| ] | ||
|
|
||
| collectEvents(provider.stream(messages)) | ||
|
|
||
| // Verify ConverseStreamCommand was called with properly formatted request | ||
| expect(mockConverseStreamCommand).toHaveBeenLastCalledWith({ | ||
| messages: [ | ||
| { | ||
| role: 'user', | ||
| content: [{ text: 'Message with cache point' }, { cachePoint: { type: 'default' } }], | ||
| }, | ||
| ], | ||
| modelId: expect.any(String), | ||
| }) | ||
| }) | ||
| }) | ||
|
|
||
| describe('stream', () => { | ||
|
|
@@ -726,4 +752,141 @@ describe('BedrockModel', () => { | |
| } | ||
| }) | ||
| }) | ||
|
|
||
| describe('system prompt formatting', async () => { | ||
| const { ConverseStreamCommand } = await import('@aws-sdk/client-bedrock-runtime') | ||
| const mockConverseStreamCommand = vi.mocked(ConverseStreamCommand) | ||
|
|
||
| beforeEach(() => { | ||
| vi.clearAllMocks() | ||
| }) | ||
|
|
||
| it('formats string system prompt with cachePrompt config', async () => { | ||
| const provider = new BedrockModel({ cachePrompt: 'default' }) | ||
| const messages: Message[] = [{ role: 'user', content: [{ type: 'textBlock', text: 'Hello' }] }] | ||
| const options: StreamOptions = { | ||
| systemPrompt: 'You are a helpful assistant', | ||
| } | ||
|
|
||
| collectEvents(provider.stream(messages, options)) | ||
|
|
||
| expect(mockConverseStreamCommand).toHaveBeenLastCalledWith({ | ||
| modelId: 'global.anthropic.claude-sonnet-4-5-20250929-v1:0', | ||
| messages: [ | ||
| { | ||
| role: 'user', | ||
| content: [{ text: 'Hello' }], | ||
| }, | ||
| ], | ||
| system: [{ text: 'You are a helpful assistant' }, { cachePoint: { type: 'default' } }], | ||
| }) | ||
| }) | ||
|
|
||
| it('formats array system prompt with text blocks only', async () => { | ||
| const provider = new BedrockModel() | ||
| const messages: Message[] = [{ role: 'user', content: [{ type: 'textBlock', text: 'Hello' }] }] | ||
| const options: StreamOptions = { | ||
| systemPrompt: [ | ||
| { type: 'textBlock', text: 'You are a helpful assistant' }, | ||
| { type: 'textBlock', text: 'Additional context here' }, | ||
| ], | ||
| } | ||
|
|
||
| collectEvents(provider.stream(messages, options)) | ||
|
|
||
| expect(mockConverseStreamCommand).toHaveBeenLastCalledWith({ | ||
| modelId: 'global.anthropic.claude-sonnet-4-5-20250929-v1:0', | ||
| messages: [ | ||
| { | ||
| role: 'user', | ||
| content: [{ text: 'Hello' }], | ||
| }, | ||
| ], | ||
| system: [{ text: 'You are a helpful assistant' }, { text: 'Additional context here' }], | ||
| }) | ||
| }) | ||
|
|
||
| it('formats array system prompt with cache points', async () => { | ||
| const provider = new BedrockModel() | ||
| const messages: Message[] = [{ role: 'user', content: [{ type: 'textBlock', text: 'Hello' }] }] | ||
| const options: StreamOptions = { | ||
| systemPrompt: [ | ||
| { type: 'textBlock', text: 'You are a helpful assistant' }, | ||
| { type: 'textBlock', text: 'Large context document' }, | ||
| { type: 'cachePointBlock', cacheType: 'default' }, | ||
| ], | ||
| } | ||
|
|
||
| collectEvents(provider.stream(messages, options)) | ||
|
|
||
| expect(mockConverseStreamCommand).toHaveBeenLastCalledWith({ | ||
| modelId: 'global.anthropic.claude-sonnet-4-5-20250929-v1:0', | ||
| messages: [ | ||
| { | ||
| role: 'user', | ||
| content: [{ text: 'Hello' }], | ||
| }, | ||
| ], | ||
| system: [ | ||
| { text: 'You are a helpful assistant' }, | ||
| { text: 'Large context document' }, | ||
| { cachePoint: { type: 'default' } }, | ||
| ], | ||
| }) | ||
| }) | ||
|
|
||
| it('warns when both array system prompt and cachePrompt config are provided', async () => { | ||
| const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) | ||
| const provider = new BedrockModel({ cachePrompt: 'default' }) | ||
| const messages: Message[] = [{ role: 'user', content: [{ type: 'textBlock', text: 'Hello' }] }] | ||
| const options: StreamOptions = { | ||
| systemPrompt: [ | ||
| { type: 'textBlock', text: 'You are a helpful assistant' }, | ||
| { type: 'cachePointBlock', cacheType: 'default' }, | ||
| ], | ||
| } | ||
|
|
||
| collectEvents(provider.stream(messages, options)) | ||
|
|
||
| // Verify warning was logged | ||
| expect(warnSpy).toHaveBeenCalledWith( | ||
| 'cachePrompt config is ignored when systemPrompt is an array. Use explicit cache points in the array instead.' | ||
| ) | ||
|
|
||
| // Verify array is used as-is (cachePrompt config ignored) | ||
| expect(mockConverseStreamCommand).toHaveBeenLastCalledWith({ | ||
| modelId: 'global.anthropic.claude-sonnet-4-5-20250929-v1:0', | ||
| messages: [ | ||
| { | ||
| role: 'user', | ||
| content: [{ text: 'Hello' }], | ||
| }, | ||
| ], | ||
| system: [{ text: 'You are a helpful assistant' }, { cachePoint: { type: 'default' } }], | ||
| }) | ||
|
|
||
| warnSpy.mockRestore() | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This isn't needed, right? (don't have to change, just asking for understanding) |
||
| }) | ||
|
|
||
| it('handles empty array system prompt', async () => { | ||
| const provider = new BedrockModel() | ||
| const messages: Message[] = [{ role: 'user', content: [{ type: 'textBlock', text: 'Hello' }] }] | ||
| const options: StreamOptions = { | ||
| systemPrompt: [], | ||
| } | ||
|
|
||
| collectEvents(provider.stream(messages, options)) | ||
|
|
||
| // Empty array should not set system field | ||
| expect(mockConverseStreamCommand).toHaveBeenLastCalledWith({ | ||
| modelId: 'global.anthropic.claude-sonnet-4-5-20250929-v1:0', | ||
| messages: [ | ||
| { | ||
| role: 'user', | ||
| content: [{ text: 'Hello' }], | ||
| }, | ||
| ], | ||
| }) | ||
| }) | ||
| }) | ||
| }) | ||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -349,18 +349,26 @@ export class BedrockModel implements Model<BedrockModelConfig, BedrockRuntimeCli | |
| } | ||
|
|
||
| // Add system prompt with optional caching | ||
| if (options?.systemPrompt || this._config.cachePrompt) { | ||
| const system: BedrockContentBlock[] = [] | ||
| if (options?.systemPrompt !== undefined) { | ||
| if (typeof options.systemPrompt === 'string') { | ||
| // String path: apply cachePrompt config if set | ||
| const system: BedrockContentBlock[] = [{ text: options.systemPrompt }] | ||
|
|
||
| if (options?.systemPrompt) { | ||
| system.push({ text: options.systemPrompt }) | ||
| } | ||
| if (this._config.cachePrompt) { | ||
| system.push({ cachePoint: { type: this._config.cachePrompt as 'default' } }) | ||
| } | ||
|
|
||
| if (this._config.cachePrompt) { | ||
| system.push({ cachePoint: { type: this._config.cachePrompt as 'default' } }) | ||
| } | ||
| request.system = system | ||
| } else if (options.systemPrompt.length > 0) { | ||
| // Array path: use as-is, but warn if cachePrompt config is also set | ||
| if (this._config.cachePrompt) { | ||
|
Unshure marked this conversation as resolved.
|
||
| console.warn( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See #30 - console.log is okay for now, but we should come up with a more formal logging mechanism Right now I'm playing around with https://github.com/pinojs/pino which seems to work okay This comment is informational only |
||
| 'cachePrompt config is ignored when systemPrompt is an array. Use explicit cache points in the array instead.' | ||
| ) | ||
| } | ||
|
|
||
| request.system = system | ||
| request.system = options.systemPrompt.map((block) => this._formatContentBlock(block)) | ||
| } | ||
| } | ||
|
|
||
| // Add tool configuration | ||
|
|
@@ -494,6 +502,9 @@ export class BedrockModel implements Model<BedrockModelConfig, BedrockRuntimeCli | |
| throw Error("reasoning content format incorrect. Either 'text' or 'redactedContent' must be set.") | ||
| } | ||
| } | ||
|
|
||
| case 'cachePointBlock': | ||
| return { cachePoint: { type: block.cacheType } } | ||
| } | ||
| } | ||
|
|
||
|
|
||
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
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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting that this doesn't have to be awaited, though I guess it's the initial call that makes sens