diff --git a/src/agent/core/AgentCore.ts b/src/agent/core/AgentCore.ts index 509cab2..1837cc5 100644 --- a/src/agent/core/AgentCore.ts +++ b/src/agent/core/AgentCore.ts @@ -177,6 +177,7 @@ export class AgentCore { type: 'tool_result', content: '', toolResult: { + toolCallId: toolCall.id, name: toolCall.function.name, success: result.success, result: result.result, diff --git a/src/agent/llm/StreamingTypes.ts b/src/agent/llm/StreamingTypes.ts index 4600a5f..18255a7 100644 --- a/src/agent/llm/StreamingTypes.ts +++ b/src/agent/llm/StreamingTypes.ts @@ -13,7 +13,7 @@ export interface StreamChunk { type: 'text' | 'tool_call' | 'tool_result' | 'done'; content: string; toolCall?: ToolCall; - toolResult?: { name: string; success: boolean; result: string }; + toolResult?: { toolCallId?: string; name: string; success: boolean; result: string }; performanceInfo?: PerformanceInfo; finishReason?: string; } diff --git a/src/agent/tools/AddNotesTool.test.ts b/src/agent/tools/AddNotesTool.test.ts new file mode 100644 index 0000000..fd825a2 --- /dev/null +++ b/src/agent/tools/AddNotesTool.test.ts @@ -0,0 +1,70 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { AddNotesTool } from './AddNotesTool'; +import { KGCore } from '../../core/KGCore'; +import { KGProject } from '../../core/KGProject'; +import { KGMidiTrack } from '../../core/track/KGMidiTrack'; +import { KGMidiRegion } from '../../core/region/KGMidiRegion'; + +const storeState = { + activeRegionId: null as string | null, +}; + +vi.mock('../../stores/projectStore', () => ({ + useProjectStore: { + getState: () => storeState, + }, +})); + +describe('AddNotesTool', () => { + beforeEach(() => { + storeState.activeRegionId = null; + vi.restoreAllMocks(); + }); + + it('builds a compact summary for successful note creation', () => { + const track = new KGMidiTrack('Lead', 1); + const region = new KGMidiRegion('region-1', track.getId().toString(), track.getTrackIndex(), 'Verse Melody', 0, 32); + track.setRegions([region]); + const project = new KGProject('summary-project', 8, 0, 120, { numerator: 4, denominator: 4 }, 'C major'); + project.setTracks([track]); + storeState.activeRegionId = region.getId(); + + vi.spyOn(KGCore, 'instance').mockReturnValue({ + getCurrentProject: () => project, + getSelectedItems: () => [], + } as unknown as KGCore); + + const tool = new AddNotesTool(); + const summary = tool.buildToolResultDisplayContent( + { + notes: [ + { pitch: 'C4', start: 16, length: 4 }, + { pitch: 'E4', start: 20, length: 8 }, + ], + }, + { success: true, result: 'raw result' }, + ); + + expect(summary).toBe( + 'Successfully created 2 notes in region **Verse Melody** on track **Lead**, spanning bars 5 to 7.' + ); + }); + + it('returns no compact summary when the target region cannot be resolved', () => { + const project = new KGProject('summary-project', 8, 0, 120, { numerator: 4, denominator: 4 }, 'C major'); + vi.spyOn(KGCore, 'instance').mockReturnValue({ + getCurrentProject: () => project, + getSelectedItems: () => [], + } as unknown as KGCore); + + const tool = new AddNotesTool(); + const summary = tool.buildToolResultDisplayContent( + { + notes: [{ pitch: 'C4', start: 0, length: 4 }], + }, + { success: true, result: 'raw result' }, + ); + + expect(summary).toBeUndefined(); + }); +}); diff --git a/src/agent/tools/AddNotesTool.ts b/src/agent/tools/AddNotesTool.ts index 76a7b60..e0b94fc 100644 --- a/src/agent/tools/AddNotesTool.ts +++ b/src/agent/tools/AddNotesTool.ts @@ -6,6 +6,14 @@ import { KGMidiRegion } from '../../core/region/KGMidiRegion'; import { useProjectStore } from '../../stores/projectStore'; import { KGCore } from '../../core/KGCore'; +interface AddNotesSummaryData { + noteCount: number; + regionName: string; + trackName: string; + earliestNoteStartBar: number; + latestNoteEndBar: number; +} + /** * Tool for adding notes to MIDI regions * Integrates with the existing command system for undo/redo support @@ -53,6 +61,19 @@ export class AddNotesTool extends BaseTool { } }; + buildToolResultDisplayContent(args: Record | null, toolResult: ToolResult): string | undefined { + if (!toolResult.success || !args) { + return undefined; + } + + const summary = this.buildSummaryData(args); + if (!summary) { + return undefined; + } + + return `Successfully created ${summary.noteCount} ${summary.noteCount === 1 ? 'note' : 'notes'} in region **${summary.regionName}** on track **${summary.trackName}**, spanning bars ${summary.earliestNoteStartBar} to ${summary.latestNoteEndBar}.`; + } + async execute(params: Record): Promise { try { // Validate parameters @@ -149,48 +170,84 @@ export class AddNotesTool extends BaseTool { * Priority: 1) Specified regionId, 2) Active piano roll region, 3) Selected regions, 4) Error if none found */ private findTargetRegion(regionId?: string): KGMidiRegion | null { + return this.findTargetRegionContext(regionId)?.region ?? null; + } + + private findTargetRegionContext(regionId?: string): { region: KGMidiRegion; trackName: string } | null { const project = this.getCurrentProject(); const tracks = project.getTracks(); - + if (regionId) { - // Find specific region by ID for (const track of tracks) { const regions = track.getRegions(); const region = regions.find(r => r.getId() === regionId); if (region && region instanceof KGMidiRegion) { - return region; + return { + region, + trackName: track.getName() || `Track ${track.getTrackIndex() + 1}`, + }; } } return null; } else { - // Smart region finding: try different sources in priority order - - // 1. Try active piano roll region const storeState = useProjectStore.getState(); if (storeState.activeRegionId) { for (const track of tracks) { const regions = track.getRegions(); const region = regions.find(r => r.getId() === storeState.activeRegionId); if (region && region instanceof KGMidiRegion) { - return region; + return { + region, + trackName: track.getName() || `Track ${track.getTrackIndex() + 1}`, + }; } } } - - // 2. Try selected regions + const core = this.getKGCore(); const selectedItems = core.getSelectedItems(); for (const item of selectedItems) { if (item instanceof KGMidiRegion) { - return item; + const track = tracks.find(candidate => candidate.getId().toString() === item.getTrackId()); + return { + region: item, + trackName: track?.getName() || `Track ${item.getTrackIndex() + 1}`, + }; } } - - // 3. No fallback - return null to trigger error + return null; } } + private buildSummaryData(args: Record): AddNotesSummaryData | null { + const typedArgs = args as { + notes?: Array<{ start: number; length: number }>; + region_id?: string; + }; + + if (!Array.isArray(typedArgs.notes) || typedArgs.notes.length === 0) { + return null; + } + + const targetRegion = this.findTargetRegionContext(typedArgs.region_id); + if (!targetRegion) { + return null; + } + + const beatsPerBar = this.getCurrentProject().getTimeSignature().numerator; + const earliestNoteStartBeat = Math.min(...typedArgs.notes.map(note => note.start)); + const latestNoteEndBeat = Math.max(...typedArgs.notes.map(note => note.start + note.length)); + + return { + noteCount: typedArgs.notes.length, + regionName: targetRegion.region.getName(), + trackName: targetRegion.trackName, + earliestNoteStartBar: Math.floor(earliestNoteStartBeat / beatsPerBar) + 1, + latestNoteEndBar: Math.max(1, Math.ceil(latestNoteEndBeat / beatsPerBar)), + }; + } + /** * Get KGCore instance for selection access */ @@ -232,4 +289,4 @@ export class AddNotesTool extends BaseTool { return midiNote; } -} \ No newline at end of file +} diff --git a/src/agent/tools/BaseTool.ts b/src/agent/tools/BaseTool.ts index 494cbb6..1011b25 100644 --- a/src/agent/tools/BaseTool.ts +++ b/src/agent/tools/BaseTool.ts @@ -67,6 +67,17 @@ export abstract class BaseTool { */ abstract execute(params: Record): Promise; + /** + * Optionally build a compact UI summary for a successful tool result. + * The raw tool result remains the canonical output stored in agent history. + */ + buildToolResultDisplayContent( + _args: Record | null, + _toolResult: ToolResult, + ): string | undefined { + return undefined; + } + /** * Get the tool definition in OpenAI function calling format */ diff --git a/src/agent/tools/ReadChordProgressionTool.test.ts b/src/agent/tools/ReadChordProgressionTool.test.ts index 5edf0ea..fe33296 100644 --- a/src/agent/tools/ReadChordProgressionTool.test.ts +++ b/src/agent/tools/ReadChordProgressionTool.test.ts @@ -108,4 +108,19 @@ describe('ReadChordProgressionTool', () => { expect(result.success).toBe(false); expect(result.result).toContain('No active or selected MIDI region found'); }); + + it('builds a compact summary for the resolved region span', () => { + const { project, midiRegion } = buildProjectWithRegionAndOptionalChords(['Am']); + storeState.activeRegionId = midiRegion.getId(); + + vi.spyOn(KGCore, 'instance').mockReturnValue({ + getCurrentProject: () => project, + getSelectedItems: () => [], + } as unknown as KGCore); + + const tool = new ReadChordProgressionTool(); + const summary = tool.buildToolResultDisplayContent({}, { success: true, result: 'raw result' }); + + expect(summary).toBe('Read the chord progression from bars 1 to 8.'); + }); }); diff --git a/src/agent/tools/ReadChordProgressionTool.ts b/src/agent/tools/ReadChordProgressionTool.ts index 46e10b4..d577a11 100644 --- a/src/agent/tools/ReadChordProgressionTool.ts +++ b/src/agent/tools/ReadChordProgressionTool.ts @@ -14,6 +14,25 @@ export class ReadChordProgressionTool extends BaseTool { readonly parameters: Record = {}; + buildToolResultDisplayContent(args: Record | null, toolResult: ToolResult): string | undefined { + if (!toolResult.success) { + return undefined; + } + + const targetRegion = this.findTargetRegion(); + if (!targetRegion) { + return undefined; + } + + const beatsPerBar = this.getCurrentProject().getTimeSignature().numerator; + const startBar = Math.floor(targetRegion.getStartFromBeat() / beatsPerBar) + 1; + const endBar = Math.max(1, Math.ceil((targetRegion.getStartFromBeat() + targetRegion.getLength()) / beatsPerBar)); + const barRange = startBar === endBar ? `bar ${startBar}` : `bars ${startBar} to ${endBar}`; + void args; + + return `Read the chord progression from ${barRange}.`; + } + async execute(_params: Record): Promise { try { const targetRegion = this.findTargetRegion(); diff --git a/src/agent/tools/ReadMusicTool.test.ts b/src/agent/tools/ReadMusicTool.test.ts new file mode 100644 index 0000000..e7d5956 --- /dev/null +++ b/src/agent/tools/ReadMusicTool.test.ts @@ -0,0 +1,122 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; +import { ReadMusicTool } from './ReadMusicTool'; +import { KGCore } from '../../core/KGCore'; +import { KGProject } from '../../core/KGProject'; +import { KGMidiTrack } from '../../core/track/KGMidiTrack'; +import { KGMidiRegion } from '../../core/region/KGMidiRegion'; +import { KGMidiNote } from '../../core/midi/KGMidiNote'; + +function buildTrack(name: string, id: number, regionStartBeat: number, regionLength: number): KGMidiTrack { + const track = new KGMidiTrack(name, id); + const region = new KGMidiRegion(`region-${id}`, track.getId().toString(), track.getTrackIndex(), `${name} Region`, regionStartBeat, regionLength); + region.addNote(new KGMidiNote(`note-${id}`, 0, 4, 60)); + track.setRegions([region]); + return track; +} + +describe('ReadMusicTool', () => { + beforeEach(() => { + vi.restoreAllMocks(); + }); + + it('builds a compact summary for a single track read', () => { + const project = new KGProject('read-music-project', 8, 0, 120, { numerator: 4, denominator: 4 }, 'C major'); + const leadTrack = buildTrack('Lead', 1, 0, 16); + project.setTracks([leadTrack]); + + vi.spyOn(KGCore, 'instance').mockReturnValue({ + getCurrentProject: () => project, + } as unknown as KGCore); + + const tool = new ReadMusicTool(); + const summary = tool.buildToolResultDisplayContent( + { + track_id: leadTrack.getId().toString(), + start: 5, + length: 6, + }, + { success: true, result: 'raw result' }, + ); + + expect(summary).toBe('Read track Lead from bars 2 to 3.'); + }); + + it('builds a compact summary for reading multiple tracks', () => { + const project = new KGProject('read-music-project', 8, 0, 120, { numerator: 4, denominator: 4 }, 'C major'); + const leadTrack = buildTrack('Lead', 1, 0, 16); + const bassTrack = buildTrack('Bass', 2, 0, 12); + project.setTracks([leadTrack, bassTrack]); + + vi.spyOn(KGCore, 'instance').mockReturnValue({ + getCurrentProject: () => project, + } as unknown as KGCore); + + const tool = new ReadMusicTool(); + const summary = tool.buildToolResultDisplayContent( + { + track_id: 'all', + start: 0, + length: 16, + }, + { success: true, result: 'raw result' }, + ); + + expect(summary).toBe('Read tracks Lead and Bass from bars 1 to 4.'); + }); + + it('returns no compact summary when the requested track cannot be resolved', () => { + const project = new KGProject('read-music-project', 8, 0, 120, { numerator: 4, denominator: 4 }, 'C major'); + project.setTracks([buildTrack('Lead', 1, 0, 16)]); + + vi.spyOn(KGCore, 'instance').mockReturnValue({ + getCurrentProject: () => project, + } as unknown as KGCore); + + const tool = new ReadMusicTool(); + const summary = tool.buildToolResultDisplayContent( + { + track_id: 'missing-track', + start: 0, + length: 4, + }, + { success: true, result: 'raw result' }, + ); + + expect(summary).toBeUndefined(); + }); + + it('returns a professional empty-project message when all MIDI tracks are empty', async () => { + const project = new KGProject('read-music-project', 8, 0, 120, { numerator: 4, denominator: 4 }, 'C major'); + const emptyTrack = new KGMidiTrack('Lead', 1); + emptyTrack.setRegions([ + new KGMidiRegion('region-1', emptyTrack.getId().toString(), emptyTrack.getTrackIndex(), 'Lead Region', 0, 36), + ]); + project.setTracks([emptyTrack]); + + vi.spyOn(KGCore, 'instance').mockReturnValue({ + getCurrentProject: () => project, + } as unknown as KGCore); + + const tool = new ReadMusicTool(); + const result = await tool.execute({ track_id: 'all', start: 0, length: 36 }); + + expect(result.success).toBe(true); + expect(result.result).toBe('No musical content is present in the project yet.'); + }); + + it('returns a professional empty-range message when the selected range has no MIDI notes', async () => { + const project = new KGProject('read-music-project', 8, 0, 120, { numerator: 4, denominator: 4 }, 'C major'); + const leadTrack = buildTrack('Lead', 1, 0, 8); + project.setTracks([leadTrack]); + + vi.spyOn(KGCore, 'instance').mockReturnValue({ + getCurrentProject: () => project, + } as unknown as KGCore); + + const tool = new ReadMusicTool(); + const result = await tool.execute({ track_id: 'all', start: 16, length: 8 }); + + expect(result.success).toBe(true); + expect(result.result).toBe('No musical content was found in the selected range.'); + }); +}); diff --git a/src/agent/tools/ReadMusicTool.ts b/src/agent/tools/ReadMusicTool.ts index 11ac81b..783a3c2 100644 --- a/src/agent/tools/ReadMusicTool.ts +++ b/src/agent/tools/ReadMusicTool.ts @@ -1,5 +1,6 @@ import { BaseTool } from './BaseTool'; import type { ToolResult, ToolParameter } from './BaseTool'; +import type { KGTrack } from '../../core/track/KGTrack'; import { KGMidiTrack } from '../../core/track/KGMidiTrack'; import { KGMidiRegion } from '../../core/region/KGMidiRegion'; import { convertRegionToABCNotation } from '../../util/abcNotationUtil'; @@ -32,6 +33,20 @@ export class ReadMusicTool extends BaseTool { } }; + buildToolResultDisplayContent(args: Record | null, toolResult: ToolResult): string | undefined { + if (!toolResult.success || !args) { + return undefined; + } + + const summary = this.buildSummaryData(args); + if (!summary) { + return undefined; + } + + const trackLabel = summary.trackNames.length === 1 ? 'track' : 'tracks'; + return `Read ${trackLabel} ${this.formatTrackNameList(summary.trackNames)} from ${this.formatBarRange(summary.startBar, summary.endBar)}.`; + } + /** * Get the display name for percussion instruments, or null if not percussion */ @@ -120,6 +135,135 @@ export class ReadMusicTool extends BaseTool { } } + private buildSummaryData(args: Record): { + trackNames: string[]; + startBar: number; + endBar: number; + } | null { + const project = this.getCurrentProject(); + const tracks = project.getTracks(); + if (tracks.length === 0) { + return null; + } + + const beatsPerBar = project.getTimeSignature().numerator; + const startBeat = (args.start as number) || 0; + const length = args.length as number | undefined; + if (startBeat < 0 || (length !== undefined && length <= 0)) { + return null; + } + + const roundedStartBeat = Math.floor(startBeat / beatsPerBar) * beatsPerBar; + const rawEndBeat = length !== undefined ? startBeat + length : undefined; + const roundedEndBeat = rawEndBeat !== undefined + ? Math.ceil(rawEndBeat / beatsPerBar) * beatsPerBar + : this.getTrackReadEndBeat(args, tracks, roundedStartBeat); + + const trackNames = this.resolveSummaryTrackNames(args, tracks); + if (trackNames.length === 0 || roundedEndBeat === undefined) { + return null; + } + + return { + trackNames, + startBar: Math.floor(roundedStartBeat / beatsPerBar) + 1, + endBar: Math.max(1, Math.ceil(roundedEndBeat / beatsPerBar)), + }; + } + + private resolveSummaryTrackNames( + args: Record, + tracks: KGTrack[], + ): string[] { + const trackId = args.track_id as string | undefined; + + if (!trackId || trackId === '' || trackId === 'all') { + const midiTracks = tracks.filter(track => track instanceof KGMidiTrack) as KGMidiTrack[]; + const tracksToSkip = this.findTracksToSkip(midiTracks); + return midiTracks + .filter(track => !tracksToSkip.includes(track)) + .map((track, index) => track.getName() || `Track ${index + 1}`); + } + + const targetTrack = tracks.find(track => track.getId().toString() === trackId); + if (!(targetTrack instanceof KGMidiTrack)) { + return []; + } + + return [targetTrack.getName() || 'Unnamed Track']; + } + + private getTrackReadEndBeat( + args: Record, + tracks: KGTrack[], + roundedStartBeat: number, + ): number | undefined { + const trackId = args.track_id as string | undefined; + + if (!trackId || trackId === '' || trackId === 'all') { + const midiTracks = tracks.filter(track => track instanceof KGMidiTrack) as KGMidiTrack[]; + const tracksToSkip = this.findTracksToSkip(midiTracks); + const visibleTracks = midiTracks.filter(track => !tracksToSkip.includes(track)); + const endBeats = visibleTracks.flatMap(track => + track.getRegions() + .filter(region => region instanceof KGMidiRegion) + .map(region => region.getStartFromBeat() + region.getLength()) + ); + return endBeats.length > 0 ? Math.max(roundedStartBeat, ...endBeats) : roundedStartBeat; + } + + const targetTrack = tracks.find(track => track.getId().toString() === trackId); + if (!(targetTrack instanceof KGMidiTrack)) { + return undefined; + } + + const endBeats = targetTrack.getRegions() + .filter(region => region instanceof KGMidiRegion) + .map(region => region.getStartFromBeat() + region.getLength()); + return endBeats.length > 0 ? Math.max(roundedStartBeat, ...endBeats) : roundedStartBeat; + } + + private formatTrackNameList(trackNames: string[]): string { + if (trackNames.length === 1) { + return trackNames[0]; + } + if (trackNames.length === 2) { + return `${trackNames[0]} and ${trackNames[1]}`; + } + + return `${trackNames.slice(0, -1).join(', ')}, and ${trackNames.at(-1)}`; + } + + private formatBarRange(startBar: number, endBar: number): string { + return startBar === endBar + ? `bar ${startBar}` + : `bars ${startBar} to ${endBar}`; + } + + private hasMidiContentInRange(track: KGMidiTrack, startBeat: number, endBeat?: number): boolean { + const rangeEndBeat = endBeat ?? Infinity; + + return track.getRegions().some(region => { + if (!(region instanceof KGMidiRegion)) { + return false; + } + + const regionStart = region.getStartFromBeat(); + const regionEnd = regionStart + region.getLength(); + const overlapsRange = regionStart < rangeEndBeat && regionEnd > startBeat; + + return overlapsRange && region.getNotes().length > 0; + }); + } + + private getEmptyProjectMessage(): string { + return 'No musical content is present in the project yet.'; + } + + private getEmptyRangeMessage(): string { + return 'No musical content was found in the selected range.'; + } + /** * Find tracks that should be skipped because they have no musical content * (no regions or regions with no notes) @@ -166,11 +310,20 @@ export class ReadMusicTool extends BaseTool { const midiTracks = tracks.filter(track => track instanceof KGMidiTrack); if (midiTracks.length === 0) { - return 'No MIDI tracks found in the project.'; + return this.getEmptyProjectMessage(); } // Find tracks to skip (tracks with no content) const tracksToSkip = this.findTracksToSkip(midiTracks); + const visibleTracks = midiTracks.filter(track => !tracksToSkip.includes(track)); + if (visibleTracks.length === 0) { + return this.getEmptyProjectMessage(); + } + + const hasContentInRange = visibleTracks.some(track => this.hasMidiContentInRange(track, startBeat, endBeat)); + if (!hasContentInRange) { + return this.getEmptyRangeMessage(); + } // Get project settings for proper notation const project = this.getCurrentProject(); @@ -252,6 +405,16 @@ export class ReadMusicTool extends BaseTool { return `Track is not a MIDI track.`; } + if (track.getRegions().length === 0 || !track.getRegions().some(region => ( + region instanceof KGMidiRegion && region.getNotes().length > 0 + ))) { + return this.getEmptyProjectMessage(); + } + + if (!this.hasMidiContentInRange(track, startBeat, endBeat)) { + return this.getEmptyRangeMessage(); + } + // Get project settings for proper notation const project = this.getCurrentProject(); const timeSignature = project.getTimeSignature(); @@ -304,4 +467,4 @@ export class ReadMusicTool extends BaseTool { return output; } -} \ No newline at end of file +} diff --git a/src/components/ChatBox.test.tsx b/src/components/ChatBox.test.tsx index 159b3a6..89a3765 100644 --- a/src/components/ChatBox.test.tsx +++ b/src/components/ChatBox.test.tsx @@ -5,11 +5,13 @@ import ChatBox from './ChatBox'; import { I18nContext } from '../i18n/I18nProvider'; import type { ResolvedLocaleCode } from '../i18n/types'; import { translate } from '../i18n/translate'; +import type { ChatMessage } from '../types/projectTypes'; const { agentCoreMock, processUserMessageMock, processStreamMock, + streamProcessorCallbacks, } = vi.hoisted(() => ({ agentCoreMock: { setLLMProvider: vi.fn(), @@ -25,6 +27,12 @@ const { }, processUserMessageMock: vi.fn(), processStreamMock: vi.fn(async () => ''), + streamProcessorCallbacks: { + onMessageAdd: undefined as ((message: ChatMessage) => void) | undefined, + onMessageUpdate: undefined as ((messageId: string, updater: (msg: ChatMessage) => ChatMessage) => void) | undefined, + onMessageRemove: undefined as ((messageId: string) => void) | undefined, + onProcessingChange: undefined as ((isProcessing: boolean) => void) | undefined, + }, })); vi.mock('./chat', () => ({ @@ -96,10 +104,17 @@ vi.mock('../util/messageFilter/UserMessageFilter', () => ({ })); vi.mock('../hooks/useStreamProcessor', () => ({ - useStreamProcessor: () => ({ - abortController: null, - processStream: processStreamMock, - }), + useStreamProcessor: (options: typeof streamProcessorCallbacks) => { + streamProcessorCallbacks.onMessageAdd = options.onMessageAdd; + streamProcessorCallbacks.onMessageUpdate = options.onMessageUpdate; + streamProcessorCallbacks.onMessageRemove = options.onMessageRemove; + streamProcessorCallbacks.onProcessingChange = options.onProcessingChange; + + return { + abortController: null, + processStream: processStreamMock, + }; + }, })); vi.mock('../utils/chatMessageUtils', () => ({ @@ -176,6 +191,10 @@ describe('ChatBox', () => { processStreamMock.mockClear(); agentCoreMock.compactConversation.mockClear(); agentCoreMock.shouldCompactBeforeNextTurn.mockResolvedValue(false); + streamProcessorCallbacks.onMessageAdd = undefined; + streamProcessorCallbacks.onMessageUpdate = undefined; + streamProcessorCallbacks.onMessageRemove = undefined; + streamProcessorCallbacks.onProcessingChange = undefined; }); it('renders the English assistant title under en_us', () => { @@ -227,4 +246,146 @@ describe('ChatBox', () => { expect(screen.queryByText('Task Checklist')).toBeNull(); }); }); + + it('removes older incomplete todo snapshots before appending a new one', async () => { + processUserMessageMock.mockResolvedValue({ + displayUserMessage: false, + sendToLLM: true, + finalMessageForLLM: 'todo prompt', + pseudoAssistantResponse: null, + metadata: null, + }); + processStreamMock.mockImplementation(async () => { + streamProcessorCallbacks.onMessageAdd?.({ + id: 'todo-1', + role: 'assistant', + content: 'todo 1', + toolName: 'update_todo_list', + toolSuccess: true, + todoSnapshot: [ + { id: '1', text: 'Inspect melody', status: 'completed', updatedAt: 1 }, + { id: '2', text: 'Write harmony', status: 'in_progress', updatedAt: 2 }, + ], + }); + streamProcessorCallbacks.onMessageAdd?.({ + id: 'todo-2', + role: 'assistant', + content: 'todo 2', + toolName: 'update_todo_list', + toolSuccess: true, + todoSnapshot: [ + { id: '1', text: 'Inspect melody', status: 'completed', updatedAt: 3 }, + { id: '2', text: 'Write bass', status: 'pending', updatedAt: 4 }, + ], + }); + return ''; + }); + + renderWithLocale('en_us'); + + const input = screen.getByPlaceholderText('Press Enter to send message, Shift + Enter for new line'); + fireEvent.change(input, { target: { value: 'todo cleanup' } }); + fireEvent.keyDown(input, { key: 'Enter', shiftKey: false }); + + await waitFor(() => { + expect(screen.queryByText('TODO SNAPSHOT: Inspect melody, Write harmony')).toBeNull(); + expect(screen.getByText('TODO SNAPSHOT: Inspect melody, Write bass')).toBeTruthy(); + }); + }); + + it('preserves completed todo snapshots when a new incomplete snapshot is added', async () => { + processUserMessageMock.mockResolvedValue({ + displayUserMessage: false, + sendToLLM: true, + finalMessageForLLM: 'todo prompt', + pseudoAssistantResponse: null, + metadata: null, + }); + processStreamMock.mockImplementation(async () => { + streamProcessorCallbacks.onMessageAdd?.({ + id: 'todo-complete', + role: 'assistant', + content: 'done snapshot', + toolName: 'update_todo_list', + toolSuccess: true, + todoSnapshot: [ + { id: '1', text: 'Inspect melody', status: 'completed', updatedAt: 1 }, + { id: '2', text: 'Write harmony', status: 'completed', updatedAt: 2 }, + ], + }); + streamProcessorCallbacks.onMessageAdd?.({ + id: 'todo-active', + role: 'assistant', + content: 'active snapshot', + toolName: 'update_todo_list', + toolSuccess: true, + todoSnapshot: [ + { id: '1', text: 'Mix stems', status: 'completed', updatedAt: 3 }, + { id: '2', text: 'Render bounce', status: 'in_progress', updatedAt: 4 }, + ], + }); + return ''; + }); + + renderWithLocale('en_us'); + + const input = screen.getByPlaceholderText('Press Enter to send message, Shift + Enter for new line'); + fireEvent.change(input, { target: { value: 'todo preserve' } }); + fireEvent.keyDown(input, { key: 'Enter', shiftKey: false }); + + await waitFor(() => { + expect(screen.getByText('TODO SNAPSHOT: Inspect melody, Write harmony')).toBeTruthy(); + expect(screen.getByText('TODO SNAPSHOT: Mix stems, Render bounce')).toBeTruthy(); + }); + }); + + it('does not remove non-todo assistant messages during todo cleanup', async () => { + processUserMessageMock.mockResolvedValue({ + displayUserMessage: false, + sendToLLM: true, + finalMessageForLLM: 'todo prompt', + pseudoAssistantResponse: null, + metadata: null, + }); + processStreamMock.mockImplementation(async () => { + streamProcessorCallbacks.onMessageAdd?.({ + id: 'assistant-note', + role: 'assistant', + content: 'Normal assistant message', + }); + streamProcessorCallbacks.onMessageAdd?.({ + id: 'todo-1', + role: 'assistant', + content: 'todo 1', + toolName: 'update_todo_list', + toolSuccess: true, + todoSnapshot: [ + { id: '1', text: 'Inspect melody', status: 'pending', updatedAt: 1 }, + ], + }); + streamProcessorCallbacks.onMessageAdd?.({ + id: 'todo-2', + role: 'assistant', + content: 'todo 2', + toolName: 'update_todo_list', + toolSuccess: true, + todoSnapshot: [ + { id: '1', text: 'Render bounce', status: 'in_progress', updatedAt: 2 }, + ], + }); + return ''; + }); + + renderWithLocale('en_us'); + + const input = screen.getByPlaceholderText('Press Enter to send message, Shift + Enter for new line'); + fireEvent.change(input, { target: { value: 'todo cleanup keep assistant' } }); + fireEvent.keyDown(input, { key: 'Enter', shiftKey: false }); + + await waitFor(() => { + expect(screen.getByText('Normal assistant message')).toBeTruthy(); + expect(screen.queryByText('TODO SNAPSHOT: Inspect melody')).toBeNull(); + expect(screen.getByText('TODO SNAPSHOT: Render bounce')).toBeTruthy(); + }); + }); }); diff --git a/src/components/ChatBox.tsx b/src/components/ChatBox.tsx index f4b1e73..35c78bc 100644 --- a/src/components/ChatBox.tsx +++ b/src/components/ChatBox.tsx @@ -3,6 +3,7 @@ import './ChatBox.css'; import { FaPlus, FaBan, FaDownload } from 'react-icons/fa'; import { UserMessage, AssistantMessage } from './chat'; import { AgentCore } from '../agent/core/AgentCore'; +import { summarizeTodoCounts } from '../agent/core/todo'; import { OpenAICompatibleLLMProvider, type LLMProvider } from '../agent/llm/LLMProvider'; import { LocalBrowserLLMProvider } from '../agent/llm/LocalBrowserLLMProvider'; import { ConfigManager } from '../core/config/ConfigManager'; @@ -23,6 +24,16 @@ import type { ChatMessage } from '../types/projectTypes'; // Module-level guard to avoid duplicate welcome in React StrictMode dev remounts let hasShownWelcomeOnceInRuntime = false; +const TODO_TOOL_NAME = 'update_todo_list'; + +const isCompletedTodoSnapshotMessage = (message: ChatMessage): boolean => { + if (message.toolName !== TODO_TOOL_NAME || !Array.isArray(message.todoSnapshot)) { + return false; + } + + const counts = summarizeTodoCounts(message.todoSnapshot); + return counts.total > 0 && counts.completed === counts.total; +}; /** * Create the LLM provider from current configuration @@ -151,7 +162,18 @@ const ChatBox: React.FC = ({ isVisible }) => { }, []); const handleMessageAdd = useCallback((message: ChatMessage) => { - setMessages(prev => [...prev, message]); + setMessages((prev) => { + if (message.toolName === TODO_TOOL_NAME && Array.isArray(message.todoSnapshot)) { + const preservedMessages = prev.filter((existingMessage) => ( + existingMessage.toolName !== TODO_TOOL_NAME + || !Array.isArray(existingMessage.todoSnapshot) + || isCompletedTodoSnapshotMessage(existingMessage) + )); + return [...preservedMessages, message]; + } + + return [...prev, message]; + }); }, []); const handleMessageRemove = useCallback((messageId: string) => { diff --git a/src/hooks/useStreamProcessor.test.ts b/src/hooks/useStreamProcessor.test.ts index 97f6eb3..e68a17b 100644 --- a/src/hooks/useStreamProcessor.test.ts +++ b/src/hooks/useStreamProcessor.test.ts @@ -138,6 +138,7 @@ describe('useStreamProcessor', () => { type: 'tool_result', content: '', toolResult: { + toolCallId: 'todo-call-1', name: 'update_todo_list', success: true, result: 'todo fallback content', @@ -201,6 +202,7 @@ describe('useStreamProcessor', () => { type: 'tool_result', content: '', toolResult: { + toolCallId: 'read-call-1', name: 'read_music', success: true, result: 'music data', @@ -239,7 +241,7 @@ describe('useStreamProcessor', () => { expect(addedMessages.some(message => message.toolName === 'update_todo_list')).toBe(false); }); -it('attaches add_notes summary metadata for chat-only rendering while preserving raw content', async () => { + it('uses tool-provided add_notes summary metadata for chat-only rendering while preserving raw content', async () => { const selectedRegion = new KGMidiRegion('region-1', '1', 0, 'Verse Melody'); vi.spyOn(AgentCore, 'instance').mockReturnValue({ @@ -268,6 +270,7 @@ it('attaches add_notes summary metadata for chat-only rendering while preserving type: 'tool_result', content: '', toolResult: { + toolCallId: 'add-notes-call-1', name: 'add_notes', success: true, result: 'Successfully created 2 notes: C4 (beat 16, length 4), E4 (beat 20, length 8)', @@ -325,4 +328,64 @@ it('attaches add_notes summary metadata for chat-only rendering while preserving 'Successfully created 2 notes in region **Verse Melody** on track **Lead**, spanning bars 5 to 7.' ); }); + + it('falls back to the raw tool result when tool summary generation cannot resolve context', async () => { + vi.spyOn(AgentCore, 'instance').mockReturnValue({ + getAgentState: () => ({ + getTodos: () => [], + }), + processUserInput: async function* () { + yield { + type: 'tool_call', + content: '', + toolCall: { + id: 'read-call-fallback', + type: 'function', + function: { + name: 'read_music', + arguments: JSON.stringify({}), + }, + }, + }; + yield { + type: 'tool_result', + content: '', + toolResult: { + toolCallId: 'read-call-fallback', + name: 'read_music', + success: true, + result: 'raw music result', + }, + }; + yield { type: 'done', content: '' }; + }, + } as unknown as AgentCore); + + const messages = new Map(); + + const { result } = renderHook(() => useStreamProcessor({ + onMessageAdd: (message) => { + messages.set(message.id, message); + }, + onMessageUpdate: (messageId, updater) => { + const current = messages.get(messageId); + if (!current) { + throw new Error(`Missing message ${messageId}`); + } + messages.set(messageId, updater(current)); + }, + onMessageRemove: (messageId) => { + messages.delete(messageId); + }, + onProcessingChange: () => undefined, + })); + + await act(async () => { + await result.current.processStream('read fallback prompt'); + }); + + const toolResultMessage = [...messages.values()].find(message => message.toolName === 'read_music'); + expect(toolResultMessage?.toolRawResult).toBe('raw music result'); + expect(toolResultMessage?.toolResultDisplayContent).toBe('raw music result'); + }); }); diff --git a/src/hooks/useStreamProcessor.ts b/src/hooks/useStreamProcessor.ts index 0f55c74..ed8417d 100644 --- a/src/hooks/useStreamProcessor.ts +++ b/src/hooks/useStreamProcessor.ts @@ -1,129 +1,17 @@ import { useState, useCallback } from 'react'; import { AgentCore } from '../agent/core/AgentCore'; -import { KGCore } from '../core/KGCore'; -import { KGMidiRegion } from '../core/region/KGMidiRegion'; +import { AVAILABLE_TOOLS } from '../agent/tools'; import { createStreamingMessage, createMessage } from '../utils/chatMessageUtils'; -import { useProjectStore } from '../stores/projectStore'; import type { ChatMessage } from '../types/projectTypes'; const TODO_TOOL_NAME = 'update_todo_list'; -const ADD_NOTES_TOOL_NAME = 'add_notes'; interface PendingToolCall { + id: string; name: string; arguments: Record | null; } -interface AddNotesToolArguments { - notes: Array<{ - pitch: string; - start: number; - length: number; - velocity?: number; - }>; - region_id?: string; -} - -const getBarNumberFromStartBeat = (beat: number, beatsPerBar: number): number => ( - Math.floor(beat / beatsPerBar) + 1 -); - -const getBarNumberFromEndBeat = (beat: number, beatsPerBar: number): number => ( - Math.max(1, Math.ceil(beat / beatsPerBar)) -); - -interface AddNotesSummaryData { - noteCount: number; - regionName: string; - trackName: string; - earliestNoteStartBar: number; - latestNoteEndBar: number; -} - -const resolveTargetMidiRegion = ( - regionId: string | undefined, - storeState: ReturnType, -): { region: KGMidiRegion; trackName: string } | undefined => { - const project = KGCore.instance().getCurrentProject(); - const tracks = project.getTracks(); - - const findById = (candidateRegionId: string): { region: KGMidiRegion; trackName: string } | undefined => { - for (const track of tracks) { - const region = track.getRegions().find(candidate => candidate.getId() === candidateRegionId); - if (region instanceof KGMidiRegion) { - return { - region, - trackName: track.getName(), - }; - } - } - - return undefined; - }; - - if (regionId) { - return findById(regionId); - } - - if (storeState.activeRegionId) { - const activeRegion = findById(storeState.activeRegionId); - if (activeRegion) { - return activeRegion; - } - } - - const selectedRegionId = storeState.selectedRegionIds.at(-1); - if (selectedRegionId) { - const selectedRegion = findById(selectedRegionId); - if (selectedRegion) { - return selectedRegion; - } - } - - const selectedItems = KGCore.instance().getSelectedItems(); - for (const item of selectedItems) { - if (item instanceof KGMidiRegion) { - const track = tracks.find(candidate => candidate.getId() === item.getTrackId()); - return { - region: item, - trackName: track?.getName() ?? `Track ${item.getTrackIndex() + 1}`, - }; - } - } - - return undefined; -}; - -const buildAddNotesSummary = (args: Record | null): AddNotesSummaryData | undefined => { - if (!args) { - return undefined; - } - - const typedArgs = args as AddNotesToolArguments; - if (!Array.isArray(typedArgs.notes) || typedArgs.notes.length === 0) { - return undefined; - } - - const storeState = useProjectStore.getState(); - const project = KGCore.instance().getCurrentProject(); - const beatsPerBar = project.getTimeSignature().numerator ?? storeState.timeSignature.numerator; - const targetRegion = resolveTargetMidiRegion(typedArgs.region_id, storeState); - if (!targetRegion) { - return undefined; - } - - const earliestNoteStartBeat = Math.min(...typedArgs.notes.map(note => note.start)); - const latestNoteEndBeat = Math.max(...typedArgs.notes.map(note => note.start + note.length)); - - return { - noteCount: typedArgs.notes.length, - regionName: targetRegion.region.getName(), - trackName: targetRegion.trackName, - earliestNoteStartBar: getBarNumberFromStartBeat(earliestNoteStartBeat, beatsPerBar), - latestNoteEndBar: getBarNumberFromEndBeat(latestNoteEndBeat, beatsPerBar), - }; -}; - const buildToolResultDisplayContent = ( toolName: string, success: boolean, @@ -134,14 +22,17 @@ const buildToolResultDisplayContent = ( return rawResult; } - if (toolName === ADD_NOTES_TOOL_NAME) { - const summary = buildAddNotesSummary(toolArgs); - if (summary) { - return `Successfully created ${summary.noteCount} ${summary.noteCount === 1 ? 'note' : 'notes'} in region **${summary.regionName}** on track **${summary.trackName}**, spanning bars ${summary.earliestNoteStartBar} to ${summary.latestNoteEndBar}.`; - } + const ToolClass = AVAILABLE_TOOLS[toolName as keyof typeof AVAILABLE_TOOLS]; + if (!ToolClass) { + return rawResult; } - return rawResult; + try { + const toolInstance = new ToolClass(); + return toolInstance.buildToolResultDisplayContent(toolArgs, { success, result: rawResult }) ?? rawResult; + } catch { + return rawResult; + } }; interface StreamProcessorOptions { @@ -204,6 +95,10 @@ export const useStreamProcessor = (options: StreamProcessorOptions): StreamProce tokenCount })); } else if (chunk.type === 'tool_call' && chunk.toolCall) { + console.log('------------ ASSISTANT TOOL CALL ------------'); + console.log(JSON.stringify(chunk.toolCall, null, 2)); + console.log('---------------------------------------------'); + // Finalize or remove the current streaming message if (hasTextContent) { onMessageUpdate(currentStreamingId, (msg) => ({ @@ -231,10 +126,10 @@ export const useStreamProcessor = (options: StreamProcessorOptions): StreamProce try { const args = JSON.parse(chunk.toolCall.function.arguments); argsDisplay = JSON.stringify(args, null, 2); - pendingToolCalls.push({ name: toolName, arguments: args }); + pendingToolCalls.push({ id: chunk.toolCall.id, name: toolName, arguments: args }); } catch { argsDisplay = chunk.toolCall.function.arguments; - pendingToolCalls.push({ name: toolName, arguments: null }); + pendingToolCalls.push({ id: chunk.toolCall.id, name: toolName, arguments: null }); } const toolCallMsg = { ...createMessage('assistant', `🔧 **Calling tool: ${toolName}**\n\n\`\`\`json\n${argsDisplay}\n\`\`\``), @@ -242,9 +137,13 @@ export const useStreamProcessor = (options: StreamProcessorOptions): StreamProce }; onMessageAdd(toolCallMsg); } else if (chunk.type === 'tool_result' && chunk.toolResult) { + console.log('------------ TOOL RESULT ------------'); + console.log(JSON.stringify(chunk.toolResult, null, 2)); + console.log('-------------------------------------'); + // Show tool result in UI - const { name, success, result } = chunk.toolResult; - const pendingToolCallIndex = pendingToolCalls.findIndex(toolCall => toolCall.name === name); + const { toolCallId, name, success, result } = chunk.toolResult; + const pendingToolCallIndex = pendingToolCalls.findIndex(toolCall => toolCall.id === toolCallId); const pendingToolCall = pendingToolCallIndex >= 0 ? pendingToolCalls.splice(pendingToolCallIndex, 1)[0] : undefined;