feat: implemented confirmation mechanism for tool invokation
This commit is contained in:
@@ -50,6 +50,32 @@ describe('AddNotesTool', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('builds a confirmation summary for 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();
|
||||
|
||||
expect(tool.isReadOnlyTool()).toBe(false);
|
||||
expect(tool.buildConfirmationContent({
|
||||
notes: [
|
||||
{ pitch: 'C4', start: 16, length: 4 },
|
||||
{ pitch: 'E4', start: 20, length: 8 },
|
||||
],
|
||||
})).toBe(
|
||||
'Allow creating 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({
|
||||
|
||||
@@ -22,6 +22,10 @@ export class AddNotesTool extends BaseTool {
|
||||
readonly name = 'add_notes';
|
||||
readonly description = 'Add one or more MIDI notes to the current region. Use this to create melodies, chords, or any musical content. Notes use absolute beat positions on the project timeline — not relative to the region start.';
|
||||
|
||||
override isReadOnlyTool(): boolean {
|
||||
return false;
|
||||
}
|
||||
|
||||
readonly parameters: Record<string, ToolParameter> = {
|
||||
notes: {
|
||||
type: 'array',
|
||||
@@ -74,6 +78,19 @@ export class AddNotesTool extends BaseTool {
|
||||
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}.`;
|
||||
}
|
||||
|
||||
override buildConfirmationContent(args: Record<string, unknown> | null): string | undefined {
|
||||
if (!args) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const summary = this.buildSummaryData(args);
|
||||
if (!summary) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
return `Allow creating ${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<string, unknown>): Promise<ToolResult> {
|
||||
try {
|
||||
// Validate parameters
|
||||
|
||||
@@ -67,6 +67,13 @@ export abstract class BaseTool {
|
||||
*/
|
||||
abstract execute(params: Record<string, unknown>): Promise<ToolResult>;
|
||||
|
||||
/**
|
||||
* Whether the tool only reads state and can execute without user approval.
|
||||
*/
|
||||
isReadOnlyTool(): boolean {
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Optionally build a compact UI summary for a successful tool result.
|
||||
* The raw tool result remains the canonical output stored in agent history.
|
||||
@@ -78,6 +85,16 @@ export abstract class BaseTool {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Optionally build a user-facing confirmation summary before execution.
|
||||
* Non-read-only tools should override this with a concise approval prompt.
|
||||
*/
|
||||
buildConfirmationContent(
|
||||
_args: Record<string, unknown> | null,
|
||||
): string | undefined {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get the tool definition in OpenAI function calling format
|
||||
*/
|
||||
|
||||
@@ -0,0 +1,56 @@
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { RemoveNotesTool } from './RemoveNotesTool';
|
||||
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';
|
||||
|
||||
const storeState = {
|
||||
activeRegionId: null as string | null,
|
||||
};
|
||||
|
||||
vi.mock('../../stores/projectStore', () => ({
|
||||
useProjectStore: {
|
||||
getState: () => storeState,
|
||||
},
|
||||
}));
|
||||
|
||||
describe('RemoveNotesTool', () => {
|
||||
beforeEach(() => {
|
||||
storeState.activeRegionId = null;
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it('builds confirmation and result summaries for note removal', () => {
|
||||
const track = new KGMidiTrack('Lead', 1);
|
||||
const region = new KGMidiRegion('region-1', track.getId().toString(), track.getTrackIndex(), 'Verse Melody', 0, 32);
|
||||
region.setNotes([
|
||||
new KGMidiNote('note-1', 16, 20, 60, 100),
|
||||
new KGMidiNote('note-2', 20, 28, 64, 100),
|
||||
]);
|
||||
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 RemoveNotesTool();
|
||||
const args = {
|
||||
start: 16,
|
||||
end: 24,
|
||||
};
|
||||
|
||||
expect(tool.isReadOnlyTool()).toBe(false);
|
||||
expect(tool.buildConfirmationContent(args)).toBe(
|
||||
'Allow removing 2 notes from beats 16-24, in region **Verse Melody** on track **Lead**, spanning bars 5 to 7?'
|
||||
);
|
||||
expect(tool.buildToolResultDisplayContent(args, { success: true, result: 'raw result' })).toBe(
|
||||
'Successfully removed 2 notes from beats 16-24, in region **Verse Melody** on track **Lead**, spanning bars 5 to 7.'
|
||||
);
|
||||
});
|
||||
});
|
||||
@@ -5,6 +5,16 @@ import { KGMidiRegion } from '../../core/region/KGMidiRegion';
|
||||
import { useProjectStore } from '../../stores/projectStore';
|
||||
import { KGCore } from '../../core/KGCore';
|
||||
|
||||
interface RemoveNotesSummaryData {
|
||||
noteCount: number;
|
||||
startBeat: number;
|
||||
endBeat: number;
|
||||
regionName: string;
|
||||
trackName: string;
|
||||
earliestNoteStartBar: number;
|
||||
latestNoteEndBar: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* Tool for removing notes from MIDI regions within a specified beat range
|
||||
* Integrates with the existing command system for undo/redo support
|
||||
@@ -13,6 +23,10 @@ export class RemoveNotesTool extends BaseTool {
|
||||
readonly name = 'remove_notes';
|
||||
readonly description = 'Remove all MIDI notes whose start position falls within the specified beat range. Use this to clear a section before rewriting it, or to delete unwanted notes. Beat positions are absolute on the project timeline.';
|
||||
|
||||
override isReadOnlyTool(): boolean {
|
||||
return false;
|
||||
}
|
||||
|
||||
readonly parameters: Record<string, ToolParameter> = {
|
||||
start: {
|
||||
type: 'number',
|
||||
@@ -31,6 +45,32 @@ export class RemoveNotesTool extends BaseTool {
|
||||
}
|
||||
};
|
||||
|
||||
override buildToolResultDisplayContent(args: Record<string, unknown> | null, toolResult: ToolResult): string | undefined {
|
||||
if (!toolResult.success || !args) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const summary = this.buildSummaryData(args);
|
||||
if (!summary || summary.noteCount === 0) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
return `Successfully removed ${summary.noteCount} ${summary.noteCount === 1 ? 'note' : 'notes'} from beats ${summary.startBeat}-${summary.endBeat}, in region **${summary.regionName}** on track **${summary.trackName}**, spanning bars ${summary.earliestNoteStartBar} to ${summary.latestNoteEndBar}.`;
|
||||
}
|
||||
|
||||
override buildConfirmationContent(args: Record<string, unknown> | null): string | undefined {
|
||||
if (!args) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
const summary = this.buildSummaryData(args);
|
||||
if (!summary) {
|
||||
return undefined;
|
||||
}
|
||||
|
||||
return `Allow removing ${summary.noteCount} ${summary.noteCount === 1 ? 'note' : 'notes'} from beats ${summary.startBeat}-${summary.endBeat}, in region **${summary.regionName}** on track **${summary.trackName}**, spanning bars ${summary.earliestNoteStartBar} to ${summary.latestNoteEndBar}?`;
|
||||
}
|
||||
|
||||
async execute(params: Record<string, unknown>): Promise<ToolResult> {
|
||||
try {
|
||||
// Validate parameters
|
||||
@@ -105,6 +145,10 @@ export class RemoveNotesTool 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();
|
||||
|
||||
@@ -114,7 +158,10 @@ export class RemoveNotesTool extends BaseTool {
|
||||
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;
|
||||
@@ -128,7 +175,10 @@ export class RemoveNotesTool extends BaseTool {
|
||||
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}`,
|
||||
};
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -138,7 +188,11 @@ export class RemoveNotesTool extends BaseTool {
|
||||
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}`,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
@@ -147,6 +201,47 @@ export class RemoveNotesTool extends BaseTool {
|
||||
}
|
||||
}
|
||||
|
||||
private buildSummaryData(args: Record<string, unknown>): RemoveNotesSummaryData | null {
|
||||
const typedArgs = args as {
|
||||
start?: number;
|
||||
end?: number;
|
||||
region_id?: string;
|
||||
};
|
||||
|
||||
if (typeof typedArgs.start !== 'number' || typeof typedArgs.end !== 'number' || typedArgs.end <= typedArgs.start) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const targetRegion = this.findTargetRegionContext(typedArgs.region_id);
|
||||
if (!targetRegion) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const regionStartBeat = targetRegion.region.getStartFromBeat();
|
||||
const adjustedStartBeat = typedArgs.start - regionStartBeat;
|
||||
const adjustedEndBeat = typedArgs.end - regionStartBeat;
|
||||
const notesToRemove = this.findNotesInRange(targetRegion.region, adjustedStartBeat, adjustedEndBeat);
|
||||
const beatsPerBar = this.getCurrentProject().getTimeSignature().numerator;
|
||||
|
||||
let earliestBeat = typedArgs.start;
|
||||
let latestBeat = typedArgs.end;
|
||||
|
||||
if (notesToRemove.length > 0) {
|
||||
earliestBeat = Math.min(...notesToRemove.map(note => note.getStartBeat() + regionStartBeat));
|
||||
latestBeat = Math.max(...notesToRemove.map(note => note.getEndBeat() + regionStartBeat));
|
||||
}
|
||||
|
||||
return {
|
||||
noteCount: notesToRemove.length,
|
||||
startBeat: typedArgs.start,
|
||||
endBeat: typedArgs.end,
|
||||
regionName: targetRegion.region.getName(),
|
||||
trackName: targetRegion.trackName,
|
||||
earliestNoteStartBar: Math.floor(earliestBeat / beatsPerBar) + 1,
|
||||
latestNoteEndBar: Math.max(1, Math.ceil(latestBeat / beatsPerBar)),
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Get KGCore instance for selection access
|
||||
*/
|
||||
@@ -165,4 +260,4 @@ export class RemoveNotesTool extends BaseTool {
|
||||
return noteStartBeat >= startBeat && noteStartBeat < endBeat;
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
// Base tool system
|
||||
import { BaseTool } from './BaseTool';
|
||||
export { BaseTool } from './BaseTool';
|
||||
export type { ToolResult, ToolParameter, ToolDefinition, OpenAIToolDefinition, OpenAIFunctionParameters } from './BaseTool';
|
||||
|
||||
@@ -21,3 +22,8 @@ export const AVAILABLE_TOOLS = {
|
||||
} as const;
|
||||
|
||||
export type ToolName = keyof typeof AVAILABLE_TOOLS;
|
||||
|
||||
export const createToolInstance = (toolName: string): BaseTool | null => {
|
||||
const ToolClass = AVAILABLE_TOOLS[toolName as ToolName];
|
||||
return ToolClass ? new ToolClass() : null;
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user