feat(stt): Always-On Push To Transcribe Mode#397
Closed
Coldaine wants to merge 1 commit intotauri-basefrom
Closed
Conversation
Code Review SummaryStatus: 1 WARNING, 1 SUGGESTION | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
SUGGESTION
Other Observations (not in diff)
Files Reviewed (2 files changed in this diff)
Files Reviewed (Previous findings on unchanged files)
Reviewed by trinity-large-thinking · 439,912 tokens |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR prunes legacy agent/prompt docs and deprecated scripts, consolidates AI instruction anchors into AGENTS.md, adds shadow-mode agentic CI reviewers (evidence assessor and northstar reviewer) with a GitHub Actions workflow, migrates STT references from Whisper to Moonshine/Parakeet, and implements an AlwaysOnPushToTranscribe activation mode with rolling-buffer pre-roll audio handling plus related runtime and test updates. Changes
Sequence DiagramsequenceDiagram
participant Mic as Microphone
participant RB as Rolling Buffer
participant AP as Audio Processor
participant SE as Session Event
participant STT as STT Engine
participant Out as Transcription Output
rect rgba(100,150,200,0.5)
Note over Mic,RB: Idle — AlwaysOnPushToTranscribe
Mic->>RB: Stream audio
RB->>RB: Accumulate (~2s), cap size
end
rect rgba(150,100,200,0.5)
Note over Mic,AP: Hotkey pressed
Mic->>AP: Immediate audio frames
RB->>AP: Drain pre-roll into processor
AP->>SE: Emit SessionEvent::Start / begin_utterance
AP->>STT: Async send pre-roll (begin_utterance → process_audio)
AP->>STT: Stream live speech audio
end
rect rgba(200,150,100,0.5)
Note over STT,Out: Transcription
STT->>STT: Process pre-roll + live audio
STT->>Out: Emit transcription result
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4fd3525f5a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Self::send_event_static(&self.event_tx, &self.metrics, err_event).await; | ||
| } | ||
| } | ||
| } else if self.settings.activation_mode == crate::stt::session::ActivationMode::AlwaysOnPushToTranscribe { |
There was a problem hiding this comment.
This change inserts an else if chain here after removing the prior incremental else { let should_process = ... } block, but the function still references if should_process later. With STT features enabled (moonshine, parakeet, or http-remote), handle_audio_frame now has an undefined guard and fails to compile, blocking the new mode entirely.
Useful? React with 👍 / 👎.
| )? | ||
| } | ||
| ActivationMode::Hotkey => crate::hotkey::spawn_hotkey_listener(self.raw_vad_tx.clone()), | ||
| ActivationMode::Hotkey | ActivationMode::AlwaysOnPushToTranscribe => crate::hotkey::spawn_hotkey_listener(self.raw_vad_tx.clone()), |
There was a problem hiding this comment.
This runtime-switch path treats AlwaysOnPushToTranscribe the same as Hotkey by only swapping the trigger listener, but the STT processor’s settings.activation_mode is set once at startup and never updated. In practice, toggling to Always-On from the dashboard won’t enable rolling pre-roll buffering, so users get plain push-to-talk behavior instead of the new mode.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Implements an “Always-On Push To Transcribe” activation mode (2s pre-roll) and performs a broad cleanup/migration of legacy STT artifacts, agent instruction duplication, and documentation/CI scaffolding.
Changes:
- Add
AlwaysOnPushToTranscribeactivation mode and introduce a rolling pre-roll audio buffer that flushes on hotkey start. - Rename STT model env var to
STT_MODEL_PATHand update related docs/comments/config references away from removed Whisper/Vosk artifacts. - Add “shadow mode” agentic PR reviewer workflows/prompts and consolidate agent instruction sources while pruning outdated scripts/docs.
Reviewed changes
Copilot reviewed 74 out of 92 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| test_enigo_live.rs | Remove loose root-level Enigo smoke test. |
| scripts/verify_vosk_model.sh | Remove dead Vosk model verification wrapper. |
| scripts/start-headless.sh | Remove obsolete Xvfb/headless helper script. |
| scripts/ensure_venv.sh | Remove pip-based venv bootstrap (uv-only direction). |
| scripts/ensure_agent_hardlinks.sh | Switch hardlink source to .github/copilot-instructions.md and mirror to AGENTS.md + Kilocode rules. |
| scripts/ci/setup-vosk-cache.sh | Remove dead whisper/vosk cache setup script. |
| scripts/check_markdown_placement.py | Update approved exceptions list (now includes AGENTS.md). |
| README.md | Update “current status” links and remove CLAUDE.md pointer. |
| plugins.json | Remove duplicate root plugin selection config. |
| GEMINI.md | Remove duplicate agent instructions file. |
| docs/todo.md | Update plan links to docs/plans/current-status.md. |
| docs/tasks/ci-runner-readiness-proposal.md | Delete superseded CI runner readiness proposal doc. |
| docs/standards/agent-rules.md | Update “what works” anchor to current-status.md. |
| docs/standards.md | Update allowed-root-doc exceptions list and agent onboarding source statement. |
| docs/revision_log.csv | Remove empty revision log CSV (header-only). |
| docs/reviews/reviewer_driven_evidence.md | Add reviewer-driven evidence workflow doc. |
| docs/reviews/portable_standard_critique.md | Add critique doc for the evidence standard. |
| docs/research/exhaustive-dependency-audit-2026-03-24.md | Update plan link to current-status.md. |
| docs/reference/crates/coldvox-gui.md | Update execution-priorities link to current-status.md. |
| docs/prompts/review-and-implement-evidence-assessor.md | Add meta-prompt for reviewing/implementing the assessor. |
| docs/playbooks/organizational/pr_playbook.md | Delete empty stub playbook. |
| docs/playbooks/organizational/github_governance.md | Delete empty stub playbook. |
| docs/plans/windows-multi-agent-recovery.md | Remove outdated plan (replaced by current-status.md). |
| docs/plans/parakeet-http-remote-integration-spec.md | Update plan anchor to current-status.md. |
| docs/plans/current-status.md | Expand “current reality” including branch strategy (tauri-base mainline). |
| docs/plans/cleanup-plan.md | Add cleanup plan documenting deletions/archival and reference fixes. |
| docs/plans/agentic-evidence-preview.md | Add spec/architecture for agentic evidence assessor CI job. |
| docs/index.md | Regenerate docs index reflecting pruning/moves and new invalid/missing-frontmatter items. |
| docs/history/2025-11-06_04-16Z-branch-status-and-work-in-progress-overview.md | Delete historical transcript marked for deletion. |
| docs/domains/stt/stt-overview.md | Remove Whisper from supported backend list. |
| docs/domains/gui/gui-design-overview.md | Update execution-priorities link to current-status.md. |
| docs/domains/foundation/fdn-voice-pipeline-core-requirements.md | Add “always-on push-to-transcribe” requirements text. |
| docs/domains/foundation/fdn-testing-guide.md | Add note about Parakeet tests not validated; suggest moonshine. |
| docs/domains/audio/aud-user-config-design.md | Correct Moonshine/Parakeet descriptions (PyO3 fragility / not validated). |
| docs/archive/root/WINDOWS_IMPLEMENTATION_SUMMARY.md | Add archived Windows implementation summary doc. |
| docs/archive/root/VERIFICATION_REPORT.md | Add archived verification report doc. |
| docs/archive/root/PYO3_DLL_TROUBLESHOOTING.md | Add archived PyO3 DLL troubleshooting guide. |
| docs/archive/root/observability-playbook.md | Add archived organizational observability playbook with frontmatter. |
| docs/archive/root/logging.md | Add archived logging reference with frontmatter. |
| docs/archive/root/FINAL_REPORT.md | Add archived final report doc. |
| docs/archive/root/DEPENDENCY_AUDIT_ISSUE.md | Add archived dependency-audit action items doc. |
| docs/archive/research/pr-reports/PR-temp-injection-path-alignment.md | Delete expired PR temp report. |
| docs/archive/research/pr-reports/PR-temp-comprehensive-testing-report.md | Delete expired PR temp report. |
| docs/archive/research/pr-reports/PR-temp-clipboard-test-timeout-fixes.md | Delete expired PR temp report. |
| docs/archive/research/dependency-audit-report-2025-02-09.md | Add archived dependency audit report doc. |
| docs/archive/research/audio-quality-monitoring.md | Add archived research doc for audio quality monitoring. |
| docs/archive/reference/crates/coldvox-stt.md | Delete summarize-marked stub index doc. |
| docs/archive/plans/roadmap.md | Add archived roadmap doc with frontmatter. |
| docs/archive/plans/gui/raw-gui-plan.md | Delete dead Qt GUI plan doc. |
| docs/archive/plans/gui/aspirational-gui-plan.md | Delete dead Qt GUI plan doc. |
| docs/archive/domains/tele-observability-playbook.md | Add archived telemetry observability playbook with frontmatter. |
| docs/archive/domains/aud-pipewire-design.md | Add archived PipeWire/ALSA design doc with frontmatter. |
| docs/architecture/adr/index.md | Delete empty ADR index placeholder. |
| docs/architecture.md | Update execution state link to current-status.md (navigation still references removed paths). |
| deny.toml | Add cargo-deny advisory ignores for known-unmaintained transitive GTK/Tauri deps. |
| crates/coldvox-stt/src/types.rs | Rename env var WHISPER_MODEL_PATH → STT_MODEL_PATH in defaults. |
| crates/coldvox-stt/src/plugin.rs | Update docs/examples from Whisper to Moonshine/Parakeet/http-remote. |
| crates/coldvox-stt/src/plugin_types.rs | Update accuracy-level examples (docs). |
| crates/coldvox-stt/Cargo.toml | Remove stale comment about faster-whisper. |
| crates/app/tests/integration/text_injection_integration_test.rs | Change forced STT plugin from whisper → moonshine. |
| crates/app/tests/integration/capture_integration_test.rs | Change forced STT plugin from whisper → moonshine. |
| crates/app/src/stt/tests/mod.rs | Update model env var references to STT_MODEL_PATH. |
| crates/app/src/stt/session.rs | Add AlwaysOnPushToTranscribe activation mode. |
| crates/app/src/stt/processor.rs | Add rolling pre-roll buffer + pre-roll flush logic (currently introduces compile-breaking control-flow). |
| crates/app/src/runtime.rs | Add runtime activation mode variant + propagate into STT processor settings and hotkey handling. |
| crates/app/src/lib.rs | Derive Default for SttRemoteAuthSettings (remove manual impl). |
| crates/app/src/bin/tui_dashboard.rs | Add UI label + toggle cycle for new activation mode. |
| crates/app/plugins.json | Remove legacy app-local plugin selection config (whisper). |
| CLAUDE.md | Remove duplicate agent instructions file. |
| CHANGELOG.md | Update anchors from windows-multi-agent-recovery → current-status; remove CLAUDE/GEMINI references. |
| AGENTS.md | Expand into full canonical agent instructions content. |
| .kilocode/rules/agents.md | Update anchors to current-status.md and wording. |
| .gitignore | Add _tmp_*.md ignore with comment about Gemini CLI boundaries (comment currently contradicts behavior). |
| .github/workflows/agentic-evidence-preview.yml | Add shadow-mode “agentic reviewers” workflow (complexity scorer + Gemini assessor jobs). |
| .github/prompts/northstar-alignment-reviewer.md | Add prompt for northstar-alignment reviewer agent. |
| .github/prompts/evidence-assessor.md | Add prompt for evidence assessor agent. |
| .github/prompts/drive-project.prompt.md | Remove legacy project-driver mega-prompt. |
| .github/copilot-instructions.md | Update anchors to current-status.md and align agent instructions. |
| .github/agents/tester.agent.md | Remove legacy agent definition. |
| .github/agents/researcher.agent.md | Remove legacy agent definition. |
| .github/agents/project-driver.agent.md | Remove legacy agent definition. |
| .github/agents/implementer.agent.md | Remove legacy agent definition. |
Comments suppressed due to low confidence (1)
crates/app/src/stt/processor.rs:281
handle_audio_framecurrently referencesshould_processinside thebehavior != HotkeyBehavior::Incrementalbranch, butshould_processis not defined in this scope. This looks like the incremental-mode else-branch was removed without moving/retaining theshould_processcalculation, which will not compile and also mixes batch vs incremental processing logic. Consider restoring an explicitelsebranch for incremental behavior that computesshould_process(e.g., by checkingstate.state == UtteranceState::SpeechActivewithout holding the lock across.await), and keep the batch-mode branch limited to buffering only.
if behavior != HotkeyBehavior::Incremental {
// Batch mode: lock, buffer, and return.
let mut state = self.state.lock();
if state.state == UtteranceState::SpeechActive {
state.buffer.extend_from_slice(samples_slice);
if state.buffer.len() > BUFFER_CEILING_SAMPLES {
tracing::warn!(target: "stt", "Audio buffer ceiling reached. Defensively finalizing.");
self.handle_session_end(state.source, false, &mut state);
}
}
if should_process {
tracing::trace!(target: "stt_debug", "Dispatching {} samples to plugin.process_audio()", samples_slice.len());
match self
.plugin_manager
Comment on lines
+68
to
+70
| RUST_FILES=$(echo "$CHANGED" | grep "^crates/" | wc -l | tr -d '[:space:]') | ||
| CI_WORKFLOW_FILES=$(echo "$CHANGED" | grep "^\.github/workflows/" | wc -l | tr -d '[:space:]') | ||
| TOTAL_FILES=$(echo "$CHANGED" | wc -l | tr -d '[:space:]') |
There was a problem hiding this comment.
With set -euo pipefail, these pipelines will fail the step when there are zero matches because grep exits 1 (e.g., docs-only PRs). That will fail the entire workflow and prevent downstream jobs (since they needs: complexity-score). Use a non-failing count method (e.g., grep -c ... || true, or awk), or disable pipefail just for these pipelines.
Suggested change
| RUST_FILES=$(echo "$CHANGED" | grep "^crates/" | wc -l | tr -d '[:space:]') | |
| CI_WORKFLOW_FILES=$(echo "$CHANGED" | grep "^\.github/workflows/" | wc -l | tr -d '[:space:]') | |
| TOTAL_FILES=$(echo "$CHANGED" | wc -l | tr -d '[:space:]') | |
| RUST_FILES=$(printf '%s\n' "$CHANGED" | awk '/^crates\// { count++ } END { print count + 0 }') | |
| CI_WORKFLOW_FILES=$(printf '%s\n' "$CHANGED" | awk '/^\.github\/workflows\// { count++ } END { print count + 0 }') | |
| TOTAL_FILES=$(printf '%s\n' "$CHANGED" | awk 'NF { count++ } END { print count + 0 }') |
| pull_request: | ||
| types: [opened, synchronize, ready_for_review] | ||
| branches: | ||
| - main |
There was a problem hiding this comment.
This workflow only triggers for PRs targeting main, but docs/plans/current-status.md states tauri-base is the integration branch/future mainline and that feature branches should target it. If PRs are opened against tauri-base, this shadow reviewer workflow won't run. Consider adding tauri-base (and/or removing the branch filter) so it runs on the branches the repo expects contributors to use.
Suggested change
| - main | |
| - main | |
| - tauri-base |
Comment on lines
18
to
+22
| ## Navigation | ||
|
|
||
| - [Architecture Roadmap](./architecture/roadmap.md) | ||
| - [Architecture Decisions](./architecture/adr/) | ||
| - [Current Execution Plan](./plans/windows-multi-agent-recovery.md) - Current broken features tracking and near-term work | ||
| - [Current Execution State](./plans/current-status.md) - Current broken features tracking and near-term work |
There was a problem hiding this comment.
The navigation links point to docs/architecture/roadmap.md and docs/architecture/adr/, but the docs/architecture/ directory (and docs/architecture/adr/index.md) were removed in this PR. This leaves broken links in a top-level doc. Update these links to the new locations (e.g., the archived roadmap) or remove them if those sections no longer exist.
Comment on lines
+86
to
87
| - The system shall manage STT memory usage dynamically, unloading large models during extended idle periods while keeping lightweight engines ready for fast activation. | ||
| - The system shall manage STT memory usage dynamically, unloading large models during extended idle periods while keeping lightweight engines ready for fast activation. |
There was a problem hiding this comment.
There are two identical bullets back-to-back about dynamic STT memory management. This looks like an accidental duplication introduced in this edit; remove one of them to avoid redundant requirements.
Comment on lines
8
to
13
| APPROVED_EXCEPTIONS = { | ||
| Path("README.md"), | ||
| Path("CHANGELOG.md"), | ||
| Path("CLAUDE.md"), | ||
| Path("AGENTS.md"), | ||
| Path(".github/pull_request_template.md"), | ||
| } |
There was a problem hiding this comment.
docs/standards.md now lists .github/copilot-instructions.md as an approved Markdown exception outside /docs, but this hook's APPROVED_EXCEPTIONS does not include it. As written, a commit that edits .github/copilot-instructions.md would be rejected by this hook. Add .github/copilot-instructions.md to APPROVED_EXCEPTIONS (or otherwise align the hook with the documented exceptions).
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 26
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
deny.toml (1)
11-11:⚠️ Potential issue | 🟡 MinorRemove the unmatched license allowance to clear CI noise.
cargo denyalready warns thatUnicode-DFS-2016is not encountered, so this entry is stale in the current dependency graph.Suggested cleanup
- "Unicode-DFS-2016",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@deny.toml` at line 11, Remove the stale license allowance entry for "Unicode-DFS-2016" from deny.toml: locate the allowance list containing the string "Unicode-DFS-2016" and delete that entry so cargo-deny no longer reports an unmatched license allowance. Ensure the resulting deny.toml remains valid TOML (remove any trailing comma if it was the last item).crates/app/src/stt/processor.rs (2)
148-176:⚠️ Potential issue | 🟠 MajorPre-roll is not guaranteed to be prepended in incremental mode.
Line 152 moves the processor to
SpeechActivebefore the spawned task callsbegin_utterance()andprocess_audio(&pre_roll). Any live frame that reacheshandle_audio_frame()first can hit the plugin before the reset and before the pre-roll, which defeats the whole “prepend 2 seconds” guarantee. Becausebegin_utterance()is the reset path incrates/app/src/stt/plugin_manager.rs:1257-1260, this also leaves the session in a bad state if that async init fails. Gate live dispatch behind aStartingstate, or buffer frames until pre-roll has been flushed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/app/src/stt/processor.rs` around lines 148 - 176, Change the start path so live frames cannot reach plugins before the plugin is initialized: in the SessionEvent::Start handler set state.state to a new UtteranceState::Starting (instead of UtteranceState::SpeechActive) and keep incoming frames buffered (state.buffer / any new frames in handle_audio_frame should check for Starting and append to state.buffer rather than dispatching to plugin). Spawn the task that calls plugin_manager.write().await.begin_utterance() and, if successful, process pre_roll with plugin_manager.write().await.process_audio(&pre_roll); only after both succeed transition state.state to UtteranceState::SpeechActive and then flush any buffered frames to the plugin; on begin_utterance/process_audio error revert state (e.g., back to Idle or error state) and ensure buffered frames are handled/cleared to avoid inconsistent session state.
263-313:⚠️ Potential issue | 🔴 CriticalDefine the missing
should_processvariable before use.
should_processis referenced at line 278 but never declared or defined in this function. This prevents compilation. The braces are balanced in the control flow structure; remove the misleading claim about "unclosed delimiters."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/app/src/stt/processor.rs` around lines 263 - 313, In handle_audio_frame, the boolean should_process is referenced but not defined; fix by declaring it before use by inspecting the session state (e.g., let should_process = { let state = self.state.lock(); state.state == UtteranceState::SpeechActive }); this uses the existing self.state.lock() and UtteranceState enum to determine whether to call plugin_manager.process_audio; place this declaration before the if behavior != HotkeyBehavior::Incremental block (or at the top of that branch) so subsequent logic (including the plugin_manager.write().await call and rolling_buffer handling) can use the computed value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/prompts/evidence-assessor.md:
- Around line 18-23: Update the hard-coded ground-truth bullets (the assertions
about Moonshine, Parakeet, the feature flags "whisper, coqui, leopard,
silero-stt", and "STT streaming partial transcription") so they are synchronized
with the repository's authoritative status document (current-status.md): remove
or revise the stale fixed claims (lines that freeze older facts about Parakeet
and the feature-flag stubs) and instead reflect the current-state wording from
current-status.md (or reference it dynamically), ensuring the bullets for
Moonshine, Parakeet, the listed feature flags, and STT streaming match that
file’s descriptions.
In @.github/prompts/northstar-alignment-reviewer.md:
- Around line 97-112: The prompt contains a contradictory instruction set: the
"Missed Opportunity" section header asks for recommendations (lines with "Missed
Opportunity") while the "Critical Constraints" block (the DO NOT recommend line)
forbids recommending changes; update the prompt to remove the contradiction by
either renaming the "Missed Opportunity" header to a neutral term like "Gap
Summary" or changing the prohibition in the Critical Constraints to allow a
brief factual gap description; edit the
.github/prompts/northstar-alignment-reviewer.md file to choose one approach and
make the two pieces consistent so the reviewer is not forced to violate
constraints (refer to the "Missed Opportunity" header and the "DO NOT recommend
what the PR should have done differently" constraint when making the change).
In @.github/workflows/agentic-evidence-preview.yml:
- Around line 68-70: The scorer fails on no-match cases and miscounts Rust
files; change the RUST_FILES and CI_WORKFLOW_FILES computations to match file
extensions and avoid grep causing non-zero exits: use anchored regexes that
require .rs for crates (e.g., grep -E '^crates/.*\.rs$' || true) and require
.yml/.yaml for workflows (e.g., grep -E '^\.github/workflows/.*\.(ya?ml)$' ||
true) before piping to wc -l, keep TOTAL_FILES as-is, and assign to the same
variables RUST_FILES, CI_WORKFLOW_FILES, TOTAL_FILES so the downstream tier
logic gets correct counts without aborting on empty results.
In @.gitignore:
- Around line 42-46: The comment "NOTE: NOT gitignored — Gemini CLI respects
.gitignore..." conflicts with the actual ignore pattern `_tmp_*.md` present in
the file; pick one intent and make them consistent by either removing the
`_tmp_*.md` entry if these files must not be ignored, or updating the comment to
state that `_tmp_*.md` are intentionally ignored; modify the comment text
referencing the Gemini CLI behavior and the `_tmp_*.md` pattern accordingly and
ensure the final comment and the `_tmp_*.md` entry both reflect the chosen
intent.
In `@AGENTS.md`:
- Around line 3-110: AGENTS.md and .github/copilot-instructions.md are out of
sync; update the copilot instructions to match the canonical content in
AGENTS.md (or vice versa) so PR validation passes: ensure headings and key
sections (Anchor, Current Product Direction & Reality including STT Backend and
Python Environment, Working Rules, Commands, Feature Flags, CI Environment, Key
Files, PR Checklist) are identical, reconcile differences under symbols like
"STT Backend", "Working Rules", "Feature Flags" and verify command examples
(cargo/cargo run scripts) match; commit the synchronized
.github/copilot-instructions.md in the same PR.
In `@crates/app/src/bin/tui_dashboard.rs`:
- Around line 344-345: The new ActivationMode variant was added to the toggle
cycle (ActivationMode::AlwaysOnPushToTranscribe) but the CLI and startup logic
still use CliActivationMode and the controls hint text which lack this variant,
so add a corresponding variant to CliActivationMode and update the CLI
parsing/initialization code to map that variant to
ActivationMode::AlwaysOnPushToTranscribe (wherever the current match/convert
between CliActivationMode and ActivationMode occurs), and update the controls
hint string shown on startup to list all three modes (VAD / PTT /
AlwaysOnPushToTranscribe) so CLI startup and UI controls are consistent and
discoverable.
In `@crates/app/src/runtime.rs`:
- Around line 578-583: The runtime currently only respawns the trigger task in
set_activation_mode(), leaving processor_settings and the activation_mode used
in SessionEvent translation stale; update set_activation_mode() to update the
shared processor_settings.activation_mode and the activation_mode used by
SessionEvent translation (the values originally copied from opts.activation_mode
at startup), then ensure the processor/fanout is either restarted or signaled to
pick up the new mode (not just the trigger task). Locate and modify the code
paths that construct Settings::default() / processor_settings and the
SessionEvent translation logic to read from the updated activation_mode (or a
shared atomic/Mutex value) so mode switches like AlwaysOnPushToTranscribe take
effect runtime-wide.
- Around line 42-49: Replace the manual Into implementation by implementing From
for ActivationMode: remove the impl Into<crate::stt::session::ActivationMode>
for ActivationMode block and add impl From<ActivationMode> for
crate::stt::session::ActivationMode with fn from(self) ->
crate::stt::session::ActivationMode that matches ActivationMode::Vad,
ActivationMode::Hotkey, and ActivationMode::AlwaysOnPushToTranscribe to the
corresponding crate::stt::session::ActivationMode variants so existing .into()
call sites continue to work.
In `@crates/app/src/stt/tests/mod.rs`:
- Around line 20-24: The test mutates the global env var STT_MODEL_PATH via
std::env::remove_var which can leak state; before removing it, capture the
original value with std::env::var("STT_MODEL_PATH") and then restore it at the
end of the test (using std::env::set_var if present or std::env::remove_var if
it was absent), or wrap that logic in a small RAII guard; update the test around
TranscriptionConfig::default() to save the original, perform remove_var, run the
assertions, and restore the original value so other tests are not affected.
In `@crates/app/tests/integration/capture_integration_test.rs`:
- Line 25: The test sets a global env var with
std::env::set_var("COLDVOX_STT_PREFERRED", "moonshine") but never restores it;
modify the test to capture the previous value using
std::env::var_os("COLDVOX_STT_PREFERRED") before calling set_var, and ensure you
restore it at the end (if Some(prev) call std::env::set_var with prev, otherwise
call std::env::remove_var). Do this inside the test scope (or via a small RAII
guard struct) so the override is always reverted even on test failure.
In `@crates/app/tests/integration/text_injection_integration_test.rs`:
- Line 21: The test sets the process-global env var COLDVOX_STT_PREFERRED but
never restores it; capture the prior value with
std::env::var_os("COLDVOX_STT_PREFERRED") before calling std::env::set_var, and
ensure you restore it at the end of the test (if prior is Some(v) call
std::env::set_var with that v, otherwise call std::env::remove_var) — do this
either with an explicit restore at the test end or by creating a small RAII
guard that restores in Drop so the original value is always reinstated even on
panic.
In `@crates/coldvox-stt/src/plugin_types.rs`:
- Line 12: Update the doc comment that currently lists "Moonshine, Parakeet" as
examples of high-accuracy engines to distinguish Parakeet as not
production-ready; locate the comment in plugin_types.rs (the doc string near the
STT accuracy description) and replace or edit the phrase to something like
"Moonshine (current), Parakeet (planned)" so Moonshine is shown as the current
working backend and Parakeet is presented as a planned successor.
In `@crates/coldvox-stt/src/plugin.rs`:
- Line 4: Update the crate-level documentation comment that lists example STT
backends by clarifying Parakeet's status: change the phrase "Moonshine,
Parakeet, etc." in the module/doc comment (the top-line comment in plugin.rs) to
explicitly mark Moonshine as the current supported backend and Parakeet as
planned (for example: "Moonshine (current), Parakeet (planned), etc."); keep the
rest of the comment intact and ensure the wording communicates that Moonshine
should be used for current implementations while Parakeet is a
future/experimental target.
In `@crates/coldvox-stt/src/types.rs`:
- Line 64: Document the breaking env var rename by updating the user-facing
release notes and migration guide to state that WHISPER_MODEL_PATH has been
renamed to STT_MODEL_PATH, show the exact new env var name (STT_MODEL_PATH) and
the fallback default ("base.en") used in types.rs (the
std::env::var(...).unwrap_or_else(|_| "base.en".to_string()) call), and include
a short migration snippet instructing users how to rename their
environment/configuration and how to set the new variable to preserve previous
behavior.
In `@deny.toml`:
- Around line 38-58: The deny.toml suppressions for multiple RUSTSEC IDs (e.g.,
RUSTSEC-2024-0370, RUSTSEC-2024-0413, RUSTSEC-2025-0057, RUSTSEC-2025-0081,
etc.) lack a maintenance guardrail; add a short policy entry in deny.toml (or a
linked docs/OWNERS) that records an owner, review cadence (e.g., review date or
next-review field), and an associated tracking issue/PR URL for each grouped
suppression, and update CI to fail if the referenced crate (Tauri) is removed or
Cargo.lock no longer contains the suppressed crate (e.g., add a script step that
verifies each listed RUSTSEC id maps to a crate in Cargo.lock and that the
tracking issue is open), so stale ignores are detected and removed.
In `@docs/domains/foundation/fdn-voice-pipeline-core-requirements.md`:
- Around line 86-87: Remove the duplicated requirement sentence that reads "The
system shall manage STT memory usage dynamically, unloading large models during
extended idle periods while keeping lightweight engines ready for fast
activation" so only one instance remains; search for that exact sentence in
fdn-voice-pipeline-core-requirements and delete the redundant line, then run a
quick scan for any other accidental duplicate requirements to ensure no repeats
remain.
In `@docs/index.md`:
- Around line 18-20: The docs index shows a big rise in "**Invalid/needs
attention**" (9 → 20) likely caused by missing frontmatter or expired
gracePeriod fields on recently archived pages; either update the affected
markdown files to include the required frontmatter fields (title, status,
archivedDate/gracePeriod or equivalent used by the doc pipeline) or, if the
cleanup intentionally archived them, regenerate the site metrics and update the
three summary bullets ("**Active docs**", "**Archived docs**", "**Invalid/needs
attention**") so they reflect the true state; identify affected files by running
the doc validation script (or linter) that reports missing frontmatter and fix
those files' frontmatter entries, then re-run the index generation to produce
corrected counts.
In `@docs/plans/agentic-evidence-preview.md`:
- Around line 70-78: The doc incorrectly implies actions/checkout always
provides the PR base ref locally; update the workflow instructions to require an
explicit fetch of the base branch before running the diff: add a `git fetch
origin "${{ github.event.pull_request.base.ref }}"` (or equivalent) step prior
to running `git diff "origin/${{ github.event.pull_request.base.ref }}...HEAD"`,
and mention that this fetch is required even when using `actions/checkout` to
ensure `origin/${{ github.event.pull_request.base.ref }}` exists for the diff
command.
In `@docs/plans/cleanup-plan.md`:
- Around line 19-21: Update the "Guiding Principle" text so Parakeet is not the
sole criterion for retention: replace the pipeline string "mic → VAD → Parakeet
STT → text injection → Tauri GUI on Windows 11" with guidance that prioritizes
the current working STT path (Moonshine/PyO3) for retention and labels Parakeet
as a planned successor; specifically mention keeping Moonshine and PyO3
artifacts as active for current implementations and treat Parakeet as future
work (pure‑Rust/Windows‑native) rather than using it to decide deletions.
In `@docs/plans/current-status.md`:
- Around line 11-15: Change the document text that instructs agents to target
"tauri-base" back to a neutral or conditional statement (e.g., continue
targeting "main" until "tauri-base" is officially promoted) and add a short note
that contributors should follow the repo's current PR target; also update the CI
reviewer dashboard reference to reflect the same canonical branch policy (ensure
"main" is still described as the target until promotion of "tauri-base") so
automated contributors and docs remain consistent.
In `@docs/reviews/reviewer_driven_evidence.md`:
- Around line 74-82: Update the "How Reviewers Use the Report" wording in the
Step Summary section: remove the directive "Skip reading lines of code that
implement well-evidenced claims — the agent verified them" and replace it with a
conditional guidance that the report highlights claims with supporting evidence
but does not guarantee correctness, e.g., state that reviewers can prioritize
attention away from claims marked well-evidenced while still performing normal
code correctness and safety checks; ensure the text explicitly clarifies the
agent only checks for existence of evidence (not correctness) to align with the
non-goals.
In `@docs/standards.md`:
- Line 135: The docs list `.github/copilot-instructions.md` as an allowed
exception but the pre-commit check in scripts/check_markdown_placement.py does
not include it, causing CI/hooks to block; update the APPROVED_EXCEPTIONS
set/tuple in scripts/check_markdown_placement.py to add the string
".github/copilot-instructions.md" (or the existing constant representing
exceptions) so the linter matches the docs, and run the pre-commit check to
verify the new exception is accepted by the check_markdown_placement logic.
In `@docs/visuals/agentic-workflow-dashboard.html`:
- Around line 389-393: The tab control buttons (the <button> elements with
data-target="timeline", "wiring", "prompt", "risk" and the one with class
"active") are currently missing an explicit type and default to type="submit";
update each of those button elements used as tabs to include type="button" so
they do not submit forms when clicked and to satisfy linters and reuse safety.
In `@docs/visuals/ci-reviewer-dashboard.html`:
- Around line 376-380: The nav tab buttons are missing explicit types and will
default to type="submit"; update each <button> in the tablist (the ones with
data-target="cadence", "prompt", "flow", and "risk" inside the element with
class "nav" role="tablist") to include type="button" so they don't trigger form
submission when placed inside a form context; ensure every tab button element
gets the attribute type="button".
In `@scripts/ensure_agent_hardlinks.sh`:
- Around line 23-26: In scripts/ensure_agent_hardlinks.sh the source/destination
variables are reversed: change the mapping so AGENTS.md is the canonical source
and the other files are targets — set src to "$repo_root/AGENTS.md" and set dst1
and dst2 to the two target paths (e.g.
"$repo_root/.github/copilot-instructions.md" and
"$repo_root/.kilocode/rules/agents.md"), then ensure the hardlink creation logic
uses src -> dst1 and src -> dst2.
---
Outside diff comments:
In `@crates/app/src/stt/processor.rs`:
- Around line 148-176: Change the start path so live frames cannot reach plugins
before the plugin is initialized: in the SessionEvent::Start handler set
state.state to a new UtteranceState::Starting (instead of
UtteranceState::SpeechActive) and keep incoming frames buffered (state.buffer /
any new frames in handle_audio_frame should check for Starting and append to
state.buffer rather than dispatching to plugin). Spawn the task that calls
plugin_manager.write().await.begin_utterance() and, if successful, process
pre_roll with plugin_manager.write().await.process_audio(&pre_roll); only after
both succeed transition state.state to UtteranceState::SpeechActive and then
flush any buffered frames to the plugin; on begin_utterance/process_audio error
revert state (e.g., back to Idle or error state) and ensure buffered frames are
handled/cleared to avoid inconsistent session state.
- Around line 263-313: In handle_audio_frame, the boolean should_process is
referenced but not defined; fix by declaring it before use by inspecting the
session state (e.g., let should_process = { let state = self.state.lock();
state.state == UtteranceState::SpeechActive }); this uses the existing
self.state.lock() and UtteranceState enum to determine whether to call
plugin_manager.process_audio; place this declaration before the if behavior !=
HotkeyBehavior::Incremental block (or at the top of that branch) so subsequent
logic (including the plugin_manager.write().await call and rolling_buffer
handling) can use the computed value.
In `@deny.toml`:
- Line 11: Remove the stale license allowance entry for "Unicode-DFS-2016" from
deny.toml: locate the allowance list containing the string "Unicode-DFS-2016"
and delete that entry so cargo-deny no longer reports an unmatched license
allowance. Ensure the resulting deny.toml remains valid TOML (remove any
trailing comma if it was the last item).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: cc6aebe3-3f68-42bc-a51c-ae4f6fd611f1
⛔ Files ignored due to path filters (2)
docs/revision_log.csvis excluded by!**/*.csvvendor/vosk/lib/libvosk.sois excluded by!**/*.so
📒 Files selected for processing (90)
.github/agents/implementer.agent.md.github/agents/project-driver.agent.md.github/agents/researcher.agent.md.github/agents/tester.agent.md.github/copilot-instructions.md.github/prompts/drive-project.prompt.md.github/prompts/evidence-assessor.md.github/prompts/northstar-alignment-reviewer.md.github/workflows/agentic-evidence-preview.yml.gitignore.kilocode/rules/agents.mdAGENTS.mdCHANGELOG.mdCLAUDE.mdGEMINI.mdREADME.mdcrates/app/plugins.jsoncrates/app/src/bin/tui_dashboard.rscrates/app/src/lib.rscrates/app/src/runtime.rscrates/app/src/stt/processor.rscrates/app/src/stt/session.rscrates/app/src/stt/tests/mod.rscrates/app/tests/integration/capture_integration_test.rscrates/app/tests/integration/text_injection_integration_test.rscrates/coldvox-stt/Cargo.tomlcrates/coldvox-stt/src/plugin.rscrates/coldvox-stt/src/plugin_types.rscrates/coldvox-stt/src/types.rsdeny.tomldocs/architecture.mddocs/architecture/adr/index.mddocs/archive/domains/aud-pipewire-design.mddocs/archive/domains/tele-observability-playbook.mddocs/archive/plans/gui/aspirational-gui-plan.mddocs/archive/plans/gui/comprehensive-gui-plan.mddocs/archive/plans/gui/raw-gui-plan.mddocs/archive/plans/phase1-audio-quality-monitoring.mddocs/archive/plans/roadmap.mddocs/archive/reference/crates/coldvox-stt.mddocs/archive/research/audio-quality-monitoring.mddocs/archive/research/dependency-audit-report-2025-02-09.mddocs/archive/research/pr-reports/PR-temp-clipboard-test-timeout-fixes.mddocs/archive/research/pr-reports/PR-temp-comprehensive-testing-report.mddocs/archive/research/pr-reports/PR-temp-injection-path-alignment.mddocs/archive/root/DEPENDENCY_AUDIT_ISSUE.mddocs/archive/root/FINAL_REPORT.mddocs/archive/root/PYO3_DEPENDENCY_AUDIT_PLAN.mddocs/archive/root/PYO3_DLL_TROUBLESHOOTING.mddocs/archive/root/VERIFICATION_REPORT.mddocs/archive/root/WINDOWS_IMPLEMENTATION_SUMMARY.mddocs/archive/root/logging.mddocs/archive/root/observability-playbook.mddocs/domains/audio/aud-user-config-design.mddocs/domains/foundation/fdn-testing-guide.mddocs/domains/foundation/fdn-voice-pipeline-core-requirements.mddocs/domains/gui/gui-design-overview.mddocs/domains/stt/stt-overview.mddocs/history/2025-11-06_04-16Z-branch-status-and-work-in-progress-overview.mddocs/history/2025-11-06_04-33Z-reviewing-implementation-of-golden-test-branch.mddocs/history/2025-11-06_05-33Z-git-history-inquiry-for-compat-rs-file.mddocs/index.mddocs/plans/agentic-evidence-preview.mddocs/plans/cleanup-plan.mddocs/plans/current-status.mddocs/plans/parakeet-http-remote-integration-spec.mddocs/plans/windows-multi-agent-recovery.mddocs/playbooks/organizational/github_governance.mddocs/playbooks/organizational/pr_playbook.mddocs/prompts/review-and-implement-evidence-assessor.mddocs/reference/crates/coldvox-gui.mddocs/research/exhaustive-dependency-audit-2026-03-24.mddocs/reviews/portable_standard_critique.mddocs/reviews/reviewer_driven_evidence.mddocs/standards.mddocs/standards/agent-rules.mddocs/tasks/ci-runner-readiness-proposal.mddocs/todo.mddocs/visuals/agentic-workflow-dashboard.htmldocs/visuals/ci-reviewer-dashboard.htmlplugins.jsonpr_365_details.jsonscripts/check_markdown_placement.pyscripts/ci/setup-vosk-cache.shscripts/ensure_agent_hardlinks.shscripts/ensure_venv.shscripts/start-headless.shscripts/verify_vosk_model.shtest_enigo_live.rsvendor/vosk/model/vosk-model-en-us-0.22
💤 Files with no reviewable changes (33)
- .github/agents/researcher.agent.md
- docs/domains/stt/stt-overview.md
- GEMINI.md
- .github/agents/implementer.agent.md
- crates/coldvox-stt/Cargo.toml
- docs/architecture/adr/index.md
- vendor/vosk/model/vosk-model-en-us-0.22
- CLAUDE.md
- scripts/verify_vosk_model.sh
- docs/archive/research/pr-reports/PR-temp-clipboard-test-timeout-fixes.md
- docs/playbooks/organizational/github_governance.md
- scripts/start-headless.sh
- scripts/ensure_venv.sh
- docs/playbooks/organizational/pr_playbook.md
- crates/app/plugins.json
- docs/archive/plans/gui/comprehensive-gui-plan.md
- docs/archive/research/pr-reports/PR-temp-injection-path-alignment.md
- docs/history/2025-11-06_04-33Z-reviewing-implementation-of-golden-test-branch.md
- .github/agents/tester.agent.md
- test_enigo_live.rs
- docs/archive/research/pr-reports/PR-temp-comprehensive-testing-report.md
- docs/plans/windows-multi-agent-recovery.md
- pr_365_details.json
- docs/archive/plans/gui/raw-gui-plan.md
- docs/history/2025-11-06_05-33Z-git-history-inquiry-for-compat-rs-file.md
- .github/prompts/drive-project.prompt.md
- docs/tasks/ci-runner-readiness-proposal.md
- scripts/ci/setup-vosk-cache.sh
- docs/archive/plans/gui/aspirational-gui-plan.md
- plugins.json
- docs/history/2025-11-06_04-16Z-branch-status-and-work-in-progress-overview.md
- docs/archive/reference/crates/coldvox-stt.md
- .github/agents/project-driver.agent.md
Comment on lines
+18
to
+23
| Known ground truths (treat as facts unless the PR explicitly updates them): | ||
| - **Moonshine** is the *only* working STT backend. It is fragile (PyO3). | ||
| - **Parakeet** is planned but NOT production-ready. | ||
| - Feature flags `whisper`, `coqui`, `leopard`, `silero-stt` are dead stubs — any PR claiming these work is making an unverified claim. | ||
| - STT streaming partial transcription is **not yet implemented**. | ||
|
|
There was a problem hiding this comment.
Keep the hard-coded “ground truths” aligned with current-status.md.
This prompt tells the agent that docs/plans/current-status.md is a ground-truth anchor, but Lines 19-22 then freeze older repo facts inline anyway. In this PR, docs/plans/current-status.md now describes HTTP-remote Parakeet as the forward path on tauri-base; leaving this prompt stale guarantees false semantic-drift findings on the very execution state the repo now documents. Based on learnings: Check docs/plans/current-status.md for what currently works and what's broken before starting work.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/prompts/evidence-assessor.md around lines 18 - 23, Update the
hard-coded ground-truth bullets (the assertions about Moonshine, Parakeet, the
feature flags "whisper, coqui, leopard, silero-stt", and "STT streaming partial
transcription") so they are synchronized with the repository's authoritative
status document (current-status.md): remove or revise the stale fixed claims
(lines that freeze older facts about Parakeet and the feature-flag stubs) and
instead reflect the current-state wording from current-status.md (or reference
it dynamically), ensuring the bullets for Moonshine, Parakeet, the listed
feature flags, and STT streaming match that file’s descriptions.
Comment on lines
+97
to
+112
| ### Missed Opportunity | ||
| [What is the single most valuable thing this PR could have done toward a northstar goal but didn't? Or "None — this PR is well-targeted."] | ||
|
|
||
| ### Alignment Notes | ||
| [3-5 sentences of reasoning. Cite specific files and goals. Do not speculate.] | ||
| ``` | ||
|
|
||
| --- | ||
|
|
||
| ## Critical Constraints | ||
|
|
||
| - **Use your tools.** Run `git diff`, read files, search the workspace. Do not guess. | ||
| - **DO NOT hallucinate alignment.** If a change does not relate to a goal, say so. | ||
| - **DO NOT comment on code quality, style, or testing.** That is not your role. | ||
| - **DO NOT recommend what the PR should have done differently.** Only assess what it did. | ||
| - **Treat the alignment verdict as a classification, not a judgment.** Humans decide priority. |
There was a problem hiding this comment.
The report template and the constraints disagree about counterfactuals.
Lines 97-99 ask the agent to describe a “Missed Opportunity,” but Line 111 says “DO NOT recommend what the PR should have done differently.” Those instructions are mutually exclusive, so the reviewer has to violate one of them on every run. Rename that section to a neutral gap summary, or drop the prohibition.
🧰 Tools
🪛 LanguageTool
[style] ~111-~111: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...r testing.** That is not your role. - **DO NOT recommend what the PR should have d...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/prompts/northstar-alignment-reviewer.md around lines 97 - 112, The
prompt contains a contradictory instruction set: the "Missed Opportunity"
section header asks for recommendations (lines with "Missed Opportunity") while
the "Critical Constraints" block (the DO NOT recommend line) forbids
recommending changes; update the prompt to remove the contradiction by either
renaming the "Missed Opportunity" header to a neutral term like "Gap Summary" or
changing the prohibition in the Critical Constraints to allow a brief factual
gap description; edit the .github/prompts/northstar-alignment-reviewer.md file
to choose one approach and make the two pieces consistent so the reviewer is not
forced to violate constraints (refer to the "Missed Opportunity" header and the
"DO NOT recommend what the PR should have done differently" constraint when
making the change).
Comment on lines
+68
to
+70
| RUST_FILES=$(echo "$CHANGED" | grep "^crates/" | wc -l | tr -d '[:space:]') | ||
| CI_WORKFLOW_FILES=$(echo "$CHANGED" | grep "^\.github/workflows/" | wc -l | tr -d '[:space:]') | ||
| TOTAL_FILES=$(echo "$CHANGED" | wc -l | tr -d '[:space:]') |
There was a problem hiding this comment.
This scorer can fail on simple PRs and it isn’t actually counting Rust files.
Because the step runs with set -euo pipefail, either grep exits 1 when there are no matches, which aborts docs-only/simple PRs before a tier is produced. Also, ^crates/ counts every file under crates/, not just changed .rs files, so the Northstar reviewer can be triggered or skipped on the wrong threshold.
🛠️ Suggested fix
- RUST_FILES=$(echo "$CHANGED" | grep "^crates/" | wc -l | tr -d '[:space:]')
- CI_WORKFLOW_FILES=$(echo "$CHANGED" | grep "^\.github/workflows/" | wc -l | tr -d '[:space:]')
+ RUST_FILES=$(printf '%s\n' "$CHANGED" | awk '/^crates\/.*\.rs$/ { count++ } END { print count + 0 }')
+ CI_WORKFLOW_FILES=$(printf '%s\n' "$CHANGED" | awk '/^\.github\/workflows\/.*\.ya?ml$/ { count++ } END { print count + 0 }')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| RUST_FILES=$(echo "$CHANGED" | grep "^crates/" | wc -l | tr -d '[:space:]') | |
| CI_WORKFLOW_FILES=$(echo "$CHANGED" | grep "^\.github/workflows/" | wc -l | tr -d '[:space:]') | |
| TOTAL_FILES=$(echo "$CHANGED" | wc -l | tr -d '[:space:]') | |
| RUST_FILES=$(printf '%s\n' "$CHANGED" | awk '/^crates\/.*\.rs$/ { count++ } END { print count + 0 }') | |
| CI_WORKFLOW_FILES=$(printf '%s\n' "$CHANGED" | awk '/^\.github\/workflows\/.*\.ya?ml$/ { count++ } END { print count + 0 }') | |
| TOTAL_FILES=$(echo "$CHANGED" | wc -l | tr -d '[:space:]') |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/agentic-evidence-preview.yml around lines 68 - 70, The
scorer fails on no-match cases and miscounts Rust files; change the RUST_FILES
and CI_WORKFLOW_FILES computations to match file extensions and avoid grep
causing non-zero exits: use anchored regexes that require .rs for crates (e.g.,
grep -E '^crates/.*\.rs$' || true) and require .yml/.yaml for workflows (e.g.,
grep -E '^\.github/workflows/.*\.(ya?ml)$' || true) before piping to wc -l, keep
TOTAL_FILES as-is, and assign to the same variables RUST_FILES,
CI_WORKFLOW_FILES, TOTAL_FILES so the downstream tier logic gets correct counts
without aborting on empty results.
Comment on lines
+42
to
+46
| # NOTE: NOT gitignored — Gemini CLI respects .gitignore as a security boundary | ||
| # and will refuse to read/write files that match gitignore patterns. | ||
| # These files are auto-generated at runtime and never committed (CI doesn't commit). | ||
| # They are excluded via .git/info/exclude or manual cleanup instead. | ||
| _tmp_*.md |
There was a problem hiding this comment.
Comment-to-rule contradiction needs immediate correction.
Line 42-Line 45 state _tmp_*.md is not gitignored, but Line 46 adds _tmp_*.md to .gitignore, which does the opposite. This can cause CI-agent file access failures and maintenance confusion.
Suggested fix (pick one intent and make code/comments consistent)
# CI agent working files (written by Gemini CLI during review jobs)
-# NOTE: NOT gitignored — Gemini CLI respects .gitignore as a security boundary
-# and will refuse to read/write files that match gitignore patterns.
-# These files are auto-generated at runtime and never committed (CI doesn't commit).
-# They are excluded via .git/info/exclude or manual cleanup instead.
-_tmp_*.md
+# NOTE: NOT gitignored — Gemini CLI respects .gitignore as a security boundary
+# and will refuse to read/write files that match gitignore patterns.
+# These files are auto-generated at runtime and never committed (CI doesn't commit).
+# They are excluded via .git/info/exclude or manual cleanup instead.or, if you do want them ignored, update the comments instead:
# CI agent working files (written by Gemini CLI during review jobs)
-# NOTE: NOT gitignored — Gemini CLI respects .gitignore as a security boundary
-# and will refuse to read/write files that match gitignore patterns.
-# These files are auto-generated at runtime and never committed (CI doesn't commit).
-# They are excluded via .git/info/exclude or manual cleanup instead.
+# NOTE: gitignored intentionally.
+# Any tooling that must read/write these files should use a different filename pattern.
_tmp_*.md📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| # NOTE: NOT gitignored — Gemini CLI respects .gitignore as a security boundary | |
| # and will refuse to read/write files that match gitignore patterns. | |
| # These files are auto-generated at runtime and never committed (CI doesn't commit). | |
| # They are excluded via .git/info/exclude or manual cleanup instead. | |
| _tmp_*.md | |
| # CI agent working files (written by Gemini CLI during review jobs) | |
| # NOTE: NOT gitignored — Gemini CLI respects .gitignore as a security boundary | |
| # and will refuse to read/write files that match gitignore patterns. | |
| # These files are auto-generated at runtime and never committed (CI doesn't commit). | |
| # They are excluded via .git/info/exclude or manual cleanup instead. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.gitignore around lines 42 - 46, The comment "NOTE: NOT gitignored — Gemini
CLI respects .gitignore..." conflicts with the actual ignore pattern `_tmp_*.md`
present in the file; pick one intent and make them consistent by either removing
the `_tmp_*.md` entry if these files must not be ignored, or updating the
comment to state that `_tmp_*.md` are intentionally ignored; modify the comment
text referencing the Gemini CLI behavior and the `_tmp_*.md` pattern accordingly
and ensure the final comment and the `_tmp_*.md` entry both reflect the chosen
intent.
Comment on lines
+3
to
+110
| Canonical AI agent instructions for ColdVox. This file is the source of truth for all agent tools. | ||
|
|
||
| ## Anchor Precedence | ||
| ## Anchor | ||
|
|
||
| If guidance conflicts, use this order: | ||
| 1. `docs/northstar.md` - Product vision | ||
| 2. `docs/plans/current-status.md` - Current execution state | ||
| 3. `docs/architecture.md` - Technical architecture | ||
| 4. `docs/dev/CI/policy.md` - CI standards | ||
| - Product and technical anchor: `docs/northstar.md` | ||
| - Current execution state: `docs/plans/current-status.md` | ||
| - Architecture direction: `docs/architecture.md` | ||
| - CI source of truth: `docs/dev/CI/architecture.md` | ||
|
|
||
| ## Quick Reference | ||
| If guidance conflicts, use this precedence: | ||
| 1. `docs/northstar.md` | ||
| 2. `docs/plans/current-status.md` | ||
| 3. `docs/dev/CI/architecture.md` | ||
|
|
||
| - Build commands: See `docs/dev/commands.md` | ||
| - Working standards: See `docs/standards/agent-rules.md` | ||
| ## Current Product Direction & Reality | ||
|
|
||
| - **Target OS:** Windows 11 priority. | ||
| - **Python Environment:** Exclusively managed by `uv`. Do NOT use `mise` or raw `pip` for Python packages. Ensure `.python-version` is respected. | ||
| - **STT Backend:** | ||
| - **Moonshine:** The current working backend, but considered a fragile dependency due to PyO3. | ||
| - **Parakeet:** The designated successor for a pure-Rust/Windows-native STT pipeline (CUDA/DirectML). It *does* compile; focus on runtime validation. | ||
| - **Vaporware:** The `whisper`, `coqui`, `leopard`, and `silero-stt` feature flags are dead stubs. Do not attempt to use them. | ||
|
|
||
| ## Project Overview | ||
|
|
||
| ColdVox is a Rust voice pipeline: audio capture -> VAD -> STT -> text injection. | ||
| Multi-crate Cargo workspace under `crates/`. | ||
|
|
||
| Key crates to know: | ||
| - `coldvox-app` (Main execution and binaries) | ||
| - `coldvox-audio` (Capture and resampling via rubato) | ||
| - `coldvox-stt` (STT Plugin logic) | ||
| - `coldvox-text-injection` (Output injection logic) | ||
|
|
||
| ## Working Rules | ||
|
|
||
| **DO:** | ||
| - Use `cargo {cmd} -p {crate}` for iteration speed, but finish with `cargo check --workspace --all-targets`. | ||
| - Only use live testing (real microphone/`.wav` files) to test VAD and STT. Do not mock audio buffers. | ||
| - Check `docs/plans/current-status.md` for what currently works and what's broken. | ||
|
|
||
| **DO NOT:** | ||
| - Claim Whisper or Parakeet are currently production-ready. | ||
| - Modify Python dependencies without using `uv`. | ||
| - Auto-run commands that destroy data or commit unverified changes. | ||
|
|
||
| ## Commands | ||
|
|
||
| File-scoped (preferred): | ||
| ```bash | ||
| cargo check -p coldvox-stt | ||
| cargo clippy -p coldvox-audio | ||
| cargo test -p coldvox-text-injection | ||
| cargo fmt --all -- --check | ||
| ``` | ||
|
|
||
| Workspace (when needed): | ||
| ```bash | ||
| ./scripts/local_ci.sh | ||
| cargo clippy --workspace --all-targets --locked | ||
| cargo test --workspace --locked | ||
| cargo build --workspace --locked | ||
| ``` | ||
|
|
||
| Run: | ||
| ```bash | ||
| cargo run -p coldvox-app --bin coldvox | ||
| cargo run -p coldvox-app --bin tui_dashboard | ||
| cargo run --features text-injection,moonshine | ||
| ``` | ||
|
|
||
| ## Feature Flags | ||
|
|
||
| - `silero`: Silero VAD | ||
| - `text-injection`: text injection backends | ||
| - `moonshine`: Current working STT backend (Python-based, CPU/GPU) | ||
| - `parakeet`: planned backend work; not current reliable path | ||
| - `examples`: example binaries | ||
| - `live-hardware-tests`: hardware test suites | ||
|
|
||
| ## CI Environment | ||
|
|
||
| Canonical CI policy is `docs/dev/CI/architecture.md`. | ||
|
|
||
| Principle: | ||
| - GitHub-hosted runners handle fast general CI work. | ||
| - Self-hosted Fedora/Nobara runner handles hardware-dependent tests. | ||
|
|
||
| Do not use: | ||
| - Xvfb on self-hosted runner | ||
| - `apt-get` on Fedora runner | ||
| - `DISPLAY=:99` in self-hosted jobs | ||
|
|
||
| ## Key Files | ||
|
|
||
| - Main entry: `crates/app/src/main.rs` | ||
| - Audio capture: `crates/coldvox-audio/src/capture.rs` | ||
| - VAD engine: `crates/coldvox-vad-silero/src/silero_wrapper.rs` | ||
| - STT plugins: `crates/coldvox-stt/src/plugins/` | ||
| - Text injection manager: `crates/coldvox-text-injection/src/manager.rs` | ||
| - Build detection: `crates/app/build.rs` | ||
|
|
||
| ## PR Checklist | ||
|
|
||
| - `./scripts/local_ci.sh` passes (or equivalent crate-scoped checks) | ||
| - Docs updated for behavior/direction changes | ||
| - `CHANGELOG.md` updated for user-visible changes | ||
| - No secrets committed |
There was a problem hiding this comment.
Sync .github/copilot-instructions.md in the same PR.
Docs Validation is already failing because this file now diverges from .github/copilot-instructions.md. Leaving them out of sync creates conflicting agent instructions and blocks the PR.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AGENTS.md` around lines 3 - 110, AGENTS.md and
.github/copilot-instructions.md are out of sync; update the copilot instructions
to match the canonical content in AGENTS.md (or vice versa) so PR validation
passes: ensure headings and key sections (Anchor, Current Product Direction &
Reality including STT Backend and Python Environment, Working Rules, Commands,
Feature Flags, CI Environment, Key Files, PR Checklist) are identical, reconcile
differences under symbols like "STT Backend", "Working Rules", "Feature Flags"
and verify command examples (cargo/cargo run scripts) match; commit the
synchronized .github/copilot-instructions.md in the same PR.
Comment on lines
+74
to
+82
| ## How Reviewers Use the Report | ||
|
|
||
| The Step Summary report is intended to reduce cognitive load for human reviewers: | ||
|
|
||
| 1. **Skip** reading lines of code that implement well-evidenced claims — the agent verified them. | ||
| 2. **Focus** on claims marked `EVIDENCE_WEAK` or `EVIDENCE_MISSING` — these are the review risk areas. | ||
| 3. **Investigate** any semantic drift the agent flags — these are documentation debt that should be resolved. | ||
|
|
||
| The agent does NOT tell reviewers what to do. It gives them a structured starting point. |
There was a problem hiding this comment.
Don’t tell reviewers to skip code review here.
“The agent verified them” is too strong for this workflow: the assessor only checks whether evidence exists for claims, not whether the implementation is correct or safe. That instruction conflicts with the non-goals below and will push reviewers to miss bugs in “well-evidenced” changes.
✏️ Suggested wording
-1. **Skip** reading lines of code that implement well-evidenced claims — the agent verified them.
-2. **Focus** on claims marked `EVIDENCE_WEAK` or `EVIDENCE_MISSING` — these are the review risk areas.
-3. **Investigate** any semantic drift the agent flags — these are documentation debt that should be resolved.
+1. **Prioritize** claims marked `EVIDENCE_WEAK` or `EVIDENCE_MISSING` — these are the highest review-risk areas.
+2. Use well-evidenced claims to focus your review, not to skip implementation review entirely.
+3. **Investigate** any semantic drift the agent flags — these are documentation debt that should be resolved.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ## How Reviewers Use the Report | |
| The Step Summary report is intended to reduce cognitive load for human reviewers: | |
| 1. **Skip** reading lines of code that implement well-evidenced claims — the agent verified them. | |
| 2. **Focus** on claims marked `EVIDENCE_WEAK` or `EVIDENCE_MISSING` — these are the review risk areas. | |
| 3. **Investigate** any semantic drift the agent flags — these are documentation debt that should be resolved. | |
| The agent does NOT tell reviewers what to do. It gives them a structured starting point. | |
| ## How Reviewers Use the Report | |
| The Step Summary report is intended to reduce cognitive load for human reviewers: | |
| 1. **Prioritize** claims marked `EVIDENCE_WEAK` or `EVIDENCE_MISSING` — these are the highest review-risk areas. | |
| 2. Use well-evidenced claims to focus your review, not to skip implementation review entirely. | |
| 3. **Investigate** any semantic drift the agent flags — these are documentation debt that should be resolved. | |
| The agent does NOT tell reviewers what to do. It gives them a structured starting point. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/reviews/reviewer_driven_evidence.md` around lines 74 - 82, Update the
"How Reviewers Use the Report" wording in the Step Summary section: remove the
directive "Skip reading lines of code that implement well-evidenced claims — the
agent verified them" and replace it with a conditional guidance that the report
highlights claims with supporting evidence but does not guarantee correctness,
e.g., state that reviewers can prioritize attention away from claims marked
well-evidenced while still performing normal code correctness and safety checks;
ensure the text explicitly clarifies the agent only checks for existence of
evidence (not correctness) to align with the non-goals.
| - `AGENTS.md` - Canonical AI agent instructions and doc-routing entry point | ||
| - `CLAUDE.md` - AI assistant context and guidelines | ||
| - `GEMINI.md` - AI assistant context and guidelines | ||
| - `.github/copilot-instructions.md` - Full agent onboarding (source of truth, synced to AGENTS.md) |
There was a problem hiding this comment.
Policy/enforcement drift: pre-commit script still blocks this new exception.
Line 135 documents .github/copilot-instructions.md as allowed, but scripts/check_markdown_placement.py (Lines 8-13 in the provided snippet) doesn’t include it in APPROVED_EXCEPTIONS. This will cause false blocking in hooks/CI.
Suggested fix (outside this file)
diff --git a/scripts/check_markdown_placement.py b/scripts/check_markdown_placement.py
@@
APPROVED_EXCEPTIONS = {
Path("README.md"),
Path("CHANGELOG.md"),
Path("AGENTS.md"),
+ Path(".github/copilot-instructions.md"),
Path(".github/pull_request_template.md"),
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/standards.md` at line 135, The docs list
`.github/copilot-instructions.md` as an allowed exception but the pre-commit
check in scripts/check_markdown_placement.py does not include it, causing
CI/hooks to block; update the APPROVED_EXCEPTIONS set/tuple in
scripts/check_markdown_placement.py to add the string
".github/copilot-instructions.md" (or the existing constant representing
exceptions) so the linter matches the docs, and run the pre-commit check to
verify the new exception is accepted by the check_markdown_placement logic.
Comment on lines
+389
to
+393
| <div class="nav" role="tablist" aria-label="Visualization sections"> | ||
| <button class="active" data-target="timeline">1. Provenance timeline</button> | ||
| <button data-target="wiring">2. Runtime wiring map</button> | ||
| <button data-target="prompt">3. Prompt anatomy</button> | ||
| <button data-target="risk">4. Applicability & CI risk</button> |
There was a problem hiding this comment.
Declare the tab controls as non-submit buttons.
These controls default to type="submit", so the current markup is relying on page context not to misbehave. Adding type="button" resolves the lint warnings and makes the component safe to reuse.
🧰 Tools
🪛 HTMLHint (1.9.2)
[warning] 390-390: The type attribute must be present on elements.
(button-type-require)
[warning] 391-391: The type attribute must be present on
elements.(button-type-require)
[warning] 392-392: The type attribute must be present on
elements.(button-type-require)
[warning] 393-393: The type attribute must be present on
elements.(button-type-require)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/visuals/agentic-workflow-dashboard.html` around lines 389 - 393, The tab
control buttons (the <button> elements with data-target="timeline", "wiring",
"prompt", "risk" and the one with class "active") are currently missing an
explicit type and default to type="submit"; update each of those button elements
used as tabs to include type="button" so they do not submit forms when clicked
and to satisfy linters and reuse safety.
Comment on lines
+376
to
+380
| <div class="nav" role="tablist" aria-label="CI reviewer sections"> | ||
| <button class="active" data-target="cadence">1. Activation cadence</button> | ||
| <button data-target="prompt">2. Prompt anatomy</button> | ||
| <button data-target="flow">3. What it does</button> | ||
| <button data-target="risk">4. Implications</button> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and read the specified lines
head -380 docs/visuals/ci-reviewer-dashboard.html | tail -10Repository: Coldaine/ColdVox
Length of output: 451
Explicitly declare tab control buttons with type="button".
These buttons currently default to type="submit" per HTML specification, which can cause unintended form submission if the markup is embedded in a form context. Adding type="button" makes the intent explicit and avoids fragility.
Suggested fix
- <button class="active" data-target="cadence">1. Activation cadence</button>
- <button data-target="prompt">2. Prompt anatomy</button>
- <button data-target="flow">3. What it does</button>
- <button data-target="risk">4. Implications</button>
+ <button type="button" class="active" data-target="cadence">1. Activation cadence</button>
+ <button type="button" data-target="prompt">2. Prompt anatomy</button>
+ <button type="button" data-target="flow">3. What it does</button>
+ <button type="button" data-target="risk">4. Implications</button>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <div class="nav" role="tablist" aria-label="CI reviewer sections"> | |
| <button class="active" data-target="cadence">1. Activation cadence</button> | |
| <button data-target="prompt">2. Prompt anatomy</button> | |
| <button data-target="flow">3. What it does</button> | |
| <button data-target="risk">4. Implications</button> | |
| <div class="nav" role="tablist" aria-label="CI reviewer sections"> | |
| <button type="button" class="active" data-target="cadence">1. Activation cadence</button> | |
| <button type="button" data-target="prompt">2. Prompt anatomy</button> | |
| <button type="button" data-target="flow">3. What it does</button> | |
| <button type="button" data-target="risk">4. Implications</button> |
🧰 Tools
🪛 HTMLHint (1.9.2)
[warning] 377-377: The type attribute must be present on elements.
(button-type-require)
[warning] 378-378: The type attribute must be present on
elements.(button-type-require)
[warning] 379-379: The type attribute must be present on
elements.(button-type-require)
[warning] 380-380: The type attribute must be present on
elements.(button-type-require)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/visuals/ci-reviewer-dashboard.html` around lines 376 - 380, The nav tab
buttons are missing explicit types and will default to type="submit"; update
each <button> in the tablist (the ones with data-target="cadence", "prompt",
"flow", and "risk" inside the element with class "nav" role="tablist") to
include type="button" so they don't trigger form submission when placed inside a
form context; ensure every tab button element gets the attribute type="button".
Comment on lines
+23
to
26
| src="$repo_root/.github/copilot-instructions.md" | ||
|
|
||
| dst1="$repo_root/.github/copilot-instructions.md" | ||
| dst1="$repo_root/AGENTS.md" | ||
| dst2="$repo_root/.kilocode/rules/agents.md" |
There was a problem hiding this comment.
Source/destination mapping is reversed for canonical agent rules.
Line 23-26 currently treats .github/copilot-instructions.md as source and AGENTS.md as destination. That makes the canonical file drift-prone and can overwrite intended source-of-truth content.
Proposed fix
-src="$repo_root/.github/copilot-instructions.md"
-
-dst1="$repo_root/AGENTS.md"
+src="$repo_root/AGENTS.md"
+
+dst1="$repo_root/.github/copilot-instructions.md"
dst2="$repo_root/.kilocode/rules/agents.md"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| src="$repo_root/.github/copilot-instructions.md" | |
| dst1="$repo_root/.github/copilot-instructions.md" | |
| dst1="$repo_root/AGENTS.md" | |
| dst2="$repo_root/.kilocode/rules/agents.md" | |
| src="$repo_root/AGENTS.md" | |
| dst1="$repo_root/.github/copilot-instructions.md" | |
| dst2="$repo_root/.kilocode/rules/agents.md" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/ensure_agent_hardlinks.sh` around lines 23 - 26, In
scripts/ensure_agent_hardlinks.sh the source/destination variables are reversed:
change the mapping so AGENTS.md is the canonical source and the other files are
targets — set src to "$repo_root/AGENTS.md" and set dst1 and dst2 to the two
target paths (e.g. "$repo_root/.github/copilot-instructions.md" and
"$repo_root/.kilocode/rules/agents.md"), then ensure the hardlink creation logic
uses src -> dst1 and src -> dst2.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| // Use i16 samples directly from SharedAudioFrame | ||
| let samples_slice: &[i16] = &frame.samples; | ||
|
|
||
| let should_process = { |
There was a problem hiding this comment.
WARNING: Potential race condition in should_process logic
The should_process variable is computed by locking the state, then released. The decision to send audio to the STT plugin is made based on this potentially stale value. If the state changes between the lock release and the plugin call, audio could be sent when the state is not SpeechActive, leading to errors. Consider re-checking the state inside the if should_process block or restructuring to hold the lock for the decision.
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/app/src/stt/processor.rs (1)
268-318:⚠️ Potential issue | 🔴 CriticalInvert the
HotkeyBehaviorsplit; this block is currently dead.With
HotkeyBehaviorcurrently only definingIncrementalincrates/app/src/stt/session.rs:34-44, Line 273 is always false.handle_audio_frame()therefore never dispatches live audio and never fillsrolling_buffer, so both normal incremental dictation and the new Always-On pre-roll path stop working.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@crates/app/src/stt/processor.rs` around lines 268 - 318, The hotkey behavior check is inverted so the live/incremental path in handle_audio_frame never runs; change the conditional that currently reads "if behavior != HotkeyBehavior::Incremental" to check for equality (i.e., run the block when behavior == HotkeyBehavior::Incremental) so that should_process, buffering, rolling_buffer maintenance and the plugin_manager.process_audio(...) dispatch occur for Incremental mode; update any related comments and verify that rolling_buffer logic remains in the appropriate branch so AlwaysOnPushToTranscribe still populates pre-roll when state.state == UtteranceState::Idle.
♻️ Duplicate comments (1)
deny.toml (1)
37-57:⚠️ Potential issue | 🟠 MajorAdd enforceable revalidation for advisory ignores.
These suppressions still lack an automated stale-ignore guardrail (owner/cadence/tracking + lockfile presence validation), so they can silently outlive the dependency paths they were added for.
#!/bin/bash set -euo pipefail # Verify ignored advisories still correspond to crates present in Cargo.lock. python - <<'PY' import pathlib, re, tomllib, sys deny = tomllib.loads(pathlib.Path("deny.toml").read_text()) ignored = { item["id"]: item.get("reason", "") for item in deny.get("advisories", {}).get("ignore", []) if isinstance(item, dict) and "id" in item } # Current mapping used by this PR rationale. id_to_crate = { "RUSTSEC-2024-0370": "proc-macro-error", "RUSTSEC-2024-0413": "atk", "RUSTSEC-2024-0416": "atk-sys", "RUSTSEC-2024-0412": "gdk", "RUSTSEC-2024-0418": "gdk-sys", "RUSTSEC-2024-0411": "gdkwayland-sys", "RUSTSEC-2024-0417": "gdkx11", "RUSTSEC-2024-0414": "gdkx11-sys", "RUSTSEC-2024-0415": "gtk", "RUSTSEC-2024-0420": "gtk-sys", "RUSTSEC-2024-0419": "gtk3-macros", "RUSTSEC-2025-0057": "fxhash", "RUSTSEC-2025-0081": "unic-char-property", "RUSTSEC-2025-0075": "unic-char-range", "RUSTSEC-2025-0080": "unic-common", "RUSTSEC-2025-0100": "unic-ucd-ident", "RUSTSEC-2025-0098": "unic-ucd-version", } lock_path = pathlib.Path("Cargo.lock") if not lock_path.exists(): print("Cargo.lock missing; cannot validate suppressions.") sys.exit(1) lock = lock_path.read_text() stale = [] for adv, crate in id_to_crate.items(): if adv in ignored: present = re.search(rf'(?m)^name = "{re.escape(crate)}"$', lock) is not None if not present: stale.append((adv, crate)) if stale: print("Stale advisory ignores detected:") for adv, crate in stale: print(f"- {adv} -> {crate} not found in Cargo.lock") sys.exit(2) print("All mapped ignored advisories still match crates present in Cargo.lock.") PYAs per coding guidelines: "Ensure
./scripts/local_ci.shpasses (or equivalent crate-scoped checks) before PR submission".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@deny.toml` around lines 37 - 57, Add an automated revalidation step that fails CI if advisory ignores in deny.toml no longer map to crates in Cargo.lock: implement a check (e.g., add to ./scripts/local_ci.sh or a new script invoked by CI) that loads deny.toml, extracts the advisories under advisories.ignore (matching the structure in deny.toml), uses a mapping like id_to_crate (same IDs as in the diff: RUSTSEC-2024-0370, RUSTSEC-2024-0413, etc.) to resolve expected crate names, then verifies each mapped crate exists in Cargo.lock and exits nonzero with a clear message if any are missing (stale ignores); ensure the check references the deny.toml advisory keys and the Cargo.lock search logic so it will catch stale suppressions and run in CI/local_ci.sh.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/app/src/stt/processor.rs`:
- Around line 155-169: The current code drains state.rolling_buffer into
pre_roll before calling begin_utterance, which loses the only copy if
begin_utterance fails; modify the flow in processor.rs so you either (A) move
the drain/extend_into state.buffer to after
pm.write().await.begin_utterance().await returns Ok (i.e., acquire pre_roll only
once startup succeeds), or (B) if you must drain before calling
begin_utterance(), preserve and restore the samples on error: push pre_roll back
into state.rolling_buffer (or extend state.buffer as appropriate) and reset the
processor state (e.g., clear SpeechActive or set to the prior state) when
pm.write().await.begin_utterance() returns Err; reference symbols:
state.rolling_buffer, pre_roll, state.buffer, self.plugin_manager /
pm.write().await.begin_utterance(), and the SpeechActive state to locate where
to change the logic.
- Around line 164-175: The pre-roll process_audio(&pre_roll) call currently
ignores Ok(Some(event)) so any event emitted for the prepended audio is dropped;
change the tokio::spawn block (using self.plugin_manager / pm) to capture the
result of pm.write().await.process_audio(&pre_roll).await, and if it returns
Ok(Some(event)) forward that event into the same handling path used for live
frames (so metrics like SttMetrics see it) — i.e., call the same
event-processing/dispatch logic (or the function that handles live-frame events)
for the returned event and handle Err/None as before; ensure this uses the
existing begin_utterance and incremental flow around process_audio and preserves
error logging.
---
Outside diff comments:
In `@crates/app/src/stt/processor.rs`:
- Around line 268-318: The hotkey behavior check is inverted so the
live/incremental path in handle_audio_frame never runs; change the conditional
that currently reads "if behavior != HotkeyBehavior::Incremental" to check for
equality (i.e., run the block when behavior == HotkeyBehavior::Incremental) so
that should_process, buffering, rolling_buffer maintenance and the
plugin_manager.process_audio(...) dispatch occur for Incremental mode; update
any related comments and verify that rolling_buffer logic remains in the
appropriate branch so AlwaysOnPushToTranscribe still populates pre-roll when
state.state == UtteranceState::Idle.
---
Duplicate comments:
In `@deny.toml`:
- Around line 37-57: Add an automated revalidation step that fails CI if
advisory ignores in deny.toml no longer map to crates in Cargo.lock: implement a
check (e.g., add to ./scripts/local_ci.sh or a new script invoked by CI) that
loads deny.toml, extracts the advisories under advisories.ignore (matching the
structure in deny.toml), uses a mapping like id_to_crate (same IDs as in the
diff: RUSTSEC-2024-0370, RUSTSEC-2024-0413, etc.) to resolve expected crate
names, then verifies each mapped crate exists in Cargo.lock and exits nonzero
with a clear message if any are missing (stale ignores); ensure the check
references the deny.toml advisory keys and the Cargo.lock search logic so it
will catch stale suppressions and run in CI/local_ci.sh.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a924830e-0a2a-465d-8c76-66a2309bf36d
📒 Files selected for processing (2)
crates/app/src/stt/processor.rsdeny.toml
Comment on lines
+155
to
+169
| // Flush the rolling buffer into the main pipeline if we have pre-roll data | ||
| let pre_roll: Vec<i16> = state.rolling_buffer.drain(..).collect(); | ||
| if !pre_roll.is_empty() { | ||
| tracing::debug!(target: "stt_debug", "Flushing {} samples of pre-roll audio", pre_roll.len()); | ||
| if self.settings.hotkey_behavior != crate::stt::session::HotkeyBehavior::Incremental { | ||
| state.buffer.extend_from_slice(&pre_roll); | ||
| } | ||
| } | ||
|
|
||
| let pm = self.plugin_manager.clone(); | ||
| let incremental = self.settings.hotkey_behavior == crate::stt::session::HotkeyBehavior::Incremental; | ||
| tokio::spawn(async move { | ||
| if let Err(e) = pm.write().await.begin_utterance().await { | ||
| tracing::error!(target: "stt", "Plugin begin_utterance failed: {}", e); | ||
| return; |
There was a problem hiding this comment.
Don't discard the only copy of pre-roll before startup succeeds.
Lines 156-169 drain rolling_buffer before begin_utterance() has actually succeeded, but the failure path only logs and returns. If startup fails, the rescued audio is gone and the processor remains in SpeechActive until some later end/abort, so the next hotkey press has no pre-roll left to recover. Drain after a successful start, or restore rolling_buffer and reset state on the error path.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/app/src/stt/processor.rs` around lines 155 - 169, The current code
drains state.rolling_buffer into pre_roll before calling begin_utterance, which
loses the only copy if begin_utterance fails; modify the flow in processor.rs so
you either (A) move the drain/extend_into state.buffer to after
pm.write().await.begin_utterance().await returns Ok (i.e., acquire pre_roll only
once startup succeeds), or (B) if you must drain before calling
begin_utterance(), preserve and restore the samples on error: push pre_roll back
into state.rolling_buffer (or extend state.buffer as appropriate) and reset the
processor state (e.g., clear SpeechActive or set to the prior state) when
pm.write().await.begin_utterance() returns Err; reference symbols:
state.rolling_buffer, pre_roll, state.buffer, self.plugin_manager /
pm.write().await.begin_utterance(), and the SpeechActive state to locate where
to change the logic.
Comment on lines
164
to
175
| let pm = self.plugin_manager.clone(); | ||
| let incremental = self.settings.hotkey_behavior == crate::stt::session::HotkeyBehavior::Incremental; | ||
| tokio::spawn(async move { | ||
| if let Err(e) = pm.write().await.begin_utterance().await { | ||
| tracing::error!(target: "stt", "Plugin begin_utterance failed: {}", e); | ||
| return; | ||
| } | ||
| if incremental && !pre_roll.is_empty() { | ||
| if let Err(e) = pm.write().await.process_audio(&pre_roll).await { | ||
| tracing::error!(target: "stt", "Plugin process_audio failed on pre-roll: {}", e); | ||
| } | ||
| } |
There was a problem hiding this comment.
Forward the event returned from the pre-roll chunk.
Unlike the live-frame path below, this new process_audio(&pre_roll) call drops Ok(Some(event)). If the backend emits a partial/final for the prepended audio, the first event of the utterance disappears and SttMetrics never sees it.
Possible fix
let pm = self.plugin_manager.clone();
+ let event_tx = self.event_tx.clone();
+ let metrics = self.metrics.clone();
let incremental = self.settings.hotkey_behavior == crate::stt::session::HotkeyBehavior::Incremental;
tokio::spawn(async move {
if let Err(e) = pm.write().await.begin_utterance().await {
tracing::error!(target: "stt", "Plugin begin_utterance failed: {}", e);
return;
}
if incremental && !pre_roll.is_empty() {
- if let Err(e) = pm.write().await.process_audio(&pre_roll).await {
- tracing::error!(target: "stt", "Plugin process_audio failed on pre-roll: {}", e);
- }
+ match pm.write().await.process_audio(&pre_roll).await {
+ Ok(Some(event)) => {
+ Self::send_event_static(&event_tx, &metrics, event).await;
+ }
+ Ok(None) => {}
+ Err(e) => {
+ tracing::error!(target: "stt", "Plugin process_audio failed on pre-roll: {}", e);
+ }
+ }
}
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let pm = self.plugin_manager.clone(); | |
| let incremental = self.settings.hotkey_behavior == crate::stt::session::HotkeyBehavior::Incremental; | |
| tokio::spawn(async move { | |
| if let Err(e) = pm.write().await.begin_utterance().await { | |
| tracing::error!(target: "stt", "Plugin begin_utterance failed: {}", e); | |
| return; | |
| } | |
| if incremental && !pre_roll.is_empty() { | |
| if let Err(e) = pm.write().await.process_audio(&pre_roll).await { | |
| tracing::error!(target: "stt", "Plugin process_audio failed on pre-roll: {}", e); | |
| } | |
| } | |
| let pm = self.plugin_manager.clone(); | |
| let event_tx = self.event_tx.clone(); | |
| let metrics = self.metrics.clone(); | |
| let incremental = self.settings.hotkey_behavior == crate::stt::session::HotkeyBehavior::Incremental; | |
| tokio::spawn(async move { | |
| if let Err(e) = pm.write().await.begin_utterance().await { | |
| tracing::error!(target: "stt", "Plugin begin_utterance failed: {}", e); | |
| return; | |
| } | |
| if incremental && !pre_roll.is_empty() { | |
| match pm.write().await.process_audio(&pre_roll).await { | |
| Ok(Some(event)) => { | |
| Self::send_event_static(&event_tx, &metrics, event).await; | |
| } | |
| Ok(None) => {} | |
| Err(e) => { | |
| tracing::error!(target: "stt", "Plugin process_audio failed on pre-roll: {}", e); | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/app/src/stt/processor.rs` around lines 164 - 175, The pre-roll
process_audio(&pre_roll) call currently ignores Ok(Some(event)) so any event
emitted for the prepended audio is dropped; change the tokio::spawn block (using
self.plugin_manager / pm) to capture the result of
pm.write().await.process_audio(&pre_roll).await, and if it returns
Ok(Some(event)) forward that event into the same handling path used for live
frames (so metrics like SttMetrics see it) — i.e., call the same
event-processing/dispatch logic (or the function that handles live-frame events)
for the returned event and handle Err/None as before; ensure this uses the
existing begin_utterance and incremental flow around process_audio and preserves
error logging.
Coldaine
dismissed stale reviews from coderabbitai[bot] and coderabbitai[bot]
Addressed: clippy errors fixed and code verified.
- Adds ActivationMode::AlwaysOnPushToTranscribe with ~2s rolling audio buffer - Prevents hotkey start mechanical clipping of transcription - Includes agentic evidence assessor CI infrastructure - Comprehensive repo cleanup: dead docs, dead code, dead references Closes #384 (contained)
Coldaine
force-pushed
the
feat/always-on-push-to-transcribe
branch
from
254707d to
6a77c38
Compare
Owner
Author
|
Changes landed via tauri-base in #400. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements an Always-On dictation mode. Maintains a 2-second pre-roll rolling buffer of audio locally. When the push-to-talk hotkey is engaged, the buffer prepends transparently to eliminate the mechanical delay dropoff clipping early words.
Always-On Push-To-Transcribe Mode Implementation
Core feature
Implements an Always-On Push-To-Transcribe activation mode that keeps a ~2s rolling pre-roll buffer of recent audio and prepends that audio when the push-to-talk hotkey is pressed to avoid clipping early words.
Key implementation
Processing flow & runtime wiring
UI
Tests & backend/config touches
Repository-wide ancillary changes (high level)
Files of note
Code review effort, risks & focus areas
Summary verdict
Adds a focused Always-On Push-To-Transcribe mode with a local ~2s pre-roll buffer and full integration into UI and runtime. The change set also includes substantial doc/CI cleanup and new shadow-mode agentic review infra; reviewers should prioritize correctness of buffering/pre-roll delivery and ensure repository cleanup did not remove critical operational knowledge or testing workflows.