win-dictate/FINDINGS-FIXES-TESTS2.md

# Win Dictation — Findings, Fixes & Test Guide

A consolidated record of everything diagnosed and changed while turning the slow,
crashing build into a fast, stable push-to-talk dictation tool. Four parts:

1. **Findings** — what was actually wrong, in the order we discovered it.
2. **Fixes required** — the concrete code changes, by file.
3. **Reasoning** — why each fix is correct (the non-obvious calls).
4. **How to build & run tests** — making the core testable, plus regression tests for every bug here.

Target machine: Dell, Intel Core i5 7th-gen (2 cores / 4 threads), 16 GB RAM, CPU-only.
Stack: native C++ / Win32 + SDL2 + whisper.cpp (`examples/win-dictation`).

---

## Part 1 — Findings

### F1. The original app was slow because of its *architecture*, not its model
The first build used whisper.cpp's live-streaming pattern: it re-ran `whisper_full` on a
rolling 5–6 second window every ~0.4 s (`length_ms` / `step_ms` in `worker_loop`). To keep
up, the CPU must transcribe ~6 s of audio in well under 1 s — i.e. **>6× real-time**.

- The README's "10–15× real-time" numbers were measured on a **24-thread** box.
- A 2-core i5-7th-gen does tiny.en at roughly **2–4× real-time**.

So each 6 s window took ~1.5–2.5 s while a new one was demanded every 0.4 s → the buffer
backed up without bound, audio was re-transcribed in overlapping chunks (duplicated/garbled
text), the CPU pegged, and 4 Whisper threads on 2 physical cores starved the UI. tiny.en was
already the right model — **the streaming design was the bottleneck.**

**Conclusion:** for dictation (not live captioning), switch to **batch / push-to-talk**:
buffer audio cheaply while recording, run `whisper_full` exactly **once** on Stop.

### F2. Build 1 crashed on Stop — null Whisper context
After the batch rewrite, the app recorded fine (VU moved) but crashed the instant you hit
Stop. Two defects combined:

1. The preload thread set `g_modelLoaded = true` **even when `preload()` returned false**
   (model file not found), so the UI showed "Ready" and let you record with no model.
2. `transcribe_worker()` called `whisper_reset_timings(m_ctx)` **before** the `if (m_ctx)`
   check. With `m_ctx == nullptr` that's an access violation. Recording never touches Whisper
   — which is exactly why capture/VU worked and only Stop crashed.

The empty model dropdown in the first screenshot was the tell: no model files were found at
the new exe-relative path, so `preload()` failed silently.

### F3. Build 1 — window wouldn't resize, and looked boxy
- The style was `WS_POPUP | WS_CAPTION | WS_SYSMENU` — **no sizing border** — and there was
  **no `WM_SIZE` handler**, so controls never reflowed. Frozen at ~360×200, Copy/Paste cut off.
- The gray 3-D system buttons + chunky title bar were the "boxy Windows" look the brief
  wanted to avoid.

### F4. Build 2 — startup data race (found and fixed by the user)
`RefreshModelList()` calls `CB_SETCURSEL`, which fires `CBN_SELCHANGE`. That spawned a
`reload()` background thread that raced the initial `preload()` thread — both writing
`m_cfg` (a `std::string`) and `m_ctx` at once. The corrupted context pointer made
`whisper_full` crash ~15 s into processing. Fixed with a `g_initializing` guard (skip the
combo handler during startup) and an `m_cfg_mtx` mutex around `m_cfg`/`m_ctx` writes.

### F5. Build 3 — the "crash after transcribing" was **not a crash**
Reported: "crashes after transcribing; relaunching the exe shows the last transcription, and
a second run replaced the first instead of appending."

That behavior is **impossible for a crashed process** — a dead process can't remember the
last transcription (nothing is persisted to disk/registry), and a *fresh* launch couldn't
show it. The only explanation: **the original process never died.**

What actually happened:
1. On a successful result, `WM_APP_RESULT` ran `ShowWindow(hWnd, SW_HIDE)` in the auto-paste
   branch → the window **vanished** (looked like a crash).
2. Double-clicking the `.exe` again hit the **single-instance guard**, which
   `PostMessage(WM_APP_SHOW)` to the existing **hidden** window → it re-appeared, still holding
   the last transcription.
3. `SetDlgItemTextW` **replaced** the edit text, so run #2 overwrote run #1.

Also: clicking the **Record button** (vs. the global hotkey) set `g_prevForeground` to the
dictation window *itself*, so it hid and "pasted" into its own read-only box — nothing landed
in a useful app.

10-second confirmation: when it "crashes," the tray icon is still present and
`win-dictation.exe` is still in Task Manager.

### F6. The test harness is stale and never built
`src/test-audio.cpp` calls `m_transcriber.init(...)` and `is_using_gpu()` — methods that the
batch rewrite **removed** (`transcriber.h` now exposes `preload`/`reload`, no `init`). So it
won't compile against the current code. And the **root** `CMakeLists.txt` (the one `build.ps1`
actually uses) defines only the `win-dictation` target — the `test-audio` target lives only in
the unused `src/CMakeLists.txt`. So there are currently **no working tests**. Part 4 fixes this.

---

## Part 2 — Fixes required

Status legend: ✅ done in current code · ⬜ still to apply.

### `transcriber.cpp` / `transcriber.h`

- ✅ **Batch architecture** — `start_recording()` buffers audio cheaply; `stop_and_transcribe()`
  runs `whisper_full` once on a worker thread; result delivered via `PostMessage` (F1).
- ✅ **Null-context guard** in `transcribe_worker` — `if (!m_ctx) { m_busy=false; if(m_on_result) m_on_result(""); return; }`,
  and the `whisper_reset_timings` call removed (F2).
- ✅ **`reload()`** frees + re-inits under a mutex; **`m_cfg_mtx`** protects `m_cfg`/`m_ctx`;
  **`threads()`** getter (F4 + thread-count display).
- ⬜ **Refactor for testability** — extract the inference core so it can be called
  synchronously from a test (see Part 4):

  ```cpp
  // transcriber.h  (public)
  std::string transcribe_sync(std::vector<float> audio);   // headless / tests
  // transcriber.h  (private)
  std::string run_inference(std::vector<float>& audio);
  ```

  ```cpp
  // transcriber.cpp
  std::string Transcriber::run_inference(std::vector<float>& audio) {
      if (!m_ctx) return "";
      if (m_cfg.trim_silence) trim_silence(audio);
      whisper_full_params wp = whisper_full_default_params(WHISPER_SAMPLING_GREEDY);
      wp.print_progress = wp.print_realtime = wp.print_timestamps = false;
      wp.no_timestamps = true; wp.translate = false;
      wp.language = m_cfg.language.c_str();
      wp.n_threads = m_cfg.n_threads;
      wp.no_context = true; wp.suppress_blank = true; wp.suppress_nst = true;
      wp.temperature = 0.0f;
      std::string out;
      if (whisper_full(m_ctx, wp, audio.data(), (int)audio.size()) == 0) {
          int n = whisper_full_n_segments(m_ctx);
          for (int i = 0; i < n; ++i) { const char* t = whisper_full_get_segment_text(m_ctx, i); if (t) out += t; }
          out = clean_text(out);
      }
      return out;
  }
  void Transcriber::transcribe_worker(std::vector<float> audio) {
      std::string out = run_inference(audio);
      m_busy = false;
      if (m_on_result) m_on_result(out);
  }
  std::string Transcriber::transcribe_sync(std::vector<float> audio) {
      return run_inference(audio);   // assumes preload() already succeeded
  }
  ```

### `main.cpp`

- ✅ **Honest load state** — separate `g_modelOk` atomic; record is gated and shows
  "Model not found: …\models\ggml-tiny.en.bin" instead of crashing (F2).
- ✅ **Resizable** — `WS_OVERLAPPEDWINDOW` (has `WS_THICKFRAME`), `WM_SIZE → LayoutControls`,
  `WM_GETMINMAXINFO` min size (F3).
- ✅ **Startup race guard** — `g_initializing` skips the model-combo handler during startup (F4).
- ⬜ **`WM_APP_RESULT` rewrite** — the F5 fix: **don't hide**, **append** (not replace),
  clipboard = latest utterance, paste only into a *different* valid window:

  ```cpp
  case WM_APP_RESULT:
  {
      std::string* res = (std::string*)wParam;
      if (res && !res->empty()) {
          HWND hEdit = GetDlgItem(hWnd, ID_EDIT_TEXT);
          int len = GetWindowTextLengthW(hEdit);
          std::wstring cur;
          if (len > 0) { cur.resize(len + 1); int g = GetWindowTextW(hEdit, &cur[0], len + 1); cur.resize(g); }
          std::wstring combined = cur;
          if (!combined.empty()) combined += L" ";
          combined += to_w(*res);
          SetWindowTextW(hEdit, combined.c_str());
          SendMessageW(hEdit, EM_SETSEL, (WPARAM)combined.size(), (LPARAM)combined.size());
          SendMessageW(hEdit, EM_SCROLLCARET, 0, 0);
          UpdatePlaceholder(hWnd);

          SetClipboardTextUtf8(hWnd, *res);   // clipboard = just the latest utterance

          bool pasted = false;
          if (g_autoPaste && g_prevForeground &&
              g_prevForeground != hWnd && IsWindow(g_prevForeground)) {
              PasteIntoWindow(g_prevForeground);
              pasted = true;
          }
          SetStatus(hWnd, pasted ? L"Pasted ✓" : L"Copied ✓");
      } else {
          SetStatus(hWnd, L"No speech detected");
      }
      delete res;
  }
  break;
  ```

- ⬜ **Clear button** — `ID_BTN_CLEAR` is handled in `WM_COMMAND` but no button is created in
  the modern layout, so the (now appending) transcript can't be cleared. Create it owner-drawn,
  place it in `LayoutControls`, and call `UpdatePlaceholder(hWnd)` after clearing.
- ⬜ **Optional `g_autoHide`** (default `false`) — only if you want the window to tuck away after
  a successful paste into another app: `if (pasted && g_autoHide) ShowWindow(hWnd, SW_HIDE);`

### Modern UI (separate doc)
The flat dark restyle (GDI+ owner-drawn buttons, rounded panel, custom VU, dark caption +
rounded corners) is fully specified in **`MODERN-UI-AND-FIXES.md`**, Part 2.

---

## Part 3 — Reasoning

**Why batch beats streaming here (F1).** Streaming only wins if you need words *as you speak*
(live captioning). Dictation doesn't: you speak a sentence, then want the text. Batch removes
the real-time deadline entirely — Whisper processes each second of audio exactly once, at its
own pace, after you stop. That means: near-zero CPU while speaking (just buffering), predictable
"a few seconds after Stop" latency that **doesn't grow with clip length**, no overlap
re-processing, and better accuracy (full context, no chunk-boundary word splits). On 2 cores
this is the difference between unusable and snappy.

**Why threads = physical cores (2), not logical (4).** Whisper's matmuls are memory-bandwidth
bound. Two extra hyperthreads share the same execution ports and cache, so they add little
throughput while stealing cycles from the UI/audio threads (janky window, laggy VU). Two
threads leaves headroom for a responsive UI.

**Why the Stop crash was a null deref (F2), and why the guard is the right fix.** Recording is
pure SDL + a `std::vector` append — it never calls into Whisper, which is why only Stop crashed.
`whisper_reset_timings(nullptr)` dereferences the context. The guard makes the worker a no-op
when no model is loaded; the separate `g_modelOk` flag makes that state *visible* (a MessageBox
with the path) instead of letting you walk into it. Defensive + diagnostic.

**Why the "crash after transcribing" was actually a hide (F5) — the logic is conclusive.** A
crashed process loses all memory. There is no on-disk persistence in this app. Therefore a
re-launched process **cannot** display the previous transcription, and a second run **cannot**
replace text in "the same box." The observed behavior (prior text reappears; second replaces
first) is only possible if it's the *same* live process — which means it hid, and the
single-instance guard re-showed it. This is why "is the process still in Task Manager?" is the
decisive test, not "did the window disappear?"

**Why clipboard = latest utterance but the box appends.** Two different jobs. The on-screen box
is your running log (you want history). The clipboard is what you paste into another app — you
want *just what you last said*, not the whole session. Splitting them gives both.

**Why paste must exclude our own window.** Auto-paste replays Ctrl+V into the foreground window
captured before the popup. If you triggered recording by clicking the Record button, that
"previous" window is the dictation app itself — pasting into its read-only box is a no-op and
(with the old hide) made the app appear to swallow the text. Restricting to
`g_prevForeground != hWnd && IsWindow(...)` makes the global-hotkey workflow the reliable path
and the button a safe "copy only."

---

## Part 4 — How to build & run tests

The goal: catch the bugs above automatically, without a human watching a window. UI behavior
(resize, hide, paste) needs a short **manual** checklist, but the **core** (model load,
transcription, null-safety, text append) can and should be tested headlessly.

### 4.1 Make the core testable
Apply the `run_inference` / `transcribe_sync` refactor from Part 2 so a test can feed a buffer
and get text synchronously. Also extract the append rule as a pure function so it can be tested
with zero Win32:

```cpp
// text_util.h  (new, tiny, UI-free)
#pragma once
#include <string>
inline std::wstring append_transcript(const std::wstring& cur, const std::wstring& add) {
    if (add.empty()) return cur;
    if (cur.empty()) return add;
    return cur + L" " + add;
}
```
Use it in `WM_APP_RESULT` (`combined = append_transcript(cur, to_w(*res));`) so the UI and the
test exercise the *same* rule.

### 4.2 The test program (`tests/test_core.cpp`)
Replaces the stale `src/test-audio.cpp`. Compiles against the **current** API and tests the real
code path (`preload` + `transcribe_sync`).

```cpp
#include "transcriber.h"
#include "text_util.h"
#include <cstdio>
#include <cstdint>
#include <fstream>
#include <vector>
#include <string>
#include <cctype>

static int g_fail = 0;
#define CHECK(cond, msg) do { if (!(cond)) { printf("  FAIL: %s\n", msg); ++g_fail; } \
                              else            printf("  ok:   %s\n", msg); } while(0)

// Minimal 16-bit PCM mono WAV loader -> float [-1,1]. Assumes 16 kHz mono (whisper.cpp samples are).
static bool load_wav(const std::string& path, std::vector<float>& out) {
    std::ifstream f(path, std::ios::binary);
    if (!f) return false;
    char hdr[44];
    f.read(hdr, 44);
    if (std::string(hdr, 4) != "RIFF") return false;
    std::vector<int16_t> pcm((std::istreambuf_iterator<char>(f)), {});  // crude: rest of file
    out.clear(); out.reserve(pcm.size());
    for (int16_t s : pcm) out.push_back(s / 32768.0f);
    return !out.empty();
}

static std::string lower(std::string s) { for (char& c : s) c = (char)tolower((unsigned char)c); return s; }

int main(int argc, char** argv) {
    std::string model = (argc > 1) ? argv[1] : "models/ggml-tiny.en.bin";
    std::string wav   = (argc > 2) ? argv[2] : "samples/jfk.wav";

    // --- Pure logic: append rule (no model needed) ---
    CHECK(append_transcript(L"", L"hello") == L"hello",        "append: empty + a");
    CHECK(append_transcript(L"hello", L"world") == L"hello world", "append: a + b spaced");
    CHECK(append_transcript(L"hello", L"") == L"hello",        "append: a + empty");

    // --- Regression F2: bad model path must NOT crash, returns "" ---
    {
        Transcriber t; WhisperConfig bad; bad.model_path = "models\\does-not-exist.bin";
        bool ok = t.preload(bad);
        CHECK(!ok, "bad model path: preload returns false");
        std::vector<float> a(16000, 0.0f);               // 1 s of silence
        std::string r = t.transcribe_sync(a);            // must be a safe no-op
        CHECK(r.empty(), "bad model path: transcribe_sync returns empty, no crash");
    }

    // --- Happy path: real model + known clip -> non-empty, expected words ---
    {
        Transcriber t; WhisperConfig cfg; cfg.model_path = model; cfg.n_threads = 4;
        bool ok = t.preload(cfg);
        CHECK(ok, "model loads");
        if (ok) {
            std::vector<float> audio;
            bool loaded = load_wav(wav, audio);
            CHECK(loaded, "wav loads");
            if (loaded) {
                std::string text = lower(t.transcribe_sync(audio));
                CHECK(!text.empty(), "transcription is non-empty");
                // jfk.wav: "...ask not what your country can do for you..."
                CHECK(text.find("country") != std::string::npos, "transcription contains 'country'");
            }
        }
    }

    // --- Edge: sub-300ms / empty audio -> "" , no crash ---
    {
        Transcriber t; WhisperConfig cfg; cfg.model_path = model;
        if (t.preload(cfg)) {
            std::vector<float> tiny(100, 0.1f);
            std::string r = t.transcribe_sync(tiny);
            CHECK(true, "short audio did not crash");   // reaching here = no crash
        }
    }

    printf("\n%s (%d failure%s)\n", g_fail ? "TESTS FAILED" : "ALL TESTS PASSED",
           g_fail, g_fail == 1 ? "" : "s");
    return g_fail ? 1 : 0;
}
```

> The WAV loader above is deliberately minimal (good enough for whisper.cpp's 16 kHz mono
> `samples/*.wav`). If you test arbitrary WAVs, parse the `fmt `/`data` chunks properly.

### 4.3 Build the tests (root `CMakeLists.txt`)
The active build file builds only `win-dictation`. Add a test target next to it:

```cmake
# --- tests ---
add_executable(test-core
    tests/test_core.cpp
    src/transcriber.cpp
    src/transcriber.h
)
target_link_libraries(test-core PRIVATE whisper ${SDL2_LIBRARIES})
target_include_directories(test-core PRIVATE
    ${CMAKE_CURRENT_SOURCE_DIR}/src
    ${CMAKE_CURRENT_SOURCE_DIR}/common
)
target_compile_definitions(test-core PRIVATE UNICODE _UNICODE)
```
(`transcriber.cpp` references SDL symbols even though the test never opens a device, so the test
still links `${SDL2_LIBRARIES}`.)

Build and run:
```powershell
cmake --build build --config Release --target test-core
# run from a dir where SDL2.dll + models/ + samples/ are reachable:
cd build\bin\Release
copy ..\..\..\deps\SDL2-2.28.5\lib\x64\SDL2.dll .   # if not already beside the exe
.\test-core.exe models\ggml-tiny.en.bin ..\..\..\samples\jfk.wav
```
Exit code `0` = all passed (usable in CI / a pre-commit hook).

### 4.4 What each test guards
| Test | Guards against |
|---|---|
| append rule (3 cases) | F5 regression — replace-instead-of-append |
| bad model path → false + empty | F2 — null-context crash on Stop |
| model loads / clip → non-empty, has "country" | core transcription works end to end |
| short/empty audio → no crash | the `< 0.3 s` guard + null-safety |

### 4.5 Manual UI checklist (can't be unit-tested)
- [ ] **Not a crash:** after Stop, the window **stays visible**; if it ever vanishes, the tray
      icon / `win-dictation.exe` in Task Manager confirms whether it's hiding vs. truly gone.
- [ ] **Append:** two dictations in a row → second is appended after the first, view scrolls down.
- [ ] **Clipboard:** after a dictation, paste into Notepad → only the *latest* utterance appears.
- [ ] **Auto-paste:** focus Notepad, press the global hotkey, speak, press it again → text lands
      in Notepad. Triggering via the Record button instead → "Copied ✓" (no self-paste).
- [ ] **Resize:** drag edges; transcript grows; min size respected.
- [ ] **Performance:** a ~10 s clip transcribes in a few seconds and the UI stays responsive;
      latency does **not** grow with longer recordings.

### 4.6 Performance smoke test (optional)
`test-core` can also print timing — wrap `transcribe_sync` in `std::chrono::steady_clock` and
assert `elapsed < audio_seconds * k` for a sanity bound (e.g. `k = 1.0`, i.e. faster than
real-time on tiny.en). Useful to catch a future regression that quietly reintroduces the
streaming-style slowdown.

---

## Part 5 — Status & remaining work

**Verified fixed:** F1 (batch), F2 (null-context crash), F3 (resize), F4 (startup race).

**Apply next (⬜ in Part 2):**
1. `run_inference` / `transcribe_sync` refactor (unblocks tests).
2. `WM_APP_RESULT` rewrite (the F5 fix — no-hide + append + safe paste). **Highest priority** —
   it's what's making the app *look* like it crashes.
3. Add the Clear button (needed now that text appends).
4. Add `tests/test_core.cpp` + the CMake target; run it.

**Then re-run** the Part 4 manual checklist. Expected result: the window no longer disappears,
transcriptions append, the clipboard holds the latest utterance, and `test-core.exe` exits `0`.