diff --git a/crates/livecap-core/src/audio/device.rs b/crates/livecap-core/src/audio/device.rs index 45005b3..cbfd226 100644 --- a/crates/livecap-core/src/audio/device.rs +++ b/crates/livecap-core/src/audio/device.rs @@ -33,14 +33,20 @@ impl AudioDevice { return Err(anyhow!("Device name cannot be empty")); } - let (name, device_type) = if name.to_lowercase().ends_with("(input)") { + // Detect and strip on the SAME case-insensitive basis (#178): the suffix + // is ASCII, so its char-length equals its byte-length in any casing, and + // slicing the ORIGINAL by that length removes the exact matched bytes — + // whereas `trim_end_matches("(input)")` on the original would silently + // leave a differently-cased "(Input)"/"(OUTPUT)" suffix stuck on. + let lower = name.to_lowercase(); + let (name, device_type) = if lower.ends_with("(input)") { ( - name.trim_end_matches("(input)").trim().to_string(), + name[..name.len() - "(input)".len()].trim().to_string(), DeviceType::Input, ) - } else if name.to_lowercase().ends_with("(output)") { + } else if lower.ends_with("(output)") { ( - name.trim_end_matches("(output)").trim().to_string(), + name[..name.len() - "(output)".len()].trim().to_string(), DeviceType::Output, ) } else { @@ -164,6 +170,21 @@ mod tests { assert_eq!(d.device_type, DeviceType::Output); } + #[test] + fn strips_the_suffix_regardless_of_case() { + // #178: detection is case-insensitive, so stripping must be too — a + // differently-cased suffix must not be left stuck on the device name (which + // would fail the exact-match lookup in get_device_and_config). + for raw in ["Studio Mic (Input)", "Studio Mic (INPUT)", "Studio Mic (input)"] { + let d = AudioDevice::from_name(raw).unwrap(); + assert_eq!(d.name, "Studio Mic", "case variant {raw:?} must strip cleanly"); + assert_eq!(d.device_type, DeviceType::Input); + } + let d = AudioDevice::from_name("Big Speaker (Output)").unwrap(); + assert_eq!(d.name, "Big Speaker"); + assert_eq!(d.device_type, DeviceType::Output); + } + #[test] fn rejects_unqualified_names() { assert!(AudioDevice::from_name("Just A Device").is_err()); diff --git a/crates/livecap-core/src/pipeline.rs b/crates/livecap-core/src/pipeline.rs index 21d38b2..9b53bcd 100644 --- a/crates/livecap-core/src/pipeline.rs +++ b/crates/livecap-core/src/pipeline.rs @@ -81,13 +81,18 @@ impl PipelineConfig { /// Set the spoken/source transcription language (#94). A BCP-47 / ISO-639-1 /// code forces whisper to that language; `"auto"` (or empty) keeps the - /// current per-utterance auto-detection (`language = None`). Whisper only - /// understands a primary subtag, so a tag like `"pt-br"` is reduced to its + /// current per-utterance auto-detection (`language = None`). The special value + /// `"auto-translate"` (per-utterance detect + translate to English) is passed + /// through verbatim (#178) — the engine matches it exactly (`whisper/engine.rs` + /// `set_translate`); without this it would hit the subtag reduction below and + /// be mangled to `"auto"`, silently disabling translation. Whisper otherwise + /// only understands a primary subtag, so a tag like `"pt-br"` is reduced to its /// primary subtag (`"pt"`) before being handed to the engine. pub fn with_source_language(mut self, code: &str) -> Self { let normalized = code.trim().to_lowercase(); self.language = match normalized.as_str() { "" | "auto" => None, + "auto-translate" => Some("auto-translate".to_string()), other => Some(other.split('-').next().unwrap_or(other).to_string()), }; self @@ -775,6 +780,22 @@ mod tests { assert_eq!(PipelineConfig::new(&dir).with_source_language(" AUTO ").language, None); } + #[test] + fn source_language_preserves_auto_translate() { + // #178: "auto-translate" (detect + translate to English) must survive the + // setter verbatim — the subtag split would otherwise mangle it to "auto" + // and silently disable translation. The engine matches the exact string. + let dir = std::path::PathBuf::from("/tmp/livecap-models"); + assert_eq!( + PipelineConfig::new(&dir).with_source_language("auto-translate").language, + Some("auto-translate".to_string()) + ); + assert_eq!( + PipelineConfig::new(&dir).with_source_language(" Auto-Translate ").language, + Some("auto-translate".to_string()) + ); + } + #[test] fn with_model_overrides_the_default() { // #110: the Settings pick lands in PipelineConfig.model; the default diff --git a/crates/livecap-core/src/resample.rs b/crates/livecap-core/src/resample.rs index 0e81e71..9b5232a 100644 --- a/crates/livecap-core/src/resample.rs +++ b/crates/livecap-core/src/resample.rs @@ -42,7 +42,7 @@ impl StreamResampler { // Pass-through; drop any residue from a previous rate. self.inner = None; self.from_rate = from_rate; - self.input_buf.clear(); + self.drop_residue(); return Ok(samples.to_vec()); } @@ -60,7 +60,7 @@ impl StreamResampler { 1, )?); self.from_rate = from_rate; - self.input_buf.clear(); + self.drop_residue(); } self.input_buf.extend_from_slice(samples); @@ -74,6 +74,24 @@ impl StreamResampler { } Ok(out) } + + /// Discard the sub-`CHUNK_IN` residue buffered at the OLD rate on a rate change + /// (#178). This is a DELIBERATE, correct drop, NOT a bug to "fix": those + /// samples were captured at the old rate, so they can neither be fed through + /// the freshly-rebuilt new-rate resampler (a rate mismatch would itself corrupt + /// the output) nor emitted as new-rate pass-through. The loss is bounded (at + /// most CHUNK_IN-1 samples, ~21ms at 48kHz) and only occurs at a mid-meeting + /// device/rate switch. The count (never any audio) is logged so the drop is + /// observable rather than silent — the one thing the finding actually asks for. + fn drop_residue(&mut self) { + if !self.input_buf.is_empty() { + log::debug!( + "Resampler: dropping {} buffered old-rate sample(s) at a rate change (#178)", + self.input_buf.len() + ); + self.input_buf.clear(); + } + } } #[cfg(test)] diff --git a/packages/archive/src/render.ts b/packages/archive/src/render.ts index be675d3..23973ab 100644 --- a/packages/archive/src/render.ts +++ b/packages/archive/src/render.ts @@ -35,10 +35,18 @@ function renderMetaLine(m: ArchiveModel): string { } function renderBoard(board: BoardData): string { + // Items are joined with " · " and split back on it by parse.ts, so an item that + // itself contains " · " (natural writing; "·" is also a common Korean bullet) + // would round-trip as TWO items. Swap an item-internal " · " middot (U+00B7) for + // a look-alike U+2027 (‧) before joining so it no longer matches the separator + // (#178); a bare "·" is untouched, and the parser never unescapes, so the item + // reads back with the look-alike (near-invisible) instead of being fragmented. + const join = (items: string[]): string => + items.map((item) => item.split(" · ").join(" ‧ ")).join(" · "); let out = ""; - if (board.decisions.length > 0) out += `**Decisions** — ${board.decisions.join(" · ")}\n`; - if (board.actionItems.length > 0) out += `**Action items** — ${board.actionItems.join(" · ")}\n`; - if (board.openQuestions.length > 0) out += `**Open questions** — ${board.openQuestions.join(" · ")}\n`; + if (board.decisions.length > 0) out += `**Decisions** — ${join(board.decisions)}\n`; + if (board.actionItems.length > 0) out += `**Action items** — ${join(board.actionItems)}\n`; + if (board.openQuestions.length > 0) out += `**Open questions** — ${join(board.openQuestions)}\n`; return out; } @@ -83,12 +91,26 @@ export function sanitizeBlock(text: string): string { .join("\n"); } +/** The trailing low-confidence marker appended to a source line; parse.ts strips + * it back off (its LOW_CONFIDENCE_SUFFIX). */ +const LOW_CONF_MARKER = " (?)"; + /** Render one entry's two lines (header line + `>` translation), trailing \n. */ export function renderEntryBody(e: CaptionEntry): string { const pin = e.pinned ? "📌 " : ""; const speaker = e.speaker === "me" ? "Me" : "Them"; - const confidence = e.lowConfidence ? " (?)" : ""; - return `${pin}**${speaker}** (${e.timestamp}) — ${sanitizeInline(e.source)}${confidence}\n> ${sanitizeInline(e.target)}\n`; + const confidence = e.lowConfidence ? LOW_CONF_MARKER : ""; + // Defuse a source that ITSELF ends with the marker " (?)" (STT punctuation / a + // spoken aside) so parse.ts can't mistake it for the appended marker and both + // truncate the caption AND falsely flag it low-confidence (#178): swap the + // trailing ASCII "?" for a look-alike U+FF1F (？). Runs after sanitizeInline + // (which never adds a trailing " (?)"), so it only fires on genuine text; the + // parser reads it back verbatim instead of stripping it. + const sanitized = sanitizeInline(e.source); + const source = sanitized.endsWith(LOW_CONF_MARKER) + ? `${sanitized.slice(0, -LOW_CONF_MARKER.length)} (？)` + : sanitized; + return `${pin}**${speaker}** (${e.timestamp}) — ${source}${confidence}\n> ${sanitizeInline(e.target)}\n`; } /** diff --git a/packages/archive/src/sanitize.ts b/packages/archive/src/sanitize.ts index 6486206..462a0cc 100644 --- a/packages/archive/src/sanitize.ts +++ b/packages/archive/src/sanitize.ts @@ -31,6 +31,13 @@ function truncateToByteBudget(text: string, maxBytes: number): string { /** Fallback when a title sanitizes to nothing. */ export const FALLBACK_TITLE = "Untitled session"; +/** The in-progress working-file title sentinel — a finalized archive must never + * reuse it as a title, or its ` — (recording).md` filename would be + * misread as a crashed recording by retention/adopt (#178). Defined here (not in + * writer.ts) so [`sanitizeTitle`] can guard against it without a circular import; + * re-exported from writer.ts for the retention sweep's marker. */ +export const WORKING_TITLE = "(recording)"; + // Characters that must never reach a filename: // - the NUL/C0 control range and DEL // - ASCII path separators @@ -66,7 +73,12 @@ export function sanitizeTitle(raw: string): string { // Truncate by UTF-8 bytes (not chars) at a code-point boundary (#32), then // re-trim any trailing space the cut may have exposed. title = truncateToByteBudget(title, MAX_TITLE_BYTES).replace(/[.\s]+$/, ""); - return title === "" ? FALLBACK_TITLE : title; + // A finalized title must never BE the in-progress sentinel (#178): otherwise the + // finalized file ` — (recording).md` collides with the working-file + // grammar and retention/adopt misread this real session as a crashed orphan + // (exempt from the sweep, then re-titled from its first summary line). + if (title === "" || title === WORKING_TITLE) return FALLBACK_TITLE; + return title; } /** Build the archive filename for a sanitized title: ` — .md`. */ diff --git a/packages/archive/src/writer.ts b/packages/archive/src/writer.ts index bb656fe..5f26326 100644 --- a/packages/archive/src/writer.ts +++ b/packages/archive/src/writer.ts @@ -16,9 +16,15 @@ import type { ArchiveFs } from "./fs"; import { uniquePath } from "./paths"; import { renderDocument, renderEntryAppend, type ArchiveModel } from "./render"; -import { sanitizeTitle } from "./sanitize"; +import { sanitizeTitle, WORKING_TITLE } from "./sanitize"; import type { BriefUpdate, CaptionEntry, CoachingData, FinalBrief, SessionMeta } from "./types"; +/** The in-progress working-file title sentinel (`<prefix> — (recording).md`), + * re-exported so the retention sweep can recognize — and never reap — an + * unfinalized recording (#63). Defined in sanitize.ts so [`sanitizeTitle`] can + * refuse to produce it as a finalized title (#178). */ +export { WORKING_TITLE }; + export interface SessionArchiveWriterOptions { fs: ArchiveFs; /** Destination folder (user data). The writer never writes outside it. */ @@ -28,10 +34,6 @@ export interface SessionArchiveWriterOptions { workingTitle?: string; } -/** Title shown in the in-progress working file's name (`<prefix> — (recording).md`) - * until the session finalizes. Exported so the retention sweep can recognize — - * and never reap — an unfinalized recording (#63). */ -export const WORKING_TITLE = "(recording)"; export class SessionArchiveWriter { private readonly fs: ArchiveFs; diff --git a/packages/archive/test/parse.test.ts b/packages/archive/test/parse.test.ts index 2d8271f..e233c8a 100644 --- a/packages/archive/test/parse.test.ts +++ b/packages/archive/test/parse.test.ts @@ -106,6 +106,42 @@ describe("parseSession — round-trips the real writer output", () => { expect(parsed.entries).toEqual(entries); }); + it("does not misread a source that itself ends with ' (?)' as low-confidence (#178)", () => { + const entries: CaptionEntry[] = [ + // NOT low-confidence, but the caption text ends with the marker glyphs. + { speaker: "them", timestamp: "09:00", source: "is that ok? (?)", target: "괜찮아요?" }, + // Low-confidence AND ends with the marker glyphs. + { speaker: "me", timestamp: "09:01", source: "wait what? (?)", target: "뭐?", lowConfidence: true }, + ]; + const parsed = parseSession(writeSession(META, entries, FINAL)); + // (1) not flagged low-confidence and not truncated — the trailing "?" is a + // look-alike, so the parser doesn't strip it as the marker (pre-fix this + // parsed as lowConfidence=true with the last four chars deleted). + expect(parsed.entries[0]?.lowConfidence).toBeFalsy(); + expect(parsed.entries[0]?.source).toBe("is that ok? (？)"); + // (2) correctly low-confidence, appended marker stripped, the caption's own + // trailing marker preserved (as the look-alike). + expect(parsed.entries[1]?.lowConfidence).toBe(true); + expect(parsed.entries[1]?.source).toBe("wait what? (？)"); + }); + + it("does not fragment a board item that itself contains the ' · ' separator (#178)", () => { + const final: FinalBrief = { + ...FINAL, + board: { + decisions: ["Ship v2 · notify design team"], // ONE decision with a middot + actionItems: [], + openQuestions: [], + }, + }; + const parsed = parseSession(writeSession(META, ENTRIES, final)); + // Stays ONE decision — pre-fix it split into "Ship v2" + "notify design team". + expect(parsed.board.decisions).toHaveLength(1); + // The middot is swapped for a look-alike U+2027 so it can't be mistaken for + // the separator; the item is otherwise intact. + expect(parsed.board.decisions[0]).toBe("Ship v2 ‧ notify design team"); + }); + it("preserves a source line that itself contains an em-dash separator", () => { const entries: CaptionEntry[] = [ { speaker: "them", timestamp: "09:00", source: "it works — mostly — for now", target: "대체로 작동" }, diff --git a/packages/archive/test/sanitize.test.ts b/packages/archive/test/sanitize.test.ts index 08fb2bd..89d35ea 100644 --- a/packages/archive/test/sanitize.test.ts +++ b/packages/archive/test/sanitize.test.ts @@ -1,6 +1,12 @@ import { describe, it, expect } from "vitest"; -import { sanitizeTitle, archiveFileName, MAX_TITLE_BYTES, FALLBACK_TITLE } from "../src/sanitize"; +import { + sanitizeTitle, + archiveFileName, + MAX_TITLE_BYTES, + FALLBACK_TITLE, + WORKING_TITLE, +} from "../src/sanitize"; const utf8 = (s: string) => new TextEncoder().encode(s).length; @@ -75,6 +81,17 @@ describe("sanitizeTitle — path traversal defense (SECURITY.md)", () => { it("preserves ordinary unicode titles (e.g. Korean)", () => { expect(sanitizeTitle("주간 회의 요약")).toBe("주간 회의 요약"); }); + + it("never produces the in-progress sentinel as a finalized title (#178)", () => { + // A final title of "(recording)" would make `<prefix> — (recording).md` + // collide with the working-file grammar and be misread as a crashed orphan. + expect(WORKING_TITLE).toBe("(recording)"); + expect(sanitizeTitle("(recording)")).toBe(FALLBACK_TITLE); + // Whitespace/case that normalizes onto the sentinel is also refused. + expect(sanitizeTitle(" (recording) ")).toBe(FALLBACK_TITLE); + // A title merely CONTAINING the word stays intact (only the exact sentinel is refused). + expect(sanitizeTitle("Notes on (recording) laws")).toBe("Notes on (recording) laws"); + }); }); describe("archiveFileName", () => { diff --git a/packages/engine/src/fallback-router.ts b/packages/engine/src/fallback-router.ts index 7aee618..f1c01ba 100644 --- a/packages/engine/src/fallback-router.ts +++ b/packages/engine/src/fallback-router.ts @@ -70,12 +70,19 @@ export class FallbackRouter implements TranslationEngine { // returned by onUsage() — NOT cleared here. Both engines keep their listeners // across stop(), so accounting (e.g. accountant.attach(router) once) keeps // working after a stop/start cycle (#38). - // Stop both; the fallback may have been started on a switch. - await Promise.all([this.primary.stop(), this.fallback.stop()]); - // Reset routing so the NEXT session begins on the primary again — otherwise - // a Stop/Start after an auto-fallback would route to the stopped fallback. + // Stop both; the fallback may have been started on a switch. `allSettled` so + // ONE engine's stop() rejecting can't skip the state reset below (#178): with + // `Promise.all`, a rejection stranded `usingFallback = true`, and the next + // session's start() would then silently route to the (stopped) fallback tier + // even after credit recovered. Neither shipped engine's stop() rejects today, + // but the TranslationEngine contract does not forbid it. + const results = await Promise.allSettled([this.primary.stop(), this.fallback.stop()]); + // Reset routing UNCONDITIONALLY so the NEXT session begins on the primary + // again, regardless of a partial stop() failure. this.active = this.primary; this.usingFallback = false; + const rejected = results.find((r) => r.status === "rejected"); + if (rejected) throw (rejected as PromiseRejectedResult).reason; } /** Synchronous force-kill of both tiers' OS children (#66 process teardown). */ diff --git a/packages/engine/test/fallback-router.test.ts b/packages/engine/test/fallback-router.test.ts index ed1d4b1..c1c43ca 100644 --- a/packages/engine/test/fallback-router.test.ts +++ b/packages/engine/test/fallback-router.test.ts @@ -132,6 +132,32 @@ describe("FallbackRouter", () => { expect((await collect(router.translate(batch, { pairs: [] }))).at(-1)?.text).toBe("fallback:final"); }); + it("resets routing even when one engine's stop() rejects (#178)", async () => { + // A partial stop() failure must not strand the router on the fallback: the + // NEXT session must still begin on the primary. The error still propagates. + const primary = new StubEngine("primary"); + class FailStopEngine extends StubEngine { + override async stop(): Promise<void> { + throw new Error("fallback stop failed"); + } + } + const fallback = new FailStopEngine("fallback"); + const router = new FallbackRouter({ primary, fallback }); + + await router.start(); + await router.switchToFallback(); + expect(router.onFallback).toBe(true); + + // stop() rejects (fallback.stop threw) — but routing state is reset anyway. + await expect(router.stop()).rejects.toThrow("fallback stop failed"); + expect(router.onFallback).toBe(false); + + // The next session starts on the PRIMARY, not the stopped fallback. + await router.start(); + expect(router.onFallback).toBe(false); + expect((await collect(router.translate(batch, { pairs: [] }))).at(-1)?.text).toBe("primary:final"); + }); + it("begins on the fallback when startOnFallback() is true at launch (restart-while-below)", async () => { const primary = new StubEngine("primary"); const fallback = new StubEngine("fallback"); diff --git a/src-tauri/src/overlay.rs b/src-tauri/src/overlay.rs index 37ddc9c..2f86fbc 100644 --- a/src-tauri/src/overlay.rs +++ b/src-tauri/src/overlay.rs @@ -44,6 +44,11 @@ pub struct Shell { pub click_through: AtomicBool, pub pinned: AtomicBool, drag_generation: AtomicU64, + /// Epoch for the `transitioning` clear-timer (#178): each apply_mode bumps it, + /// and a spawned clear-timer only clears `transitioning` if its captured epoch + /// is still current — so a rapid second apply_mode's timer can't clear the flag + /// mid-way through the first (mirrors `drag_generation`). + transition_generation: AtomicU64, dirty: AtomicBool, transitioning: AtomicBool, ignoring_cursor: AtomicBool, @@ -61,6 +66,7 @@ impl Shell { click_through: AtomicBool::new(click_through), pinned: AtomicBool::new(pinned), drag_generation: AtomicU64::new(0), + transition_generation: AtomicU64::new(0), dirty: AtomicBool::new(false), // Starts true so window-creation Moved/Resized events cannot // record geometry before the launch restore in `apply_mode` @@ -78,6 +84,28 @@ impl Shell { self.dirty.store(true, Ordering::Relaxed); } + /// Begin a mode transition (#178): set `transitioning` and return a fresh + /// epoch. The caller hands this epoch to the clear-timer. + pub fn begin_transition(&self) -> u64 { + let generation = self.transition_generation.fetch_add(1, Ordering::SeqCst) + 1; + self.transitioning.store(true, Ordering::Relaxed); + generation + } + + /// Clear `transitioning` ONLY if `generation` is still the current epoch — + /// so a stale timer from a superseded transition can't reopen geometry + /// recording mid-way through a newer one (#178). + pub fn end_transition_if_current(&self, generation: u64) { + if self.transition_generation.load(Ordering::SeqCst) == generation { + self.transitioning.store(false, Ordering::Relaxed); + } + } + + #[cfg(test)] + fn is_transitioning(&self) -> bool { + self.transitioning.load(Ordering::Relaxed) + } + pub fn save_now(&self) { let snapshot = self.config.lock().expect("config lock").clone(); if let Err(e) = config::save_atomic(&self.config_path, &snapshot) { @@ -212,7 +240,7 @@ pub fn apply_mode(app: &AppHandle, new_mode: Mode) { if shell.mode() != new_mode { record_geometry(&window, shell); } - shell.transitioning.store(true, Ordering::Relaxed); + let generation = shell.begin_transition(); *shell.mode.lock().expect("mode lock") = new_mode; let display = display_key(&window); @@ -257,13 +285,15 @@ pub fn apply_mode(app: &AppHandle, new_mode: Mode) { let _ = window.emit(EVENT_MODE, shell_state(shell)); crate::tray::sync_mode(app, new_mode); - // Let the Moved/Resized events from this transition drain before - // geometry recording resumes. + // Let the Moved/Resized events from this transition drain before geometry + // recording resumes. Guard the clear by epoch (#178): if a second apply_mode + // ran within 250ms, this timer's epoch is stale and it leaves `transitioning` + // set for the newer transition instead of clobbering its geometry. let app2 = app.clone(); std::thread::spawn(move || { std::thread::sleep(Duration::from_millis(250)); let shell = app2.state::<Shell>(); - shell.transitioning.store(false, Ordering::Relaxed); + shell.end_transition_if_current(generation); }); } @@ -491,3 +521,48 @@ pub fn spawn_config_saver(app: AppHandle) { } }); } + +#[cfg(test)] +mod tests { + use super::*; + + fn test_shell() -> Shell { + Shell::new(PathBuf::from("/tmp/livecap-test.json"), ShellConfig::default(), Mode::Panel) + } + + #[test] + fn a_stale_transition_timer_does_not_clear_the_flag() { + // #178: two apply_mode calls within 250ms. The FIRST call's clear-timer + // must NOT clear `transitioning` while the SECOND transition is in flight, + // or record_geometry would persist the in-flight (animated) rect. + let shell = test_shell(); + + let g1 = shell.begin_transition(); + assert!(shell.is_transitioning()); + + // A rapid second transition supersedes the first (new epoch). + let g2 = shell.begin_transition(); + assert_ne!(g1, g2, "each transition gets a fresh epoch"); + assert!(shell.is_transitioning()); + + // The FIRST timer fires late with its now-stale epoch → must be a no-op. + shell.end_transition_if_current(g1); + assert!( + shell.is_transitioning(), + "a superseded transition's timer must not clear the flag mid-transition" + ); + + // The SECOND (current) timer fires → clears cleanly. + shell.end_transition_if_current(g2); + assert!(!shell.is_transitioning()); + } + + #[test] + fn a_single_transition_timer_clears_the_flag() { + let shell = test_shell(); + let g = shell.begin_transition(); + assert!(shell.is_transitioning()); + shell.end_transition_if_current(g); + assert!(!shell.is_transitioning()); + } +}