Skip to content

Log local TTS volume and alert lifecycle diagnostics - #583

Draft
nedtwigg wants to merge 5 commits into
redact-speechfrom
alert-diagnostics
Draft

Log local TTS volume and alert lifecycle diagnostics#583
nedtwigg wants to merge 5 commits into
redact-speechfrom
alert-diagnostics

Conversation

@nedtwigg

@nedtwigg nedtwigg commented Sep 6, 2026

Copy link
Copy Markdown
Member

Local speech currently leaves no usage record, and an alert heard after a long focus gap cannot be traced back to its manager decision, renderer timer, or speech-engine queue. Add local JSONL diagnostics to both desktop hosts and a daily character-volume report for planning ElevenLabs usage.

Diagnostics default off behind the shared alertDiagnosticsConfig.enabled source flag in lib/src/lib/alert-diagnostics-config.ts. Set it to true, rebuild the desktop frontend and host/sidecar bundles, and restart to collect a sample. The disabled path emits no diagnostic IPC, installs no diagnostic focus listeners, and writes or prunes no log files, including direct lifecycle records. docs/alert-diagnostics-removal.md lists the removal steps.

The journal records the final redacted spoken text, character counts, separate initial/requeue/test attempts, and engine outcomes. Alert traces include dismissal and attention, completion suppression, detector output counters, deferral deadlines, timer lateness, renderer snapshots, and focus/visibility. No PTY bytes, raw commands, or notification bodies are recorded. Bounded queues, rotation, retention, and loss markers keep diagnostics best-effort and separate from alert state.

Usage: node scripts/summarize-alert-log.mjs <alert-logs-directory>. Standalone writes under its app-data directory; VS Code writes under extension global storage and reports the path in the Dormouse output channel. This adds observability only; ElevenLabs delivery/fallback and fixes to alert behavior are follow-up work.

Stacked on #582 (redact-speech).

Validation: full pnpm test; focused logging tests covering hour-long gaps, rearmed animation deferral, delayed speech starts/timers, redaction, retries, disk failure, permissions and rotation; standalone sidecar bundling; pnpm exec cargo check --manifest-path standalone/src-tauri/Cargo.toml.

Default-off validation: 53 targeted tests, including normal speech with no diagnostic emission, no focus listeners, and no journal directory; lib and extension TypeScript checks; spec lint; sidecar bundling.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Sep 6, 2026

Copy link
Copy Markdown

Deploying mouseterm with  Cloudflare Pages  Cloudflare Pages

Latest commit: 34bbd09
Status: ✅  Deploy successful!
Preview URL: https://2771b0cd.mouseterm.pages.dev
Branch Preview URL: https://alert-diagnostics.mouseterm.pages.dev

View logs

@dormouse-bot dormouse-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewing as a draft — flagging anything that looks worth a quick fix. Mark ready for a full review.

The journal/emitter bounds match the spec exactly (100/200 per second, 512 × 8 KiB, 4 MiB / 24 h rotation, 30-day + 64 MiB − one-file headroom), the redaction path is right (toSpokenText caps at 120 code points, well under the validator's 512, so text can't be silently rejected), and the nlink === 0 reopen for the multi-window case is a nice catch. Seven notes inline; the first is the one I'd fix before this lands.

Comment thread vscode-ext/src/extension.ts Outdated
Comment thread standalone/src-tauri/src/lib.rs Outdated
Comment thread lib/src/lib/alert-manager.ts Outdated
Comment thread lib/src/lib/alert-manager.ts Outdated
Comment thread lib/src/host/alert-journal.ts Outdated
Comment thread standalone/sidecar/main.js Outdated
Comment thread standalone/scripts/build-sidecar-proxy.mjs
@dormouse-bot

Copy link
Copy Markdown
Collaborator

Re-reviewed aa93d197 — all seven are addressed, and each fix does what the reply says.

The two I checked hardest: the loss marker now clears this module's own isAlertDiagnostic (journal:<uuid> source under the 100-char cap, seq >= 1, journal.dropped matching ^[a-zA-Z.]+$, finite monotonicMs), and the overflow test pins it with expect(all.every(isAlertDiagnostic)).toBe(true). The suppression ladder now reads the branch actually taken, and the new test picks the case where the old mirrored ladder would have lied — unarmed and attended reports not-armed, matching the switch. alertDiagnosticsEnabled() is exactly sink !== undefined, the same guard alertDiagnostic already applied, so gating trace and setTerminalActivity on it changes nothing but the allocation.

One consequence of tracing on the way out rather than up front, worth knowing before someone reads a journal rather than worth changing: manager.completion now lands after the manager.notify records its decision caused, so within a single dispatch the effect has a lower seq than the decision.

Per-finding verification
Finding Fix at aa93d197
vscode-ext/src/extension.ts — close before teardown Both calls moved below gracefulKillAll/killAll, wrapped in a 250ms Promise.race with clearTimeout, matching the sidecar's shutdown(). docs/specs/vscode.md records the ordering; pnpm lint:specs is green.
standalone/src-tauri/src/lib.rs — comment adjacency alert_diagnostic moved below burrow_command, so the burrow-bridge docblock heads burrow_command again, and the moved fn has its own header.
lib/src/lib/alert-manager.ts — mirrored reason ladder traceDecision(reason) fires once at the end; every break sets its own reason. All arms break, so no path skips the trace, and claimed traces before its early return.
lib/src/lib/alert-manager.ts — eager snapshot trace early-returns on alertDiagnosticsEnabled() before touching this.entries, so manager.onResize no longer builds a snapshot on hosts without a journal.
lib/src/host/alert-journal.ts — partial loss envelope Writer-owned source/seq/monotonicMs on the marker; the outer source is the one used in the drain loop, not the shadowed destructure in append.
standalone/sidecar/main.js — above the header docblock Require moved beside the other generated .cjs requires with the provenance comment.
standalone/scripts/build-sidecar-proxy.mjs — stale index alert-journal.ts → sidecar/alert-journal.cjs added.

Not run locally: the suite (node_modules isn't installed in this checkout) — Build & Test was still pending at the time of writing. pnpm lint:specs ran clean.

@dormouse-bot dormouse-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewing as a draft — flagging anything that looks worth a quick fix. Mark ready for a full review.

All seven earlier notes are addressed, and the two structural ones came out well: every dispatchCompletion path now falls through to a single traceDecision, so the mirrored predicate ladder is gone, and the loss marker carries a real envelope (journal:<uuid>, seq >= 1, monotonicMs) that isAlertDiagnostic accepts — the new expect(all.every(isAlertDiagnostic)).toBe(true) pins it. alertDiagnosticsEnabled() gating in both AlertManager.trace and setTerminalActivity removes the discarded-snapshot cost on Pocket, the demo, and unsink'd tests. The Rust move, the sidecar require + provenance comment, and the bundler header index all check out; pnpm lint:specs passes with the new vscode.md sentence.

Three notes inline. The first two are on the shutdown/trace reordering — one of them is my earlier suggestion going one line too far, and I would unwind that half.

Comment thread lib/src/lib/alert-manager.ts
Comment thread vscode-ext/src/extension.ts Outdated
Comment thread lib/src/host/alert-journal.ts Outdated
@dormouse-bot

Copy link
Copy Markdown
Collaborator

Re-verified 4f49e385 — all three are addressed.

The decision snapshot is the one I cared about most, and the new test picks exactly the case the old code got wrong: a deferred OSC 9 waiting when commandFinished arrives. manager.completion now reports pendingNotification: 'OSC 9', ringSeq: 0, todo: false (the inputs), and manager.publish reports ALERT_RINGING, pendingNotification: null, todo: true (the outputs) — so the record that explains where the ring's detail came from finally shows it. traceFields returning {} when the sink is absent keeps the no-journal hosts allocation-free, and alertDiagnostic applies the same sink !== undefined guard trace did, so dropping through it changes nothing else.

Both hosts now bracket teardown, and the asymmetry is the right one: host.stopping sits beside step('starting') / at the top of shutdown(), before the ~5 s of teardown that VS Code's budget usually cuts short, so it is the marker that actually gets written; host.stopped stays at the far end where reaching it is itself the signal. The bounded 250 ms close() is unchanged in both.

One consequence worth knowing before someone reads a standalone journal rather than worth changing: the sidecar's lifecycle markers carry the writer identity (journal:<uuid>) because that process has no emitter of its own — renderer records arrive over IPC already stamped with the renderer's source — and summarizeAlertLogs now excludes exactly that prefix. So host.stopping / host.stopped are attributable to a counted source on the VS Code host and to a deliberately-uncounted one on standalone.

Per-finding verification
Finding Fix at 4f49e385
lib/src/lib/alert-manager.ts — eligible branches logged outputs as inputs traceFields(id) extracted from trace; const atDecision = this.traceFields(id) runs before the switch, and traceDecision spreads it first. claimed is still computed ahead of it, which is unavoidable — it is a decision output.
vscode-ext/src/extension.tshost.stop written where nothing reaches Split into host.stopping at extension.ts:246 (beside step('starting')) and host.stopped at extension.ts:304; closeAlertJournal() and its 250 ms race stay below both. No host.stop remains anywhere in the tree.
standalone/sidecar/main.js — no shutdown marker at all alertJournal.recordLifecycle('host.stopping') at the shuttingDown latch and 'host.stopped' after mgr.killAll(), both before the close race. recordLifecycle builds a full writer envelope through writerRecord, so isAlertDiagnostic accepts it (journal:<uuid> under the 100-char cap, seq >= 1, ^[a-zA-Z.]+$ event).
scripts/summarize-alert-log.mjs — writer identities inflating sources typeof record.source === 'string' && !record.source.startsWith('journal:'), the meaning stated in docs/specs/alert.md → Local alert diagnostics, and assert.equal(summary.sources, 1) over a fixture whose loss marker uses a real writer envelope.

Ran locally: node scripts/spec-lint.mjs (OK, 33 specs / 65 files) and node scripts/summarize-alert-log.test.mjs (1/1 pass — it needs no install). Not run: the vitest suites, since node_modules isn't installed in this checkout; Build & Test was still pending at the time of writing, so alert-diagnostics.test.ts and alert-journal.test.ts are unverified here.

@dormouse-bot dormouse-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewing as a draft — flagging anything that looks worth a quick fix. Mark ready for a full review.

The three earlier notes are all closed out correctly. traceFields is captured before the switch, so manager.completion now carries the decision's inputs on every branch — the new alert-diagnostics.test.ts case is a good pin for it: attention expires on the 100 ms timer during the first advanceTimersByTime(500), which is what arms the exit and makes eligible (not attended) the right expectation, and the decision/publication pair shows pendingNotification: 'OSC 9'null across flushDeferredNotification. host.stopping at the top of both hosts' shutdown answers the "clean quit or killed host" question without depending on [deactivate] done ever being reached, and journal.recordLifecycle keeps the journal open across teardown. summarize-alert-log.test.mjs passes against the new journal: filter (ran it directly — it has no node_modules dependency); the vitest suites are on CI.

One note inline, on the standalone side of the new markers.

Comment thread standalone/sidecar/main.js

@dormouse-bot dormouse-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewing as a draft — flagging anything that looks worth a quick fix. Mark ready for a full review.

The gate is complete across all three bundles, and I checked each of the three claims in the new spec bullet against the code rather than the tests. No IPC: the renderer's only route to recordAlertDiagnostic is sink inside alertDiagnostic, so both vscode-adapter.ts and tauri-adapter.ts go quiet. No listeners: observeAlertFocus returns the no-op before touching window, and platform/index.ts calls it only after configureAlertDiagnostics, so ordering is right on an enabled build too. No files, no pruning: append is the sole entry to queue, pump runs only from append, and drain is the only caller of mkdir and prune — so nothing reaches disk, including recordLifecycle, which routes through append rather than around it. build-sidecar-proxy.mjs runs bundle: true, so alert-diagnostics-config.ts is inlined into alert-journal.cjs and the sidecar carries its own baked copy, which is what makes "rebuild the host/sidecar bundles" the accurate instruction. pnpm lint:specs is green with the ratcheted budget.

Two notes inline. One more, on text this PR doesn't touch:

docs/specs/vscode.md → Local alert diagnostics still opens "Must append host decisions and renderer diagnostic messages to the same local journal", and docs/specs/standalone.md says alert diagnostics "live in <app_data_dir>/alert-logs/, written by the Node sidecar through alert_diagnostic". Both read as unconditional present tense, and above the fold that means "the code as it is" — but a default build now creates no directory on either host, and on VS Code initAlertJournal returns before createAlertJournal is even called. Each already points at alert.md for behavior, so the fix is the same scoping clause you added to the alert.md bullet one line up: "when diagnostics are enabled". Outside the diff, so no inline suggestion — happy to push it if you'd rather not hand-edit two specs.

Comment on lines +34 to +35
const windowListener = vi.spyOn(window, 'addEventListener');
const documentListener = vi.spyOn(document, 'addEventListener');

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This case passes with alertDiagnosticsConfig.enabled = true, so it doesn't pin the rule the spec says it pins.

No sink is configured here — the afterEach above leaves configureAlertDiagnostics('off'), so sink is undefined. alertDiagnosticsEnabled() is alertDiagnosticsConfig.enabled && sink !== undefined, and the second term alone already makes it false. Flip the flag to true and this test still goes green while a real enabled build installs all five listeners.

The other two cases don't have this problem: the first configures a sink explicitly, and the third's gate is !alertDiagnosticsConfig.enabled on its own inside append. Configuring a sink first makes this one isolate the flag the same way:

Suggested change
const windowListener = vi.spyOn(window, 'addEventListener');
const documentListener = vi.spyOn(document, 'addEventListener');
configureAlertDiagnostics('renderer', vi.fn());
const windowListener = vi.spyOn(window, 'addEventListener');
const documentListener = vi.spyOn(document, 'addEventListener');

}

export function configureAlertDiagnostics(label: string, write?: (record: AlertDiagnostic) => void): void {
sink = alertDiagnosticsConfig.enabled ? write : undefined;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This second gate is redundant, and it's the one that can strand you.

sink has exactly one consumer — alertDiagnostic, which already returns on !alertDiagnosticsEnabled(), and that reads the flag live. So capturing it here changes nothing about what gets recorded; it only makes the flag order-dependent: set it after configureAlertDiagnostics has run and sink stays undefined forever, with alertDiagnosticsEnabled() reporting false and no error anywhere. Every diagnostics test already has to set the flag in beforeEach above its configureAlertDiagnostics call for that reason, which is a quiet trap for the next one added.

Dropping it also makes a runtime flip work for anyone collecting a sample without a full rebuild, and the gate test still passes — case 1 asserts sink is never called, which the live check in alertDiagnostic delivers on its own.

Suggested change
sink = alertDiagnosticsConfig.enabled ? write : undefined;
sink = write;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants