From a39bdbe6fc5148fe38bc27d38a98bac413643384 Mon Sep 17 00:00:00 2001 From: diyaa Date: Sun, 13 Sep 2026 21:13:52 +0200 Subject: [PATCH] Implement optional discussion mode --- .../Presentation/ProjectInspectorView.swift | 5 + .../Integrations/OpenAI/OpenAIClient.swift | 13 +++ .../Services/AI/AIService.swift | 31 ++++++ .../Services/AI/ApplicationRules.swift | 10 ++ .../AI/SongProjectDiscussionDirector.swift | 48 ++++++++++ .../AIServiceTests.swift | 25 +++++ .../ApplicationRuleInjectionTests.swift | 12 +++ .../OpenAIClientTests.swift | 43 +++++++++ .../SongProjectDiscussionDirectorTests.swift | 94 +++++++++++++++++++ docs/ARCHITECTURE.md | 3 + docs/DATA_MODEL.md | 6 ++ docs/TASKS.md | 2 +- 12 files changed, 291 insertions(+), 1 deletion(-) create mode 100644 Sources/MusicAssistantCore/Services/AI/SongProjectDiscussionDirector.swift create mode 100644 Tests/MusicAssistantCoreTests/SongProjectDiscussionDirectorTests.swift diff --git a/Sources/MusicAssistantApp/Presentation/ProjectInspectorView.swift b/Sources/MusicAssistantApp/Presentation/ProjectInspectorView.swift index 8720c27..df83c5a 100644 --- a/Sources/MusicAssistantApp/Presentation/ProjectInspectorView.swift +++ b/Sources/MusicAssistantApp/Presentation/ProjectInspectorView.swift @@ -38,6 +38,11 @@ struct ProjectInspectorView: View { TextField("Title", text: $project.title) TextField("Idea", text: $project.idea, axis: .vertical) .lineLimit(2...4) + Picker("AI mode", selection: $project.conversationMode) { + Text("Auto").tag(ConversationMode.auto) + Text("Discuss").tag(ConversationMode.discuss) + } + .pickerStyle(.segmented) TextField("Duration seconds", text: durationSecondsBinding) TextField("Duration note", text: durationDescriptionBinding) } diff --git a/Sources/MusicAssistantCore/Integrations/OpenAI/OpenAIClient.swift b/Sources/MusicAssistantCore/Integrations/OpenAI/OpenAIClient.swift index 756a462..586047d 100644 --- a/Sources/MusicAssistantCore/Integrations/OpenAI/OpenAIClient.swift +++ b/Sources/MusicAssistantCore/Integrations/OpenAI/OpenAIClient.swift @@ -49,6 +49,13 @@ public protocol OpenAIClientAdapter: Sendable { func decodeProjectGenerationResult(from data: Data) throws -> SongProjectGenerationResult + func makeSongProjectDiscussionRequest( + _ request: SongProjectDiscussionRequest, + configuration: OpenAIClientConfiguration + ) throws -> OpenAIClientRequest + + func decodeSongProjectDiscussionResult(from data: Data) throws -> SongProjectDiscussionResult + func makeLyricsRevisionRequest( _ request: LyricsRevisionRequest, configuration: OpenAIClientConfiguration @@ -99,6 +106,12 @@ public final class OpenAIClient: AIService, Sendable { return try adapter.decodeProjectGenerationResult(from: data) } + public func discussSongProject(from request: SongProjectDiscussionRequest) async throws -> SongProjectDiscussionResult { + let clientRequest = try adapter.makeSongProjectDiscussionRequest(request, configuration: configuration) + let data = try await perform(clientRequest) + return try adapter.decodeSongProjectDiscussionResult(from: data) + } + public func reviseLyrics(from request: LyricsRevisionRequest) async throws -> LyricsRevisionResult { let clientRequest = try adapter.makeLyricsRevisionRequest(request, configuration: configuration) let data = try await perform(clientRequest) diff --git a/Sources/MusicAssistantCore/Services/AI/AIService.swift b/Sources/MusicAssistantCore/Services/AI/AIService.swift index c57af24..52f132c 100644 --- a/Sources/MusicAssistantCore/Services/AI/AIService.swift +++ b/Sources/MusicAssistantCore/Services/AI/AIService.swift @@ -2,10 +2,21 @@ import Foundation public protocol AIService: Sendable { func generateSongProject(from request: SongProjectGenerationRequest) async throws -> SongProjectGenerationResult + func discussSongProject(from request: SongProjectDiscussionRequest) async throws -> SongProjectDiscussionResult func reviseLyrics(from request: LyricsRevisionRequest) async throws -> LyricsRevisionResult func proposeProjectUpdate(from request: SongProjectUpdateRequest) async throws -> SongProjectUpdateResult } +public extension AIService { + func discussSongProject(from request: SongProjectDiscussionRequest) async throws -> SongProjectDiscussionResult { + throw AIServiceCapabilityError.discussionNotSupported + } +} + +public enum AIServiceCapabilityError: Error, Equatable, Sendable { + case discussionNotSupported +} + public struct AIConversationMessage: Equatable, Identifiable, Sendable { public let id: String public var role: AIConversationRole @@ -82,6 +93,26 @@ public struct SongProjectGenerationResult: Equatable, Sendable { } } +public struct SongProjectDiscussionRequest: Equatable, Sendable { + public var context: AIRequestContext + public var project: SongProject + + public init(context: AIRequestContext, project: SongProject) { + self.context = context + self.project = project + } +} + +public struct SongProjectDiscussionResult: Equatable, Sendable { + public var questions: [String] + public var notes: [String] + + public init(questions: [String], notes: [String] = []) { + self.questions = questions + self.notes = notes + } +} + public struct LyricsRevisionRequest: Equatable, Sendable { public var context: AIRequestContext public var project: SongProject diff --git a/Sources/MusicAssistantCore/Services/AI/ApplicationRules.swift b/Sources/MusicAssistantCore/Services/AI/ApplicationRules.swift index bf232a8..99ee923 100644 --- a/Sources/MusicAssistantCore/Services/AI/ApplicationRules.swift +++ b/Sources/MusicAssistantCore/Services/AI/ApplicationRules.swift @@ -45,6 +45,10 @@ public final class ApplicationRuleInjectingAIService: AIService, Sendable { try await baseService.generateSongProject(from: requestWithInjectedRules(request)) } + public func discussSongProject(from request: SongProjectDiscussionRequest) async throws -> SongProjectDiscussionResult { + try await baseService.discussSongProject(from: requestWithInjectedRules(request)) + } + public func reviseLyrics(from request: LyricsRevisionRequest) async throws -> LyricsRevisionResult { try await baseService.reviseLyrics(from: requestWithInjectedRules(request)) } @@ -59,6 +63,12 @@ public final class ApplicationRuleInjectingAIService: AIService, Sendable { return request } + private func requestWithInjectedRules(_ request: SongProjectDiscussionRequest) throws -> SongProjectDiscussionRequest { + var request = request + request.context.injectPrivateApplicationRules(try ruleProvider.privateApplicationRules()) + return request + } + private func requestWithInjectedRules(_ request: LyricsRevisionRequest) throws -> LyricsRevisionRequest { var request = request request.context.injectPrivateApplicationRules(try ruleProvider.privateApplicationRules()) diff --git a/Sources/MusicAssistantCore/Services/AI/SongProjectDiscussionDirector.swift b/Sources/MusicAssistantCore/Services/AI/SongProjectDiscussionDirector.swift new file mode 100644 index 0000000..ce64b41 --- /dev/null +++ b/Sources/MusicAssistantCore/Services/AI/SongProjectDiscussionDirector.swift @@ -0,0 +1,48 @@ +import Foundation + +public final class SongProjectDiscussionDirector: Sendable { + private let aiService: any AIService + + public init(aiService: any AIService) { + self.aiService = aiService + } + + public func discuss( + project: SongProject, + userMessage: String, + conversation: [AIConversationMessage] = [], + localeIdentifier: String? = nil + ) async throws -> SongProjectDiscussionResult { + guard project.conversationMode == .discuss else { + throw SongProjectDiscussionDirectorError.discussionModeNotEnabled + } + + let trimmedMessage = userMessage.trimmingCharacters(in: .whitespacesAndNewlines) + guard !trimmedMessage.isEmpty else { + throw SongProjectDiscussionDirectorError.emptyUserMessage + } + + let result = try await aiService.discussSongProject( + from: SongProjectDiscussionRequest( + context: AIRequestContext( + userInstruction: trimmedMessage, + conversation: conversation, + localeIdentifier: localeIdentifier + ), + project: project + ) + ) + + return SongProjectDiscussionResult( + questions: result.questions + .map { $0.trimmingCharacters(in: .whitespacesAndNewlines) } + .filter { !$0.isEmpty }, + notes: result.notes + ) + } +} + +public enum SongProjectDiscussionDirectorError: Error, Equatable, Sendable { + case discussionModeNotEnabled + case emptyUserMessage +} diff --git a/Tests/MusicAssistantCoreTests/AIServiceTests.swift b/Tests/MusicAssistantCoreTests/AIServiceTests.swift index 0e32b7b..bfe2f78 100644 --- a/Tests/MusicAssistantCoreTests/AIServiceTests.swift +++ b/Tests/MusicAssistantCoreTests/AIServiceTests.swift @@ -24,6 +24,24 @@ final class AIServiceTests: XCTestCase { XCTAssertEqual(result.followUpQuestions, ["Should the chorus be bigger?"]) } + func testProviderIndependentServiceReturnsDiscussionQuestionsWithoutProjectUpdate() async throws { + let service = MockAIService() + let project = SongProject( + title: "Discussion Project", + idea: "Plan the chorus", + conversationMode: .discuss + ) + let request = SongProjectDiscussionRequest( + context: AIRequestContext(userInstruction: "Ask what is still missing."), + project: project + ) + + let result = try await service.discussSongProject(from: request) + + XCTAssertEqual(result.questions, ["Which vocal delivery should lead the chorus?"]) + XCTAssertEqual(result.notes, ["Discussion only; no project update was proposed."]) + } + func testProviderIndependentServiceRevisesLyricsWithStructuredResult() async throws { let service = MockAIService() let project = SongProject(title: "Lyric Project", idea: "Improve words") @@ -71,6 +89,13 @@ private struct MockAIService: AIService { ) } + func discussSongProject(from request: SongProjectDiscussionRequest) async throws -> SongProjectDiscussionResult { + SongProjectDiscussionResult( + questions: ["Which vocal delivery should lead the chorus?"], + notes: ["Discussion only; no project update was proposed."] + ) + } + func reviseLyrics(from request: LyricsRevisionRequest) async throws -> LyricsRevisionResult { LyricsRevisionResult( lyrics: Lyrics(text: "\(request.sourceLyrics.text)\n\(request.context.userInstruction)"), diff --git a/Tests/MusicAssistantCoreTests/ApplicationRuleInjectionTests.swift b/Tests/MusicAssistantCoreTests/ApplicationRuleInjectionTests.swift index 22ca901..1b24e0d 100644 --- a/Tests/MusicAssistantCoreTests/ApplicationRuleInjectionTests.swift +++ b/Tests/MusicAssistantCoreTests/ApplicationRuleInjectionTests.swift @@ -17,6 +17,12 @@ final class ApplicationRuleInjectionTests: XCTestCase { context: AIRequestContext(userInstruction: "Generate.") ) ) + _ = try await service.discussSongProject( + from: SongProjectDiscussionRequest( + context: AIRequestContext(userInstruction: "Discuss."), + project: project + ) + ) _ = try await service.reviseLyrics( from: LyricsRevisionRequest( context: AIRequestContext(userInstruction: "Improve."), @@ -37,6 +43,7 @@ final class ApplicationRuleInjectionTests: XCTestCase { XCTAssertEqual( recordedRuleContents, [ + "Use private product rules.", "Use private product rules.", "Use private product rules.", "Use private product rules." @@ -90,6 +97,11 @@ private actor RecordingAIService: AIService { ) } + func discussSongProject(from request: SongProjectDiscussionRequest) async throws -> SongProjectDiscussionResult { + contexts.append(request.context) + return SongProjectDiscussionResult(questions: ["Which direction should we take?"]) + } + func reviseLyrics(from request: LyricsRevisionRequest) async throws -> LyricsRevisionResult { contexts.append(request.context) return LyricsRevisionResult(lyrics: request.sourceLyrics) diff --git a/Tests/MusicAssistantCoreTests/OpenAIClientTests.swift b/Tests/MusicAssistantCoreTests/OpenAIClientTests.swift index fdf238a..f3e3a4f 100644 --- a/Tests/MusicAssistantCoreTests/OpenAIClientTests.swift +++ b/Tests/MusicAssistantCoreTests/OpenAIClientTests.swift @@ -68,6 +68,34 @@ final class OpenAIClientTests: XCTestCase { XCTAssertEqual(recordedURL, overrideURL) } + func testDiscussSongProjectUsesAdapterAndReturnsQuestions() async throws { + let transport = RecordingOpenAITransport( + statusCode: 200, + responseData: try JSONEncoder().encode(MockOpenAIResponse(text: "Should the chorus be intimate or anthemic?")) + ) + let configuration = OpenAIClientConfiguration( + apiKey: "test-api-key", + endpointURL: URL(string: "https://api.example.test/v1/configured")!, + model: "configured-model" + ) + let client = OpenAIClient( + configuration: configuration, + adapter: MockOpenAIClientAdapter(), + transport: transport + ) + + let result = try await client.discussSongProject( + from: SongProjectDiscussionRequest( + context: AIRequestContext(userInstruction: "Ask before deciding."), + project: SongProject(title: "Discussion", idea: "Plan this", conversationMode: .discuss) + ) + ) + let requestBodyString = await transport.requestBodyString + + XCTAssertEqual(result.questions, ["Should the chorus be intimate or anthemic?"]) + XCTAssertEqual(requestBodyString, "discuss|configured-model|Ask before deciding.") + } + func testClientRejectsMissingAPIKeyBeforeSendingRequest() async throws { let transport = RecordingOpenAITransport( statusCode: 200, @@ -175,6 +203,21 @@ private struct MockOpenAIClientAdapter: OpenAIClientAdapter { ) } + func makeSongProjectDiscussionRequest( + _ request: SongProjectDiscussionRequest, + configuration: OpenAIClientConfiguration + ) throws -> OpenAIClientRequest { + OpenAIClientRequest( + url: overrideURL, + body: Data("discuss|\(configuration.model ?? "")|\(request.context.userInstruction)".utf8) + ) + } + + func decodeSongProjectDiscussionResult(from data: Data) throws -> SongProjectDiscussionResult { + let response = try JSONDecoder().decode(MockOpenAIResponse.self, from: data) + return SongProjectDiscussionResult(questions: [response.text]) + } + func makeLyricsRevisionRequest( _ request: LyricsRevisionRequest, configuration: OpenAIClientConfiguration diff --git a/Tests/MusicAssistantCoreTests/SongProjectDiscussionDirectorTests.swift b/Tests/MusicAssistantCoreTests/SongProjectDiscussionDirectorTests.swift new file mode 100644 index 0000000..339d60b --- /dev/null +++ b/Tests/MusicAssistantCoreTests/SongProjectDiscussionDirectorTests.swift @@ -0,0 +1,94 @@ +import MusicAssistantCore +import XCTest + +final class SongProjectDiscussionDirectorTests: XCTestCase { + func testDiscussBuildsARequestAndReturnsTrimmedQuestionsWithoutChangingProject() async throws { + let project = SongProject( + id: "discussion-project", + title: "Discussion Project", + idea: "Plan a cinematic chorus", + conversationMode: .discuss, + createdAt: Date(timeIntervalSince1970: 10), + updatedAt: Date(timeIntervalSince1970: 20) + ) + let aiService = RecordingDiscussionAIService( + result: SongProjectDiscussionResult( + questions: [" Should the chorus use a choir? ", " "], + notes: ["Awaiting the user's choice."] + ) + ) + let director = SongProjectDiscussionDirector(aiService: aiService) + let conversation = [AIConversationMessage(role: .assistant, content: "What mood should lead?")] + + let result = try await director.discuss( + project: project, + userMessage: " Keep the verses intimate. ", + conversation: conversation, + localeIdentifier: "en_US" + ) + let request = await aiService.recordedRequests.first + + XCTAssertEqual(request?.context.userInstruction, "Keep the verses intimate.") + XCTAssertEqual(request?.context.conversation, conversation) + XCTAssertEqual(request?.context.localeIdentifier, "en_US") + XCTAssertEqual(request?.project, project) + XCTAssertEqual(result.questions, ["Should the chorus use a choir?"]) + XCTAssertEqual(result.notes, ["Awaiting the user's choice."]) + } + + func testDiscussRejectsAutoModeAndBlankMessagesBeforeCallingAI() async throws { + let aiService = RecordingDiscussionAIService( + result: SongProjectDiscussionResult(questions: ["Unused"]) + ) + let director = SongProjectDiscussionDirector(aiService: aiService) + + do { + _ = try await director.discuss( + project: SongProject(title: "Auto", idea: "Auto", conversationMode: .auto), + userMessage: "Ask a question." + ) + XCTFail("Expected discussion mode validation to fail.") + } catch let error as SongProjectDiscussionDirectorError { + XCTAssertEqual(error, .discussionModeNotEnabled) + } + + do { + _ = try await director.discuss( + project: SongProject(title: "Discuss", idea: "Discuss", conversationMode: .discuss), + userMessage: " \n " + ) + XCTFail("Expected blank message validation to fail.") + } catch let error as SongProjectDiscussionDirectorError { + XCTAssertEqual(error, .emptyUserMessage) + } + + let requestCount = await aiService.recordedRequests.count + XCTAssertEqual(requestCount, 0) + } +} + +private actor RecordingDiscussionAIService: AIService { + private(set) var recordedRequests: [SongProjectDiscussionRequest] = [] + private let result: SongProjectDiscussionResult + + init(result: SongProjectDiscussionResult) { + self.result = result + } + + func generateSongProject(from request: SongProjectGenerationRequest) async throws -> SongProjectGenerationResult { + SongProjectGenerationResult(project: request.seedProject ?? SongProject(title: "Unused", idea: "Unused")) + } + + func discussSongProject(from request: SongProjectDiscussionRequest) async throws -> SongProjectDiscussionResult { + recordedRequests.append(request) + return result + } + + func reviseLyrics(from request: LyricsRevisionRequest) async throws -> LyricsRevisionResult { + LyricsRevisionResult(lyrics: request.sourceLyrics) + } + + func proposeProjectUpdate(from request: SongProjectUpdateRequest) async throws -> SongProjectUpdateResult { + SongProjectUpdateResult(project: request.project) + } +} diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 5e084f7..2e442b2 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -43,6 +43,9 @@ data wherever possible, not treated as an unstructured chat transcript. For automatic decisions, the AI Director requests only enabled automatic scopes and merges the response into the current project while preserving manual structure, arrangement, musical-parameter and production choices. +When a Song Project uses Discuss mode, the AI Director sends the current +project and conversation context to the provider and returns follow-up +questions without applying a project update. ## Prompt Compiler diff --git a/docs/DATA_MODEL.md b/docs/DATA_MODEL.md index 80b0e22..f038c2c 100644 --- a/docs/DATA_MODEL.md +++ b/docs/DATA_MODEL.md @@ -70,6 +70,12 @@ Auto-decision behavior - The AI may update production directions only when productionMode is auto. - Manual values remain authoritative when automatic decisions are applied. +Discuss mode behavior +- When conversationMode is discuss, the AI receives the current Song Project + and conversation context, then returns follow-up questions before a project + decision is applied. +- A discussion response does not itself modify the Song Project. + InstrumentPlacement - sectionId (optional) - startTime (optional) diff --git a/docs/TASKS.md b/docs/TASKS.md index ab46b01..e594b67 100644 --- a/docs/TASKS.md +++ b/docs/TASKS.md @@ -61,7 +61,7 @@ requirement is missing and blocks implementation, record it in - [x] Implement existing lyrics → correction/improvement flow. - [x] Implement Auto mode for structure, arrangement, BPM/key/maqam and production decisions. -- [ ] Implement optional Discuss mode. +- [x] Implement optional Discuss mode. - [ ] Enforce user-lock/manual-value precedence over AI output. - [ ] Add error, retry, cancellation and rate-limit handling.