-
-
Notifications
You must be signed in to change notification settings - Fork 243
Fix: send AI enhancement instructions in system role, not user message #439
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.
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
Kayaba-Attribution
wants to merge
3
commits into
altic-dev:main
Choose a base branch
from
Kayaba-Attribution:fix/issue-388-system-prompt-placement
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+102
−21
Open
Changes from all commits
Commits
Show all changes
3 commits
Select commit
Hold shift + click to select a range
ded39d9
Fix AI enhancement instructions sent in system prompt, not user messa…
Kayaba-Attribution 4e92e4e
Fix system prompt placement in ContentView.processTextWithAI (#388)
Kayaba-Attribution a7a0d1c
fix: restore \${transcript} placeholder substitution in system role
Kayaba-Attribution 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
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
81 changes: 81 additions & 0 deletions
81
Tests/FluidDictationIntegrationTests/DictationSystemPromptTests.swift
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 |
|---|---|---|
| @@ -0,0 +1,81 @@ | ||
| @testable import FluidVoice_Debug | ||
| import XCTest | ||
|
|
||
| // Regression tests for https://github.com/altic-dev/FluidVoice/issues/388 | ||
| // AI enhancement instructions must be sent in the system role, not the user message. | ||
| // Previously, DictationPostProcessingService hardcoded systemPrompt = "" and folded | ||
| // the instruction text into the user message alongside the transcript. | ||
|
|
||
| @MainActor | ||
| final class DictationSystemPromptTests: XCTestCase { | ||
| // MARK: - effectiveDictationSystemPrompt | ||
|
|
||
| func testEffectiveDictationSystemPrompt_returnsConfiguredPrompt() { | ||
| self.withPromptSettingsRestored { | ||
| let settings = SettingsStore.shared | ||
| let custom = SettingsStore.DictationPromptProfile( | ||
| name: "Test Profile", | ||
| prompt: "Clean up the transcript. Remove filler words.", | ||
| mode: .dictate | ||
| ) | ||
| settings.dictationPromptProfiles = [custom] | ||
| settings.selectedDictationPromptID = custom.id | ||
|
|
||
| let result = settings.effectiveDictationSystemPrompt(for: .primary) | ||
| XCTAssertFalse(result.isEmpty, "effectiveDictationSystemPrompt must return the configured prompt, not an empty string") | ||
| XCTAssertTrue(result.contains("Clean up the transcript"), "system prompt must include the custom instruction text") | ||
| } | ||
| } | ||
|
|
||
| func testEffectiveDictationSystemPrompt_offSelection_returnsDefault() { | ||
| self.withPromptSettingsRestored { | ||
| let settings = SettingsStore.shared | ||
| settings.setDictationPromptSelection(.off) | ||
|
|
||
| // When off, effectiveDictationSystemPrompt falls back to the built-in default, | ||
| // which is non-empty. This ensures the system field is never silently blank. | ||
| let result = settings.effectiveDictationSystemPrompt(for: .primary) | ||
| XCTAssertFalse(result.isEmpty, "built-in default prompt must be non-empty") | ||
| } | ||
| } | ||
|
|
||
| // MARK: - renderSystemPrompt (${transcript} placeholder in the system role) | ||
|
|
||
| func testRenderSystemPrompt_noPlaceholder_returnsPromptUnchanged() { | ||
| let prompt = "Clean up the transcript." | ||
| let result = SettingsStore.renderSystemPrompt(promptText: prompt, transcript: "hello world") | ||
| XCTAssertEqual(result, prompt, "prompt without placeholder must be returned unchanged") | ||
| } | ||
|
|
||
| func testRenderSystemPrompt_transcriptPlaceholder_isSubstitutedInSystemRole() { | ||
| // Users can embed ${transcript} in their system prompt template so the transcript | ||
| // appears inline in their instructions. renderSystemPrompt must substitute it before | ||
| // the prompt is sent to the provider as the system message. | ||
| let prompt = "Rewrite cleanly: \(SettingsStore.transcriptPlaceholder)" | ||
| let transcript = "um so like yeah" | ||
| let result = SettingsStore.renderSystemPrompt(promptText: prompt, transcript: transcript) | ||
| XCTAssertEqual(result, "Rewrite cleanly: um so like yeah") | ||
| } | ||
|
|
||
| // MARK: - Helpers | ||
|
|
||
| private func withPromptSettingsRestored(_ run: () -> Void) { | ||
| let keys = [ | ||
| "DictationPromptProfiles", | ||
| "SelectedDictationPromptID", | ||
| "DictationPromptOff", | ||
| ] | ||
| let defaults = UserDefaults.standard | ||
| var snapshot: [String: Any] = [:] | ||
| for key in keys { | ||
| if let v = defaults.object(forKey: key) { snapshot[key] = v } | ||
| } | ||
| defer { | ||
| for key in keys { | ||
| if let v = snapshot[key] { defaults.set(v, forKey: key) } | ||
| else { defaults.removeObject(forKey: key) } | ||
| } | ||
| } | ||
| run() | ||
| } | ||
| } |
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.
For dictation prompts that contain
${transcript}, this replacement puts the transcript into the system prompt, while both new call sites still send the same text as the user turn (ContentViewsetsuserMessageContent = inputText, andDictationPostProcessingServicesets it totrimmed). Those prompts used to include the transcript only once viarenderDictationUserMessage; now long dictations can double token usage or exceed context, and templates that relied on the placeholder to control where the sole transcript appears will see duplicate input. Please either avoid expanding the placeholder once the transcript is a separate user message, or suppress the extra user turn for placeholder templates.Useful? React with 👍 / 👎.