diff --git a/dikte/app.py b/dikte/app.py index 5bf335b..a840356 100644 --- a/dikte/app.py +++ b/dikte/app.py @@ -118,6 +118,10 @@ class Dikte: def __init__(self, app): self.app = app self.conf = cfg.Config() + # run_app hands the one-instance lock over after construction; a Dikte + # built without one (the tests) restarts without touching a lock. + self.instance_lock = None + self._registry_shortcuts = True # settled by _apply_settings below self.state = IDLE self.ask_state = IDLE # Which of the two the microphone is currently serving, or None. @@ -168,9 +172,16 @@ class Dikte: # Before anything of ours is started: a server from a Dikte that was # killed outright is still holding a model in memory. ggml.sweep() + # The first dictation with no microphone picked would otherwise pay for + # the ffmpeg device listing on the key press itself, seconds of nothing + # happening at the least explicable moment. Warmed the way the models + # are, off the main thread; the listing caches itself. + if sys.platform == "win32" and not self.conf["mic_target"]: + threading.Thread(target=audio.list_sources, daemon=True).start() self.recorder.level.connect(self._on_level) self.recorder.stopped.connect(self._on_recorded) + self.recorder.died.connect(self._on_recorder_died) self.recorder.failed.connect(self._on_recorder_error) self.pipeline.stage.connect(self.overlay.show_busy) self.pipeline.finished.connect(self._on_finished) @@ -192,7 +203,7 @@ class Dikte: self.elapsed = QElapsedTimer() self.meeting_elapsed = QElapsedTimer() - self.last_toggle = QElapsedTimer() + self.last_toggle = {} # action name -> QElapsedTimer, see _repeated self.last_evdev = {} self.ticker = QTimer() self.ticker.setInterval(100) @@ -437,7 +448,10 @@ class Dikte: # Where nothing was installed there is no shortcut to catch up, and # retiring the listener would leave the keys with nowhere to arrive. timer = self.last_evdev.get(name) - if (hotkey.installs_shortcuts() and self.evdev.running + # The snapshot from _apply_settings: which desktop this is cannot + # change under a running process, and asking hotkey again here costs a + # PATH scan on every key press. + if (self._registry_shortcuts and self.evdev.running and timer is not None and timer.elapsed() < ECHO_MS): self._retire_listener() return @@ -506,10 +520,6 @@ class Dikte: def _dictation_request(self, cmd, request, reply): before = self.state - # Only a request that said something about pasting changes it, so that - # the stop half of a `start --paste` does not undo the start half. - if "paste" in request: - self.paste_override[DICTATION] = request["paste"] if cmd == "toggle": self.toggle() elif cmd == "stop": @@ -520,13 +530,20 @@ class Dikte: if seconds > 0 and self.state == RECORDING: run = self._run_id QTimer.singleShot(int(seconds * 1000), lambda: self._auto_stop(run)) + # Armed only when the request actually moved this run along: a request + # that no-opped (the microphone held by the other one, or nothing to + # stop) must not leave a preference behind for some later, unrelated + # run to pick up. A stop that lands keeps changing the run it ends, + # so the stop half of a `start --paste` still does not undo the start. + if "paste" in request and self.state != before: + self.paste_override[DICTATION] = request["paste"] self._answer(DICTATION, before, self.state, request, reply) def _ask_request(self, request, reply): before = self.ask_state - if "paste" in request: - self.paste_override[ASK] = request["paste"] self.toggle_ask() + if "paste" in request and self.ask_state != before: + self.paste_override[ASK] = request["paste"] self._answer(ASK, before, self.ask_state, request, reply) def _meeting_request(self, cmd, request, reply): @@ -591,7 +608,7 @@ class Dikte: def _toggle(self): # Two /dev/input nodes can carry the same keyboard, and a menu click can # land on top of a key press; swallow the immediate repeat. - if self._repeated(): + if self._repeated("toggle"): return if self.state == RECORDING: self.stop() @@ -600,17 +617,23 @@ class Dikte: # a request during its own BUSY is ignored; nothing queues up def _toggle_ask(self): - if self._repeated(): + if self._repeated("ask"): return if self.ask_state == RECORDING: self.stop_ask() elif self.ask_state == IDLE: self.start_ask() - def _repeated(self): - if self.last_toggle.isValid() and self.last_toggle.elapsed() < 400: + def _repeated(self, name): + # Per action, the way last_evdev already is: the window is meant to + # swallow a duplicate delivery of the same press, not a pause landing + # right after the toggle that started the recording. + timer = self.last_toggle.get(name) + if timer is None: + timer = self.last_toggle[name] = QElapsedTimer() + if timer.isValid() and timer.elapsed() < 400: return True - self.last_toggle.restart() + timer.restart() return False def _the_front(self): @@ -628,6 +651,12 @@ class Dikte: self.front_before = self._the_front() self.overlay.show_recording() self._begin_recording(DICTATION) + # A recorder that could not start has already said so, synchronously, + # and the error handler put everything back; setting RECORDING on top + # of that would strand the state machine with no signal ever coming. + # The same guard start_meeting has always had. + if not self.recorder.active: + return self._set_state(RECORDING) def start_ask(self): @@ -636,6 +665,8 @@ class Dikte: self.front_before = self._the_front() self.ask_overlay.show_recording(asking=True) self._begin_recording(ASK) + if not self.recorder.active: + return self._set_ask_state(RECORDING) def _begin_recording(self, owner): @@ -747,7 +778,7 @@ class Dikte: the phone call in the middle of a dictation never reaches the model and the sentence around it is still one sentence. """ - if not self.recording or self._repeated(): + if not self.recording or self._repeated("pause"): return self.paused = not self.paused if self.paused: @@ -779,6 +810,8 @@ class Dikte: self.ticker.stop() self._clear_pause() self.recorder.cancel() + # The preference dies with the run it was given for. + self.paste_override.pop(ASK if asking else DICTATION, None) self.recorder_owner = None # What goes over the socket is read by a program as often as by a # person, so it stays in one language; only what a run itself said @@ -1035,9 +1068,28 @@ class Dikte: def _on_recorder_error(self, message): """The microphone itself could not run, so it belongs to whoever asked.""" owner, self.recorder_owner = self.recorder_owner, None + self.paste_override.pop(owner, None) self.ticker.stop() (self._on_ask_error if owner == ASK else self._on_error)(message) + def _on_recorder_died(self): + """The capture quit under a live recording: keep what it caught. + + Ended the way a key press would end it, so the captured half is + transcribed rather than thrown away, and said out loud, because the + user is still talking at a microphone nobody is reading. + """ + owner = self.recorder_owner + self.tray.showMessage( + "Dikte", + t("The recording stopped on its own; transcribing what was captured."), + QSystemTrayIcon.MessageIcon.Warning, 8000, + ) + if owner == ASK and self.ask_state == RECORDING: + self.stop_ask() + elif self.state == RECORDING: + self.stop() + def _on_error(self, message): self._report(message, self.overlay) self._set_state(IDLE) @@ -1194,10 +1246,13 @@ class Dikte: self._apply_local() self._build_tray() self._refresh_tray() + # Taken once here for _external: the answer cannot change under a + # running process, and re-deriving it there is a PATH scan per press. + self._registry_shortcuts = hotkey.installs_shortcuts() # Where the desktop has no shortcut registry of its own, the listener is # not the fallback the setting offers to turn on: it is the only way the # keys arrive at all, so it runs whatever the setting says. - if self.conf["evdev_hotkey"] or not hotkey.installs_shortcuts(): + if self.conf["evdev_hotkey"] or not self._registry_shortcuts: self.evdev.start({name: self.conf[spec.setting] for name, spec in hotkey.SHORTCUTS.items()}) else: @@ -1219,22 +1274,25 @@ class Dikte: if self.server is not None: self.server.close() QLocalServer.removeServer(SERVER_NAME) - args = ipc.launcher() + ["--gui"] - if sys.platform == "win32": - # execv on Windows mangles arguments with spaces and leaves the two - # processes sharing a console; a detached start does neither. - subprocess.Popen( - args, - creationflags=(subprocess.DETACHED_PROCESS - | subprocess.CREATE_NEW_PROCESS_GROUP), - close_fds=True, - ) - QApplication.instance().quit() - return - os.execv(args[0], args) + # The lock too, or the replacement would take this restart for a + # double start and hand the attention back to a process on its way out. + if self.instance_lock is not None: + self.instance_lock.unlock() + ipc.respawn(["--gui"]) + # respawn only returns on Windows, where the replacement was started + # detached and this process still has to leave on its own. + QApplication.instance().quit() def shutdown(self): self._quitting = True + # Waiters first, while the connections still work: a `--wait` left + # unanswered reads to the terminal as an instance too old to answer, + # which points the user at a version problem that does not exist. + # In one language, like every other error that goes over the socket: + # a script reads these as often as a person does. + for kind in (DICTATION, ASK, MEETING): + self._settle(kind, {"ok": False, + "error": "the instance is shutting down"}) self.evdev.stop() if self.recording: self.recorder.cancel() @@ -1357,9 +1415,44 @@ def _stay_out_of_the_dock(): pass +def _hand_over(command): + """Give the running instance the attention this start was asking for. + + A start carrying a verb forwards only that verb; a bare double start asks + for the Settings window as the sign of life the click was looking for. + Retried for a moment, because the copy that won the lock may not be + listening yet. + """ + verb = command or "settings" + deadline = time.monotonic() + 5 + while time.monotonic() < deadline: + if ipc.send(verb) is not None: + return + time.sleep(0.2) + print("dikte: another copy holds the lock but never answered") + + def run_app(args): command = args[0] if args else "" + # One Dikte per user. The lock closes the simultaneous-start window two + # probes would both fall through; the probe still runs behind it, because + # an instance from before the lock existed holds only the socket. Both + # sit before the QApplication, so a second copy costs a moment and not a + # second tray icon. The lock lives in this frame, which app.exec() below + # keeps alive for exactly the process's lifetime. + lock = ipc.instance_lock() + if lock is not None and not lock.tryLock(0): + _hand_over(command) + return 0 + if ipc.already_serving(): + print("dikte: already running; handing it the attention") + if command: + ipc.send(command) + else: + ipc.send("settings") + return 0 + app = QApplication(sys.argv) app.setApplicationName("Dikte") app.setDesktopFileName("dikte") @@ -1387,6 +1480,9 @@ def run_app(args): print("dikte: no system tray found, running anyway") dikte = Dikte(app) + # Handed over so that restart() can let go of it before the replacement + # tries to take it. + dikte.instance_lock = lock server = QLocalServer() # Qt puts the socket in /tmp, so keep it to this user: commands like diff --git a/dikte/assistant.py b/dikte/assistant.py index b57fd06..21ff241 100644 --- a/dikte/assistant.py +++ b/dikte/assistant.py @@ -24,13 +24,17 @@ while they work. import json import os +import re import shutil +import signal import subprocess +import tempfile import threading import time from . import api from . import config as cfg +from . import paths from .i18n import t SESSION_FILE = cfg.DATA_DIR / "assistant.json" @@ -114,13 +118,19 @@ def display_name(conf): # one along costs tokens and invites an answer to the wrong question. Switching # provider drops it too, since none of them can pick up another's thread. -def _read_row(name, max_age_seconds): +def _read_session(): + """The stored conversation row, or {} however the file fails to read.""" try: with open(SESSION_FILE, encoding="utf-8") as fh: row = json.load(fh) except (OSError, json.JSONDecodeError, ValueError): return {} - if not isinstance(row, dict) or row.get("provider") != name: + return row if isinstance(row, dict) else {} + + +def _read_row(name, max_age_seconds): + row = _read_session() + if row.get("provider") != name: return {} if max_age_seconds and time.time() - row.get("ts", 0) > max_age_seconds: return {} @@ -159,22 +169,13 @@ def clear_session(): def stored_provider(): """Whose conversation is on disk, whatever the setting says now.""" - try: - with open(SESSION_FILE, encoding="utf-8") as fh: - row = json.load(fh) - except (OSError, json.JSONDecodeError, ValueError): - return "" - return str(row.get("provider", "")) if isinstance(row, dict) else "" + return str(_read_session().get("provider", "")) def session_age(): """Seconds since the stored conversation was last used, or None.""" - try: - with open(SESSION_FILE, encoding="utf-8") as fh: - row = json.load(fh) - except (OSError, json.JSONDecodeError, ValueError): - return None - if not isinstance(row, dict) or not (row.get("session") or row.get("messages")): + row = _read_session() + if not (row.get("session") or row.get("messages")): return None return time.time() - row.get("ts", 0) @@ -373,14 +374,24 @@ def _stream(cmd, conf, on_event, should_stop): Returns (exit code, stderr). Raises Cancelled when the stop was asked for, and AssistantError when the clock ran out. """ + # stderr lands in a file rather than a pipe: nobody drains it while stdout + # is being read, and a CLI chatty enough on stderr would fill the pipe's + # buffer and wedge both of us. A file has no such limit, and is read once + # at the end, which is the only moment stderr matters. + stderr_file = tempfile.TemporaryFile() + # On POSIX the run gets its own session, so that ending it can take down + # every subprocess it started, not just the CLI itself. + grouped = {"start_new_session": True} if os.name == "posix" else {} try: proc = subprocess.Popen( cmd, cwd=working_dir(conf), stdin=subprocess.DEVNULL, - stdout=subprocess.PIPE, stderr=subprocess.PIPE, + stdout=subprocess.PIPE, stderr=stderr_file, text=True, encoding="utf-8", errors="replace", bufsize=1, - creationflags=getattr(subprocess, "CREATE_NO_WINDOW", 0), + creationflags=paths.NO_WINDOW, + **grouped, ) except OSError as exc: + stderr_file.close() raise AssistantError(t("Could not run {binary}: {error}", binary=cmd[0], error=exc)) from exc @@ -409,7 +420,7 @@ def _stream(cmd, conf, on_event, should_stop): if isinstance(event, dict): on_event(event) finally: - stderr = _finish(proc) + stderr = _finish(proc, stderr_file) watchdog.join(timeout=1) if ended["cancelled"]: @@ -420,12 +431,31 @@ def _stream(cmd, conf, on_event, should_stop): return proc.returncode, stderr +# Failures that a fresh session cannot cure: an exhausted quota, a signed-out +# CLI, a network that is down. A resumed run that dies with one of these is +# reported as what it is, not retried without the session, because the retry +# would fail the same way after making the user wait through a second run. +_API_TROUBLE = re.compile( + r"(?i)rate.?limit|quota|overloaded|too many requests|credit|billing|" + r"insufficient|unauthorized|forbidden|authentication|invalid.{0,8}key|" + r"log ?in|logged.?out|network|connection|ECONN|ENOTFOUND|ETIMEDOUT|" + r"\b(401|403|429|5\d\d)\b") + + def _conclude(found, code, stderr, session, service): """Turn what the stream said into an answer, or into the reason there is none.""" if code != 0 and not found["answer"]: - if session and _session_missing(stderr): + # A resumed run that died with nothing to show is treated as the + # session being gone, whatever the wording: this code used to look for + # "session ... not found" in stderr, but a CLI update or another + # language rewords that and the recovery stops working. Retrying costs + # one clean start, and cannot loop because the retry resumes nothing. + # Recognised API trouble is the exception: it is not the session's + # fault, and the retry would only repeat it. + blame = last_line(stderr) or found["failure"] or "" + if session and not _API_TROUBLE.search(blame): raise _SessionGone() - raise AssistantError(last_line(stderr) or found["failure"] or t( + raise AssistantError(blame or t( "{service} exited with code {code}.", service=service, code=code)) if found["failure"] and not found["answer"]: raise AssistantError(found["failure"]) @@ -436,14 +466,6 @@ def _conclude(found, code, stderr, session, service): return found["answer"], found["warning"] -def _session_missing(stderr): - lowered = (stderr or "").lower() - if "session" in lowered or "thread" in lowered or "conversation" in lowered: - return any(word in lowered for word in ("not found", "no such", "unknown", - "does not exist", "no conversation")) - return False - - def _watch(proc, deadline, should_stop, ended): while proc.poll() is None: if should_stop is not None and should_stop(): @@ -454,29 +476,60 @@ def _watch(proc, deadline, should_stop, ended): break time.sleep(0.25) if ended["cancelled"] or ended["timed_out"]: - _kill(proc) + kill_tree(proc) -def _kill(proc): +def kill_tree(proc): + """End the process and everything it started. + + A CLI runs tools as subprocesses of its own, and ending only the CLI would + leave those behind, still working on a question nobody is waiting for. + Shared with cleanup, which runs the same two programs. Every failure here + is swallowed: the process being already gone is the outcome being asked for. + """ + if os.name == "nt": + # There is no process group to signal on Windows; taskkill walks the + # tree instead. The wait after it is best-effort, so a tree that will + # not die does not hang the caller on top of everything else. + subprocess.run( + ["taskkill", "/T", "/F", "/PID", str(proc.pid)], + capture_output=True, + creationflags=paths.NO_WINDOW, + ) + try: + proc.wait(timeout=3) + except (subprocess.TimeoutExpired, OSError): + pass + return + # The Popen was started with start_new_session=True, so the pid names a + # whole session to signal. SIGTERM first for a clean exit, SIGKILL for a + # tree that ignored it. + try: + os.killpg(proc.pid, signal.SIGTERM) + except (ProcessLookupError, PermissionError, OSError): + return try: - proc.terminate() proc.wait(timeout=3) except subprocess.TimeoutExpired: - proc.kill() - except OSError: - pass + try: + os.killpg(proc.pid, signal.SIGKILL) + except (ProcessLookupError, PermissionError, OSError): + pass -def _finish(proc): - try: - stderr = proc.stderr.read() or "" - except (OSError, ValueError): - stderr = "" +def _finish(proc, stderr_file): try: proc.wait(timeout=5) except subprocess.TimeoutExpired: - proc.kill() - for stream in (proc.stdout, proc.stderr): + kill_tree(proc) + # Read back what the CLI wrote to its stderr file, decoded leniently: a + # dying CLI is exactly the one likely to print something half-encoded. + try: + stderr_file.seek(0) + stderr = stderr_file.read().decode("utf-8", "replace") + except (OSError, ValueError): + stderr = "" + for stream in (proc.stdout, stderr_file): try: stream.close() except OSError: diff --git a/dikte/audio.py b/dikte/audio.py index ef24461..6736045 100644 --- a/dikte/audio.py +++ b/dikte/audio.py @@ -31,11 +31,21 @@ import wave from PyQt6.QtCore import QObject, pyqtSignal +from . import paths from .i18n import t -# Console programs started from a windowless process would otherwise each open -# a console window of their own on Windows. -NO_WINDOW = getattr(subprocess, "CREATE_NO_WINDOW", 0) if sys.platform == "win32" else 0 +# Squaring a chunk sample by sample in Python is the most expensive thing the +# level meter does, and it does it for every chunk of every recording. sumprod +# stays in C for the whole sum; it arrived in 3.12 and the floor here is 3.11, +# so the plain loop remains as the fallback. Both produce the same integer. +try: + from math import sumprod +except ImportError: + sumprod = None + +# See paths.NO_WINDOW; re-exported here because this module's callers and +# tests have always read it under this name. +NO_WINDOW = paths.NO_WINDOW RATE = 16000 CHANNELS = 1 @@ -79,12 +89,15 @@ class Recorder(QObject): level = pyqtSignal(float) # 0.0 - 1.0, for the waveform stopped = pyqtSignal(str, float, object) # wav path, duration (s), per-chunk RMS + died = pyqtSignal() # the capture quit mid-recording failed = pyqtSignal(str) def __init__(self, parent=None): super().__init__(parent) self._proc = None self._thread = None + self._log = None + self._run = None self._buffer = bytearray() self._rms = [] self._cancelled = False @@ -126,12 +139,17 @@ class Recorder(QObject): self.failed.emit(t(sound().missing)) return + # The recorder keeps talking to stderr for as long as it runs; a pipe + # nobody drains would eventually block it, so it writes to a file. + self._drop_log() + self._log = tempfile.TemporaryFile() try: self._proc = subprocess.Popen( - cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE, bufsize=0, + cmd, stdout=subprocess.PIPE, stderr=self._log, bufsize=0, creationflags=NO_WINDOW, ) except OSError as exc: + self._drop_log() self.failed.emit(t("Could not start recording: {error}", error=exc)) return @@ -141,15 +159,21 @@ class Recorder(QObject): self._stopping = False self._paused = False self._max_bytes = int(max_seconds * RATE * SAMPLE_WIDTH * CHANNELS) - self._thread = threading.Thread(target=self._pump, daemon=True) + # The pump is handed this run's objects rather than reading them off + # self, and a token to say whose run it still is: a pump that outlives + # its 2 s join must not touch the recording that comes after it. + self._run = object() + self._thread = threading.Thread( + target=self._pump, daemon=True, + args=(self._run, self._proc, self._proc.stdout, + self._buffer, self._rms, self._max_bytes), + ) self._thread.start() - def _pump(self): - proc = self._proc - stdout = proc.stdout + def _pump(self, run, proc, stdout, buffer, rms, max_bytes): try: while True: - chunk = stdout.read(CHUNK_BYTES) + chunk = _read_exact(stdout, CHUNK_BYTES) if not chunk: break if self._paused: @@ -157,33 +181,58 @@ class Recorder(QObject): # nobody empties fills up, and the capture program blocks on # a full one instead of waiting quietly for the resume. continue - peak, rms = chunk_levels(chunk) + peak, chunk_rms = chunk_levels(chunk) with self._lock: - self._buffer.extend(chunk) - self._rms.append(rms) - too_long = len(self._buffer) >= self._max_bytes + buffer.extend(chunk) + rms.append(chunk_rms) + too_long = len(buffer) >= max_bytes + if self._run is not run: + # This recording was given up on; whatever happens now + # belongs to the run that replaced it, not to this one. + return self.level.emit(peak) if too_long: self._terminate() break except (OSError, ValueError): pass + if self._run is not run: + return + if self._stopping or self._cancelled: + return + with self._lock: + captured = bool(buffer) + if captured: + # Sound had already arrived and nobody asked it to end: the device + # went away, or the recorder fell over mid-dictation. That has to + # be said while there is still something worth keeping. + self.died.emit() + return # Nobody asked it to end and it captured nothing: the recorder is not # installed properly, or the device was refused. Said out loud here, # because stop() would otherwise report it as a recording that was too # short, which sends the user looking in the wrong place. - with self._lock: - captured = bool(self._buffer) - if self._stopping or self._cancelled or captured: - return - try: - detail = proc.stderr.read().decode("utf-8", "replace").strip() - except (AttributeError, OSError): - detail = "" - self.failed.emit(t( - "Audio recorder stopped before receiving sound: {error}", - error=detail or f"exit code {proc.returncode}", - )) + detail = self._error_tail() + # poll() first, because returncode stays None until somebody reaps the + # process, and "exit code None" answers nothing. + code = proc.poll() + if not detail and code is not None: + detail = f"exit code {code}" + if detail: + self.failed.emit(t( + "Audio recorder stopped before receiving sound: {error}", + error=detail, + )) + else: + self.failed.emit(t("Audio recorder stopped before receiving sound")) + + def _error_tail(self): + log = self._log + return _last_log_line(log) if log is not None else "" + + def _drop_log(self): + log, self._log = self._log, None + _close_log(log) def _terminate(self): self._stopping = True @@ -195,7 +244,10 @@ class Recorder(QObject): except (subprocess.TimeoutExpired, OSError): try: proc.kill() - except OSError: + # Reaped even after a kill, or the child stays a zombie + # holding its slot in the process table. + proc.wait(timeout=1) + except (subprocess.TimeoutExpired, OSError): pass def cancel(self): @@ -205,6 +257,8 @@ class Recorder(QObject): self._thread.join(timeout=2) self._thread = None self._proc = None + self._run = None + self._drop_log() with self._lock: self._buffer = bytearray() @@ -217,7 +271,11 @@ class Recorder(QObject): self._thread.join(timeout=2) self._thread = None self._proc = None + self._run = None + self._drop_log() + # The same buffer object the pump was handed, harvested under the same + # lock it appends with. with self._lock: pcm = bytes(self._buffer) rms = list(self._rms) @@ -231,7 +289,13 @@ class Recorder(QObject): self.failed.emit(t("Recording too short, speak for at least 0.3 s")) return - path = write_wav(pcm) + try: + path = write_wav(pcm) + except (OSError, wave.Error) as exc: + # A full disk or an unwritable temp directory costs this recording + # either way; a message beats a traceback in the journal. + self.failed.emit(t("Could not write the recording: {error}", error=exc)) + return self.stopped.emit(path, frames / RATE, rms) @@ -284,6 +348,7 @@ class MeetingRecorder(QObject): def __init__(self, parent=None): super().__init__(parent) self._procs = [] + self._interrupted = set() self._thread = None self._wav = None self._logs = [] @@ -336,6 +401,7 @@ class MeetingRecorder(QObject): # nobody drains would eventually block it, so it writes to a file. self._logs = [tempfile.TemporaryFile() for _ in commands] self._procs = [] + self._interrupted = set() for command, log in zip(commands, self._logs): self._procs.append(subprocess.Popen( command, stdout=subprocess.PIPE, stderr=log, bufsize=0, @@ -440,14 +506,20 @@ class MeetingRecorder(QObject): try: _interrupt(proc) except OSError: - pass + continue + # ffmpeg reports being interrupted as a failure; stop() needs to + # know which exits were our own doing and which were real deaths. + self._interrupted.add(proc) for proc in running: try: proc.wait(timeout=2) except (subprocess.TimeoutExpired, OSError): try: proc.kill() - except OSError: + # Reaped even after a kill, or the child stays a zombie + # holding its slot in the process table. + proc.wait(timeout=1) + except (subprocess.TimeoutExpired, OSError): pass def _close_file(self): @@ -460,24 +532,23 @@ class MeetingRecorder(QObject): pass def _error_tail(self): - tails = [] - for log in self._logs: - try: - log.seek(0) - text = log.read().decode("utf-8", "replace").strip() - except OSError: - continue - lines = [line for line in text.splitlines() if line.strip()] - if lines: - tails.append(lines[-1]) - return " | ".join(tails) + tails = [_last_log_line(log) for log in self._logs] + return " | ".join(tail for tail in tails if tail) def _finish_process(self): self._terminate() if self._thread: self._thread.join(timeout=3) self._thread = None - codes = [proc.poll() for proc in self._procs] + codes = [] + for proc in self._procs: + code = proc.poll() + # A nonzero exit from a process we interrupted ourselves is ffmpeg + # complaining about our own stop; one that had already died on its + # own keeps its code, because that one is the story. + if code and proc in self._interrupted: + code = 0 + codes.append(code) code = next((value for value in codes if value), 0) self._procs = [] self._close_file() @@ -534,13 +605,30 @@ class MeetingRecorder(QObject): def _drop_log(self): for log in self._logs: - try: - log.close() - except OSError: - pass + _close_log(log) self._logs = [] +def _last_log_line(log): + """The last thing a recorder said before it ended, or ''.""" + try: + log.seek(0) + text = log.read().decode("utf-8", "replace").strip() + except (OSError, ValueError): + return "" + lines = [line for line in text.splitlines() if line.strip()] + return lines[-1] if lines else "" + + +def _close_log(log): + if log is None: + return + try: + log.close() + except OSError: + pass + + def chunk_levels(chunk): """(peak, rms) in 0..1. Peak drives the waveform, RMS drives the silence check.""" samples = array.array("h") @@ -549,7 +637,9 @@ def chunk_levels(chunk): return 0.0, 0.0 samples.frombytes(chunk[:usable]) peak = max(abs(min(samples)), abs(max(samples))) / 32768.0 - rms = math.sqrt(sum(s * s for s in samples) / len(samples)) / 32768.0 + power = (sumprod(samples, samples) if sumprod is not None + else sum(s * s for s in samples)) + rms = math.sqrt(power / len(samples)) / 32768.0 return min(1.0, peak), min(1.0, rms) @@ -638,6 +728,13 @@ MERGE_FILTER = ( ) +# Whether pw-record takes --raw, asked of the binary once per process: the +# probe costs a subprocess, and the answer cannot change under a running +# application. Kept here rather than inside _pw_record_raw_option so the probe +# itself stays testable against different binaries. +_PW_RAW = None + + def _pulse_record(target): """parec, or pw-record where PulseAudio's tools were left out. @@ -645,6 +742,7 @@ def _pulse_record(target): service, and its source names are the same ones shown by list_sources(). Keep pw-record as the fallback for minimal native-PipeWire installations. """ + global _PW_RAW if shutil.which("parec"): cmd = [ "parec", "--record", "--raw", f"--rate={RATE}", @@ -660,8 +758,10 @@ def _pulse_record(target): cmd.append(f"--device={target}") return cmd if shutil.which("pw-record"): + if _PW_RAW is None: + _PW_RAW = _pw_record_raw_option() cmd = [ - "pw-record", *_pw_record_raw_option(), f"--rate={RATE}", + "pw-record", *_PW_RAW, f"--rate={RATE}", f"--channels={CHANNELS}", "--format=s16", ] if target: @@ -682,8 +782,11 @@ def _pw_record_raw_option(): that line, so ask the installed binary which form it understands. """ try: + # utf-8 spelled out: subprocess otherwise decodes with the locale's + # codec, and help text through a codec it was not written in raises. result = subprocess.run( - ["pw-record", "--help"], capture_output=True, text=True, timeout=2 + ["pw-record", "--help"], capture_output=True, text=True, + encoding="utf-8", errors="replace", timeout=2, ) help_text = (result.stdout or "") + (result.stderr or "") except (subprocess.SubprocessError, OSError): @@ -708,9 +811,12 @@ def _pactl_sources(): if not shutil.which("pactl"): return [] try: + # utf-8 spelled out: device descriptions carry whatever alphabet the + # machine speaks, and the locale's codec is not always able to say so. out = subprocess.run( ["pactl", "-f", "json", "list", "sources"], - capture_output=True, text=True, timeout=5, check=True, + capture_output=True, text=True, encoding="utf-8", errors="replace", + timeout=5, check=True, ).stdout return json.loads(out) except (subprocess.SubprocessError, OSError, json.JSONDecodeError): @@ -739,7 +845,8 @@ def _pulse_default_output(): try: sink = subprocess.run( ["pactl", "get-default-sink"], - capture_output=True, text=True, timeout=5, check=True, + capture_output=True, text=True, encoding="utf-8", errors="replace", + timeout=5, check=True, ).stdout.strip() except (subprocess.SubprocessError, OSError): return "" diff --git a/dikte/cleanup.py b/dikte/cleanup.py index 96eab1d..14e44df 100644 --- a/dikte/cleanup.py +++ b/dikte/cleanup.py @@ -21,6 +21,7 @@ import tempfile from . import api from . import assistant from . import ggml +from . import paths from .i18n import t PROVIDERS = ("openrouter", "local", "claude", "codex") @@ -194,21 +195,43 @@ def _output(cmd, timeout, service): "{binary} not found. Install it, or have OpenRouter clean up " "instead, under Settings → API and models.", binary=binary, )) + # Both streams land in files rather than pipes: nobody drains a pipe while + # the process is being waited out, and a CLI chatty enough would fill the + # buffer and wedge. And a timeout must end the CLI's tool subprocesses too, + # not just the CLI, which subprocess.run's timeout does not do; hence the + # own session on POSIX and assistant.kill_tree on the way out. + out_file = tempfile.TemporaryFile() + err_file = tempfile.TemporaryFile() + grouped = {"start_new_session": True} if os.name == "posix" else {} try: - done = subprocess.run( - cmd, cwd=os.path.expanduser("~"), stdin=subprocess.DEVNULL, - capture_output=True, text=True, encoding="utf-8", errors="replace", - timeout=timeout, - creationflags=getattr(subprocess, "CREATE_NO_WINDOW", 0), - ) - except subprocess.TimeoutExpired: - raise CleanupError(t("{service} did not finish within {seconds} seconds.", - service=service, seconds=timeout)) from None - except OSError as exc: - raise CleanupError(t("Could not run {binary}: {error}", - binary=binary, error=exc)) from exc - if done.returncode != 0: - raise CleanupError(assistant.last_line(done.stderr) or t( + try: + proc = subprocess.Popen( + cmd, cwd=os.path.expanduser("~"), stdin=subprocess.DEVNULL, + stdout=out_file, stderr=err_file, + creationflags=paths.NO_WINDOW, + **grouped, + ) + except OSError as exc: + raise CleanupError(t("Could not run {binary}: {error}", + binary=binary, error=exc)) from exc + try: + proc.wait(timeout=timeout) + except subprocess.TimeoutExpired: + assistant.kill_tree(proc) + raise CleanupError(t("{service} did not finish within {seconds} seconds.", + service=service, seconds=timeout)) from None + out_file.seek(0) + stdout = out_file.read().decode("utf-8", "replace") + err_file.seek(0) + stderr = err_file.read().decode("utf-8", "replace") + finally: + for handle in (out_file, err_file): + try: + handle.close() + except OSError: + pass + if proc.returncode != 0: + raise CleanupError(assistant.last_line(stderr) or t( "{service} exited with code {code}.", - service=service, code=done.returncode)) - return (done.stdout or "").strip() + service=service, code=proc.returncode)) + return stdout.strip() diff --git a/dikte/cli.py b/dikte/cli.py index 3109348..e9b0c45 100644 --- a/dikte/cli.py +++ b/dikte/cli.py @@ -136,21 +136,10 @@ def _ask_instance(opts, cmd, wait=False, **args): def launch_gui(verb=""): """No instance running, so become the application itself.""" - args = ipc.launcher() - if verb: - args.append(verb) - args.append("--gui") - if sys.platform == "win32": - # execv on Windows mangles arguments with spaces and would leave the - # application tied to this console; start it detached instead. - subprocess.Popen( - args, - creationflags=(subprocess.DETACHED_PROCESS - | subprocess.CREATE_NEW_PROCESS_GROUP), - close_fds=True, - ) - sys.exit(0) - os.execv(args[0], args) + ipc.respawn(([verb] if verb else []) + ["--gui"]) + # respawn only returns on Windows, where the application was started + # detached and this console process's job is over. + sys.exit(0) def _not_running(opts): @@ -519,7 +508,7 @@ def cmd_history_clear(opts): # --- settings --------------------------------------------------------------- -SECRET_KEYS = ("openai_api_key", "openrouter_api_key") +SECRET_KEYS = ("openai_api_key", "groq_api_key", "openrouter_api_key") def _mask(key, value): @@ -869,27 +858,51 @@ def cmd_doctor(opts): programs = {name: shutil.which(name) or "" for name in wanted if name} target = conf.transcribe_target() cleaner = cleanup.provider(conf) + # What each provider actually needs: the local ones have no key to check, + # and marking them by the key they do not use reported every fully local + # setup as broken. + transcribe_ready = conf.transcribe_ready() + if cleaner == "openrouter": + cleanup_ready = bool(conf.openrouter_key()) + elif cleaner == "local": + cleanup_ready = conf.local_llm_ready() + else: + cleanup_ready = bool(programs.get(cleanup.executable(cleaner), "")) checks = { "programs": programs, "transcription": {"provider": target.provider, "model": target.model, - "key": bool(target.api_key)}, + "key": bool(target.api_key), + "ready": transcribe_ready}, "cleanup": {"enabled": conf["cleanup_enabled"], "provider": cleaner, "model": cleanup.model(conf), - "key": bool(conf.openrouter_key())}, + "key": bool(conf.openrouter_key()), + "ready": cleanup_ready}, "agent": {"provider": assistant.provider(conf), "directory": assistant.working_dir(conf)}, "running": ipc.send("status") is not None, } + if target.provider == "local": + transcribe_line = (f"{'✓' if transcribe_ready else '✗'} {target.service}, " + f"transcribing on {target.model or 'no model yet'}") + else: + transcribe_line = (f"{'✓' if transcribe_ready else '✗'} {target.service} " + f"key, transcribing on {target.model}") + if cleaner == "openrouter": + cleanup_line = (f"{'✓' if cleanup_ready else '✗'} OpenRouter key, " + f"cleaning up on {conf['cleanup_model']}") + elif cleaner == "local": + cleanup_line = (f"{'✓' if cleanup_ready else '✗'} Local model, " + f"cleaning up on {conf['local_llm_model'] or 'no model yet'}") + else: + # Cleanup on a CLI needs no key, so what is checked is the program. + cleanup_line = (f"{'✓' if cleanup_ready else '✗'} " + f"{cleanup.executable(cleaner)}, cleaning up on " + f"{cleanup.model(conf)}") lines = [f"{'✓' if path else '✗'} {name:14} {path or 'not on your PATH'}" for name, path in programs.items()] lines += [ - f"{'✓' if target.api_key else '✗'} {target.service} key, transcribing on " - f"{target.model}", - # Cleanup on a CLI needs no key, so what is checked is the program. - (f"{'✓' if conf.openrouter_key() else '✗'} OpenRouter key, cleaning up on " - f"{conf['cleanup_model']}") if cleaner == "openrouter" else - (f"{'✓' if programs[cleanup.executable(cleaner)] else '✗'} " - f"{cleanup.executable(cleaner)}, cleaning up on {cleanup.model(conf)}"), + transcribe_line, + cleanup_line, f"{'✓' if checks['running'] else '·'} application " + ("running" if checks["running"] else "not running"), ] @@ -1012,13 +1025,14 @@ def build_parser(): transcribe.set_defaults(func=cmd_transcribe) # --- meetings --------------------------------------------------------- - for name, help_text in (("meeting", "start a meeting, or end it and write it up"), - ("meeting-cancel", "")): - page = leaf(subs, name, help_text) - page.add_argument("--wait", action="store_true", - help="wait for the minutes to be written") - page.add_argument("--timeout", type=float, default=0) - page.set_defaults(func=cmd_meeting) + page = leaf(subs, "meeting", "start a meeting, or end it and write it up") + page.add_argument("--wait", action="store_true", + help="wait for the minutes to be written") + page.add_argument("--timeout", type=float, default=0) + page.set_defaults(func=cmd_meeting) + # No --wait here: a cancel is answered on the spot, and a flag the server + # would ignore is a promise the help text cannot keep. + leaf(subs, "meeting-cancel", "").set_defaults(func=cmd_meeting) meetings = leaf(subs, "meetings", "recorded meetings and their minutes") inner = meetings.add_subparsers(dest="meetings", metavar="") @@ -1037,12 +1051,14 @@ def build_parser(): delete.add_argument("which", nargs="+") delete.set_defaults(func=cmd_meetings_delete) for name, verb, help_text in (("start", "meeting-start", "start recording one"), - ("stop", "meeting-stop", "end it and write it up"), - ("cancel", "meeting-cancel", "throw the recording away")): + ("stop", "meeting-stop", "end it and write it up")): page = leaf(inner, name, help_text) page.add_argument("--wait", action="store_true") page.add_argument("--timeout", type=float, default=0) page.set_defaults(func=cmd_meeting, verb=verb) + # cancel takes no --wait: see the top-level meeting-cancel. + leaf(inner, "cancel", "throw the recording away").set_defaults( + func=cmd_meeting, verb="meeting-cancel") # --- history ---------------------------------------------------------- history = leaf(subs, "history", "past dictations") @@ -1153,6 +1169,16 @@ def _needs_subcommand(parser): def run(argv): global _app + # A redirected stdout on Windows falls back to the console codepage, + # strict, and a transcript (or doctor's ✓) with a character outside it + # would then fail the run after the work succeeded. Interactively nothing + # changes: the console is written through its own Unicode API. + if sys.platform == "win32" and not os.environ.get("PYTHONIOENCODING"): + for stream in (sys.stdout, sys.stderr): + try: + stream.reconfigure(errors="replace") + except (AttributeError, OSError): + pass parser = build_parser() opts = parser.parse_args(argv) # No verb at all is the plain `dikte`, which means the settings window. diff --git a/dikte/config.py b/dikte/config.py index fb7683a..3b904c1 100644 --- a/dikte/config.py +++ b/dikte/config.py @@ -5,6 +5,8 @@ import hashlib import json import os import sys +import threading +import time from . import api from . import ggml @@ -528,6 +530,30 @@ TRANSCRIBERS = { "openrouter_base_url", "openrouter_transcribe_model"), } +# One lock for the history file and the meeting index both, rather than one +# each: the files are a few kilobytes, the writes happen a handful of times an +# hour, and a second lock would only add a way to take them in the wrong order. +_FILES_LOCK = threading.Lock() + + +def _replace_with_retry(tmp, target): + """The atomic swap, tried again briefly when the target is held. + + On Windows an antivirus or sync tool opens a freshly written file to look + at it, and a rename over the file fails for as long as it is held. The + hold lasts milliseconds, so three tries with a short sleep cover it; a + file held longer than that is a real error and is raised as one. + """ + for attempt in range(3): + try: + tmp.replace(target) + return + except OSError: + if attempt == 2: + raise + time.sleep(0.05) + + # Corners used to be stored with Turkish names. _CORNER_MIGRATION = { "sol-alt": "bottom-left", "sağ-alt": "bottom-right", @@ -548,7 +574,19 @@ class Config: self.data.update({k: v for k, v in stored.items() if k in DEFAULTS}) except FileNotFoundError: pass - except (json.JSONDecodeError, OSError) as exc: + except json.JSONDecodeError as exc: + # Set aside rather than left in place: the next save would write + # the defaults over it, and whatever broke the file deserves to + # still be there to look at. Best effort; a rename that fails + # changes nothing about falling back to the defaults. + broken = CONFIG_FILE.with_suffix(".json.broken") + try: + CONFIG_FILE.replace(broken) + except OSError: + pass + print(f"dikte: could not read settings ({exc}), using defaults; " + f"the unreadable file was kept as {broken}") + except OSError as exc: print(f"dikte: could not read settings ({exc}), using defaults") self.data["overlay_corner"] = _CORNER_MIGRATION.get( self.data["overlay_corner"], self.data["overlay_corner"] @@ -563,8 +601,13 @@ class Config: tmp = CONFIG_FILE.with_suffix(".json.tmp") with open(tmp, "w", encoding="utf-8") as fh: json.dump(self.data, fh, ensure_ascii=False, indent=2) + # Pushed to the disk before the rename: swapping in a file that + # still lives in the page cache turns a power cut into a settings + # wipe, which the atomic replace exists to prevent. + fh.flush() + os.fsync(fh.fileno()) os.chmod(tmp, 0o600) - tmp.replace(CONFIG_FILE) + _replace_with_retry(tmp, CONFIG_FILE) i18n.set_language(self.data["ui_language"]) def __getitem__(self, key): @@ -730,12 +773,22 @@ def default_assistant_prompt(): def append_history(entry): DATA_DIR.mkdir(parents=True, exist_ok=True) - with open(HISTORY_FILE, "a", encoding="utf-8") as fh: - fh.write(json.dumps(entry, ensure_ascii=False) + "\n") + with _FILES_LOCK: + with open(HISTORY_FILE, "a", encoding="utf-8") as fh: + fh.write(json.dumps(entry, ensure_ascii=False) + "\n") def read_history(limit=None): """Newest last. A limit of None (or 0) reads the whole file.""" + # Locked even though the rewrites are atomic: it costs nothing, and a read + # that waits out a rewrite in flight hands back the settled file rather + # than whichever side of the swap it happened to land on. + with _FILES_LOCK: + return _read_history(limit) + + +def _read_history(limit=None): + """The body of read_history, for callers already holding the lock.""" try: with open(HISTORY_FILE, encoding="utf-8") as fh: lines = fh.readlines() @@ -758,40 +811,66 @@ def _write_history(lines): tmp = HISTORY_FILE.with_suffix(".jsonl.tmp") with open(tmp, "w", encoding="utf-8") as fh: fh.writelines(lines) - tmp.replace(HISTORY_FILE) + fh.flush() + os.fsync(fh.fileno()) + _replace_with_retry(tmp, HISTORY_FILE) def trim_history(limit): """Drop the oldest entries once the file passes `limit` rows. 0 means keep all.""" if not limit or limit < 0: return - try: - with open(HISTORY_FILE, encoding="utf-8") as fh: - lines = fh.readlines() - except OSError: - return - if len(lines) <= limit: - return - _write_history(lines[-limit:]) + # Read and rewrite under one lock, so a dictation appended in between the + # two is not erased by a rewrite that never saw it. + with _FILES_LOCK: + try: + with open(HISTORY_FILE, encoding="utf-8") as fh: + lines = fh.readlines() + except OSError: + return + if len(lines) <= limit: + return + _write_history(lines[-limit:]) def _row_key(row): return json.dumps(row, ensure_ascii=False, sort_keys=True) +def amend_history(entry, **changes): + """Patch one entry in place, matched on its whole content like delete_history. + + For the caller that learns something after its row is already written: the + row goes in before the paste is attempted, and a paste that then fails + still has to end up in the record. None when the row is gone, which a trim + in between can legitimately make true.""" + wanted = _row_key(entry) + with _FILES_LOCK: + rows = _read_history() + for row in rows: + if _row_key(row) == wanted: + row.update(changes) + _write_history([json.dumps(r, ensure_ascii=False) + "\n" + for r in rows]) + return row + return None + + def delete_history(rows): """Remove the given entries, matched on their whole content rather than on a line number: the worker may have appended a new one since the list was read.""" doomed = {_row_key(row) for row in rows} if not doomed: return - kept = [json.dumps(row, ensure_ascii=False) + "\n" - for row in read_history() if _row_key(row) not in doomed] - _write_history(kept) + with _FILES_LOCK: + kept = [json.dumps(row, ensure_ascii=False) + "\n" + for row in _read_history() if _row_key(row) not in doomed] + _write_history(kept) def clear_history(): - HISTORY_FILE.unlink(missing_ok=True) + with _FILES_LOCK: + HISTORY_FILE.unlink(missing_ok=True) # --- meetings ------------------------------------------------------------- @@ -807,6 +886,12 @@ def meeting_paths(base): def read_meetings(): """Newest last.""" + with _FILES_LOCK: + return _read_meetings() + + +def _read_meetings(): + """The body of read_meetings, for callers already holding the lock.""" try: with open(MEETINGS_FILE, encoding="utf-8") as fh: lines = fh.readlines() @@ -829,29 +914,33 @@ def _write_meetings(rows): with open(tmp, "w", encoding="utf-8") as fh: for row in rows: fh.write(json.dumps(row, ensure_ascii=False) + "\n") - tmp.replace(MEETINGS_FILE) + fh.flush() + os.fsync(fh.fileno()) + _replace_with_retry(tmp, MEETINGS_FILE) def save_meeting(entry): """Insert the row, or replace the one with the same base.""" - rows = read_meetings() - for index, row in enumerate(rows): - if row["base"] == entry["base"]: - rows[index] = entry - break - else: - rows.append(entry) - _write_meetings(rows) + with _FILES_LOCK: + rows = _read_meetings() + for index, row in enumerate(rows): + if row["base"] == entry["base"]: + rows[index] = entry + break + else: + rows.append(entry) + _write_meetings(rows) def update_meeting(base, **changes): """Patch one row and hand it back, or None when it is gone.""" - rows = read_meetings() - for row in rows: - if row["base"] == base: - row.update(changes) - _write_meetings(rows) - return row + with _FILES_LOCK: + rows = _read_meetings() + for row in rows: + if row["base"] == base: + row.update(changes) + _write_meetings(rows) + return row return None @@ -860,7 +949,9 @@ def delete_meetings(bases): doomed = set(bases) if not doomed: return - _write_meetings([row for row in read_meetings() if row["base"] not in doomed]) + with _FILES_LOCK: + _write_meetings([row for row in _read_meetings() + if row["base"] not in doomed]) for base in doomed: for path in meeting_paths(base): try: diff --git a/dikte/filetranscribe.py b/dikte/filetranscribe.py index 084bb8f..c69bc2f 100644 --- a/dikte/filetranscribe.py +++ b/dikte/filetranscribe.py @@ -31,6 +31,7 @@ from PyQt6.QtCore import QObject, pyqtSignal from . import api from . import cleanup from . import ggml +from . import paths from .i18n import t UPLOAD_LIMIT = 24 * 1024 * 1024 # the APIs take 25 MB; leave the form its room @@ -337,8 +338,11 @@ def _ffmpeg(args, out, aborter=None): proc = subprocess.Popen( ["ffmpeg", "-nostdin", "-y", *args], stdin=subprocess.DEVNULL, stdout=subprocess.PIPE, stderr=subprocess.PIPE, - text=True, - creationflags=getattr(subprocess, "CREATE_NO_WINDOW", 0), + # ffmpeg writes UTF-8 whatever the locale says; read as the Windows + # codepage its messages mojibake, and a byte the codepage cannot place + # raises from inside communicate itself. + text=True, encoding="utf-8", errors="replace", + creationflags=paths.NO_WINDOW, ) # A two hour film is a minute of ffmpeg, which is a minute of a Stop button # doing nothing unless the abort reaches the process itself. diff --git a/dikte/ggml.py b/dikte/ggml.py index 9256f0f..c2f8da8 100644 --- a/dikte/ggml.py +++ b/dikte/ggml.py @@ -63,6 +63,10 @@ MODELS_DIR = DATA_DIR / "models" # Loading a large model onto a GPU is the slow part of a start, and on a cold # page cache a large LLM read from a spinning disk is slower still. STARTUP_TIMEOUT = 180.0 +# A child that loses the bind race fails and exits at once; a model that fails +# to load takes longer than this to be read in first. The line between "worth +# another port" and "would fail the same way again" is drawn on time. +EARLY_EXIT_WINDOW = 5.0 DOWNLOAD_CHUNK = 1 << 20 # `health` is the path that answers only once the model is in memory. whisper @@ -193,7 +197,16 @@ def download(item, target, on_progress=None, should_stop=None, require_hash=True part.unlink(missing_ok=True) raise LocalError(t("{name} does not match its published checksum. " "Nothing was installed.", name=item.name)) - part.replace(target) + try: + part.replace(target) + except PermissionError as exc: + # Windows refuses to replace a file something has open, and a + # running server holds its model and its binary open. The bytes + # are complete and verified: keeping the .part costs a retry, + # deleting it costs the whole download again. + raise LocalError(t("{name} downloaded, but the old file is held " + "open by the running server. Stop it and try " + "again.", name=item.name)) from exc return True except urllib.error.HTTPError as exc: part.unlink(missing_ok=True) @@ -268,22 +281,23 @@ def _install_record(program): return BIN_DIR / program.name / "installed.json" -def installed_program(program): - """The binary Dikte downloaded, or "" when there is none that still runs.""" +def _read_record(program): + """The install record, or {} however it fails to read.""" try: record = json.loads(_install_record(program).read_text(encoding="utf-8")) - path = record.get("binary") or "" except (OSError, ValueError): - return "" + return {} + return record if isinstance(record, dict) else {} + + +def installed_program(program): + """The binary Dikte downloaded, or "" when there is none that still runs.""" + path = _read_record(program).get("binary") or "" return path if os.path.isfile(path) and os.access(path, os.X_OK) else "" def installed_version(program): - try: - record = json.loads(_install_record(program).read_text(encoding="utf-8")) - return record.get("tag") or "" - except (OSError, ValueError): - return "" + return _read_record(program).get("tag") or "" def program_path(program, custom=""): @@ -339,6 +353,19 @@ def _extract(archive, into): name=os.path.basename(str(archive)), error=exc)) from exc +def _under(path, root): + """Whether `path` lies inside `root`, symlinks and case resolved. + + Resolved on both sides, because the same directory can be reached under + two spellings and this answer decides whether a server gets stopped. + """ + try: + pathlib.Path(path).resolve().relative_to(pathlib.Path(root).resolve()) + return True + except (OSError, ValueError): + return False + + def install_program(program, tag="", on_progress=None, should_stop=None, refresh=False): """Fetch and unpack a release. The path to the binary, or "" when stopped. @@ -374,17 +401,52 @@ def install_program(program, tag="", on_progress=None, should_stop=None, repo=program.repo, tag=tag)) into = BIN_DIR / program.name / tag - shutil.rmtree(into, ignore_errors=True) + fresh = into.with_name(tag + ".new") archive = BIN_DIR / program.name / item.name + try: if not download(item, archive, on_progress, should_stop): return "" - _extract(archive, into) - binary = _find_binary(into, _binary_file(program)) - if binary is None: - raise LocalError(t("{name} was not in the download.", - name=program.binary)) - binary.chmod(binary.stat().st_mode | 0o111) + try: + # Unpacked into a sibling and swapped in only once the binary is + # known to be inside: a failure anywhere in here leaves the + # previous install, and its record, exactly as they were. + shutil.rmtree(fresh, ignore_errors=True) + _extract(archive, fresh) + binary = _find_binary(fresh, _binary_file(program)) + if binary is None: + raise LocalError(t("{name} was not in the download.", + name=program.binary)) + binary.chmod(binary.stat().st_mode | 0o111) + # A running server holds its binary open, and Windows will not + # delete an open file: whichever of our servers runs out of this + # program's directory is stopped here, after the download and the + # unpack are known good, so the outage is the swap and not the + # whole transfer. + for server in SERVERS: + current = program_path(server.program, + server.settings().get("binary", "")) + if current and _under(current, BIN_DIR / program.name): + server.stop() + if into.exists(): + try: + shutil.rmtree(into) + except OSError as exc: + # Not ignore_errors: silently losing this would rename the + # new version somewhere it can never land, and the user can + # actually fix it by closing whatever holds the directory. + raise LocalError(t( + "Could not replace {path}: a file in it is still " + "open: {error}", path=into, error=exc)) from exc + fresh.rename(into) + except BaseException: + # Half an unpacked sibling is not worth keeping, and the swap + # never ran, so the previous install is still whole. + shutil.rmtree(fresh, ignore_errors=True) + raise + # Found under the sibling, run from the final directory. + binary = into / binary.relative_to(fresh) + # Written last, so the record never points at anything half-made. _install_record(program).write_text( json.dumps({"tag": tag, "binary": str(binary)}), encoding="utf-8") except OSError as exc: @@ -404,8 +466,15 @@ def _drop_old_versions(program, keep): root = BIN_DIR / program.name try: for path in root.iterdir(): - if path.is_dir() and path.name != keep: - shutil.rmtree(path, ignore_errors=True) + if not path.is_dir() or path.name == keep: + continue + # A ".new" sibling belongs to an install mid-swap; housekeeping + # must not pull it out from under it. + if path.name.endswith(".new"): + continue + # ignore_errors on purpose: this is housekeeping, and a locked old + # version is a little wasted disk rather than a failed install. + shutil.rmtree(path, ignore_errors=True) except OSError: pass @@ -538,7 +607,12 @@ def _tail(path, lines=3): def _win_image_name(pid): - """The lower-cased file name of the process's executable, or ''.""" + """The full, lower-cased path of the process's executable, or ''. + + The full path rather than the base name, because the name alone is anyone's + whisper-server.exe and this answer decides what gets killed. MAX_PATH is a + convention rather than a limit, so the buffer grows until the query fits. + """ import ctypes kernel32 = ctypes.WinDLL("kernel32", use_last_error=True) kernel32.OpenProcess.restype = ctypes.c_void_p @@ -548,11 +622,18 @@ def _win_image_name(pid): if not handle: return "" try: - buffer = ctypes.create_unicode_buffer(260) - size = ctypes.c_uint32(len(buffer)) - ok = kernel32.QueryFullProcessImageNameW( - ctypes.c_void_p(handle), 0, buffer, ctypes.byref(size)) - return os.path.basename(buffer.value).lower() if ok else "" + length = 260 + while length <= 32768: + buffer = ctypes.create_unicode_buffer(length) + size = ctypes.c_uint32(len(buffer)) + ok = kernel32.QueryFullProcessImageNameW( + ctypes.c_void_p(handle), 0, buffer, ctypes.byref(size)) + if ok: + return buffer.value.lower() + if ctypes.get_last_error() != 122: # ERROR_INSUFFICIENT_BUFFER + return "" + length *= 2 + return "" finally: kernel32.CloseHandle(handle) @@ -578,6 +659,9 @@ class Server: self._port = 0 self._log = "" self._key = None + # The pid this instance last wrote to its pid file, so _forget never + # removes a file some other Dikte wrote after us. + self._pid = 0 # ---- settings -------------------------------------------------------- @@ -626,7 +710,9 @@ class Server: ready = self._current_url() if ready: return ready - self.stop() + # _stop_now rather than stop(): this thread already holds + # _starting, and the public stop() waits for it. + self._stop_now() with self._lock: settings, key = dict(self._settings), self._settings_key() proc, port, log = self._launch(settings) @@ -659,7 +745,7 @@ class Server: stdout=sink, stderr=subprocess.STDOUT, stdin=subprocess.DEVNULL, # No console window of its own on Windows. - creationflags=getattr(subprocess, "CREATE_NO_WINDOW", 0), + creationflags=paths.NO_WINDOW, ) except OSError as exc: raise LocalError(t("Could not start {name}: {error}", @@ -668,8 +754,9 @@ class Server: # Written before it is ready rather than after, so that a kill # during the model load leaves something for the sweep to find. self._remember(proc.pid) + began = time.monotonic() try: - ready = self._wait_ready(proc, port) + reason, listened = self._wait_ready(proc, port) except BaseException: # Whatever went wrong while waiting, the process is ours and # nothing else is left holding a reference to it. Leaving it @@ -679,31 +766,52 @@ class Server: self._kill(proc) self._forget() raise - if ready: + if reason == "ready": return proc, port, str(log) last = _tail(log) self._forget() - # A port taken between the probe and the bind is the one failure - # worth another go; anything else will fail the same way again. - if "address" not in last.lower() and "bind" not in last.lower(): + # Losing the port between the probe and the bind is the one + # failure another port fixes, and it has a shape rather than a + # message: the child died at once without the port ever having + # answered as its own. Grepping the log for "bind" would tie this + # to one program's wording in one language. + early = time.monotonic() - began < EARLY_EXIT_WINDOW + if reason != "exited" or listened or not early: break raise LocalError(t("{name} did not start: {error}", name=self.program.binary, error=last or t("no output"))) def _wait_ready(self, proc, port): + """("ready" | "exited" | "timeout", whether the port answered as ours). + + whisper binds after the model is loaded, so the open port is the + answer; llama binds first and answers /health with 503 until it is + ready. For the health-less case the open port alone is not proof: a + child that lost the bind race exits at once while the winner keeps the + port open, so "ready" also wants our child alive a beat after the port + was first seen open. + """ deadline = time.monotonic() + STARTUP_TIMEOUT + seen_open = False + listened = False while time.monotonic() < deadline: if proc.poll() is not None: - return False + return "exited", listened + if seen_open: + # Port open on the last pass and our child still alive now: + # an imposter's port would have left our child dead by here. + return "ready", True if _listening(port): - # whisper binds after the model is loaded, so the open port is - # the answer. llama binds first and answers /health with 503 - # until it is ready. - if not self.program.health or _healthy(port, self.program.health): - return True + if self.program.health: + # llama did the binding itself, so the port is its. + listened = True + if _healthy(port, self.program.health): + return "ready", True + else: + seen_open = True time.sleep(0.1) self._kill(proc) - return False + return "timeout", listened @staticmethod def _kill(proc, gently=False): @@ -724,6 +832,14 @@ class Server: pass def stop(self): + # Taking _starting means a stop cannot slide past a launch in flight: + # serve() finishes registering its child first, and the child is then + # killed here rather than surviving the shutdown unowned. + with self._starting: + self._stop_now() + + def _stop_now(self): + """stop() for a thread that already holds _starting.""" with self._lock: proc, self._proc = self._proc, None self._port, self._log, self._key = 0, "", None @@ -737,6 +853,8 @@ class Server: return DATA_DIR / f"{self.program.name}-server.pid" def _remember(self, pid): + with self._lock: + self._pid = pid try: path = self._pid_file() path.parent.mkdir(parents=True, exist_ok=True) @@ -744,28 +862,62 @@ class Server: except OSError: pass # the sweep is a safety net, not something to fail a run over - def _forget(self): + def _forget(self, pid=None): + """Remove the pid file, but only while it still holds our own pid. + + Another Dikte started after us writes its pid over ours, and removing + that file would hide its server from every future sweep. + """ + if pid is None: + with self._lock: + pid = self._pid try: - self._pid_file().unlink() - except OSError: + if int(self._pid_file().read_text().strip()) == pid: + self._pid_file().unlink() + except (OSError, ValueError): pass def _is_ours(self, pid): - """Whether that pid is still the server this Dikte started. + """True: still our server. False: definitely not. None: cannot tell now. Asked because pids are handed out again: by the time anyone looks, the number could belong to something else entirely, and killing it would be - a good deal worse than the leak being cleaned up. The program name alone - could be somebody else's copy; the name together with Dikte's own data - directory on the command line could not. Windows offers no command line - to read, so the executable's name is the whole of the answer there. + a good deal worse than the leak being cleaned up. The tri-state matters + for the pid file: a definitive "not ours" means the file is stale and + safe to drop, while "cannot tell" means it has to stay so a later start + can ask again. + + On Linux the program name alone could be somebody else's copy; the name + together with Dikte's own data directory on the command line could not. + Windows offers no command line to read, so the executable's full path + is the answer there: under our bin directory, or exactly the binary the + settings point this server at. Never the base name alone, which is + anyone's whisper-server.exe. """ if sys.platform == "win32": - return _win_image_name(pid) == _binary_file(self.program).lower() + path = _win_image_name(pid) + if not path: + # OpenProcess said nothing: the process may be gone or merely + # unreadable from here, and the difference decides whether the + # pid file may be dropped, so no verdict rather than a wrong one. + return None + path = os.path.normcase(path) + if path.startswith(os.path.normcase(str(BIN_DIR)) + os.sep): + return True + with self._lock: + custom = self._settings.get("binary", "") + configured = program_path(self.program, custom) + if configured: + resolved = os.path.normcase(str(pathlib.Path(configured).resolve())) + if path == resolved: + return True + return False try: blob = pathlib.Path(f"/proc/{pid}/cmdline").read_bytes() + except (FileNotFoundError, ProcessLookupError): + return False # the process is definitively gone except OSError: - return False + return None # /proc would not answer just now return (self.program.binary.encode() in blob and str(DATA_DIR).encode() in blob) @@ -781,13 +933,21 @@ class Server: pid = int(self._pid_file().read_text().strip()) except (OSError, ValueError): return False - self._forget() - if not self._is_ours(pid): + owned = self._is_ours(pid) + if owned is None: + # Could not be verified rather than known stale: the file stays, + # so the next start asks again instead of losing track of a server + # that may still be holding a model. + return False + if not owned: + self._forget(pid) return False try: os.kill(pid, signal.SIGTERM) except OSError: + self._forget(pid) return False + self._forget(pid) return True diff --git a/dikte/hotkey.py b/dikte/hotkey.py index 649ef36..91e4c6b 100644 --- a/dikte/hotkey.py +++ b/dikte/hotkey.py @@ -970,9 +970,7 @@ def conflicting_shortcuts(shortcut, desktop_id=DESKTOP_ID): if "=" not in line or desktop_id in section: continue key, _, value = line.partition("=") - if shortcut.lower() in value.lower().split(","): - hits.append(f"{section} → {key}") - elif any(shortcut.lower() == part.strip().lower() - for part in re.split(r"[,\t]", value)): + if any(shortcut.lower() == part.strip().lower() + for part in re.split(r"[,\t]", value)): hits.append(f"{section} → {key}") return hits diff --git a/dikte/hub.py b/dikte/hub.py index 9e6e518..f5da71a 100644 --- a/dikte/hub.py +++ b/dikte/hub.py @@ -12,19 +12,18 @@ means one for every whisper.cpp release; both of those are somebody else's news, not Dikte's. Answers are cached for a few hours, and a cache that has gone stale is still a better answer than none when the network is down. -Nothing here imports the rest of Dikte apart from the string table: this module -knows two websites and nothing about dictation. +Nothing here imports the rest of Dikte apart from two leaves, the string table +and the path map: this module knows two websites and nothing about dictation. """ import collections import json -import os -import pathlib import time import urllib.error import urllib.parse import urllib.request +from . import paths from .i18n import t GITHUB_API = "https://api.github.com" @@ -32,8 +31,7 @@ HF_API = "https://huggingface.co/api" HF_FILES = "https://huggingface.co" USER_AGENT = "dikte/1.0 (+https://github.com/yusufipk/dikte)" -CACHE_DIR = (pathlib.Path(os.environ.get("XDG_CACHE_HOME") - or os.path.expanduser("~/.cache")) / "dikte") +CACHE_DIR = paths.cache_dir() # Long enough that opening the settings window twice in an evening asks nobody # anything, short enough that a model published this morning is offered today. CACHE_TTL = 6 * 3600 diff --git a/dikte/i18n.py b/dikte/i18n.py index d8cb759..fc7c59e 100644 --- a/dikte/i18n.py +++ b/dikte/i18n.py @@ -746,4 +746,135 @@ TR = { "This one is being written up right now.": "Bunun tutanağı şu anda çıkarılıyor.", "Delete this meeting, its minutes and its recording?": "Bu toplantı, tutanağı ve ses kaydı silinsin mi?", + + # --- local models and downloads ------------------------------------ + # The whole box was born after the last translation pass, which left the + # first-run screen half English on a Turkish machine. + "Download": "İndir", + "Delete": "Sil", + "Program": "Program", + "Publisher": "Yayıncı", + "Automatic": "Otomatik", + "Threads": "İş parçacığı", + "On this machine": "Bu makinede", + "Use the graphics card": "Ekran kartını kullan", + "Load the model when Dikte starts": "Modeli Dikte açılırken yükle", + "Local whisper": "Yerel whisper", + "Local model": "Yerel model", + "Not installed.": "Kurulu değil.", + "Installed on the system: {path}": "Sistemde kurulu: {path}", + "Downloaded, version {version}.": "İndirildi, sürüm {version}.", + "Fetching the model list…": "Model listesi çekiliyor…", + "Downloading…": "İndiriliyor…", + "Downloading: {done} of {total}{share}": "İndiriliyor: {done} / {total}{share}", + "Download stopped.": "İndirme durduruldu.", + "Ready: {name}.": "Hazır: {name}.", + "Nothing downloaded yet.": "Henüz bir şey indirilmedi.", + "{name} has not been downloaded yet.": "{name} henüz indirilmedi.", + "downloaded": "indirildi", + "not downloaded": "indirilmedi", + "Delete model": "Modeli sil", + "Delete {name} from this machine?": "{name} bu makineden silinsin mi?", + "Runs on this machine, on llama.cpp.": "Bu makinede, llama.cpp üzerinde çalışır.", + "A Hugging Face repository of GGUF files. The list is fetched; any other " + "one can be typed in.": + "GGUF dosyaları içeren bir Hugging Face deposu. Liste internetten " + "çekilir; başka bir depo da yazılabilir.", + "A large model takes a second or two to load. Loading it up front spends " + "that once instead of on the first dictation, at the cost of the memory " + "it sits in.": + "Büyük bir modelin yüklenmesi bir iki saniye sürer. Baştan yüklemek bu " + "bedeli ilk diktede değil bir kez öder; karşılığı, modelin oturduğu " + "bellektir.", + "An LLM is slower to load than a whisper model and sits in more memory. " + "Off means it is loaded on the first cleanup instead.": + "Bir LLM, whisper modelinden daha geç yüklenir ve daha çok bellekte " + "oturur. Kapalı, ilk temizlemede yüklenmesi demektir.", + "A model trained to think will think unless it is told not to, and " + "spending 300 tokens of reasoning on a comma is 300 tokens of waiting. " + "Off is what cleanup wants.": + "Düşünmeye eğitilmiş bir model, aksi söylenmedikçe düşünür; bir virgül " + "için 300 token akıl yürütmek 300 token'lık bekleyiştir. Temizleme için " + "doğrusu Kapalı.", + "OpenRouter is the quickest and the only one that needs nothing " + "installed. llama.cpp runs here, on a model downloaded below. Claude Code " + "and Codex clean up on the subscription you already have, without a " + "second key, and take a few seconds longer because each one opens a " + "session to do it.": + "OpenRouter en hızlısıdır ve kurulum istemeyen tek seçenektir. " + "llama.cpp burada, aşağıda indirilen bir modelle çalışır. Claude Code " + "ve Codex, ikinci bir anahtar olmadan zaten sahip olduğun abonelikle " + "temizler; her biri bunun için bir oturum açtığından birkaç saniye " + "daha sürer.", + "whisper.cpp reaches the card through CUDA, ROCm or Vulkan when the build " + "it is running was made with one. A build without any of them runs on the " + "processor whatever this says.": + "whisper.cpp karta CUDA, ROCm ya da Vulkan üzerinden ulaşır; koştuğu " + "derleme bunlardan biriyle yapılmışsa. Hiçbiri olmadan derlenmiş bir " + "kopya, bu ne derse desin işlemcide çalışır.", + "whisper.cpp is not installed. Settings → API and models → Download.": + "whisper.cpp kurulu değil. Ayarlar → API ve modeller → İndir.", + "llama.cpp is not installed. Settings → API and models → Download.": + "llama.cpp kurulu değil. Ayarlar → API ve modeller → İndir.", + "No whisper model has been downloaded yet. Settings → API and models → " + "Download.": + "Henüz whisper modeli indirilmedi. Ayarlar → API ve modeller → İndir.", + "No local cleanup model has been downloaded yet. Settings → API and " + "models → Download.": + "Henüz yerel temizleme modeli indirilmedi. Ayarlar → API ve modeller " + "→ İndir.", + "Hugging Face did not return a model list.": + "Hugging Face model listesi döndürmedi.", + "{repo} did not return a file list.": "{repo} dosya listesi döndürmedi.", + "{repo} has no downloadable release.": + "{repo} deposunun indirilebilir bir sürümü yok.", + "{repo} {tag} has no build for this machine.": + "{repo} {tag} bu makine için derleme içermiyor.", + "{url} answered HTTP {code}.": "{url} HTTP {code} yanıtı verdi.", + "Could not reach {url}: {error}": "{url} adresine ulaşılamadı: {error}", + "Could not read the answer from {url}: {error}": + "{url} yanıtı okunamadı: {error}", + "Could not create {path}: {error}": "{path} oluşturulamadı: {error}", + "Could not download {name}: HTTP {code}": + "{name} indirilemedi: HTTP {code}", + "Could not download {name}: {error}": "{name} indirilemedi: {error}", + "Could not write {name}: {error}": "{name} yazılamadı: {error}", + "Could not unpack {name}: {error}": "{name} açılamadı: {error}", + "Could not install {name}: {error}": "{name} kurulamadı: {error}", + "Could not start {name}: {error}": "{name} başlatılamadı: {error}", + "Could not delete the model: {error}": "Model silinemedi: {error}", + "Could not replace {path}: a file in it is still open: {error}": + "{path} değiştirilemedi: içindeki bir dosya hâlâ açık: {error}", + "{name} did not start: {error}": "{name} başlamadı: {error}", + "no output": "çıktı yok", + "The download stopped early ({done} of {total}).": + "İndirme erken kesildi ({done} / {total}).", + "{name} is longer than it said it would be.": + "{name} bildirdiğinden daha uzun çıktı.", + "{name} does not match its published checksum. Nothing was installed.": + "{name} yayımlanan sağlama toplamıyla uyuşmuyor. Hiçbir şey kurulmadı.", + "{name} is published without a checksum, so there is no way to tell what " + "arrived. Nothing was installed.": + "{name} sağlama toplamı olmadan yayımlanmış; gelenin ne olduğu " + "doğrulanamaz. Hiçbir şey kurulmadı.", + "{name} was not in the download.": "{name} indirilenin içinde yoktu.", + "{name} downloaded, but the old file is held open by the running server. " + "Stop it and try again.": + "{name} indirildi ama eski dosyayı çalışan sunucu açık tutuyor. " + "Sunucuyu durdurup yeniden dene.", + "The cleanup model spent its whole reply on thinking. Set Thinking to " + "“Off”.": + "Temizleme modeli bütün yanıtını düşünmeye harcadı. Düşünme'yi " + "“Kapalı” yap.", + + # --- this pass's new messages --------------------------------------- + "Audio recorder stopped before receiving sound": + "Ses kayıt aracı veri alamadan kapandı", + "Could not write the recording: {error}": "Kayıt dosyası yazılamadı: {error}", + "Copied, but pasting failed: {error}": + "Kopyalandı ama yapıştırma başarısız: {error}", + "The recording was kept: {path}": "Kayıt saklandı: {path}", + "The recording stopped on its own; transcribing what was captured.": + "Kayıt kendi kendine durdu; yakalanan kısım yazıya dökülüyor.", + "Could not save the settings: {error}": "Ayarlar kaydedilemedi: {error}", } diff --git a/dikte/integrate.py b/dikte/integrate.py index a55fcf3..10639d0 100644 --- a/dikte/integrate.py +++ b/dikte/integrate.py @@ -56,6 +56,17 @@ def packaged(): return bool(getattr(sys, "frozen", False)) +def windowed_executable(executable=None): + """The windowed executable installed beside this one, or None. + + Beside rather than at a known place, because the setup program lays the two + executables into the same directory wherever that directory was put: asking + from either of them finds the other without knowing where the install is. + """ + windowed = pathlib.Path(executable or sys.executable).with_name(WINDOWS_APP) + return windowed if windowed.is_file() else None + + def target(): """The file a launcher has to name to start this build again. @@ -74,8 +85,8 @@ def target(): # The windowed executable, whichever of the two is running: the console # one is what the `dikte` command names, and a sign-in that started # that one would open a console window nobody asked for. - windowed = executable.with_name(WINDOWS_APP) - if windowed.is_file(): + windowed = windowed_executable(executable) + if windowed is not None: return windowed return executable @@ -572,24 +583,60 @@ def _run_entry_name(): return f"HKCU\\{RUN_KEY}\\{RUN_VALUE}" +def _run_target(value): + """The executable a Run value names, out of the quoting the setup wrote. + + Only the first word matters here: it is the file whose existence says + whether the entry still starts anything. + """ + if value.startswith('"'): + closing = value.find('"', 1) + return value[1:closing] if closing > 0 else "" + return value.split(" ", 1)[0] + + +def _startup_shortcut(): + """Where install.ps1 -Autostart puts a checkout's sign-in entry.""" + appdata = os.environ.get("APPDATA") + if not appdata: + return None + return (pathlib.Path(appdata) / "Microsoft" / "Windows" / "Start Menu" + / "Programs" / "Startup" / "Dikte.lnk") + + def _windows_install(app, force=False): """Point the autostart entry at this build. What changed. Only `force`, which is what typing `dikte integrate` means, creates one. The call on every start repairs an entry that is already there and names an - executable somewhere else, which is what an installation moved to another - drive or reinstalled into another directory leaves behind; somebody who - unticked the box in the wizard, or turned it off since, is not asked again - by every start. + executable that is gone, which is what an installation moved to another + drive or reinstalled into another directory leaves behind. An entry naming + an executable that still exists is another installation that still works, + and is stood aside for the way the Linux half stands aside for another + menu entry; somebody who unticked the box in the wizard, or turned it off + since, is not asked again by every start either. """ command = f'"{app}"' current = _run_entry() + changed = [] + if force: + # install.ps1 -Autostart wrote this for a checkout. The Run value + # written below replaces it, and both left in place would be two + # Diktes at every sign-in. Only on force: the silent call on every + # start has not been asked to move the machine off its checkout. + shortcut = _startup_shortcut() + if shortcut is not None and shortcut.is_file(): + shortcut.unlink() + changed.append(shortcut) if not current and not force: - return [] - if current == command: - return [] - _write_run_entry(command) - return [_run_entry_name()] + return changed + if current != command: + theirs = _run_target(current) if current else "" + if not force and theirs and theirs != str(app) and os.path.exists(theirs): + return changed + _write_run_entry(command) + changed.append(_run_entry_name()) + return changed def _windows_remove(): diff --git a/dikte/ipc.py b/dikte/ipc.py index 02f668a..589045e 100644 --- a/dikte/ipc.py +++ b/dikte/ipc.py @@ -11,11 +11,14 @@ shortcut may still send. import json import os import shlex +import subprocess import sys +from PyQt6.QtCore import QLockFile from PyQt6.QtNetwork import QLocalSocket from . import integrate +from . import paths SERVER_NAME = "dikte-" + ( str(os.getuid()) if hasattr(os, "getuid") @@ -54,10 +57,9 @@ def launcher(): if not getattr(sys, "frozen", False): return [sys.executable, script_path()] if sys.platform == "win32": - windowed = os.path.join(os.path.dirname(sys.executable), - integrate.WINDOWS_APP) - if os.path.isfile(windowed): - return [windowed] + windowed = integrate.windowed_executable() + if windowed is not None: + return [str(windowed)] return [os.environ.get("APPIMAGE") or sys.executable] @@ -72,6 +74,60 @@ def command_for(verb): return shlex.join(launcher() + ([verb] if verb else [])) +def already_serving(): + """Whether a running instance answers on this user's name. + + Asked before an instance opens a server of its own, because listen() is + not the check: a Windows named pipe takes a second server on the same name + rather than refusing it, and everywhere else removeServer() would first + take the live socket away from the instance holding it. Either way two + whole Diktes then run, and the newer one's sweep() kills the whisper the + older one is answering dictations with. The probe is "status" and nothing + else: a verb with a side effect here would fire it during the relaunch a + slow-to-answer instance provokes, on top of the verb being forwarded. + """ + return send("status") is not None + + +def instance_lock(): + """This user's one-Dikte lock, taken before anything else is built. + + The probe above has a hole: two copies started in the same moment both ask + before either listens, and both come up. A lock file closes it, and + QLockFile writes the holder's pid into it, so a lock a killed instance + left behind identifies itself as stale and clears. None when the data + directory cannot be made, which a start should survive: the probe still + stands guard, just without the simultaneous-start case. + """ + try: + paths.DATA_DIR.mkdir(parents=True, exist_ok=True) + except OSError: + return None + lock = QLockFile(str(paths.DATA_DIR / "dikte.lock")) + # Never presume a lock is stale by age alone; the pid check is the truth. + lock.setStaleLockTime(0) + return lock + + +def respawn(arguments): + """Start this installation again with `arguments`, leaving this process. + + execv everywhere it works the way it says: the new process takes this + pid and nothing is left behind. On Windows execv mangles arguments with + spaces and leaves the two processes sharing a console, so the replacement + is started detached instead and the caller exits on its own. + """ + args = launcher() + list(arguments) + if sys.platform == "win32": + # By value where the names are missing, so the Windows half of this is + # testable from the suite's other platforms too. + detached = (getattr(subprocess, "DETACHED_PROCESS", 0x00000008) + | getattr(subprocess, "CREATE_NEW_PROCESS_GROUP", 0x00000200)) + subprocess.Popen(args, creationflags=detached, close_fds=True) + return + os.execv(args[0], args) + + def send(cmd, wait=False, timeout=0, **args): """Send one request; the reply, or None when no instance is running. diff --git a/dikte/paths.py b/dikte/paths.py index 7670c8b..8f2e057 100644 --- a/dikte/paths.py +++ b/dikte/paths.py @@ -13,10 +13,18 @@ platform as an argument so that a test can stand on the other one. import os import pathlib +import subprocess import sys +# The one other platform constant every subprocess site needs, kept in this +# leaf so no caller has to pull the audio stack in for it: console programs +# started from a windowless process would otherwise each open a console window +# of their own on Windows. +NO_WINDOW = (getattr(subprocess, "CREATE_NO_WINDOW", 0) + if sys.platform == "win32" else 0) -def _env(var, default): + +def env_path(var, default): """The directory a variable names, or the one it stands in for.""" return pathlib.Path(os.environ.get(var) or os.path.expanduser(default)) @@ -34,11 +42,28 @@ def directories(platform=None): support = pathlib.Path.home() / "Library/Application Support/Dikte" return support, support if here == "win32": - roaming = _env("APPDATA", "~/AppData/Roaming") - local = _env("LOCALAPPDATA", "~/AppData/Local") + roaming = env_path("APPDATA", "~/AppData/Roaming") + local = env_path("LOCALAPPDATA", "~/AppData/Local") return roaming / "Dikte", local / "Dikte" - return (_env("XDG_CONFIG_HOME", "~/.config") / "dikte", - _env("XDG_DATA_HOME", "~/.local/share") / "dikte") + return (env_path("XDG_CONFIG_HOME", "~/.config") / "dikte", + env_path("XDG_DATA_HOME", "~/.local/share") / "dikte") + + +def cache_dir(platform=None): + """The directory for answers worth keeping but never worth backing up. + + A third place because a cache is neither settings nor data: losing it costs + a network request, not a model or a preference, and every system sets aside + a directory for exactly that kind of file, one that backups skip and + cleanup tools may empty. Storing it with the data would ask a backup to + carry files whose whole point is that they can be thrown away. + """ + here = platform or sys.platform + if here == "darwin": + return pathlib.Path.home() / "Library/Caches/Dikte" + if here == "win32": + return env_path("LOCALAPPDATA", "~/AppData/Local") / "Dikte" / "cache" + return env_path("XDG_CACHE_HOME", "~/.cache") / "dikte" CONFIG_DIR, DATA_DIR = directories() diff --git a/dikte/settings_ui.py b/dikte/settings_ui.py index f166140..485ddab 100644 --- a/dikte/settings_ui.py +++ b/dikte/settings_ui.py @@ -1,7 +1,9 @@ """Settings window.""" +import functools import os import shutil +import sys import threading from PyQt6.QtCore import QEvent, QObject, QRect, Qt, QUrl, pyqtSignal @@ -194,10 +196,12 @@ class LocalModelBox(QGroupBox): """ _listed = pyqtSignal(list, str) - _quants = pyqtSignal(list, str) # qint64 rather than int, which is C++'s 32-bit one: a 2.3 GB model is more # than fits in it, and the count comes out the far side negative. - _progress = pyqtSignal("qint64", "qint64") + # The tag says which label the numbers belong to: the program download and + # a model download can run at once, and routing by a flag read when the + # queued event lands put one job's bytes in the other's label. + _progress = pyqtSignal(str, "qint64", "qint64") _finished = pyqtSignal(str, str) _installed = pyqtSignal(str, str) @@ -245,7 +249,6 @@ class LocalModelBox(QGroupBox): form.addRow(self.status) self._listed.connect(self._on_listed) - self._quants.connect(self._on_listed) self._progress.connect(self._on_progress) self._finished.connect(self._on_finished) self._installed.connect(self._on_installed) @@ -348,9 +351,9 @@ class LocalModelBox(QGroupBox): def work(): try: found = self._models(repo) if self._repos is not None else self._models() - self._quants.emit([("models", found)], "") + self._listed.emit([("models", found)], "") except ggml.LocalError as exc: - self._quants.emit([], str(exc)) + self._listed.emit([], str(exc)) threading.Thread(target=work, daemon=True).start() @@ -412,7 +415,9 @@ class LocalModelBox(QGroupBox): def work(): try: - ggml.install_program(self.program, on_progress=self._report) + ggml.install_program( + self.program, + on_progress=functools.partial(self._report, "program")) self._installed.emit("", "") except ggml.LocalError as exc: self._installed.emit("", str(exc)) @@ -441,8 +446,17 @@ class LocalModelBox(QGroupBox): def work(): try: - landed = ggml.download(item, self._model_path(item.name), - on_progress=self._report, + target_path = self._model_path(item.name) + # A model the running server holds open cannot be replaced on + # Windows, and the finished download would be thrown away over + # it; a re-download lets go of the server first, and the next + # run starts it again on the fresh file. + if target_path.exists(): + (ggml.whisper if self.program is ggml.WHISPER + else ggml.llm).stop() + landed = ggml.download(item, target_path, + on_progress=functools.partial( + self._report, "model"), should_stop=lambda: self._stop) self._finished.emit(item.name if landed else "", "") except ggml.LocalError as exc: @@ -450,15 +464,15 @@ class LocalModelBox(QGroupBox): threading.Thread(target=work, daemon=True).start() - def _report(self, done, total): - self._progress.emit(done, total) + def _report(self, job, done, total): + self._progress.emit(job, done, total) - def _on_progress(self, done, total): + def _on_progress(self, job, done, total): share = f" ({done * 100 // total}%)" if total else "" text = t("Downloading: {done} of {total}{share}", done=ggml.human_size(done), total=ggml.human_size(total or done), share=share) - if self._downloading: + if job == "model": self.status.setText(text) else: self.program_label.setText(text) @@ -524,6 +538,15 @@ class SettingsWindow(QDialog): # hears about it from here rather than waiting for its own next check. update_found = pyqtSignal(object) + def _sources_once(self): + """One device listing per window build, shared by every combo. + + Three listings at open were three subprocess runs on the main thread, + which on a slow ffmpeg was most of the wait for the window.""" + if not hasattr(self, "_sources"): + self._sources = audio.list_sources() + return self._sources + _models_loaded = pyqtSignal(list, str) _transcribe_models_loaded = pyqtSignal(list, str) # Which key was tested, whether it worked, and what to write under it. @@ -650,7 +673,7 @@ class SettingsWindow(QDialog): self.mic = QComboBox() self.mic.addItem(t("Default microphone"), "") - for name, desc in audio.list_sources(): + for name, desc in self._sources_once(): self.mic.addItem(desc, name) form.addRow(t("Microphone"), self.mic) @@ -1115,7 +1138,7 @@ class SettingsWindow(QDialog): sources_form = QFormLayout(sources) self.meeting_mic = QComboBox() self.meeting_mic.addItem(t("Same as dictation"), "") - for name, desc in audio.list_sources(): + for name, desc in self._sources_once(): self.meeting_mic.addItem(desc, name) sources_form.addRow(t("Microphone"), self.meeting_mic) @@ -1628,9 +1651,20 @@ class SettingsWindow(QDialog): self.local_llm_preload.setChecked(conf["local_llm_preload"]) self._select_data(self.local_llm_reasoning, conf["local_llm_reasoning"]) self.local_llm.load(conf["local_llm_model"], conf["local_llm_repo"]) - self.cleanup_prompt.setPlainText(conf["cleanup_prompt"] or cfg.default_cleanup_prompt()) + # The defaults as they read NOW, kept for the save comparison: after a + # language switch the boxes still hold the old language's default, and + # comparing against the new one would store that text as a custom + # prompt shadowing every future improvement. + self._loaded_defaults = { + "cleanup": cfg.default_cleanup_prompt(), + "file": cfg.default_file_cleanup_prompt(), + "assistant": cfg.default_assistant_prompt(), + "meeting": cfg.default_meeting_prompt(), + } + self.cleanup_prompt.setPlainText( + conf["cleanup_prompt"] or self._loaded_defaults["cleanup"]) self.file_cleanup_prompt.setPlainText( - conf["file_cleanup_prompt"] or cfg.default_file_cleanup_prompt() + conf["file_cleanup_prompt"] or self._loaded_defaults["file"] ) self.transcribe_prompt.setPlainText(conf["transcribe_prompt"]) @@ -1648,7 +1682,7 @@ class SettingsWindow(QDialog): self.assistant_paste.setChecked(conf["assistant_paste"]) self.assistant_cleanup.setChecked(conf["assistant_cleanup"]) self.assistant_prompt.setPlainText( - conf["assistant_prompt"] or cfg.default_assistant_prompt() + conf["assistant_prompt"] or self._loaded_defaults["assistant"] ) self._select_data(self.meeting_mic, conf["meeting_mic_target"]) @@ -1660,10 +1694,11 @@ class SettingsWindow(QDialog): self._select_data(self.meeting_reasoning, conf["meeting_reasoning"]) self._select_data(self.meeting_language, conf["meeting_language"]) self.meeting_cleanup.setChecked(conf["meeting_cleanup"]) - self.meeting_max_minutes.setValue(max(5, int(conf["meeting_max_seconds"]) // 60)) + self._meeting_max_loaded = int(conf["meeting_max_seconds"]) + self.meeting_max_minutes.setValue(max(5, self._meeting_max_loaded // 60)) self.meeting_keep_audio.setChecked(conf["meeting_keep_audio"]) self.meeting_prompt.setPlainText( - conf["meeting_prompt"] or cfg.default_meeting_prompt() + conf["meeting_prompt"] or self._loaded_defaults["meeting"] ) self.file_timestamps.setChecked(conf["file_timestamps"]) @@ -1726,13 +1761,18 @@ class SettingsWindow(QDialog): conf["local_llm_preload"] = self.local_llm_preload.isChecked() conf["local_llm_reasoning"] = self.local_llm_reasoning.currentData() or "" - # Store an empty prompt when it matches the default, so switching the - # interface language also switches the prompt language. + # Store an empty prompt when it matches a default: the one it was + # loaded with, or today's (a Reset click in a session that switched + # languages fills in the latter). Both count, so switching the + # interface language keeps switching the prompt language. prompt = self.cleanup_prompt.toPlainText().strip() - conf["cleanup_prompt"] = "" if prompt == cfg.default_cleanup_prompt() else prompt + conf["cleanup_prompt"] = ("" if prompt in ( + self._loaded_defaults["cleanup"], cfg.default_cleanup_prompt()) + else prompt) file_prompt = self.file_cleanup_prompt.toPlainText().strip() - conf["file_cleanup_prompt"] = ("" if file_prompt == cfg.default_file_cleanup_prompt() - else file_prompt) + conf["file_cleanup_prompt"] = ("" if file_prompt in ( + self._loaded_defaults["file"], cfg.default_file_cleanup_prompt()) + else file_prompt) conf["transcribe_prompt"] = self.transcribe_prompt.toPlainText().strip() conf["assistant_provider"] = self.assistant_provider.currentData() or "claude" @@ -1759,8 +1799,9 @@ class SettingsWindow(QDialog): conf["assistant_paste"] = self.assistant_paste.isChecked() conf["assistant_cleanup"] = self.assistant_cleanup.isChecked() assistant_prompt = self.assistant_prompt.toPlainText().strip() - conf["assistant_prompt"] = ("" if assistant_prompt == cfg.default_assistant_prompt() - else assistant_prompt) + conf["assistant_prompt"] = ("" if assistant_prompt in ( + self._loaded_defaults["assistant"], cfg.default_assistant_prompt()) + else assistant_prompt) conf["meeting_mic_target"] = self.meeting_mic.currentData() or "" conf["meeting_system_target"] = self.meeting_system.currentData() or "" @@ -1772,11 +1813,16 @@ class SettingsWindow(QDialog): conf["meeting_reasoning"] = self.meeting_reasoning.currentData() or "" conf["meeting_language"] = self.meeting_language.currentData() or "" conf["meeting_cleanup"] = self.meeting_cleanup.isChecked() - conf["meeting_max_seconds"] = self.meeting_max_minutes.value() * 60 + # Only when the dial was actually turned: the box speaks whole minutes + # with a floor, and an unrelated Save must not rewrite a value the + # command line set in seconds. + if self.meeting_max_minutes.value() != max(5, self._meeting_max_loaded // 60): + conf["meeting_max_seconds"] = self.meeting_max_minutes.value() * 60 conf["meeting_keep_audio"] = self.meeting_keep_audio.isChecked() meeting_prompt = self.meeting_prompt.toPlainText().strip() - conf["meeting_prompt"] = ("" if meeting_prompt == cfg.default_meeting_prompt() - else meeting_prompt) + conf["meeting_prompt"] = ("" if meeting_prompt in ( + self._loaded_defaults["meeting"], cfg.default_meeting_prompt()) + else meeting_prompt) conf["file_timestamps"] = self.file_timestamps.isChecked() conf["file_cleanup"] = self.file_cleanup.isChecked() @@ -1790,7 +1836,16 @@ class SettingsWindow(QDialog): or hotkey.default_combo(which)) conf["evdev_hotkey"] = self.evdev_enabled.isChecked() conf["history_limit"] = self.history_limit.value() - conf.save() + try: + conf.save() + except OSError as exc: + # An antivirus or a sync tool holding the file for a beat is a + # message, not an exit: an exception out of a Qt slot takes the + # whole application down. + QMessageBox.warning(self, "Dikte", + t("Could not save the settings: {error}", + error=exc)) + return # A lowered limit should bite now, not on the next dictation. try: cfg.trim_history(conf["history_limit"]) @@ -2012,7 +2067,10 @@ class SettingsWindow(QDialog): """ self.conf["file_timestamps"] = self.file_timestamps.isChecked() self.conf["file_cleanup"] = self.file_cleanup.isChecked() - self.conf.save() + try: + self.conf.save() + except OSError as exc: + print(f"dikte: could not save the settings: {exc}", file=sys.stderr) def _run_file(self): if not getattr(self, "file_path", "") or self.transcriber.busy: @@ -2113,7 +2171,12 @@ class SettingsWindow(QDialog): QMessageBox.information(self, t("Shortcut"), message) if ok: self.conf[spec.setting] = combo - self.conf.save() + try: + self.conf.save() + except OSError as exc: + QMessageBox.warning(self, "Dikte", + t("Could not save the settings: {error}", + error=exc)) self._refresh_shortcut_status(which) def _remove_shortcut(self, which): diff --git a/dikte/vad.py b/dikte/vad.py index 082d14d..4f784ae 100644 --- a/dikte/vad.py +++ b/dikte/vad.py @@ -17,13 +17,15 @@ import unicodedata # Stock phrases the models produce when handed silence. Kept deliberately # narrow: only sentences nobody dictates on purpose in a two-second clip. +# Whisper does invent "you" and "bye" too, but people dictate both as whole +# answers, so a single word never belongs here. HALLUCINATIONS = { "altyazi mk", "altyazi m k", "altyazi", "altyazilar", "abone olmayi unutmayin", "izlediginiz icin tesekkurler", "izlediginiz icin tesekkur ederim", "izlediginiz icin tesekkur ederiz", "kanalima abone olmayi unutmayin", "altyazi mk altyazi mk", "thanks for watching", "thank you for watching", "thanks for watching!", - "please subscribe", "subscribe to my channel", "you", "bye", + "please subscribe", "subscribe to my channel", "mbc masr", "sous titres realises par la communaute damara org", "amara org community", "sous titrage st 501", } diff --git a/dikte/worker.py b/dikte/worker.py index b576d56..24b1e1f 100644 --- a/dikte/worker.py +++ b/dikte/worker.py @@ -112,11 +112,16 @@ class Pipeline(QObject): text = raw warning = "" + # Remembered rather than re-derived at the history write below: the + # ask path runs cleanup under a different setting, and the record + # should say what happened, not what one of the two gates implies. + cleaned = False # Claude reads through “eee” and “hani” without help, so a dictation # on its way there is normally sent as it was heard, one API call and # a second or two lighter. if (conf["assistant_cleanup"] if ask else conf["cleanup_enabled"]): self.stage.emit(t("Cleaning up…")) + cleaned = True try: text = cleanup.run(raw, conf, conf.cleanup_prompt()) except api.ApiError as exc: @@ -142,62 +147,111 @@ class Pipeline(QObject): if paste_override is not None: wants_paste = paste_override - with _paste_lock: - previous = (paste.read_clipboard() - if conf["restore_clipboard"] and wants_paste else None) - try: - paste.copy(text) - if wants_paste: - self.stage.emit(t("Pasting…")) - paste.press(conf["paste_shortcut"], focus=focus) - finally: - if previous is not None: - # Let the focused application consume the temporary - # transcription before putting every old clipboard type - # back. This also runs when key injection fails. - time.sleep(0.35) - paste.copy_bytes(previous) - - cfg.append_history({ + # Into the history before the paste is attempted: the record says + # what was dictated, not whether a key press landed, and a paste + # that fails must not take the transcript down with it. + record = { "ts": time.strftime("%Y-%m-%d %H:%M:%S"), "duration": round(duration, 1), "elapsed": round(time.monotonic() - started, 1), "model": target.model, - "cleanup_model": cleanup.model(conf) if conf["cleanup_enabled"] else "", + "cleanup_model": cleanup.model(conf) if cleaned else "", "cleanup_error": warning, "mode": "ask" if ask else "", "question": question, "assistant_model": conf["assistant_model"] if ask else "", "raw": raw, "text": text, - }) + } + cfg.append_history(record) try: cfg.trim_history(conf["history_limit"]) except OSError as exc: print(f"dikte: could not trim the history: {exc}", file=sys.stderr) + + with _paste_lock: + previous = (paste.read_clipboard() + if conf["restore_clipboard"] and wants_paste else None) + paste.copy(text) + if wants_paste: + self.stage.emit(t("Pasting…")) + try: + paste.press(conf["paste_shortcut"], focus=focus) + except paste.PasteError as exc: + # The transcript is on the clipboard and in the history; + # a key press that would not land is a warning, not a + # failure, and the old clipboard is NOT put back over + # the text the user now has to paste by hand. + previous = None + warning = "\n".join(x for x in ( + warning, + t("Copied, but pasting failed: {error}", error=exc), + ) if x) + # The row above was written before the paste, so it + # has to be told what the paste then did. + record = cfg.amend_history( + record, cleanup_error=warning) or record + if previous is not None: + # Let the focused application consume the temporary + # transcription before putting every old clipboard type + # back. + time.sleep(0.35) + paste.copy_bytes(previous) + self.finished.emit(raw, text, warning) except assistant.Cancelled: self.cancelled.emit() except (api.ApiError, paste.PasteError, assistant.AssistantError) as exc: print(f"dikte: {exc}", file=sys.stderr) - self.failed.emit(str(exc)) + self.failed.emit(self._keeping(wav_path, str(exc))) except Exception as exc: # never fail silently traceback.print_exc() - self.failed.emit(t("Unexpected error: {error}", error=exc)) + self.failed.emit(self._keeping(wav_path, t("Unexpected error: {error}", + error=exc))) finally: self._discard(wav_path) + def _keeping(self, wav_path, message): + """Put the failed run's audio somewhere a retry can find it. + + A dictation that died on the way to the model is speech the user cannot + say again from memory; deleting it because a server was down turns one + failure into two. Kept regardless of the keep_audio setting, which is + about the runs that succeeded. + """ + kept = self._keep(wav_path) + if not kept: + return message + return message + "\n" + t("The recording was kept: {path}", path=kept) + + def _keep(self, wav_path): + """Move the WAV into the recordings directory; its new path, or ''.""" + try: + cfg.RECORDINGS_DIR.mkdir(parents=True, exist_ok=True) + base = time.strftime("%Y%m%d-%H%M%S") + # Two runs can finish inside the same second; the first one kept + # must not be overwritten by the second. + for suffix in ("",) + tuple(f"-{n}" for n in range(1, 100)): + target = cfg.RECORDINGS_DIR / f"{base}{suffix}.wav" + if not target.exists(): + shutil.move(wav_path, target) + return str(target) + return "" + except OSError as exc: + print(f"dikte: could not keep the audio: {exc}", file=sys.stderr) + return "" + def _discard(self, wav_path): if not os.path.exists(wav_path): return if self.conf["keep_audio"]: - try: - cfg.RECORDINGS_DIR.mkdir(parents=True, exist_ok=True) - shutil.move(wav_path, cfg.RECORDINGS_DIR / (time.strftime("%Y%m%d-%H%M%S") + ".wav")) + if self._keep(wav_path): return - except OSError: - pass + # The move failing is no reason to delete what the user asked to + # keep: the temporary file stays where it is, named in the log. + print(f"dikte: the audio stays at {wav_path}", file=sys.stderr) + return try: os.unlink(wav_path) except OSError: diff --git a/install.ps1 b/install.ps1 index c847131..67cf071 100644 --- a/install.ps1 +++ b/install.ps1 @@ -23,9 +23,20 @@ $autostartLink = Join-Path $startup "Dikte.lnk" $cmdShim = Join-Path $env:LOCALAPPDATA "Microsoft\WindowsApps\dikte.cmd" if ($Uninstall) { - foreach ($path in @($shortcut, $autostartLink, $cmdShim)) { + foreach ($path in @($shortcut, $autostartLink)) { if (Test-Path $path) { Remove-Item $path -Force; Write-Host "removed: $path" } } + # The packaged install writes the same shim, naming its own dikte-cli.exe. + # Only the one naming this checkout is ours to delete; taking the other + # would break the `dikte` command of an install this script never made. + if (Test-Path $cmdShim) { + if ((Get-Content $cmdShim -Raw).Contains($entry)) { + Remove-Item $cmdShim -Force + Write-Host "removed: $cmdShim" + } else { + Write-Host "left alone: $cmdShim (it names another install, not this checkout)" + } + } Write-Host "Dikte's shortcuts are gone. The repository and your settings are not." exit 0 } @@ -65,6 +76,15 @@ foreach ($path in @($shortcut) + $(if ($Autostart) { @($autostartLink) } else { Write-Host "shortcut: $path" } +if ($Autostart) { + # The packaged build keeps its sign-in entry in the registry. Left there + # beside the shortcut written above, both would start a Dikte at sign-in, + # and this install is the one being asked for. + Remove-ItemProperty -Path "HKCU:\Software\Microsoft\Windows\CurrentVersion\Run" ` + -Name "Dikte" -ErrorAction SilentlyContinue + Write-Host "autostart: the Startup shortcut replaces any registry Run entry a packaged install left" +} + # --- the dikte command ------------------------------------------------------ # The interpreter by its full path rather than by name: the one checked above is # the one the command line should run, whatever a later PATH change puts first. diff --git a/packaging/dikte.iss b/packaging/dikte.iss index d16d784..d8b5b0a 100644 --- a/packaging/dikte.iss +++ b/packaging/dikte.iss @@ -59,6 +59,13 @@ Name: "autostart"; Description: "Start Dikte when I sign in" [Files] Source: "{#Source}\*"; DestDir: "{app}"; Flags: recursesubdirs ignoreversion +[InstallDelete] +; A checkout's install.ps1 -Autostart is a shortcut in the Startup folder. The +; registry entry this setup writes replaces it, and both left in place would be +; two Diktes at every sign-in. Only under the autostart task: somebody who +; unticked the box has not asked for their checkout's entry to go. +Type: files; Name: "{userstartup}\Dikte.lnk"; Tasks: autostart + [Icons] Name: "{autoprograms}\Dikte"; Filename: "{app}\Dikte.exe" @@ -111,7 +118,14 @@ begin end; procedure CurUninstallStepChanged(CurUninstallStep: TUninstallStep); +var + Shim: AnsiString; begin + { Only the shim this setup wrote, which is the one naming its dikte-cli.exe. + install.ps1 writes the same file for a checkout, naming that checkout's + Python, and a shim somebody else wrote is not this uninstaller's to take. } if CurUninstallStep = usUninstall then - DeleteFile(ShimPath()); + if LoadStringFromFile(ShimPath(), Shim) + and (Pos(ExpandConstant('{app}\dikte-cli.exe'), Shim) > 0) then + DeleteFile(ShimPath()); end; diff --git a/tests/__init__.py b/tests/__init__.py index 5b00175..094b8ec 100644 --- a/tests/__init__.py +++ b/tests/__init__.py @@ -17,6 +17,7 @@ os.environ.setdefault("QT_QPA_PLATFORM", "offscreen") _SANDBOX = tempfile.mkdtemp(prefix="dikte-tests-") os.environ["XDG_CONFIG_HOME"] = os.path.join(_SANDBOX, "config") os.environ["XDG_DATA_HOME"] = os.path.join(_SANDBOX, "data") +os.environ["XDG_CACHE_HOME"] = os.path.join(_SANDBOX, "cache") # Home goes with them: the shortcut file, the applications directory and every # macOS path start from it rather than from an XDG variable, and a test run is # not allowed to touch the real one. diff --git a/tests/test_assistant.py b/tests/test_assistant.py index 1c682c6..faf6362 100644 --- a/tests/test_assistant.py +++ b/tests/test_assistant.py @@ -10,6 +10,8 @@ import io import json import os import subprocess +import sys +import threading import time import unittest from unittest import mock @@ -19,12 +21,16 @@ from tests.support import DikteTest, fake_urlopen, only_these_tools class FakeCli: - """A CLI that prints the given events and exits.""" + """A CLI that prints the given events and exits. - def __init__(self, events=(), code=0, stderr="", noise=()): + Its stderr is not modelled: _stream hands the process a temporary file for + that, and a mocked Popen leaves the file empty, which is what a quiet CLI + writes anyway. + """ + + def __init__(self, events=(), code=0, noise=()): lines = list(noise) + [json.dumps(event) for event in events] self.stdout = io.StringIO("\n".join(lines) + "\n") - self.stderr = io.StringIO(stderr) self.returncode = code self.killed = False @@ -41,6 +47,38 @@ class FakeCli: self.killed = True +class WedgedCli: + """A CLI whose stdout produces nothing, the way a hung process's does. + + Iterating its stdout blocks until the kill arrives, because that is what + reading a silent pipe does: only the process ending closes the stream. + """ + + def __init__(self): + self.pid = 4242 + self.returncode = None + self.released = threading.Event() + self.stdout = self + + def __iter__(self): + return self + + def __next__(self): + # The 5 second cap is a safety net for the test itself; the kill is + # what is supposed to end the wait. + self.released.wait(timeout=5) + raise StopIteration + + def poll(self): + return self.returncode + + def wait(self, timeout=None): + return self.returncode + + def close(self): + pass + + class Provider(DikteTest): def test_the_default(self): self.assertEqual(assistant.provider(self.config()), "claude") @@ -221,19 +259,7 @@ class Denials(DikteTest): self.assertIn("Write", warning) -class SessionMissing(unittest.TestCase): - def test_a_session_that_is_gone(self): - for text in ("Error: session abc not found", - "No conversation with that id", - "unknown thread: abc"): - with self.subTest(text=text): - self.assertTrue(assistant._session_missing(text)) - - def test_an_unrelated_failure(self): - for text in ("", "network unreachable", "session limit exceeded"): - with self.subTest(text=text): - self.assertFalse(assistant._session_missing(text)) - +class LastLine(unittest.TestCase): def test_the_last_line_is_the_one_worth_showing(self): self.assertEqual(assistant.last_line("warning\n\nreal error\n"), "real error") @@ -269,6 +295,28 @@ class Conclude(DikteTest): assistant._conclude(self.found(), 1, "session abc not found", "abc", "Claude") + def test_the_recovery_no_longer_hangs_on_the_words_the_cli_chose(self): + # The complaint used to be matched by substring, which a CLI update or + # another language broke. A resumed run that died with nothing to show + # is now enough on its own. + for stderr in ("Oturum bulunamadı", "something else entirely", ""): + with self.subTest(stderr=stderr): + with self.assertRaises(assistant._SessionGone): + assistant._conclude(self.found(), 1, stderr, "abc", "Claude") + + def test_api_trouble_on_a_resumed_run_is_not_blamed_on_the_session(self): + # A fresh session cannot cure a spent quota, a signed-out CLI or a dead + # network: the retry would fail the same way after a second wait, and + # the user would lose the conversation thread on top. + for stderr in ("Rate limit exceeded", + "You are not logged in. Please run /login.", + "API Error: 401 Unauthorized", + "fetch failed: ECONNREFUSED 127.0.0.1"): + with self.subTest(stderr=stderr): + with self.assertRaises(assistant.AssistantError) as caught: + assistant._conclude(self.found(), 1, stderr, "abc", "Claude") + self.assertIn(stderr, str(caught.exception)) + def test_a_session_that_is_gone_only_matters_when_one_was_resumed(self): with self.assertRaises(assistant.AssistantError): assistant._conclude(self.found(), 1, "session abc not found", @@ -279,6 +327,11 @@ class Conclude(DikteTest): "", "Claude") self.assertEqual(answer, "done") + def test_an_answer_on_a_resumed_session_is_kept_rather_than_retried(self): + answer, _ = assistant._conclude(self.found(answer="done"), 1, "noise", + "abc", "Claude") + self.assertEqual(answer, "done") + def test_a_reported_failure_with_no_answer(self): with self.assertRaises(assistant.AssistantError) as caught: assistant._conclude(self.found(failure="the model refused"), 0, "", @@ -291,14 +344,53 @@ class Conclude(DikteTest): self.assertIn("Codex", str(caught.exception)) +class Stream(DikteTest): + def test_a_cli_that_floods_stderr_still_finishes(self): + # A real subprocess, because the wedge being tested is real plumbing: + # with stderr on a pipe nobody drains, 200 KB fills the pipe's buffer, + # the child blocks writing it, and the run hangs until the watchdog + # timeout. With stderr on a file the run completes at once. + script = ( + "import sys\n" + "sys.stderr.write('x' * 200000)\n" + "sys.stderr.flush()\n" + "print('{\"type\": \"result\", \"result\": \"done\"}')\n" + ) + conf = self.config(assistant_timeout=15) + events = [] + code, stderr = assistant._stream( + [sys.executable, "-c", script], conf, events.append, None) + self.assertEqual(code, 0) + self.assertEqual(len(stderr), 200000) + self.assertEqual(events[-1]["result"], "done") + + def test_the_watchdog_takes_the_whole_tree_down_on_timeout(self): + proc = WedgedCli() + + def killed(target): + # What the real kill does, as far as _stream can see: the process + # ends, and its closing stream releases the blocked read. + target.returncode = 1 + target.released.set() + + conf = self.config(assistant_timeout=0) + with mock.patch.object(subprocess, "Popen", return_value=proc), \ + mock.patch.object(assistant, "kill_tree", + side_effect=killed) as kill: + with self.assertRaises(assistant.AssistantError) as caught: + assistant._stream(["claude"], conf, lambda event: None, None) + kill.assert_called_once_with(proc) + self.assertIn("did not finish", str(caught.exception)) + + class AskClaude(DikteTest): - def run_ask(self, conf=None, events=None, code=0, stderr="", noise=(), + def run_ask(self, conf=None, events=None, code=0, noise=(), session=""): conf = conf or self.config() proc = FakeCli(events or [ {"type": "system", "subtype": "init", "session_id": "abc"}, {"type": "result", "session_id": "abc", "result": " done "}, - ], code=code, stderr=stderr, noise=noise) + ], code=code, noise=noise) stages = [] with only_these_tools("claude", "codex"), \ mock.patch.object(subprocess, "Popen", return_value=proc) as popen: @@ -533,6 +625,44 @@ class Ask(DikteTest): self.assertEqual(attempts, ["stale-id", ""]) self.assertEqual(assistant.stored_provider(), "") + def test_a_resumed_run_that_dies_is_retried_without_the_session_flag(self): + # All the way through the stream this time: the first run exits 1 with + # no answer and whatever stderr it liked, and the recovery must not + # depend on those words. + conf = self.config() + assistant.write_session("claude", "stale-id") + procs = iter([ + FakeCli(code=1), + FakeCli(events=[{"type": "result", "result": "done"}]), + ]) + cmds = [] + + def popen(cmd, **kwargs): + cmds.append(cmd) + return next(procs) + + with only_these_tools("claude"), \ + mock.patch.object(subprocess, "Popen", side_effect=popen): + answer, _ = assistant.ask("hi", conf) + self.assertEqual(answer, "done") + self.assertEqual(cmds[0][cmds[0].index("--resume") + 1], "stale-id") + self.assertNotIn("--resume", cmds[1]) + + def test_a_run_that_dies_with_an_answer_in_hand_is_not_retried(self): + conf = self.config() + assistant.write_session("claude", "stale-id") + calls = [] + + def popen(cmd, **kwargs): + calls.append(cmd) + return FakeCli(events=[{"type": "result", "result": "done"}], code=1) + + with only_these_tools("claude"), \ + mock.patch.object(subprocess, "Popen", side_effect=popen): + answer, _ = assistant.ask("hi", conf) + self.assertEqual(answer, "done") + self.assertEqual(len(calls), 1) + if __name__ == "__main__": unittest.main() diff --git a/tests/test_audio.py b/tests/test_audio.py index a7ea353..41e0621 100644 --- a/tests/test_audio.py +++ b/tests/test_audio.py @@ -81,6 +81,14 @@ class ChunkLevels(unittest.TestCase): self.assertEqual(peak, 1.0) self.assertEqual(rms, 1.0) + def test_the_fast_and_plain_rms_paths_agree(self): + """sumprod is a speedup, not a different sum: on a 3.11 machine the + loop must land on the same integers.""" + chunk = tone(0.1) + with mock.patch.object(audio, "sumprod", None): + plain = audio.chunk_levels(chunk) + self.assertEqual(audio.chunk_levels(chunk), plain) + class StereoLevels(unittest.TestCase): def test_the_channels_are_read_apart(self): @@ -280,6 +288,48 @@ class _StalledStream: self._released.set() +class _DribblingStream: + """A pipe that never fills a whole chunk in one read, the way an unbuffered + pipe hands data over under load.""" + + def __init__(self, data, piece): + self._data = io.BytesIO(data) + self._piece = piece + + def read(self, size): + return self._data.read(min(size, self._piece)) + + +class _RunSwappingStream: + """A pipe whose recorder moves on mid-read, the way a new recording starts + while a stale pump is still draining the old one.""" + + def __init__(self, data, recorder, swap_at, new_proc): + self._data = io.BytesIO(data) + self._recorder = recorder + self._swap_at = swap_at + self._new_proc = new_proc + self.reads = 0 + + def read(self, size): + if self.reads == self._swap_at: + self._recorder._run = object() + self._recorder._proc = self._new_proc + self.reads += 1 + return self._data.read(size) + + +class _SignalCrashingProcess(FakeProcess): + """An ffmpeg that calls being interrupted a failure, the way ffmpeg does.""" + + def __init__(self, data, code=255): + super().__init__(data) + self._code = code + + def poll(self): + return None if self._alive else self._code + + class _HeldStream: """A capture that is paused and taken up again partway through, the way a key press lands in the middle of a recording rather than between two.""" @@ -307,7 +357,10 @@ class RecordingCommand(OnLinux, DikteTest): super().setUp() # Whether pw-record takes --raw is read off the installed binary, and # what is being tested here is the command rather than the machine the - # test is running on. PwRecordRawOption covers the reading itself. + # test is running on. PwRecordRawOption covers the reading itself. The + # answer is remembered between calls, so it cannot be remembered + # between tests. + self.enterContext(mock.patch.object(audio, "_PW_RAW", None)) self.enterContext(mock.patch.object( audio, "_pw_record_raw_option", return_value=["--raw"])) @@ -392,11 +445,30 @@ class PwRecordRawOption(DikteTest): self.assertEqual(["--raw"], self.option(return_value=FakeCompleted())) +class PwRawMemo(OnLinux, DikteTest): + """The --raw probe runs once per process, not once per key press.""" + + def setUp(self): + super().setUp() + self.enterContext(mock.patch.object(audio, "_PW_RAW", None)) + + def test_the_probe_is_asked_once_and_remembered(self): + with only_these_tools("pw-record"), \ + mock.patch.object(audio, "_pw_record_raw_option", + return_value=["--raw"]) as probe: + first = audio.recording_command() + second = audio.recording_command() + probe.assert_called_once_with() + self.assertIn("--raw", first) + self.assertEqual(first, second) + + class RecorderChain(OnLinux, DikteTest): """Start to WAV, with pw-record faked out.""" def setUp(self): super().setUp() + self.enterContext(mock.patch.object(audio, "_PW_RAW", None)) self.enterContext(mock.patch.object( audio, "_pw_record_raw_option", return_value=["--raw"])) @@ -476,42 +548,121 @@ class RecorderChain(OnLinux, DikteTest): self.assertEqual(len(failures), 1) self.assertIn("pulseaudio-utils", failures[0]) - def pump(self, data=b"", stderr=b"", stopping=False, cancelled=False): + def pump(self, data=b"", stderr=b"", stopping=False, cancelled=False, + alive=False): """Run the pump in this thread, where a queued signal would need an event loop nobody is running here.""" recorder = audio.Recorder() failures = [] + deaths = [] recorder.failed.connect(failures.append) + recorder.died.connect(lambda: deaths.append(True)) proc = FakeProcess(data) - proc.stderr = io.BytesIO(stderr) - proc._alive = False + proc._alive = alive recorder._proc = proc - recorder._max_bytes = 10 ** 9 + recorder._log = io.BytesIO(stderr) recorder._stopping = stopping recorder._cancelled = cancelled - recorder._pump() - return failures + recorder._run = run = object() + recorder._pump(run, proc, proc.stdout, recorder._buffer, + recorder._rms, 10 ** 9) + return failures, deaths def test_a_recorder_that_died_on_its_own_says_so(self): """parec refused the device, or the sound server went away.""" - failures = self.pump(stderr=b"connection refused\n") + failures, _ = self.pump(stderr=b"connection refused\n") self.assertEqual(len(failures), 1) self.assertIn("connection refused", failures[0]) def test_a_death_with_nothing_on_stderr_still_names_the_exit_code(self): - failures = self.pump() + failures, _ = self.pump() self.assertIn("exit code", failures[0]) + def test_a_death_that_left_no_exit_code_is_not_named_none(self): + """A process nobody managed to reap has no code to show, and "exit + code None" would only puzzle the person reading it.""" + failures, _ = self.pump(alive=True) + self.assertEqual(len(failures), 1) + self.assertNotIn("None", failures[0]) + def test_a_recording_we_ended_ourselves_is_not_a_death(self): """Otherwise a stray keypress produces two errors, and the first one sends the user looking for a broken sound server.""" - self.assertEqual(self.pump(stopping=True), []) + self.assertEqual(self.pump(stopping=True), ([], [])) def test_a_cancelled_recording_is_not_a_death(self): - self.assertEqual(self.pump(cancelled=True), []) + self.assertEqual(self.pump(cancelled=True), ([], [])) - def test_a_recorder_that_captured_something_first_is_not_a_death(self): - self.assertEqual(self.pump(data=silence(0.5)), []) + def test_a_capture_that_ends_mid_recording_dies_rather_than_fails(self): + """Sound had already arrived, so this is not a broken installation: + the app is told the recording died and can rescue what there is.""" + failures, deaths = self.pump(data=silence(0.5)) + self.assertEqual(failures, []) + self.assertEqual(deaths, [True]) + + def test_short_pipe_reads_are_gathered_into_whole_chunks(self): + """Every RMS entry must stand for one full chunk, or the silence check + weighs a half-filled read as its own stretch of room tone.""" + half = audio.CHUNK_BYTES // 2 + data = pcm([1000] * (3 * half // 2)) # three half-chunk reads + recorder = audio.Recorder() + proc = FakeProcess(b"") + proc.stdout = _DribblingStream(data, half) + proc._alive = False + recorder._proc = proc + recorder._log = io.BytesIO(b"") + recorder._run = run = object() + buffer, rms = bytearray(), [] + recorder._pump(run, proc, proc.stdout, buffer, rms, 10 ** 9) + self.assertEqual(len(buffer), len(data)) + self.assertEqual(len(rms), 2) # one whole chunk, then the tail + + def test_a_stale_pump_cannot_touch_the_recording_that_replaced_it(self): + """A pump that outlives its join must not meter the next run, stop its + process, push audio into its buffer, or speak on its behalf.""" + recorder = audio.Recorder() + levels, failures, deaths = [], [], [] + recorder.level.connect(levels.append) + recorder.failed.connect(failures.append) + recorder.died.connect(lambda: deaths.append(True)) + new_proc = FakeProcess(b"") + old_proc = FakeProcess(b"") + old_proc.stdout = _RunSwappingStream(tone(0.192), recorder, + swap_at=1, new_proc=new_proc) + recorder._proc = old_proc + recorder._log = io.BytesIO(b"") + old_run = object() + recorder._run = old_run + recorder._buffer = bytearray() # the next recording's buffer + old_buffer, old_rms = bytearray(), [] + recorder._pump(old_run, old_proc, old_proc.stdout, old_buffer, old_rms, + 2 * audio.CHUNK_BYTES) + # Metered once, then the new run took over: the over-length cutoff hit + # on the next chunk and had to stand down instead of stopping a + # process that was never its own. + self.assertEqual(len(levels), 1) + self.assertEqual(len(old_buffer), 2 * audio.CHUNK_BYTES) + self.assertEqual(new_proc.signals, []) + self.assertEqual(old_proc.signals, []) + self.assertEqual(recorder._buffer, bytearray()) + self.assertEqual((failures, deaths), ([], [])) + + def test_a_wav_that_cannot_be_written_is_reported_not_raised(self): + recorder = audio.Recorder() + results, failures = [], [] + recorder.stopped.connect(lambda *args: results.append(args)) + recorder.failed.connect(failures.append) + proc = FakeProcess(tone(1.0)) + with only_these_tools("pw-record"), \ + mock.patch.object(subprocess, "Popen", return_value=proc), \ + mock.patch.object(audio, "write_wav", + side_effect=OSError("disk full")): + recorder.start() + recorder._thread.join(timeout=5) + recorder.stop() + self.assertEqual(results, []) + self.assertEqual(len(failures), 1) + self.assertIn("disk full", failures[0]) def test_a_short_recording_reports_only_that(self): _, results, failures, _ = self.record(silence(0.1)) @@ -722,6 +873,42 @@ class MacMeetingRecorder(OnMacOS, DikteTest): _, _, _, _, processes, _ = self.record(tone(0.5), tone(0.5)) self.assertTrue(all(process.signals for process in processes)) + def test_a_stop_we_asked_for_is_not_reported_as_an_ffmpeg_failure(self): + """ffmpeg exits 255 when interrupted, and the interruption was our own + stop: a meeting ended at once must say "too short", not "ffmpeg → 255".""" + path = str(self.path("meeting.wav")) + recorder = audio.MeetingRecorder() + failed = [] + recorder.failed.connect(failed.append) + processes = [_SignalCrashingProcess(tone(0.1)), + _SignalCrashingProcess(tone(0.1))] + with only_these_tools("ffmpeg"), self.devices(), \ + mock.patch.object(subprocess, "Popen", side_effect=processes): + recorder.start(path, "MacBook Pro Microphone", "BlackHole 2ch") + recorder._thread.join(timeout=5) + recorder.stop() + self.assertEqual(len(failed), 1) + self.assertIn("0.3", failed[0]) + self.assertNotIn("255", failed[0]) + + def test_an_ffmpeg_that_died_on_its_own_keeps_its_exit_code(self): + """A process nobody interrupted has a story to tell, and its code is + the only lead the user gets.""" + path = str(self.path("meeting.wav")) + recorder = audio.MeetingRecorder() + failed = [] + recorder.failed.connect(failed.append) + dead = _SignalCrashingProcess(tone(0.1)) + dead._alive = False # it fell over before stop() reached it + processes = [dead, FakeProcess(tone(0.1))] + with only_these_tools("ffmpeg"), self.devices(), \ + mock.patch.object(subprocess, "Popen", side_effect=processes): + recorder.start(path, "MacBook Pro Microphone", "BlackHole 2ch") + recorder._thread.join(timeout=5) + recorder.stop() + self.assertEqual(len(failed), 1) + self.assertIn("255", failed[0]) + def test_a_legacy_numeric_target_fails_before_recording(self): recorder = audio.MeetingRecorder() failed = [] @@ -953,9 +1140,11 @@ class WindowsDevices(OnWindows, DikteTest): def setUp(self): super().setUp() # The listing is remembered between calls, so that a dictation does not - # run ffmpeg of its own. It cannot be remembered between tests. + # run ffmpeg of its own. It cannot be remembered between tests, and + # neither can the pw-record probe's answer. audio._DSHOW_SEEN.clear() self.addCleanup(audio._DSHOW_SEEN.clear) + self.enterContext(mock.patch.object(audio, "_PW_RAW", None)) @contextlib.contextmanager def listing(self, stderr=None, tools=("ffmpeg",)): diff --git a/tests/test_cleanup.py b/tests/test_cleanup.py index 7753029..e4222a5 100644 --- a/tests/test_cleanup.py +++ b/tests/test_cleanup.py @@ -1,9 +1,9 @@ -"""Who cleans the transcript up, and what they are asked. +"""Who cleans the transcript up, and what they are asked. -The CLIs are faked at subprocess.run: what the tests read is the argument list -each one is given, where the answer is picked up from, and what happens to the -chain when the program is missing, slow or unhappy. The OpenRouter path is the -one that was always there and is checked here only for still being taken. +The CLIs are faked at subprocess.Popen: what the tests read is the argument +list each one is given, where the answer is picked up from, and what happens to +the chain when the program is missing, slow or unhappy. The OpenRouter path is +the one that was always there and is checked here only for still being taken. """ import os @@ -18,18 +18,26 @@ from tests.support import DikteTest, fake_urlopen, sent_json, url_error from tests.test_api import FakeServer, chat_reply -def fake_run(stdout="", code=0, stderr="", last_message=""): - """Stand in for subprocess.run, writing the file Codex would have written.""" +def fake_cli(stdout="", code=0, stderr="", last_message=""): + """Stand in for subprocess.Popen. + + _output hands the process a temporary file for each stream, so the fake + writes into those, plus the file Codex would have written on its way out. + """ calls = [] - def run(cmd, **kwargs): + def popen(cmd, **kwargs): calls.append(cmd) + kwargs["stdout"].write(stdout.encode("utf-8")) + kwargs["stderr"].write(stderr.encode("utf-8")) if last_message and "-o" in cmd: with open(cmd[cmd.index("-o") + 1], "w", encoding="utf-8") as fh: fh.write(last_message) - return subprocess.CompletedProcess(cmd, code, stdout, stderr) + proc = mock.Mock() + proc.returncode = code + return proc - return mock.patch.object(subprocess, "run", side_effect=run), calls + return mock.patch.object(subprocess, "Popen", side_effect=popen), calls class Provider(DikteTest): @@ -80,7 +88,7 @@ class OpenRouter(DikteTest): def test_no_cli_is_started_for_it(self): conf = self.config(openrouter_api_key="sk-or-test") - patcher, calls = fake_run(stdout="never") + patcher, calls = fake_cli(stdout="never") with patcher, mock.patch.object(api, "cleanup", return_value="Done."): cleanup.run("uh, done", conf, "the rules") self.assertEqual(calls, []) @@ -93,7 +101,7 @@ class ClaudeCode(DikteTest): self.patch_attr(cleanup.shutil, "which", lambda name: f"/usr/bin/{name}") def run_cleanup(self, text="uh, book it", **kwargs): - patcher, calls = fake_run(**kwargs) + patcher, calls = fake_cli(**kwargs) with patcher: answer = cleanup.run(text, self.conf, "the rules") return answer, calls[0] @@ -146,14 +154,18 @@ class ClaudeCode(DikteTest): self.run_cleanup(stdout="Book it.") self.assertIn("claude", str(caught.exception)) - def test_a_run_that_never_ends(self): - def run(cmd, **kwargs): - raise subprocess.TimeoutExpired(cmd, 180) + def test_a_run_that_never_ends_is_killed_with_its_whole_tree(self): + def popen(cmd, **kwargs): + proc = mock.Mock() + proc.wait.side_effect = subprocess.TimeoutExpired(cmd, 180) + return proc - with mock.patch.object(subprocess, "run", side_effect=run): + with mock.patch.object(subprocess, "Popen", side_effect=popen), \ + mock.patch.object(cleanup.assistant, "kill_tree") as kill: with self.assertRaises(cleanup.CleanupError) as caught: cleanup.run("uh, book it", self.conf, "the rules") self.assertIn("180", str(caught.exception)) + kill.assert_called_once() class Codex(DikteTest): @@ -163,7 +175,7 @@ class Codex(DikteTest): self.patch_attr(cleanup.shutil, "which", lambda name: f"/usr/bin/{name}") def run_cleanup(self, text="uh, book it", **kwargs): - patcher, calls = fake_run(**kwargs) + patcher, calls = fake_cli(**kwargs) with patcher: answer = cleanup.run(text, self.conf, "the rules") return answer, calls[0] @@ -280,7 +292,8 @@ class Here(DikteTest): self.assertIn("out of memory", str(caught.exception)) def test_no_cli_is_started_for_it(self): - patcher, calls = fake_run(stdout="never") + patcher, calls = fake_cli(stdout="never") with patcher, fake_urlopen(chat_reply("Done.")): cleanup.run("uh, done", self.conf, "the rules") self.assertEqual(calls, []) + diff --git a/tests/test_config.py b/tests/test_config.py index 9d8e372..a7b6211 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -8,7 +8,10 @@ config and now shadows the default. import json import os +import pathlib import sys +import threading +import time import unittest from unittest import mock @@ -44,6 +47,21 @@ class Loading(DikteTest): conf = cfg.Config() self.assertEqual(conf["cleanup_model"], cfg.DEFAULTS["cleanup_model"]) + def test_a_config_that_is_not_json_is_set_aside_as_evidence(self): + """Left in place it would be overwritten by the very next save.""" + cfg.CONFIG_DIR.mkdir(parents=True, exist_ok=True) + cfg.CONFIG_FILE.write_text("{not json", encoding="utf-8") + broken = cfg.CONFIG_FILE.with_suffix(".json.broken") + with mock.patch("builtins.print") as told: + conf = cfg.Config() + self.assertEqual(broken.read_text(encoding="utf-8"), "{not json") + self.assertFalse(cfg.CONFIG_FILE.exists()) + self.assertIn(str(broken), told.call_args[0][0]) + conf.save() + self.assertEqual(broken.read_text(encoding="utf-8"), "{not json") + self.assertEqual(self.read_config_file()["cleanup_model"], + cfg.DEFAULTS["cleanup_model"]) + def test_a_config_that_is_json_but_not_an_object(self): cfg.CONFIG_DIR.mkdir(parents=True, exist_ok=True) cfg.CONFIG_FILE.write_text("[1, 2]", encoding="utf-8") @@ -113,6 +131,41 @@ class Saving(DikteTest): conf.save() self.assertEqual(i18n.language(), "tr") + def test_the_settings_hit_the_disk_before_the_swap(self): + """Renaming a file still in the page cache into place makes a power + cut a settings wipe, which is what the atomic replace exists to stop.""" + with mock.patch("os.fsync") as fsync: + cfg.Config().save() + fsync.assert_called_once() + + def test_a_file_held_briefly_by_a_scanner_does_not_fail_the_save(self): + """Antivirus and sync tools on Windows hold a fresh file for a moment, + and the rename over it fails until they let go.""" + attempts = [] + real_replace = pathlib.Path.replace + + def flaky(path, target): + attempts.append(str(target)) + if len(attempts) < 3: + raise PermissionError("held by a scanner") + return real_replace(path, target) + + with mock.patch.object(pathlib.Path, "replace", flaky), \ + mock.patch("time.sleep"): + cfg.Config().save() + self.assertEqual(len(attempts), 3) + self.assertEqual(self.read_config_file()["language"], + cfg.DEFAULTS["language"]) + + def test_a_file_held_for_good_still_raises(self): + def held(path, target): + raise PermissionError("never let go") + + with mock.patch.object(pathlib.Path, "replace", held), \ + mock.patch("time.sleep"): + with self.assertRaises(PermissionError): + cfg.Config().save() + class Keys(DikteTest): def test_a_stored_key_is_used(self): @@ -350,6 +403,23 @@ class History(DikteTest): cfg.delete_history([]) self.assertEqual(len(cfg.read_history()), 1) + def test_amending_matches_on_content_and_patches_in_place(self): + rows = [self.entry("a"), self.entry("b")] + for row in rows: + cfg.append_history(row) + patched = cfg.amend_history(rows[0], cleanup_error="could not paste") + self.assertEqual(patched["cleanup_error"], "could not paste") + kept = cfg.read_history() + self.assertEqual([row["text"] for row in kept], ["a", "b"]) + self.assertEqual(kept[0]["cleanup_error"], "could not paste") + + def test_amending_a_row_a_trim_took_away_is_a_no_op(self): + row = self.entry("gone") + cfg.append_history(row) + cfg.clear_history() + self.assertIsNone(cfg.amend_history(row, cleanup_error="x")) + self.assertEqual(cfg.read_history(), []) + def test_clearing(self): cfg.append_history(self.entry("a")) cfg.clear_history() @@ -358,6 +428,40 @@ class History(DikteTest): def test_clearing_a_history_that_is_not_there(self): cfg.clear_history() # must not raise + def test_an_append_during_a_trim_is_not_lost(self): + """Trim is read, cut, rewrite; a dictation appended between the read + and the rewrite must wait rather than be erased by a rewrite that + never saw it. The rewrite is slowed down to hold the race open.""" + for index in range(10): + cfg.append_history(self.entry(str(index))) + real_write = cfg._write_history + rewriting = threading.Event() + + def slow_write(lines): + rewriting.set() + time.sleep(0.1) + real_write(lines) + + with mock.patch.object(cfg, "_write_history", slow_write): + trimmer = threading.Thread(target=cfg.trim_history, args=(3,)) + trimmer.start() + # The trim now holds the lock inside its read-cut-rewrite window. + self.assertTrue(rewriting.wait(5)) + appender = threading.Thread(target=cfg.append_history, + args=(self.entry("late"),)) + appender.start() + trimmer.join() + appender.join() + self.assertEqual([row["text"] for row in cfg.read_history()], + ["7", "8", "9", "late"]) + + def test_the_rewrite_hits_the_disk_before_the_swap(self): + for index in range(5): + cfg.append_history(self.entry(str(index))) + with mock.patch("os.fsync") as fsync: + cfg.trim_history(2) + fsync.assert_called_once() + class Meetings(DikteTest): def entry(self, base, **changes): @@ -430,6 +534,27 @@ class Meetings(DikteTest): cfg.delete_meetings([]) self.assertEqual(len(cfg.read_meetings()), 1) + def test_the_index_hits_the_disk_before_the_swap(self): + with mock.patch("os.fsync") as fsync: + cfg.save_meeting(self.entry("a")) + fsync.assert_called_once() + + def test_an_index_held_briefly_by_a_scanner_is_still_written(self): + real_replace = pathlib.Path.replace + attempts = [] + + def flaky(path, target): + attempts.append(str(target)) + if len(attempts) < 3: + raise PermissionError("held by a scanner") + return real_replace(path, target) + + with mock.patch.object(pathlib.Path, "replace", flaky), \ + mock.patch("time.sleep"): + cfg.save_meeting(self.entry("a")) + self.assertEqual(len(attempts), 3) + self.assertEqual([row["base"] for row in cfg.read_meetings()], ["a"]) + class Defaults(unittest.TestCase): """The table itself, which every command line and settings tab reads.""" diff --git a/tests/test_filetranscribe.py b/tests/test_filetranscribe.py index 5bb2f75..9931dfa 100644 --- a/tests/test_filetranscribe.py +++ b/tests/test_filetranscribe.py @@ -235,6 +235,28 @@ class ChunkSeconds(DikteTest): self.assertEqual(ft.chunk_seconds(self.file(ft.UPLOAD_LIMIT * 2), 0), 0.0) +class Ffmpeg(DikteTest): + """How the converter process is started.""" + + def test_its_output_is_read_as_utf8_whatever_the_locale_says(self): + """ffmpeg writes UTF-8; read as the locale codepage its messages + mojibake, and a byte the codepage cannot place raises from inside + communicate itself.""" + out = str(self.path("out.wav")) + with open(out, "wb") as fh: + fh.write(b"\x00") + proc = mock.Mock() + proc.communicate.return_value = ("", "") + proc.returncode = 0 + proc.poll.return_value = 0 + with mock.patch.object(ft.subprocess, "Popen", return_value=proc) as popen: + ft._ffmpeg(["-i", "in.mp4", out], out) + kwargs = popen.call_args.kwargs + self.assertTrue(kwargs["text"]) + self.assertEqual(kwargs["encoding"], "utf-8") + self.assertEqual(kwargs["errors"], "replace") + + class Chunks(DikteTest): """What each provider is handed, and in how many pieces.""" diff --git a/tests/test_ggml.py b/tests/test_ggml.py index 14a8ddf..44b9305 100644 --- a/tests/test_ggml.py +++ b/tests/test_ggml.py @@ -9,6 +9,7 @@ import contextlib import hashlib import io import os +import pathlib import signal import sys import tarfile @@ -179,6 +180,21 @@ class Download(Local): ggml.download(item("m.bin", b"x"), target) self.assertFalse(target.exists()) + def test_a_target_held_open_keeps_the_finished_download(self): + # Windows refuses to replace a file a running server holds open. The + # bytes are complete and verified by then, so the .part must survive + # the failure rather than being deleted with everything else. + data = b"a finished, verified download" + target = self.path("data", "models", "m.bin") + with fake_urlopen(body(data)): + with mock.patch.object(pathlib.Path, "replace", + side_effect=PermissionError(13, "in use")): + with self.assertRaises(ggml.LocalError) as caught: + ggml.download(item("m.bin", data), target) + self.assertIn("held open", str(caught.exception)) + self.assertEqual(target.with_name("m.bin.part").read_bytes(), data) + self.assertFalse(target.exists()) + # --- installing a program ------------------------------------------------- @@ -275,6 +291,70 @@ class InstallProgram(Local): self.install("whisper-bin-ubuntu-x64.tar.gz", archive=empty) self.assertIn("whisper-server", str(caught.exception)) + def test_a_failed_update_leaves_the_working_install_alone(self): + # The old install used to be deleted before the new bytes had even + # arrived, so a bad download left no local server at all. + path, _ = self.install("whisper-bin-ubuntu-x64.tar.gz") + with self.assertRaises(ggml.LocalError): + self.install("whisper-bin-ubuntu-x64.tar.gz", + archive=b"not an archive at all") + self.assertEqual(ggml.installed_program(ggml.WHISPER), path) + self.assertTrue(os.path.isfile(path)) + self.assertEqual(ggml.installed_version(ggml.WHISPER), "v1.9.1") + # And the half-made sibling did not linger either. + left = list(self.path("data", "bin", "whisper").glob("*.new")) + self.assertEqual(left, []) + + def test_an_update_that_never_downloaded_leaves_the_install_alone(self): + path, _ = self.install("whisper-bin-ubuntu-x64.tar.gz") + listing = self.release("whisper-bin-ubuntu-x64.tar.gz") + with serving(listing, self.archive): + with mock.patch.object(ggml, "download", + side_effect=ggml.LocalError("no route")): + with self.assertRaises(ggml.LocalError): + ggml.install_program(ggml.WHISPER) + self.assertEqual(ggml.installed_program(ggml.WHISPER), path) + self.assertTrue(os.path.isfile(path)) + + class StubServer: + """Owns the installed binary, remembers when it was told to stop.""" + + def __init__(self): + self.program = ggml.WHISPER + self.stops = 0 + self.new_version_was_ready = False + + def settings(self): + return {"binary": ""} + + def stop(self): + self.stops += 1 + self.new_version_was_ready = any( + (ggml.BIN_DIR / "whisper").glob("*.new")) + + def test_the_running_server_is_stopped_only_for_the_swap(self): + # The outage is the swap, not the transfer: the server keeps answering + # through a download that can take minutes, and is stopped only once + # the replacement is unpacked next door and known to be whole. + self.install("whisper-bin-ubuntu-x64.tar.gz") + server = self.StubServer() + with mock.patch.object(ggml, "SERVERS", (server,)): + self.install("whisper-bin-ubuntu-x64.tar.gz") + self.assertEqual(server.stops, 1) + self.assertTrue(server.new_version_was_ready) + + def test_a_download_that_fails_never_stops_the_server(self): + self.install("whisper-bin-ubuntu-x64.tar.gz") + server = self.StubServer() + listing = self.release("whisper-bin-ubuntu-x64.tar.gz") + with mock.patch.object(ggml, "SERVERS", (server,)): + with serving(listing, self.archive): + with mock.patch.object(ggml, "download", + side_effect=ggml.LocalError("no route")): + with self.assertRaises(ggml.LocalError): + ggml.install_program(ggml.WHISPER) + self.assertEqual(server.stops, 0) + def test_a_release_without_a_published_checksum_is_refused(self): # GitHub did not always publish one, and whisper.cpp v1.8.0 and older @@ -456,12 +536,14 @@ STAND_IN = textwrap.dedent(""" def opt(name, default=""): return args[args.index(name) + 1] if name in args else default + time.sleep(float(opt("--wait", "0"))) + + # After the sleep, so that --wait plus --die is a program that runs for a + # while and then crashes, the way a bad model dies mid-load. if "--die" in args: print("could not load model: no such file") sys.exit(2) - time.sleep(float(opt("--wait", "0"))) - started = time.monotonic() healthy_after = float(opt("--healthy-after", "0")) @@ -539,6 +621,14 @@ class Servers(Local): self.assertTrue(server.running) self.assertTrue(second) + def count_ports(self): + """Record every port handed to a launch, one per attempt.""" + ports = [] + real = ggml._free_port + self.patch_attr(ggml, "_free_port", + lambda: ports.append(real()) or ports[-1]) + return ports + def test_a_program_that_dies_reports_what_it_printed(self): server = self.server(extra=["--die"]) with self.assertRaises(ggml.LocalError) as caught: @@ -546,6 +636,46 @@ class Servers(Local): self.assertIn("no such file", str(caught.exception)) self.assertFalse(server.running) + def test_an_early_death_that_never_listened_is_retried(self): + # A child that loses the bind race fails and exits at once, and which + # port was lost cannot be told from the log: the shape of the failure, + # not its wording, is what earns another port. + ports = self.count_ports() + server = self.server(extra=["--die"]) + with self.assertRaises(ggml.LocalError): + server.serve() + self.assertEqual(len(ports), 3) + + def test_a_late_crash_is_not_retried(self): + # A program that ran for a while before dying was not a bind race: it + # would die the same way on any port. + self.patch_attr(ggml, "EARLY_EXIT_WINDOW", 0.2) + ports = self.count_ports() + server = self.server(extra=["--wait", "0.5", "--die"]) + with self.assertRaises(ggml.LocalError) as caught: + server.serve() + self.assertEqual(len(ports), 1) + self.assertIn("no such file", str(caught.exception)) + + def test_an_open_port_with_a_dead_child_is_not_ready(self): + # Another process winning the bind race leaves the port open while our + # child exits: the open port alone must not be read as ready. + polls = iter([None, 2]) + proc = mock.Mock() + proc.poll = lambda: next(polls) + self.patch_attr(ggml, "_listening", lambda port: True) + reason, listened = self.server()._wait_ready(proc, 1) + self.assertEqual(reason, "exited") + self.assertFalse(listened) + + def test_an_open_port_with_a_child_that_outlives_it_a_beat_is_ready(self): + proc = mock.Mock() + proc.poll = lambda: None + self.patch_attr(ggml, "_listening", lambda port: True) + reason, listened = self.server()._wait_ready(proc, 1) + self.assertEqual(reason, "ready") + self.assertTrue(listened) + def test_a_model_that_is_still_loading_is_not_ready_yet(self): # llama binds its port first and answers /health with 503 until the # model is in memory, so the open port on its own is not the signal. @@ -614,6 +744,72 @@ class Servers(Local): def test_no_pid_file_is_nothing_to_sweep(self): self.assertFalse(self.server().sweep()) + def test_an_unverifiable_pid_is_kept_for_a_later_sweep(self): + # Could not be checked is not the same as known stale: dropping the + # file here would lose track of a server that may still hold a model. + server = self.server() + server._remember(4242) + with mock.patch.object(server, "_is_ours", return_value=None): + self.assertFalse(server.sweep()) + self.assertTrue(server._pid_file().exists()) + + def test_a_pid_known_stale_is_forgotten(self): + server = self.server() + server._remember(4242) + with mock.patch.object(server, "_is_ours", return_value=False): + self.assertFalse(server.sweep()) + self.assertFalse(server._pid_file().exists()) + + def test_the_pid_file_goes_once_the_kill_was_attempted(self): + server = self.server() + server._remember(4242) + with mock.patch.object(server, "_is_ours", return_value=True): + with mock.patch.object(ggml.os, "kill") as kill: + self.assertTrue(server.sweep()) + kill.assert_called_once_with(4242, signal.SIGTERM) + self.assertFalse(server._pid_file().exists()) + + def test_forget_leaves_a_pid_file_that_is_no_longer_ours(self): + server = self.server() + server._remember(111) + # A Dikte started after us wrote its own server's pid over ours; + # removing the file would hide that server from every future sweep. + server._pid_file().write_text("222") + server._forget() + self.assertEqual(server._pid_file().read_text(), "222") + + def test_forget_removes_the_pid_it_remembered(self): + server = self.server() + server._remember(111) + server._forget() + self.assertFalse(server._pid_file().exists()) + + def test_stop_waits_out_a_start_in_flight_and_kills_it(self): + # stop_all on quit must not slide past a launch that is mid-load: the + # child would then survive Dikte with nothing left that knows its pid. + children = [] + real = ggml.subprocess.Popen + + def popen(*args, **kwargs): + proc = real(*args, **kwargs) + children.append(proc) + return proc + + self.patch_attr(ggml.subprocess, "Popen", popen) + server = self.server(extra=["--wait", "0.5"]) + thread = threading.Thread(target=server.serve) + thread.start() + try: + deadline = time.monotonic() + 10 + while not children and time.monotonic() < deadline: + time.sleep(0.01) # until the launch is truly in flight + server.stop() + finally: + thread.join(timeout=10) + self.assertEqual(len(children), 1) + self.assertIsNotNone(children[0].poll()) + self.assertFalse(server.running) + def test_a_start_that_goes_wrong_takes_its_process_with_it(self): started = [] @@ -752,6 +948,10 @@ class InstallOnWindows(Local): super().setUp() self.patch_attr(sys, "platform", "win32") self.patch_attr(ggml, "_arch", lambda: "x64") + # shutil.which cannot be allowed through to the real one: standing on + # win32 from another system, Python 3.12's Windows branch of which() + # reaches for the nt module that is not there. + self.enterContext(mock.patch("shutil.which", return_value=None)) self.archive = zipball({ "Release/whisper-server.exe": b"MZ not really a program", "Release/whisper.dll": b"not really a library", @@ -785,3 +985,43 @@ class InstallOnWindows(Local): with self.assertRaises(ggml.LocalError) as caught: ggml.install_program(ggml.WHISPER) self.assertIn("this machine", str(caught.exception)) + + +class WindowsOwnership(Local): + """Sweeping on Windows goes by the executable's full path, never its name: + the base name alone is anyone's whisper-server.exe, and this answer decides + what gets killed.""" + + def setUp(self): + super().setUp() + self.patch_attr(sys, "platform", "win32") + # See InstallOnWindows: the real which() on win32 wants the nt module. + self.enterContext(mock.patch("shutil.which", return_value=None)) + self.made = ggml.Server(ggml.WHISPER, lambda values: [], {"binary": ""}) + + def image(self, path): + self.patch_attr(ggml, "_win_image_name", lambda pid: path) + + def test_a_binary_under_our_bin_directory_is_ours(self): + self.image(str(ggml.BIN_DIR / "whisper" / "v1.9.1" / "whisper-server.exe")) + self.assertIs(self.made._is_ours(1234), True) + + def test_the_configured_binary_is_ours_wherever_it_lives(self): + mine = self.path("elsewhere", "whisper-server.exe") + mine.parent.mkdir(parents=True) + mine.write_bytes(b"MZ") + mine.chmod(0o755) + self.made._settings["binary"] = str(mine) + self.image(str(mine.resolve())) + self.assertIs(self.made._is_ours(1234), True) + + def test_the_same_name_somewhere_else_is_not_ours(self): + self.image(str(self.path("theirs", "whisper-server.exe"))) + with mock.patch("shutil.which", return_value=None): + self.assertIs(self.made._is_ours(1234), False) + + def test_a_process_that_cannot_be_read_is_no_verdict(self): + # OpenProcess answering nothing covers both "gone" and "not readable + # from here", and only one of those makes the pid file safe to drop. + self.image("") + self.assertIsNone(self.made._is_ours(1234)) diff --git a/tests/test_hub.py b/tests/test_hub.py index 84cd8d0..1b0a630 100644 --- a/tests/test_hub.py +++ b/tests/test_hub.py @@ -3,6 +3,7 @@ import json from dikte import hub +from dikte import paths from tests.support import DikteTest, fake_urlopen, http_error, url_error RELEASE = { @@ -165,6 +166,15 @@ def os_utime(path): os.utime(path, (old, old)) +class CacheLocation(DikteTest): + """Resolved at import, like every other path constant.""" + + def test_the_cache_lives_in_the_system_cache_directory(self): + # One answer for both, the same way ggml and config share DATA_DIR: + # hub asked paths once, at import, and kept what it was told. + self.assertEqual(hub.CACHE_DIR, paths.cache_dir()) + + class CacheOnDisk(DikteTest): def setUp(self): super().setUp() diff --git a/tests/test_integrate.py b/tests/test_integrate.py index f5a5f97..23c6121 100644 --- a/tests/test_integrate.py +++ b/tests/test_integrate.py @@ -475,6 +475,17 @@ class Windows(unittest.TestCase): self.installed = pathlib.Path(self.tmp.name).resolve() self.app = self.installed / "Dikte.exe" self.app.write_text("") + # APPDATA pointed into the sandbox, so that the Startup folder these + # tests delete from is never the machine's own. + appdata = mock.patch.dict(os.environ, + {"APPDATA": str(self.installed / "Roaming")}) + appdata.start() + self.addCleanup(appdata.stop) + + def startup_shortcut(self): + """Where install.ps1 -Autostart puts a checkout's sign-in entry.""" + return (self.installed / "Roaming" / "Microsoft" / "Windows" + / "Start Menu" / "Programs" / "Startup" / "Dikte.lnk") def _write(self, command): self.value = command @@ -510,6 +521,45 @@ class Windows(unittest.TestCase): self.assertEqual(len(self.install()), 1) self.assertEqual(self.value, f'"{self.app}"') + def test_an_entry_for_another_working_install_is_left_alone(self): + """The same courtesy the Linux half pays another menu entry: an entry + naming an executable that still exists is an installation that still + works, and a start of this one has no business redirecting it.""" + other = self.installed / "Elsewhere" / "Dikte.exe" + other.parent.mkdir() + other.write_text("") + self.value = f'"{other}"' + self.assertEqual(self.install(), []) + self.assertEqual(self.value, f'"{other}"') + + def test_asking_outright_overrules_a_working_other_install(self): + other = self.installed / "Elsewhere" / "Dikte.exe" + other.parent.mkdir() + other.write_text("") + self.value = f'"{other}"' + self.assertEqual(self.install(force=True), [integrate._run_entry_name()]) + self.assertEqual(self.value, f'"{self.app}"') + + def test_typing_it_sweeps_away_a_checkout_startup_shortcut(self): + """install.ps1 -Autostart writes it, the Run value replaces it, and + both left in place would be two Diktes at every sign-in.""" + shortcut = self.startup_shortcut() + shortcut.parent.mkdir(parents=True) + shortcut.write_text("") + changed = self.install(force=True) + self.assertIn(shortcut, changed) + self.assertFalse(shortcut.exists()) + + def test_a_start_leaves_a_checkout_startup_shortcut_alone(self): + """The silent call on every start has not been asked to move the + machine off its checkout.""" + shortcut = self.startup_shortcut() + shortcut.parent.mkdir(parents=True) + shortcut.write_text("") + self.value = f'"{self.app}"' + self.assertEqual(self.install(), []) + self.assertTrue(shortcut.exists()) + def test_running_it_again_changes_nothing(self): self.install(force=True) self.assertEqual(self.install(), []) @@ -521,6 +571,31 @@ class Windows(unittest.TestCase): self.assertEqual(self.remove(), []) +class WindowedExecutable(unittest.TestCase): + """The windowed executable, looked up beside whichever one is running. + + Beside rather than at a known place: the setup lays both executables into + one directory wherever that directory was put, so either can find the + other without knowing where the install is. + """ + + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + self.addCleanup(self.tmp.cleanup) + self.installed = pathlib.Path(self.tmp.name).resolve() + + def test_found_beside_the_named_executable(self): + windowed = self.installed / "Dikte.exe" + windowed.write_text("") + self.assertEqual( + integrate.windowed_executable(str(self.installed / "dikte-cli.exe")), + windowed) + + def test_none_when_no_setup_installed_one(self): + self.assertIsNone( + integrate.windowed_executable(str(self.installed / "dikte-cli.exe"))) + + class WindowsExecutableNames(unittest.TestCase): """The two Windows executables, read out of the files that name them. diff --git a/tests/test_ipc.py b/tests/test_ipc.py index 502574c..2c5d802 100644 --- a/tests/test_ipc.py +++ b/tests/test_ipc.py @@ -14,6 +14,7 @@ import unittest from unittest import mock from dikte import ipc +from tests.support import DikteTest class FakeSocket: @@ -185,5 +186,74 @@ class Send(unittest.TestCase): self.assertTrue(sock.disconnected) +class AlreadyServing(unittest.TestCase): + """The single-instance check, which listen() cannot be: a Windows pipe + takes a second server on the same name rather than refusing it.""" + + def probe(self, socket): + with mock.patch.object(ipc, "QLocalSocket", return_value=socket): + return ipc.already_serving() + + def test_nothing_running_means_go_ahead(self): + self.assertFalse(self.probe(FakeSocket(connected=False))) + + def test_an_answer_means_yield(self): + self.assertTrue(self.probe(FakeSocket(reply=b'{"ok": true}\n'))) + + def test_the_probe_has_no_side_effect(self): + """A probe that opened a window would open it during the relaunch a + slow instance provokes, on top of the verb being forwarded.""" + sock = FakeSocket(reply=b'{"ok": true}\n') + self.probe(sock) + self.assertEqual(sock.written.decode("utf-8").strip(), "status") + + def test_an_instance_too_old_to_answer_still_counts_as_running(self): + self.assertTrue(self.probe(FakeSocket(reply=b""))) + + +class InstanceLock(DikteTest): + def setUp(self): + super().setUp() + # The lock derives its home from paths, which DikteTest's cfg patches + # do not cover; without this the test would write into the real one. + from dikte import paths + self.patch_attr(paths, "DATA_DIR", self.path("data")) + + def test_one_holder_at_a_time(self): + first = ipc.instance_lock() + self.assertIsNotNone(first) + self.assertTrue(first.tryLock(0)) + second = ipc.instance_lock() + self.assertFalse(second.tryLock(0)) + first.unlock() + self.assertTrue(second.tryLock(0)) + second.unlock() + + def test_the_lock_lives_in_the_data_directory(self): + from dikte import paths + lock = ipc.instance_lock() + self.assertTrue(lock.tryLock(0)) + self.assertTrue((paths.DATA_DIR / "dikte.lock").exists()) + lock.unlock() + + +class Respawn(unittest.TestCase): + def test_windows_starts_a_detached_process_and_returns(self): + with mock.patch.object(sys, "platform", "win32"), \ + mock.patch.object(ipc, "launcher", return_value=["py", "x"]), \ + mock.patch.object(ipc.subprocess, "Popen") as popen: + ipc.respawn(["--gui"]) + self.assertEqual(popen.call_args.args[0], ["py", "x", "--gui"]) + self.assertEqual(popen.call_args.kwargs["creationflags"], + 0x00000008 | 0x00000200) + + def test_everywhere_else_the_process_is_replaced(self): + with mock.patch.object(sys, "platform", "linux"), \ + mock.patch.object(ipc, "launcher", return_value=["py", "x"]), \ + mock.patch.object(ipc.os, "execv") as execv: + ipc.respawn(["toggle", "--gui"]) + execv.assert_called_once_with("py", ["py", "x", "toggle", "--gui"]) + + if __name__ == "__main__": unittest.main() diff --git a/tests/test_paths.py b/tests/test_paths.py index 8c076f4..df84a09 100644 --- a/tests/test_paths.py +++ b/tests/test_paths.py @@ -61,6 +61,34 @@ class Directories(unittest.TestCase): self.assertTrue(data_dir.as_posix().endswith("/AppData/Local/Dikte")) +class CacheDir(unittest.TestCase): + """The third place: files whose whole point is that they can be lost.""" + + def test_linux_follows_xdg(self): + with mock.patch.dict(os.environ, {"XDG_CACHE_HOME": "/k"}): + self.assertEqual(paths.cache_dir("linux").as_posix(), "/k/dikte") + + def test_linux_without_the_variable_set(self): + with mock.patch.dict(os.environ, {}, clear=True): + self.assertTrue(paths.cache_dir("linux").as_posix() + .endswith("/.cache/dikte")) + + def test_a_mac_caches_under_library_caches(self): + """Where Time Machine already knows not to look.""" + self.assertTrue(paths.cache_dir("darwin").as_posix() + .endswith("/Library/Caches/Dikte")) + + def test_windows_caches_outside_the_roaming_profile(self): + with mock.patch.dict(os.environ, {"LOCALAPPDATA": "C:/local"}): + self.assertEqual(paths.cache_dir("win32").as_posix(), + "C:/local/Dikte/cache") + + def test_windows_without_the_variable_set(self): + with mock.patch.dict(os.environ, {}, clear=True): + self.assertTrue(paths.cache_dir("win32").as_posix() + .endswith("/AppData/Local/Dikte/cache")) + + class OnePlace(unittest.TestCase): """The programs and the models go where everything else goes. diff --git a/tests/test_ui.py b/tests/test_ui.py index 060686a..3be548b 100644 --- a/tests/test_ui.py +++ b/tests/test_ui.py @@ -1049,12 +1049,21 @@ class LocalModels(DikteTest): # Qt's int is C++'s 32-bit one, and a 2.3 GB model is more than fits in # it: the count came out the far side negative, at "-1%". box = self.window(cfg.Config()).local_llm - box._downloading = True - box._report(1_048_576, 2_489_757_856) + box._report("model", 1_048_576, 2_489_757_856) _app.processEvents() self.assertIn("2.3 GB", box.status.text()) self.assertNotIn("-", box.status.text()) + def test_each_download_reports_into_its_own_label(self): + """The two can run at once; the tag, not a flag read later, says + which label the bytes belong to.""" + box = self.window(cfg.Config()).local_llm + box._report("program", 10, 100) + box._report("model", 20, 100) + _app.processEvents() + self.assertIn("10", box.program_label.text()) + self.assertIn("20", box.status.text()) + def test_a_long_model_name_is_not_cut_in_half(self): # The list under a combo box takes the box's width and elides what does # not fit, in the middle: "ggml-org/Qwen....7B-Base-GGUF". diff --git a/tests/test_vad.py b/tests/test_vad.py index 71c6cfd..d7fb6e6 100644 --- a/tests/test_vad.py +++ b/tests/test_vad.py @@ -128,6 +128,12 @@ class Hallucinations(DikteTest): self.assertFalse(vad.looks_like_hallucination("Bugün toplantı var.", 2.0)) self.assertFalse(vad.looks_like_hallucination("Send it on Thursday.", 2.0)) + def test_a_one_word_answer_is_believed(self): + # Whisper invents both over silence, but people dictate both as whole + # answers, and losing a real answer costs more than passing a fake one. + self.assertFalse(vad.looks_like_hallucination("You.", 1.5)) + self.assertFalse(vad.looks_like_hallucination("Bye.", 1.5)) + def test_an_empty_transcript_counts_as_invented(self): self.assertTrue(vad.looks_like_hallucination(" ", 2.0)) self.assertTrue(vad.looks_like_hallucination("...", 2.0)) @@ -138,8 +144,8 @@ class Hallucinations(DikteTest): self.assertTrue(vad.looks_like_hallucination(text, 2.0)) def test_the_boundary_is_the_max_duration(self): - self.assertTrue(vad.looks_like_hallucination("you", 6.0)) - self.assertFalse(vad.looks_like_hallucination("you", 6.1)) + self.assertTrue(vad.looks_like_hallucination("thanks for watching", 6.0)) + self.assertFalse(vad.looks_like_hallucination("thanks for watching", 6.1)) if __name__ == "__main__": diff --git a/tests/test_worker.py b/tests/test_worker.py index 44102fb..c1918dd 100644 --- a/tests/test_worker.py +++ b/tests/test_worker.py @@ -30,6 +30,7 @@ class Chain(DikteTest): def run_chain(self, ask=False, paste_override=None, duration=2.0, transcript="uh, book it for Thursday", + transcribe_error=None, cleaned="Book it for Thursday.", cleanup_error=None, answer=("Booked.", ""), rms=None, clipboard=b"what was there before", paste_error=None, @@ -47,7 +48,10 @@ class Chain(DikteTest): # The chain reports its own failures on stderr, which a test run has no # use for. with contextlib.redirect_stderr(io.StringIO()), \ - mock.patch.object(api, "transcribe", return_value=transcript) as tr, \ + mock.patch.object( + api, "transcribe", + **({"side_effect": transcribe_error} if transcribe_error + else {"return_value": transcript})) as tr, \ mock.patch.object(api, "cleanup", cleanup), \ mock.patch.object(assistant, "ask", return_value=answer) as ask_call, \ mock.patch.object(paste, "copy") as copy, \ @@ -118,11 +122,57 @@ class Chain(DikteTest): run = self.run_chain() run["copy_bytes"].assert_not_called() - def test_the_clipboard_is_put_back_when_the_keypress_fails(self): + def test_a_failed_keypress_leaves_the_transcript_on_the_clipboard(self): + """The press failing is a warning, not a lost dictation: restoring the + old clipboard over the text would leave nothing to paste by hand.""" self.conf["restore_clipboard"] = True run = self.run_chain(paste_error=paste.PasteError("not trusted")) - self.assertIn("not trusted", run["failures"][0]) - run["copy_bytes"].assert_called_once_with(b"what was there before") + self.assertEqual(run["failures"], []) + raw, text, warning = run["done"][0] + self.assertIn("not trusted", warning) + run["copy_bytes"].assert_not_called() + + def test_a_failed_keypress_still_reaches_the_history(self): + self.run_chain(paste_error=paste.PasteError("not trusted")) + rows = cfg.read_history() + self.assertEqual(len(rows), 1) + self.assertEqual(rows[0]["text"], "Book it for Thursday.") + # The row goes in before the paste is attempted, so the paste failing + # has to be written back into it: the record tells the whole truth. + self.assertIn("not trusted", rows[0]["cleanup_error"]) + + def test_a_failed_transcription_keeps_the_audio(self): + """Speech the user cannot repeat from memory must survive the failure.""" + run = self.run_chain(transcribe_error=api.ApiError("server down")) + self.assertIn("server down", run["failures"][0]) + self.assertIn("kept", run["failures"][0]) + kept = list(cfg.RECORDINGS_DIR.glob("*.wav")) + self.assertEqual(len(kept), 1) + self.assertFalse(os.path.exists(self.wav)) + + def test_two_failures_in_one_second_keep_both_recordings(self): + self.run_chain(transcribe_error=api.ApiError("down")) + self.wav = make_wav(self.path("clip2.wav"), speech(2.0)) + with mock.patch.object(worker.time, "strftime", + return_value="20260820-120000"): + self.run_chain(transcribe_error=api.ApiError("down")) + self.wav = make_wav(self.path("clip3.wav"), speech(2.0)) + self.run_chain(transcribe_error=api.ApiError("down")) + self.assertEqual(len(list(cfg.RECORDINGS_DIR.glob("*.wav"))), 3) + + def test_the_history_row_says_whether_cleanup_actually_ran(self): + """The ask path cleans under its own setting; the record follows the + run, not the dictation gate.""" + self.conf["cleanup_enabled"] = False + self.conf["assistant_cleanup"] = True + self.run_chain(ask=True) + row = cfg.read_history()[0] + self.assertNotEqual(row["cleanup_model"], "") + cfg.clear_history() + self.conf["cleanup_enabled"] = True + self.conf["assistant_cleanup"] = False + self.run_chain(ask=True) + self.assertEqual(cfg.read_history()[0]["cleanup_model"], "") def test_the_transcription_is_told_the_language_and_the_glossary(self): self.conf["language"] = "tr"