diff --git a/Sources/HeardCore/AppModel.swift b/Sources/HeardCore/AppModel.swift index e06c81a..149521f 100644 --- a/Sources/HeardCore/AppModel.swift +++ b/Sources/HeardCore/AppModel.swift @@ -709,13 +709,12 @@ public var filteredSpeakers: [SpeakerProfile] { ) // Rewrite every transcript that references the placeholder. Speaker // numbers are globally unique, so this normally only touches the one - // transcript in `candidate.transcriptPath`, but scanning the output - // directory keeps the rename complete if the user moved/renamed files - // or if the placeholder ever shows up in more than one transcript. - let outputDir = URL(fileURLWithPath: settingsStore.settings.outputDirectory, isDirectory: true) - TranscriptWriter.renameSpeakerInDirectory(outputDir, from: candidate.temporaryName, to: trimmed) - if let transcriptPath = candidate.transcriptPath, - transcriptPath.deletingLastPathComponent().standardizedFileURL != outputDir.standardizedFileURL { + // transcript in `candidate.transcriptPath`, but the helper also scans + // every queued transcript so the rename lands even when the output + // directory was changed or a path-comparison quirk would otherwise + // have made the directory scan skip the file. + rewriteSpeakerAcrossTranscripts(from: candidate.temporaryName, to: trimmed) + if let transcriptPath = candidate.transcriptPath { TranscriptWriter.renameSpeaker(in: transcriptPath, from: candidate.temporaryName, to: trimmed) } namingCandidates.removeAll { $0.id == candidate.id } @@ -767,7 +766,7 @@ public var filteredSpeakers: [SpeakerProfile] { public func renameSpeaker(id: UUID, to name: String) { let trimmed = name.trimmingCharacters(in: .whitespacesAndNewlines) guard !trimmed.isEmpty else { return } - + guard let oldProfile = speakerStore.speakers.first(where: { $0.id == id }) else { return } let oldName = oldProfile.name guard oldName != trimmed else { return } @@ -776,8 +775,7 @@ public var filteredSpeakers: [SpeakerProfile] { // Prompt the user before retroactively renaming if askUserToUpdateTranscripts(oldName: oldName, newName: trimmed) { - let outputDir = URL(fileURLWithPath: settingsStore.settings.outputDirectory, isDirectory: true) - TranscriptWriter.renameSpeakerInDirectory(outputDir, from: oldName, to: trimmed) + rewriteSpeakerAcrossTranscripts(from: oldName, to: trimmed) } } @@ -799,8 +797,7 @@ public var filteredSpeakers: [SpeakerProfile] { if let primary = speakerStore.speakers.first(where: { $0.id == primaryID }), let secondary = speakerStore.speakers.first(where: { $0.id == secondaryID }) { if askUserToUpdateTranscripts(oldName: secondary.name, newName: primary.name) { - let outputDir = URL(fileURLWithPath: settingsStore.settings.outputDirectory, isDirectory: true) - TranscriptWriter.renameSpeakerInDirectory(outputDir, from: secondary.name, to: primary.name) + rewriteSpeakerAcrossTranscripts(from: secondary.name, to: primary.name) } } @@ -808,6 +805,23 @@ public var filteredSpeakers: [SpeakerProfile] { mergeSelection.removeAll() } + /// Rewrite `oldName` → `newName` in every transcript we can reasonably locate: + /// the configured output directory, every path tracked by the pipeline queue, + /// and any audit-level fallbacks. `renameSpeaker` is a literal string-replace + /// pass that's idempotent for a given file, so the redundant scans are safe + /// and just guarantee the rename actually lands even if the user moved the + /// output directory between meetings, or a URL-comparison quirk would have + /// caused the directory scan to skip the file. + private func rewriteSpeakerAcrossTranscripts(from oldName: String, to newName: String) { + let outputDir = URL(fileURLWithPath: settingsStore.settings.outputDirectory, isDirectory: true) + TranscriptWriter.renameSpeakerInDirectory(outputDir, from: oldName, to: newName) + for job in queueStore.jobs { + if let path = job.transcriptPath { + TranscriptWriter.renameSpeaker(in: path, from: oldName, to: newName) + } + } + } + private func askUserToUpdateTranscripts(oldName: String, newName: String) -> Bool { // If it's a generated placeholder, there's no need to ask, just do it. // The user only needs to be asked if they are renaming an already explicitly named speaker. diff --git a/Sources/HeardCore/Services.swift b/Sources/HeardCore/Services.swift index 253fd10..9586255 100644 --- a/Sources/HeardCore/Services.swift +++ b/Sources/HeardCore/Services.swift @@ -1575,6 +1575,12 @@ public enum TranscriptWriter { } let updated = lines.joined(separator: "\n") + // Skip the disk write when nothing changed. The popup save and the + // Speakers tab both call this function across every queued transcript + // and across every file in the output directory to make sure the + // rename actually lands, so on a typical install most invocations are + // no-ops; rewriting an unchanged file just adds unnecessary I/O. + guard updated != original else { return } try? updated.write(to: transcriptURL, atomically: true, encoding: .utf8) } diff --git a/Tests/HeardTests/TestRunner.swift b/Tests/HeardTests/TestRunner.swift index c66078a..996acd0 100644 --- a/Tests/HeardTests/TestRunner.swift +++ b/Tests/HeardTests/TestRunner.swift @@ -468,6 +468,102 @@ func runTranscriptWriterTests() { try expect(c.contains("**Speaker 8:**"), "C should be untouched") } + test("renameSpeakerInDirectory rewrites the production Speaker_ placeholder") { + // Production places `Speaker_<6 hex chars>` in transcripts when a new speaker + // is detected and the user hasn't named them yet. The legacy "Speaker 7" test + // covers the basic logic; this one pins down the underscore-based format the + // pipeline actually emits so any regression there is caught immediately. + let tmpDir = FileManager.default.temporaryDirectory + .appendingPathComponent("HeardTests-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: tmpDir, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let date = Date() + let doc = TranscriptDocument( + title: "Roadmap", + startTime: date, + endTime: date.addingTimeInterval(120), + participants: ["Speaker_ABC123", "Speaker_DEF456", "Me"], + segments: [ + TranscriptSegment(speaker: "Speaker_ABC123", startTime: 0, endTime: 30, + text: "Welcome to the meeting."), + TranscriptSegment(speaker: "Speaker_DEF456", startTime: 30, endTime: 60, + text: "Glad to be here."), + TranscriptSegment(speaker: "Me", startTime: 60, endTime: 90, text: "Hi."), + ] + ) + let url = try TranscriptWriter.write(document: doc, outputDirectory: tmpDir) + + TranscriptWriter.renameSpeakerInDirectory(tmpDir, from: "Speaker_ABC123", to: "Alice") + + let content = try String(contentsOf: url, encoding: .utf8) + try expect(content.contains("**Alice:** Welcome to the meeting."), + "Body line should be rewritten to Alice") + try expect(!content.contains("Speaker_ABC123"), + "Placeholder must be fully gone from body and participants header") + try expect(content.contains("**Speaker_DEF456:** Glad to be here."), + "Other placeholders must be left untouched") + try expect(content.contains("Alice"), "Participants header should include Alice") + } + + test("renameSpeaker rewrites a transcript outside the configured output directory") { + // Simulates the popup-after-meeting case where `candidate.transcriptPath` + // points at a file the directory scan would miss — e.g. because the user + // moved their output folder between meetings. The unconditional path-based + // pass in `saveSpeakerName` should still rewrite the file. + let tmpDir = FileManager.default.temporaryDirectory + .appendingPathComponent("HeardTests-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: tmpDir, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let date = Date() + let doc = TranscriptDocument( + title: "Standup", + startTime: date, + endTime: date.addingTimeInterval(60), + participants: ["Speaker_ABC123", "Me"], + segments: [ + TranscriptSegment(speaker: "Speaker_ABC123", startTime: 0, endTime: 30, + text: "Morning."), + ] + ) + let url = try TranscriptWriter.write(document: doc, outputDirectory: tmpDir) + + TranscriptWriter.renameSpeaker(in: url, from: "Speaker_ABC123", to: "Bob") + + let content = try String(contentsOf: url, encoding: .utf8) + try expect(content.contains("**Bob:** Morning."), "Body line should now reference Bob") + try expect(!content.contains("Speaker_ABC123"), + "Placeholder should be gone from body and participants header") + } + + test("renameSpeaker is idempotent — second pass is a no-op") { + // We deliberately invoke `renameSpeaker` multiple times from `saveSpeakerName` + // (directory scan + queue scan + explicit path) to make sure the rename actually + // lands. This relies on the function being safe to run again after a successful + // rename, since after the first pass the placeholder is gone. + let tmpDir = FileManager.default.temporaryDirectory + .appendingPathComponent("HeardTests-\(UUID().uuidString)") + try FileManager.default.createDirectory(at: tmpDir, withIntermediateDirectories: true) + defer { try? FileManager.default.removeItem(at: tmpDir) } + + let doc = TranscriptDocument( + title: "Standup", startTime: Date(), endTime: Date().addingTimeInterval(60), + participants: ["Speaker_1AB23C", "Me"], + segments: [TranscriptSegment(speaker: "Speaker_1AB23C", startTime: 0, endTime: 30, + text: "Hello.")] + ) + let url = try TranscriptWriter.write(document: doc, outputDirectory: tmpDir) + + TranscriptWriter.renameSpeaker(in: url, from: "Speaker_1AB23C", to: "Carol") + let afterFirst = try String(contentsOf: url, encoding: .utf8) + TranscriptWriter.renameSpeaker(in: url, from: "Speaker_1AB23C", to: "Carol") + let afterSecond = try String(contentsOf: url, encoding: .utf8) + + try expect(afterFirst.contains("**Carol:** Hello."), "First pass rewrites Carol") + try expectEqual(afterFirst, afterSecond) + } + test("renameSpeakerInDirectory ignores non-md files and missing dirs") { let tmpDir = FileManager.default.temporaryDirectory .appendingPathComponent("HeardTests-\(UUID().uuidString)")