Addresses pr_agent "Stale State" review on PR #389. show_capsule was
read on the audio callback thread and snapshotted into the
run_on_main_thread closure. If the user toggled the preference between
the audio-thread emit and the main-thread closure firing (one or two
frames later), the queued window show/hide could apply the old
preference and briefly flash the wrong UI state.
Move that read inside the closure so the latest user pref always wins.
Why visible / translation stay snapshotted: they're the payload of
THIS specific capsule:state event — the whole point is to tell the JS
frontend "this is the state at the moment emit_capsule was called".
Re-reading them inside the closure would mean the OS-level window state
and the JS-side payload disagree (the closure lands one frame later
than the emit_to that already shipped).
No behavior change in the steady state; eliminates a 16–33 ms stale-
pref window when the user changes show_capsule mid-dictation.
emit_capsule does two things — (a) push a CapsulePayload event to the
JS frontend via app.emit_to, and (b) flip the OS-level window visibility
via window.show/hide and reposition it via maybe_position_capsule_*.
Path (a) is safe from any thread (Tauri event bus marshals internally).
Path (b) calls NSWindow / HWND APIs that expect the main thread —
but emit_capsule is invoked at ~30 Hz from the cpal process_callback
(audio thread) via the level_handler in coordinator/dictation.rs.
On macOS this risks dispatch_assert_queue_fail SIGTRAP if the AppKit
runloop assertion fires; on Windows SendMessage can deadlock against
the GUI thread; in either case the audio callback can miss its deadline
and trigger kAudioUnitErr_TooManyFramesToProcess on macOS. The 30 Hz
throttle limits frequency but does not change per-call risk.
Fix: wrap window.show / window.hide / maybe_position_capsule_bottom_center
/ show_capsule_window_no_activate / restore_main_window_key_if_active
in app.run_on_main_thread. The closure captures an Arc<Inner> clone +
AppHandle clone + Copy primitives (state, translation, show_capsule,
visible) — no payload clone needed. emit_to stays on the calling thread
since it is internally thread-safe, preserving the existing happens-
before relationship between event payload and window state on the JS
frontend (frontend processes events asynchronously regardless).
This pattern is already established in the codebase — see
coordinator.rs::stop_qa_hotkey_listener (line 326) for the same
reasoning around Carbon hotkey drop on the wrong thread (issue #169).
Audit ID 3.2.2 (CONFIRMED 高).
Test: 31 lib tests pass. Functional verification (no SIGTRAP on long
recordings, capsule still appears at start of dictation) requires
running the app — to be done after PR ships.
Bailian uses Notify::notify_waiters when the remote task starts. send_last_frame must register the Notified future before checking the started flag so a task-started event cannot land between the check and waiter registration.\n\nConstraint: DashScope stop should not falsely time out when task-started races with user stop.\nRejected: Replace Notify with polling | broader and less direct than the Tokio enable pattern.\nConfidence: high\nScope-risk: narrow\nTested: cargo check --manifest-path openless-all/app/src-tauri/Cargo.toml\nTested: cargo test --manifest-path openless-all/app/src-tauri/Cargo.toml --lib asr::bailian::tests\nCo-authored-by: OmX <omx@oh-my-codex.dev>
Bailian startup stores the ASR instance before opening the remote session so recorder startup audio can be buffered. If the websocket setup then fails in the active startup path, the provider must be cancelled before the coordinator returns to idle. The partial-result fallback remains intentional and is now documented as matching the existing Volcengine no-loss behavior.\n\nConstraint: OpenLess ASR paths prefer not losing already recognized user speech.\nRejected: Treat every websocket close with partial text as failure | would diverge from Volcengine fallback semantics.\nConfidence: high\nScope-risk: narrow\nTested: cargo check --manifest-path openless-all/app/src-tauri/Cargo.toml\nTested: cargo test --manifest-path openless-all/app/src-tauri/Cargo.toml --lib\nTested: cargo test --manifest-path openless-all/app/src-tauri/backend-tests/Cargo.toml\nCo-authored-by: OmX <omx@oh-my-codex.dev>
Review noted that changing defaultMode after turning Style master off made the switch look on again because enabledModes still reflected the old fallback pair.\n\nWhen master is already off, default-mode changes now persist the whole style preference set through setSettings so enabledModes is recomputed to raw plus the new default. Normal enabled master behavior still uses the narrower setDefaultPolishMode IPC.\n\nConstraint: Issue #312 selected raw/default fallback semantics for master-off state.\nRejected: Add a separate persisted masterEnabled flag | broader schema and migration change for a narrow UI consistency bug.\nConfidence: high\nScope-risk: narrow\nTested: cd openless-all/app && npx tsx src/lib/stylePrefs.test.ts\nTested: cd openless-all/app && npm run build\nNot-tested: Manual Style page click flow in packaged Tauri app.\nRelated: https://github.com/appergb/openless/pull/383\nCo-authored-by: OmX <omx@oh-my-codex.dev>
coordinator.rs had accumulated dictation lifecycle, QA panel state, and shared recording resources in one file. This split keeps behavior unchanged while moving the dictation edge/session helpers, QA panel state helpers, and resource ownership helpers into child modules under coordinator/.
Constraint: Issue #296 asks for module boundaries after Rust state-machine coverage is in place
Rejected: Split every helper into separate public modules | larger churn with no behavior benefit
Confidence: high
Scope-risk: moderate
Directive: Keep cross-module wiring in coordinator; leaf modules should remain coordinator-private
Tested: cargo check --manifest-path openless-all/app/src-tauri/Cargo.toml
Tested: cargo test --manifest-path openless-all/app/src-tauri/Cargo.toml --lib
Tested: cargo test --manifest-path openless-all/app/src-tauri/backend-tests/Cargo.toml
Co-authored-by: OmX <omx@oh-my-codex.dev>
The backend-only test PR needs to cover the concrete recorder paths named in the ticket, not just empty input handling. This adds focused Rust unit coverage for non-target sample-rate resampling, emitted PCM shape, scaled RMS levels, and peak tracking while keeping the production recorder unchanged.\n\nConstraint: Scope is Rust-only backend tests; do not launch or link extra Tauri runtime paths for Windows CI.\nConfidence: high\nScope-risk: narrow\nTested: cargo test --manifest-path openless-all/app/src-tauri/backend-tests/Cargo.toml recorder:: -- --nocapture\nTested: cargo test --manifest-path openless-all/app/src-tauri/backend-tests/Cargo.toml\nTested: cargo test --manifest-path openless-all/app/src-tauri/Cargo.toml --lib\nCo-authored-by: OmX <omx@oh-my-codex.dev>
The Rust-only backend harness already ran existing hotkey, recorder, and insertion unit tests, but the review could not see target edge coverage directly in this diff. Add focused edge tests for modifier trigger filtering/re-emission, recorder callback no-op inputs, and insertion fallback status so the coverage is explicit in the changed files.\n\nConstraint: Keep scope to backend Rust unit tests and avoid app/runtime integration.\nRejected: Add UI or Tauri runtime tests | conflicts with the Rust-only backend boundary.\nConfidence: high\nScope-risk: narrow\nTested: cargo test --manifest-path openless-all/app/src-tauri/backend-tests/Cargo.toml\nTested: cargo test --manifest-path openless-all/app/src-tauri/Cargo.toml --lib\nTested: git diff --check\nNot-tested: GitHub Windows runner result after push.\n\nCo-authored-by: OmX <omx@oh-my-codex.dev>
The Vocabulary screen advertises that entries are passed to "both ASR
hotword and post-processing model context", and that contract holds for
Volcengine ASR (which receives them as hotwords) and the Polish LLM
(which receives them as a system-prompt block). Whisper-compatible
providers (whisper / siliconflow / zhipu / groq) were the gap: their
client never set the OpenAI `prompt` field, so the dictionary had no
effect on the STT stage at all.
That gap shows up as inconsistent transliteration of registered proper
nouns. A user who registers "梁山泊" still gets "両山泊" / "良山泊" /
"りょうざんぱく" from Whisper, and the Polish LLM cannot always recover
once the script choice has already drifted.
This change:
- Adds an optional `prompt: Option<String>` to `WhisperBatchASR` and
attaches it as a `prompt` multipart field when non-blank. Empty/blank
prompts are skipped defensively so they cannot trigger
provider-specific 4xx responses.
- Adds `build_prompt_from_phrases`, a pure helper that joins enabled
vocabulary phrases with `, ` and a trailing period, capped at 240
characters (Whisper's published limit is 244 tokens; CJK is worst-case
~1 token/char, so 240 chars is the safe cap). Over-budget entries are
skipped individually rather than truncating mid-list, which preserves
registration order and lets short later entries still ride along when
one earlier entry is unusually long.
- Wires `enabled_phrases(inner)` through the helper at the existing
Whisper-compatible branch in `coordinator.rs`, mirroring how the
Volcengine branch already builds its hotword list.
Existing call sites unchanged in behavior when the dictionary is empty
(`None` prompt → no `prompt` field on the request). No new dependencies.
Tests in `asr::whisper::tests` cover: empty / all-blank input returns
None; single and multi phrase joining; per-entry trimming; mid-list
blanks are skipped without breaking the join; an over-long single entry
is dropped while shorter entries that follow it are kept; the resulting
string never exceeds the budget; truncation prefers earlier entries.
The backend-tests crate is intentionally an integration-test harness, but declaring the target explicitly makes Cargo discovery and review intent unambiguous. The harness comment also records that path-included modules run their own cfg(test) unit tests for hotkey, recorder, and insertion without linking the full Tauri library.\n\nConstraint: Stay within Rust-only backend tests; do not add a Tauri lib/runtime target.\nRejected: Add src/lib.rs just to satisfy target discovery | would introduce an unused crate surface instead of the actual test harness.\nConfidence: high\nScope-risk: narrow\nTested: cargo test --manifest-path openless-all/app/src-tauri/backend-tests/Cargo.toml -- --list | grep -E '^(hotkey|recorder|insertion)::'\nTested: cargo test --manifest-path openless-all/app/src-tauri/backend-tests/Cargo.toml\nTested: git diff --check\nNot-tested: GitHub Windows runner rerun result pending after push.\n\nCo-authored-by: OmX <omx@oh-my-codex.dev>
Windows intentionally normalizes cmd/command shortcut modifiers to ctrl, while macOS and Linux keep the super bit. The Rust-only backend test crate now executes these source tests on Windows, so the assertion must follow the same platform contract as production parsing instead of assuming the non-Windows result.\n\nConstraint: Keep the fix inside Rust backend tests and avoid changing shortcut behavior.\nRejected: Change normalize_modifier_tag to keep cmd as super on Windows | would alter user-facing hotkey semantics.\nConfidence: high\nScope-risk: narrow\nTested: cargo test --manifest-path openless-all/app/src-tauri/backend-tests/Cargo.toml\nTested: cargo test --manifest-path openless-all/app/src-tauri/Cargo.toml --lib\nNot-tested: GitHub Windows runner rerun result pending after push.\n\nCo-authored-by: OmX <omx@oh-my-codex.dev>
Windows cannot reliably execute the full Tauri lib test harness in CI because the process exits before Rust tests start in the desktop runtime loader path. Split the coordinator session state transitions into a Tauri-free Rust module used by coordinator.rs, and add a small Rust-only backend test crate that path-includes coordinator/hotkey/recorder/insertion logic for real Windows execution.
Constraint: User scoped this follow-up to backend-only Rust-to-Rust tests, not app/runtime smoke tests.
Constraint: Existing Windows CI still keeps full lib --no-run cfg/link coverage.
Rejected: Keep trying to execute cargo test --lib on Windows | it starts the full Tauri-linked test binary and fails before unit tests run.
Rejected: Rework release/runtime DLL handling | outside the backend unit-test scope.
Confidence: high
Scope-risk: moderate
Directive: Keep Windows actual unit execution on the Rust-only backend-tests manifest unless the full Tauri lib harness is proven runnable on clean Windows CI.
Tested: cargo test --manifest-path openless-all/app/src-tauri/backend-tests/Cargo.toml
Tested: cargo test --manifest-path openless-all/app/src-tauri/Cargo.toml --lib
Tested: cargo check --manifest-path openless-all/app/src-tauri/Cargo.toml
Tested: python yaml parse for .github/workflows/ci.yml; git diff --check
Not-tested: Windows runner execution; intended to be verified by GitHub Actions.
Related: #295
Co-authored-by: OmX <omx@oh-my-codex.dev>
Auto-update was failing on Windows because OpenLessIme.dll is a TSF input-method
DLL: once registered, Windows maps it into every text-input client process
(browsers, IDEs, chat apps...). The installer's PREINSTALL hook tried to
overwrite the file directly while it was still mapped, hitting a sharing
violation and aborting the update.
NSIS hook (openless-ime-hooks.nsh):
- PREINSTALL now calls OPENLESS_IME_UNREGISTER_X86/X64 first to release the
COM/TSF registration. On a fresh install this is a harmless no-op (existing
unregister macros log the exit code without aborting).
- DLL bytes are written to OpenLessIme.dll.new and atomically renamed into
place. If the old DLL is still locked by a TSF client process, Delete fails
and we fall back to Rename /REBOOTOK + SetRebootFlag — Windows queues the
replacement via PendingFileRenameOperations and the new bytes activate after
the next reboot. Same TSF registration path is preserved, so re-registration
in POSTINSTALL stays valid across the reboot.
WiX (openless-ime.wxs):
- UnregisterOpenLessImeX86/X64 now also fire Before InstallFiles when a
previous version is installed (Installed AND NOT REMOVE). Original condition
(REMOVE="ALL", Before RemoveFiles) only covered the dedicated uninstall
sequence; auto-update / minor-upgrade skipped it entirely and ran straight
into the same sharing violation as NSIS.
Bumps to 1.2.24-5 (Beta-5).
Co-authored-by: baiqing <lbx12309@icloud.com>
The #295 unit suite now runs in CI, but the Windows clean runner exits before the lib test harness starts with STATUS_ENTRYPOINT_NOT_FOUND when optional native runtime entrypoints are absent. Windows still needs cfg/link coverage, while macOS and Linux can execute the shared Rust unit suite.
Constraint: Preserve the new Rust test gate without making optional Windows native runtime installation a CI prerequisite.
Rejected: Install Foundry/Windows App Runtime into every CI run | broader, slower, and unrelated to backend unit-test coverage.
Confidence: high
Scope-risk: narrow
Directive: Do not switch Windows back to executing the lib test binary until the optional native runtime dependency is present on the runner.
Tested: cargo test --manifest-path openless-all/app/src-tauri/Cargo.toml --lib
Not-tested: Windows CI rerun before push.
The first #295 pass made cargo tests visible but still left the reviewer-requested state-machine, modifier-edge, and insertion fallback contracts too implicit. This adds narrow unit coverage around those contracts without refactoring runtime flow.\n\nConstraint: Address review compliance with minimal production changes.\nRejected: Split coordinator or hotkey adapters for testability | broader than the issue and riskier for the active PR.\nConfidence: high\nScope-risk: narrow\nTested: cargo test --manifest-path openless-all/app/src-tauri/Cargo.toml --lib; cargo check --manifest-path src-tauri/Cargo.toml; npm run build\nNot-tested: Live macOS/Windows native hotkey hooks and clipboard/AX behavior.
Issue #295 calls out that core Rust paths have been too dependent on manual app runs. Keep the change narrow by covering existing pure state/audio/clipboard helpers instead of refactoring coordinator or touching hardware-facing code, then wire the existing lib test suite into the lightweight CI lane.
Constraint: Global hotkey, recorder, and insertion integration still require real OS permissions and devices.
Rejected: Split coordinator before adding tests | broader refactor would raise review risk and is tracked separately.
Confidence: high
Scope-risk: narrow
Directive: Keep Rust CI on --lib unless integration tests get explicit OS fixtures.
Tested: npm run build
Tested: cargo test --manifest-path src-tauri/Cargo.toml --lib
Tested: cargo check --manifest-path src-tauri/Cargo.toml
Not-tested: Live microphone, global hotkey, clipboard, and insertion device paths.
Related: #295
The desktop WebView no longer runs with an unexplained disabled CSP. The policy keeps scripts self-hosted, permits Tauri IPC and local Vite development connections, keeps provider/update/model networking on the Rust side, and documents the only unsafe allowance for React inline styles used by the existing pixel-aligned UI.
Constraint: Issue #226 requires csp:null to be replaced or explicitly bounded
Constraint: Provider validation, updater checks, and model downloads are Rust-side paths, not frontend fetch allowlist entries
Rejected: Keep csp:null with documentation only | weaker defense-in-depth and leaves the WebView effectively unconstrained
Confidence: high
Scope-risk: narrow
Directive: Do not add external provider domains to WebView connect-src unless a frontend fetch path is introduced
Tested: npm run build
Tested: npm run tauri -- info
Tested: cargo check --manifest-path src-tauri/Cargo.toml
The dictation coordinator used a wrapping u64 counter to guard delayed recorder and startup events. Replacing that guard with UUID-backed session ids removes the theoretical wraparound collision while keeping the existing stale-event comparison model.
Constraint: Issue #297 requests the UUID-based fix rather than checked arithmetic.\nRejected: checked_add with warning | still preserves a numeric boundary condition instead of eliminating the collision class.\nConfidence: high\nScope-risk: narrow\nTested: cargo check --manifest-path openless-all/app/src-tauri/Cargo.toml\nTested: cargo test --manifest-path openless-all/app/src-tauri/Cargo.toml coordinator::tests:: --lib\nNot-tested: Full cargo test --lib still has unrelated existing failure in types::tests::legacy_hotkey_trigger_still_produces_effective_key_codes.\nRelated: #297
Hotkey settings are a shared context for multiple pages, so one rejected IPC call must not leave the provider in a permanent loading state. The refresh path now records a localized recoverable error, keeps any successfully loaded half of the state, and always releases loading so consumers can render retry UI instead of staying blank.
Constraint: Issue #316 requires local load failures to stay visible and non-fatal.
Rejected: Keep Promise.all with one catch | it would still discard partial success from the other IPC call.
Confidence: high
Scope-risk: narrow
Tested: git diff --check; npm run build; cargo check --manifest-path src-tauri/Cargo.toml
Not-tested: Manual Tauri UI session with a deliberately corrupted preferences.json.
Related: https://github.com/appergb/openless/issues/316
baiqing