From 485c923c81e143ac8e0229cf065f22b535213997 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 22 May 2026 21:44:05 +0000 Subject: [PATCH] Make speaker rename actually land in every transcript MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The popup save and the Speakers settings tab both relied on a single directory scan to rewrite `Speaker_` placeholders into real names, and the popup's "specific transcript" fallback only ran when the transcript's directory differed from the configured output dir. That made the rename quietly skip the file whenever a URL-comparison quirk (symlink resolution, trailing-slash normalization, etc.) made the two URLs compare unequal in one direction but equal in the other — leaving `Speaker_ABC123` in the saved .md even after the user named the speaker. - Centralize the rewrite in `rewriteSpeakerAcrossTranscripts`, used by the popup save, the Speakers tab inline rename, and the merge action. In addition to scanning the output directory, also walk every `transcriptPath` tracked by the pipeline queue so a moved output folder doesn't strand old transcripts. - In `saveSpeakerName`, always rewrite `candidate.transcriptPath` unconditionally instead of only when the directory diverges from `outputDir`. The redundant pass is harmless thanks to the new no-op short-circuit below. - In `TranscriptWriter.renameSpeaker`, skip the disk write when the rebuilt content matches what's already on disk. The defensive scans now hit many transcripts that don't contain the placeholder, and rewriting unchanged files just churns the FS. - Add tests that pin down the production `Speaker_` placeholder format (the existing tests only covered "Speaker 7" with a space) and verify the file-targeted rename + idempotency. --- Sources/HeardCore/AppModel.swift | 38 ++++++++---- Sources/HeardCore/Services.swift | 6 ++ Tests/HeardTests/TestRunner.swift | 96 +++++++++++++++++++++++++++++++ 3 files changed, 128 insertions(+), 12 deletions(-) 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)")