Skip to content

Conversation history returns MessageSelect[] and formats at call site #2743

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mike-inkeep wants to merge 4 commits into
base: main
Choose a base branch
from
+287 −123
Open

Conversation history returns MessageSelect[] and formats at call site #2743

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8eea5bf
Return MessageSelect[] from history APIs and format at generation cal…
mike-inkeep Mar 18, 2026
a96c949
Merge remote-tracking branch 'origin/main' into stack/history_shape
mike-inkeep Mar 23, 2026
4e1416a
Further review feedback and tightening of types
mike-inkeep Mar 24, 2026
37a910e
Merge remote-tracking branch 'origin/main' into stack/history_shape
mike-inkeep Mar 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 36 additions & 17 deletions agents-api/src/__tests__/run/agents/Agent.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,10 @@ const {
getToolsForAgentMock,
getFunctionToolsForSubAgentMock,
buildPersistedMessageContentMock,
createDefaultConversationHistoryConfigMock,
getFormattedConversationHistoryMock,
getConversationHistoryWithCompressionMock,
formatMessagesAsConversationHistoryMock,
} = vi.hoisted(() => {
const getCredentialReferenceMock = vi.fn(() => vi.fn().mockResolvedValue(null));
const getContextConfigByIdMock = vi.fn(() => vi.fn().mockResolvedValue(null));
Expand All @@ -159,6 +163,20 @@ const {
);
const getFunctionToolsForSubAgentMock = vi.fn().mockResolvedValue([]);
const buildPersistedMessageContentMock = vi.fn();
const createDefaultConversationHistoryConfigMock = vi.fn().mockReturnValue({
mode: 'full',
limit: 50,
includeInternal: true,
messageTypes: ['chat'],
maxOutputTokens: 4000,
});
const getFormattedConversationHistoryMock = vi
.fn()
.mockResolvedValue('Mock conversation history');
const getConversationHistoryWithCompressionMock = vi.fn().mockResolvedValue([]);
const formatMessagesAsConversationHistoryMock = vi
.fn()
.mockReturnValue('Mock conversation history');

return {
getCredentialReferenceMock,
Expand All @@ -170,6 +188,10 @@ const {
getToolsForAgentMock,
getFunctionToolsForSubAgentMock,
buildPersistedMessageContentMock,
createDefaultConversationHistoryConfigMock,
getFormattedConversationHistoryMock,
getConversationHistoryWithCompressionMock,
formatMessagesAsConversationHistoryMock,
};
});

Expand Down Expand Up @@ -229,9 +251,10 @@ vi.mock('../../../domains/run/data/conversations', async (importOriginal) => {
const actual = (await importOriginal()) as any;
return {
...actual,
getConversationHistoryWithCompression: vi
.fn()
.mockResolvedValue('Mock conversation history as string'),
createDefaultConversationHistoryConfig: createDefaultConversationHistoryConfigMock,
getFormattedConversationHistory: getFormattedConversationHistoryMock,
getConversationHistoryWithCompression: getConversationHistoryWithCompressionMock,
formatMessagesAsConversationHistory: formatMessagesAsConversationHistoryMock,
};
});

Expand Down Expand Up @@ -362,20 +385,16 @@ vi.mock('../../../domains/run/agents/SystemPromptBuilder.js', () => ({
})),
}));

vi.mock('../../../domains/run/data/conversations.js', () => ({
createDefaultConversationHistoryConfig: vi.fn().mockReturnValue({
mode: 'full',
limit: 50,
includeInternal: true,
messageTypes: ['chat'],
maxOutputTokens: 4000,
}),
getFormattedConversationHistory: vi.fn().mockResolvedValue('Mock conversation history'),
getConversationScopedArtifacts: vi.fn().mockResolvedValue([]),
getConversationHistoryWithCompression: vi
.fn()
.mockResolvedValue('Mock conversation history as string'),
}));
vi.mock('../../../domains/run/data/conversations.js', async (importOriginal) => {
const actual = (await importOriginal()) as any;
return {
...actual,
createDefaultConversationHistoryConfig: createDefaultConversationHistoryConfigMock,
getFormattedConversationHistory: getFormattedConversationHistoryMock,
getConversationHistoryWithCompression: getConversationHistoryWithCompressionMock,
formatMessagesAsConversationHistory: formatMessagesAsConversationHistoryMock,
};
});

// Import the mocked module - these will automatically be mocked
import {
Expand Down
69 changes: 61 additions & 8 deletions agents-api/src/__tests__/run/data/conversations.artifact-replacement.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,12 +97,15 @@ describe('getConversationHistoryWithCompression — artifact replacement', () =>

const result = await getConversationHistoryWithCompression(baseParams);

expect(result).toContain('Artifact: "Google Doc"');
expect(result).toContain('id: art-1');
expect(result).toContain('args:');
expect(result).toContain('description: Fetched document content');
expect(result).toContain('summary:');
expect(result).not.toContain(rawContent);
const toolResult = result.find((msg) => msg.messageType === 'tool-result');
const toolResultText = toolResult?.content?.text ?? '';

expect(toolResultText).toContain('Artifact: "Google Doc"');
expect(toolResultText).toContain('id: art-1');
expect(toolResultText).toContain('Tool call args:');
expect(toolResultText).toContain('description: Fetched document content');
expect(toolResultText).toContain('summary:');
expect(toolResultText).not.toContain(rawContent);
});

it('batches toolCallId lookups in a single getLedgerArtifacts call', async () => {
Expand Down Expand Up @@ -134,7 +137,57 @@ describe('getConversationHistoryWithCompression — artifact replacement', () =>

const result = await getConversationHistoryWithCompression(baseParams);

expect(result).toContain(content);
expect(result).not.toContain('Artifact:');
const toolResult = result.find((msg) => msg.messageType === 'tool-result');
const toolResultText = toolResult?.content?.text ?? '';

expect(toolResultText).toContain(content);
expect(toolResultText).not.toContain('Artifact:');
});

it('preserves all artifact references when multiple artifacts share a toolCallId', async () => {
const rawContent = 'raw tool output';
const messages = [makeToolResultMessage('tc-shared', rawContent)];

mockGetConversationHistory.mockReturnValue(vi.fn().mockResolvedValue(messages));
mockGetLedgerArtifacts.mockReturnValue(
vi.fn().mockResolvedValue([
{
artifactId: 'art-1',
toolCallId: 'tc-shared',
name: 'First',
description: 'First artifact',
parts: [{ kind: 'data', data: { summary: { text: 'one' } } }],
metadata: {},
createdAt: new Date().toISOString(),
},
{
artifactId: 'art-2',
toolCallId: 'tc-shared',
name: 'Second',
description: 'Second artifact',
parts: [{ kind: 'data', data: { summary: { text: 'two' } } }],
metadata: {},
createdAt: new Date().toISOString(),
},
])
);

const result = await getConversationHistoryWithCompression(baseParams);
const toolResult = result.find((msg) => msg.messageType === 'tool-result');
const toolResultText = toolResult?.content?.text ?? '';

expect(toolResultText).not.toContain(rawContent);
expect(toolResultText).toContain('Tool call args:');
expect(toolResultText.match(/Tool call args:/g)?.length).toBe(1);
const argsJson = JSON.stringify({ query: 'test' });
expect(toolResultText.split(argsJson).length - 1).toBe(1);

expect(toolResultText).toContain('Artifact: "First"');
expect(toolResultText).toContain('Artifact: "Second"');
expect(toolResultText).toContain('id: art-1');
expect(toolResultText).toContain('id: art-2');
Comment on lines +175 to +188
Copy link
Contributor

@pullfrog pullfrog bot Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test verifies both artifact IDs are present but doesn't assert the \n join between them. If the join separator were accidentally changed to empty string or space, this test would still pass. Consider:

expect(toolResultText).toMatch(/id: art-1[\s\S]*\n[\s\S]*id: art-2/);

expect(toolResultText).toContain('description: First artifact');
expect(toolResultText).toContain('description: Second artifact');
expect(toolResultText).toMatch(/\]\s*\n\n\[/);
});
});
40 changes: 24 additions & 16 deletions agents-api/src/__tests__/run/data/conversations.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import type { MessageSelect } from '@inkeep/agents-core';
import { describe, expect, it } from 'vitest';
import { reconstructMessageText } from '../../../domains/run/data/conversations';

Expand All @@ -17,12 +18,19 @@ describe('reconstructMessageText', () => {
expect(reconstructMessageText(msg)).toBe('');
});

it('uses part.type when kind is omitted (legacy shape)', () => {
const msg = {
content: { parts: [{ type: 'text', text: 'legacy' } as any] },
} as Pick<MessageSelect, 'content'>;
expect(reconstructMessageText(msg)).toBe('legacy');
});

it('concatenates text parts in order', () => {
const msg = {
content: {
parts: [
{ type: 'text', text: 'Hello ' },
{ type: 'text', text: 'world' },
{ kind: 'text', text: 'Hello ' },
{ kind: 'text', text: 'world' },
],
},
};
Expand All @@ -32,7 +40,7 @@ describe('reconstructMessageText', () => {
it('converts data parts with artifactId + toolCallId to artifact:ref tags', () => {
const msg = {
content: {
parts: [{ type: 'data', data: { artifactId: 'art-1', toolCallId: 'tool-1' } }],
parts: [{ kind: 'data', data: { artifactId: 'art-1', toolCallId: 'tool-1' } }],
},
};
expect(reconstructMessageText(msg)).toBe('<artifact:ref id="art-1" tool="tool-1" />');
Expand All @@ -42,9 +50,9 @@ describe('reconstructMessageText', () => {
const msg = {
content: {
parts: [
{ type: 'text', text: 'Here is the result. ' },
{ type: 'data', data: { artifactId: 'art-abc', toolCallId: 'toolu_xyz' } },
{ type: 'text', text: ' And more text.' },
{ kind: 'text', text: 'Here is the result. ' },
{ kind: 'data', data: { artifactId: 'art-abc', toolCallId: 'toolu_xyz' } },
{ kind: 'text', text: ' And more text.' },
],
},
};
Expand All @@ -58,7 +66,7 @@ describe('reconstructMessageText', () => {
content: {
parts: [
{
type: 'data',
kind: 'data',
data: JSON.stringify({ artifactId: 'art-json', toolCallId: 'tool-json' }),
},
],
Expand All @@ -70,7 +78,7 @@ describe('reconstructMessageText', () => {
it('ignores data parts without artifactId', () => {
const msg = {
content: {
parts: [{ type: 'data', data: { toolCallId: 'tool-1' } }],
parts: [{ kind: 'data', data: { toolCallId: 'tool-1' } }],
},
};
expect(reconstructMessageText(msg)).toBe('');
Expand All @@ -79,7 +87,7 @@ describe('reconstructMessageText', () => {
it('ignores data parts without toolCallId', () => {
const msg = {
content: {
parts: [{ type: 'data', data: { artifactId: 'art-1' } }],
parts: [{ kind: 'data', data: { artifactId: 'art-1' } }],
},
};
expect(reconstructMessageText(msg)).toBe('');
Expand All @@ -88,7 +96,7 @@ describe('reconstructMessageText', () => {
it('ignores data parts with unparseable JSON string', () => {
const msg = {
content: {
parts: [{ type: 'data', data: 'not-valid-json' }],
parts: [{ kind: 'data', data: 'not-valid-json' }],
},
};
expect(reconstructMessageText(msg)).toBe('');
Expand All @@ -97,7 +105,7 @@ describe('reconstructMessageText', () => {
it('returns empty string for unknown part types', () => {
const msg = {
content: {
parts: [{ type: 'image', url: 'http://example.com/img.png' }],
parts: [{ kind: 'image' }],
},
};
expect(reconstructMessageText(msg)).toBe('');
Expand All @@ -107,10 +115,10 @@ describe('reconstructMessageText', () => {
const msg = {
content: {
parts: [
{ type: 'text', text: 'First: ' },
{ type: 'data', data: { artifactId: 'art-1', toolCallId: 'tool-1' } },
{ type: 'text', text: ' Second: ' },
{ type: 'data', data: { artifactId: 'art-2', toolCallId: 'tool-2' } },
{ kind: 'text', text: 'First: ' },
{ kind: 'data', data: { artifactId: 'art-1', toolCallId: 'tool-1' } },
{ kind: 'text', text: ' Second: ' },
{ kind: 'data', data: { artifactId: 'art-2', toolCallId: 'tool-2' } },
],
},
};
Expand All @@ -122,7 +130,7 @@ describe('reconstructMessageText', () => {
it('handles missing text property in text part gracefully', () => {
const msg = {
content: {
parts: [{ type: 'text' }],
parts: [{ kind: 'text' }],
},
};
expect(reconstructMessageText(msg)).toBe('');
Expand Down
7 changes: 5 additions & 2 deletions agents-api/src/domains/run/agents/generation/conversation-history.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { normalizeMimeType } from '@inkeep/agents-core/constants/allowed-file-fo
import { getLogger } from '../../../../logger';
import {
createDefaultConversationHistoryConfig,
formatMessagesAsConversationHistory,
getConversationHistoryWithCompression,
} from '../../data/conversations';
import {
Expand Down Expand Up @@ -62,7 +63,7 @@ export async function buildConversationHistory(
isDelegated: ctx.isDelegatedAgent,
};

conversationHistory = await getConversationHistoryWithCompression({
const historyMessages = await getConversationHistoryWithCompression({
tenantId: ctx.config.tenantId,
projectId: ctx.config.projectId,
conversationId: contextId,
Expand All @@ -74,8 +75,9 @@ export async function buildConversationHistory(
streamRequestId,
fullContextSize: initialContextBreakdown.total,
});
conversationHistory = formatMessagesAsConversationHistory(historyMessages);
} else if (historyConfig.mode === 'scoped') {
conversationHistory = await getConversationHistoryWithCompression({
const historyMessages = await getConversationHistoryWithCompression({
tenantId: ctx.config.tenantId,
projectId: ctx.config.projectId,
conversationId: contextId,
Expand All @@ -92,6 +94,7 @@ export async function buildConversationHistory(
streamRequestId,
fullContextSize: initialContextBreakdown.total,
});
conversationHistory = formatMessagesAsConversationHistory(historyMessages);
}
}

Expand Down
Loading
Loading