refactor: prune dead code and dedup (multi-agent review cleanup) - #6
Merged
Merged
Conversation
Findings from an adversarial multi-agent review, verified by grep before
removal. No behavior change.
Dead code (Tier 1):
- Delete verified-unused CSS: settings.css .hist-*/.activity/.sec-note/
.row-static; components.css .stat*/.chip/.field*/.hint.ok|error/
.small-btn.danger; onboarding.css .perm-badge.warn + a dead `.ob kbd`
selector. All had zero references in HTML/JS.
- Drop unused tokens: --state-recording/-transcribing/-refining and
--shadow-card-hover (kept the numbered --fs-*/--space-* scale intact as a
deliberate design system).
- onboarding.js: collapse the write-only `whisper` progress object to a
boolean; settings.html: drop never-read data-default attrs; recorder.js:
drop the unused returned `cancel`; remove a redundant renderInsights().
- Remove the unused jsdom devDep (suite is node-only) and its transitive tree.
Dedup (Tier 2):
- Hoist the duplicated trigger→glyph map into shortcuts.js (TRIGGER_LABEL),
imported by settings.js + onboarding.js; drop MOD_LABEL (use prettyShortcut).
- stt.rs: shared set_dictation_params() for warm_infer + transcribe.
- tts.rs: KokoroSpeaker::speak reuses load_kokoro_tts.
- lib.rs: merge emit_download_progress/error into emit_download; extract
stage_downloaded_update() for the two update-staging sites.
- commands.rs: delete download_neural_voice (identical to
retry_download("kokoro")); onboarding calls retry_download instead.
- fn_key.rs: reuse permissions::accessibility_granted() over a duplicate extern.
Plus stale-comment fixes (lib.rs, tts.rs "no espeak", constants.js).
Verified: cargo fmt/clippy/build/test (46 pass incl. IPC contract), npm test
(16 pass), npm run build, and npm run ui-diff (all sections within tolerance).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EhiLhbjf14q4Qc6Cq2Kzu3
There was a problem hiding this comment.
🟡 Changes recommended
A refactor in stt.rs orphaned the warm_infer doc comment onto set_dictation_params, so it now documents the wrong function and should be relocated.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR is a no-behavior-change cleanup that prunes dead code and deduplicates logic across the Rust backend and the webview frontend, following an adversarial multi-agent review. The bulk of the change is dead CSS/tokens and removing the unused jsdom devDependency (which accounts for most of the package-lock.json delta); the remainder consolidates duplicated Rust helpers (whisper decode params, Kokoro model load, download emit/update-staging, Accessibility check) and shared frontend maps (TRIGGER_LABEL). I verified each removal has zero remaining references and that each dedup preserves the original behavior.
Changes:
- Remove dead CSS rules, design tokens, JS state, a redundant
renderInsights()call,data-defaultattrs, the returnedcancel, and thejsdomdevDep (+ transitive tree). - Deduplicate Rust helpers:
set_dictation_params(stt),load_kokoro_ttsreuse (tts),emit_download+stage_downloaded_update(lib),permissions::accessibility_granted()reuse (fn_key), and dropdownload_neural_voicein favor ofretry_download. - Hoist the trigger→glyph map into
shortcuts.js(TRIGGER_LABEL) shared by settings + onboarding, and fix several stale comments.
File summaries
| File | Description |
|---|---|
vite.config.js |
Switch test env from jsdom to node; update comment. |
src-tauri/src/tts.rs |
Reuse load_kokoro_tts in KokoroSpeaker::speak; fix "no espeak" comment. |
src-tauri/src/stt.rs |
Extract shared set_dictation_params; doc comment for warm_infer left misattached (flagged). |
src-tauri/src/lib.rs |
Merge emit_download_* and extract stage_downloaded_update; drop removed command; update comments. |
src-tauri/src/fn_key.rs |
Reuse permissions::accessibility_granted() instead of a duplicate extern. |
src-tauri/src/commands.rs |
Delete download_neural_voice (identical to retry_download("kokoro")). |
package.json / package-lock.json |
Remove jsdom devDep and its transitive tree. |
frontend/tokens.css |
Drop dead overlay-state and shadow tokens. |
frontend/shortcuts.js |
Add shared TRIGGER_LABEL map. |
frontend/settings.js / settings.html / settings.css |
Use TRIGGER_LABEL/prettyShortcut, drop redundant renderInsights(), data-default, and dead CSS. |
frontend/recorder.js |
Drop the never-used returned cancel. |
frontend/onboarding.js / onboarding.css |
Collapse whisper progress object to whisperReady; reuse TRIGGER_LABEL; call retry_download; drop dead CSS. |
frontend/components.css |
Remove unreferenced component rules. |
frontend/constants.js |
Drop removed command; fix data-p / emit_download comments. |
Review details
- Files reviewed: 17/18 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+119
to
+129
| @@ -120,11 +120,13 @@ fn open_context(model_name: &str) -> Result<Arc<WhisperContext>> { | |||
| /// graph compile + state allocation up front, so the first real dictation hits a | |||
| /// fully-warm engine rather than paying that one-time cost. Uses the same | |||
| /// no-fallback params as `transcribe` so it stays fast and deterministic. | |||
| fn warm_infer(ctx: &WhisperContext) -> Result<()> { | |||
| let mut state = ctx | |||
| .create_state() | |||
| .map_err(|e| anyhow!("whisper warm state: {e}"))?; | |||
| let mut params = FullParams::new(SamplingStrategy::Greedy { best_of: 1 }); | |||
| // The whisper decode params shared by warm-up and real transcription, kept in | |||
| // one place so the two can't drift. Pin to English (matches the cloud path); a | |||
| // dictation clip is one self-contained utterance, so don't seed the decoder with | |||
| // prior-window text, and pin a single temperature so a hard clip can't trip | |||
| // whisper's temperature-fallback retries (which re-decode and spike latency). | |||
| // Give it the machine's cores, and silence whisper's stdout chatter. | |||
| fn set_dictation_params(params: &mut FullParams) { | |||
…action Copilot review: the warm_infer `///` doc ended up above the newly-extracted set_dictation_params, so Rust attached it to the wrong function. Give set_dictation_params its own `///` doc and move the warm-up doc back onto warm_infer. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EhiLhbjf14q4Qc6Cq2Kzu3
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.
Cleanup from an adversarial multi-agent code review (6 agents across the whole codebase), aimed at reducing/simplifying code. Every removal was grep-verified for zero references before deletion. No behavior change.
Dead code (Tier 1)
settings.css.hist-*/.activity/.act-*/.sec-note/.row-static;components.css.stat*/.chip/.field*/.hint.ok|error/.small-btn.danger;onboarding.css.perm-badge.warn+ a dead.ob kbdselector.--state-recording/-transcribing/-refining,--shadow-card-hover. (Kept the numbered--fs-*/--space-*scale intact as a deliberate design system.)whisperprogress object to a boolean; drop never-readdata-defaultattrs; drop the unused returnedcancel; remove a redundantrenderInsights().jsdomdevDep (test suite is node-only) and its transitive tree (the bulk of the lockfile delta).Dedup (Tier 2)
shortcuts.js(TRIGGER_LABEL); dropMOD_LABEL(reuseprettyShortcut).stt.rs: sharedset_dictation_params()forwarm_infer+transcribe(removes a documented drift risk).tts.rs:KokoroSpeaker::speakreusesload_kokoro_tts.lib.rs: mergeemit_download_progress/emit_download_error→emit_download; extractstage_downloaded_update()for the two update-staging sites.commands.rs: deletedownload_neural_voice(identical toretry_download("kokoro")); onboarding callsretry_downloadinstead.fn_key.rs: reusepermissions::accessibility_granted()over a duplicateAXIsProcessTrustedextern.lib.rs,tts.rs"no espeak",constants.js).Net
~230 source LOC removed (plus jsdom's dependency tree). The Rust backend was already clean (0 warnings, no dead code), so the bulk was dead CSS.
Deferred (separate PR)
The larger dead block in
components.css(~124 LOC used only by onboarding, which references none of it) is held back so its onboarding visual re-test is isolated.Verified
cargo fmt/clippy/build/test(46 pass incl. the IPC contract test),npm test(16 pass),npm run build, andnpm run ui-diff(all settings + onboarding sections within tolerance).🤖 Generated with Claude Code