Skip to content

Commit e9b6f16

Browse files
committed
docs(tui): add inline comments throughout TUI codebase
Add concise, high-signal inline comments to function bodies across the TUI implementation. Comments explain intent, edge cases, boundary math, ownership/cloning decisions, and performance trade-offs rather than restating code behavior. Key areas covered: - Config screen rendering (panels, overlays, layouts) - Dictating screen rendering (visualizer, status, controls) - Runtime event loop (channel draining, input routing, periodic tasks) - State management (initialization, selection logic, feedback) - Widget rendering (logs, hotkeys, brand header) - Override parsing and application
1 parent cb199b9 commit e9b6f16

30 files changed

Lines changed: 677 additions & 133 deletions

keyless/src/main.rs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,22 +38,27 @@ use keyless::Cli;
3838
fn handle_top_level_flags() -> Result<Option<Cli>, String> {
3939
let cli = Cli::parse();
4040
if cli.list_devices {
41-
// Enable CLI logging formatting for non-TUI output
41+
// Enable CLI logging formatting for non-TUI output (stdout is safe here).
42+
// TUI path avoids stdout logging to prevent tearing alternate screen buffer.
4243
keyless_logging::init_stdout();
4344
match keyless_audio::input::list_input_device_names() {
4445
Ok(list) => {
46+
// Empty list: show helpful message instead of blank output.
4547
if list.is_empty() {
4648
println!("(no input devices)");
4749
} else {
50+
// Print index:name pairs; enumerate provides 0-based indices for user reference.
4851
for (i, n) in list.iter().enumerate() {
4952
println!("{}: {}", i, n);
5053
}
5154
}
5255
}
5356
Err(e) => {
57+
// Print error to stdout (non-TUI path); user expects to see it.
5458
println!("failed to list devices: {}", e);
5559
}
5660
}
61+
// Early return: list-devices handled, don't start TUI.
5762
return Ok(None);
5863
}
5964
Ok(Some(cli))
@@ -65,17 +70,20 @@ fn main() -> ExitCode {
6570
match handle_top_level_flags() {
6671
Ok(None) => ExitCode::SUCCESS, // handled and printed, exit early
6772
Ok(Some(cli)) => {
68-
// Do NOT init stdout logging for the TUI path
73+
// Do NOT init stdout logging for the TUI path (prevents alternate screen corruption).
74+
// Build overrides from CLI flags and ENV vars (CLI takes precedence).
6975
let ov = keyless::overrides::overrides_from_env_and_cli(&cli);
7076
if let Err(e) = keyless::tui::run_with_overrides(Some(ov)) {
71-
// Best-effort error reporting to stderr after TUI exits
77+
// Best-effort error reporting to stderr after TUI exits (terminal restored).
78+
// Use stderr to avoid mixing with TUI output if terminal wasn't fully restored.
7279
eprintln!("tui error: {}", e);
7380
ExitCode::FAILURE
7481
} else {
7582
ExitCode::SUCCESS
7683
}
7784
}
7885
Err(e) => {
86+
// Flag parsing error: report to stderr before any TUI initialization.
7987
eprintln!("flag handling error: {}", e);
8088
ExitCode::FAILURE
8189
}

keyless/src/overrides.rs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,18 +59,22 @@ pub struct Overrides {
5959
/// set in `ov` will override the corresponding field in `config`.
6060
pub fn apply_overrides(config: &mut Config, ov: &Overrides) {
6161
if let Some(v) = ov.model.as_ref() {
62+
// Trim whitespace from model ID/path; reject empty strings (defensive).
6263
let t = v.trim();
6364
if !t.is_empty() {
6465
config.model_path = PathBuf::from(t);
6566
}
6667
}
6768
if let Some(v) = ov.language.as_ref() {
69+
// Trim whitespace from language code; reject empty strings (None = auto-detect).
6870
let t = v.trim();
6971
if !t.is_empty() {
7072
config.language = Some(t.to_string());
7173
}
7274
}
7375
if let Some(v) = ov.sink.as_ref() {
76+
// Parse sink string (paste/clipboard/file); ignore parse errors (invalid strings ignored).
77+
// File mode not set here (handled by file override below); only Paste/Clipboard applied.
7478
match OutputMode::from_str(v.trim()) {
7579
Ok(OutputMode::Paste) => config.output_mode = OutputMode::Paste,
7680
Ok(OutputMode::Clipboard) => config.output_mode = OutputMode::Clipboard,
@@ -79,23 +83,28 @@ pub fn apply_overrides(config: &mut Config, ov: &Overrides) {
7983
}
8084
}
8185
if let Some(p) = ov.file.as_ref() {
86+
// File path override: trim whitespace, reject empty strings.
87+
// Takes precedence over sink=file parsing above (explicit path provided).
8288
let t = p.trim();
8389
if !t.is_empty() {
8490
config.output_mode = OutputMode::File(PathBuf::from(t));
8591
}
8692
}
8793
if let Some(v) = ov.device.as_ref() {
94+
// Trim whitespace from device name; reject empty strings (None = use default).
8895
let t = v.trim();
8996
if !t.is_empty() {
9097
config.device_name = Some(t.to_string());
9198
}
9299
}
93100
if let Some(v) = ov.hotkey.as_ref() {
101+
// Trim whitespace from hotkey string; reject empty strings (prevent invalid config).
94102
let t = v.trim();
95103
if !t.is_empty() {
96104
config.hotkey = t.to_string();
97105
}
98106
}
107+
// VAD overrides: direct assignment (no clamping; user may want extreme values).
99108
if let Some(f) = ov.vad_start {
100109
config.vad.start_db = f;
101110
}
@@ -108,6 +117,7 @@ pub fn apply_overrides(config: &mut Config, ov: &Overrides) {
108117
if let Some(s) = ov.vad_max_silence {
109118
config.vad.max_silence_ms = s;
110119
}
120+
// EQ overrides: clamp to valid ranges to prevent invalid audio processing parameters.
111121
if let Some(b) = ov.eq_bands {
112122
config.eq.bands = b.clamp(EQ_BANDS_MIN, EQ_BANDS_MAX);
113123
}
@@ -126,21 +136,25 @@ pub fn apply_overrides(config: &mut Config, ov: &Overrides) {
126136
if let Some(d) = ov.eq_decay {
127137
config.eq.decay = d.clamp(EQ_DECAY_MIN, EQ_DECAY_MAX);
128138
}
139+
// Validate config after applying all overrides; ignore result (errors logged internally).
129140
let _ = config.validate();
130141
}
131142

132143
/// Overrides source used by the TUI; constructed in main from CLI/ENV.
133144
#[allow(clippy::module_name_repetitions)]
134145
pub fn overrides_from_env_and_cli(cli: &Cli) -> Overrides {
135-
// precedence: CLI > ENV
146+
// Precedence: CLI > ENV (CLI flags override environment variables).
147+
// Clone CLI Option<String> to move value into closure (needs owned String).
136148
let pick_str = |cli_opt: &Option<String>, env_key: &str| -> Option<String> {
137149
if let Some(v) = cli_opt.clone() {
138150
Some(v)
139151
} else {
152+
// Fallback to ENV var if CLI flag not set; ok() converts Result to Option.
140153
std::env::var(env_key).ok()
141154
}
142155
};
143156

157+
// Parse f32 from ENV if CLI flag not set; ignore parse errors (invalid strings = None).
144158
let pick_f32 = |cli_opt: Option<f32>, env_key: &str| -> Option<f32> {
145159
cli_opt.or_else(|| {
146160
std::env::var(env_key)
@@ -149,6 +163,7 @@ pub fn overrides_from_env_and_cli(cli: &Cli) -> Overrides {
149163
})
150164
};
151165

166+
// Parse u16 from ENV if CLI flag not set; ignore parse errors (invalid strings = None).
152167
let pick_u16 = |cli_opt: Option<u16>, env_key: &str| -> Option<u16> {
153168
cli_opt.or_else(|| {
154169
std::env::var(env_key)

keyless/src/tui/config/device.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,16 @@ use crate::tui::theme::{Colors, Theme};
1010

1111
/// Render the input devices list.
1212
pub fn render_devices(f: &mut ratatui::Frame, area: Rect, app: &AppState) {
13+
// Recompute list items each frame; incurs small allocs for text styling/formatting.
14+
// Acceptable given the short list size and TUI frame budget.
1315
let device_items: Vec<ListItem> = app
1416
.selections
1517
.device_names
1618
.iter()
1719
.enumerate()
1820
.map(|(idx, d)| {
21+
// Selection is driven by `app.selections.device_idx` (external state).
22+
// Assumes the index is in-bounds of `device_names`; upstream logic must enforce it.
1923
let is_selected = idx == app.selections.device_idx;
2024
let style = if is_selected {
2125
Style::default()
@@ -25,22 +29,35 @@ pub fn render_devices(f: &mut ratatui::Frame, area: Rect, app: &AppState) {
2529
Theme::text_muted()
2630
};
2731
let text = if is_selected {
32+
// Selected row shows the raw device name; the active indicator will be provided by
33+
// the widget's `highlight_symbol` below to avoid duplicate indicators.
2834
d.clone()
2935
} else {
36+
// Non-selected rows are visually de-emphasized and receive an inactive indicator.
3037
format!("{} {}", Theme::inactive_indicator(), d)
3138
};
3239
ListItem::new(Span::styled(text, style))
3340
})
3441
.collect();
42+
43+
// Local list state is ephemeral; selection is persisted in `app` not this state object.
3544
let mut device_state = ratatui::widgets::ListState::default();
45+
// No bounds guard here; relies on upstream to keep `device_idx < device_names.len()`.
46+
// Ratatui tolerates out-of-range select without panicking but may render no highlight.
3647
device_state.select(Some(app.selections.device_idx));
48+
49+
// Trailing space ensures padding between the indicator glyph and the text.
3750
let indicator_symbol = format!("{} ", Theme::active_indicator());
51+
52+
// Visual container: rounded border and accent-colored title for discoverability.
3853
let block = Block::default()
3954
.borders(Borders::ALL)
4055
.border_type(BorderType::Rounded)
4156
.border_style(Style::default().fg(Colors::border()))
4257
.title_style(Style::default().fg(Colors::device_accent()))
4358
.title("🎤 input devices");
59+
60+
// Highlight uses accent fg + selected bg for strong contrast; bold improves readability.
4461
let list = List::new(device_items)
4562
.block(block)
4663
.highlight_style(
@@ -50,5 +67,7 @@ pub fn render_devices(f: &mut ratatui::Frame, area: Rect, app: &AppState) {
5067
.add_modifier(Modifier::BOLD),
5168
)
5269
.highlight_symbol(&indicator_symbol);
70+
71+
// Render with stateful API so the selected row gets the highlight/indicator this frame.
5372
f.render_stateful_widget(list, area, &mut device_state);
5473
}

keyless/src/tui/config/draw.rs

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ pub fn draw_config(
1919
) {
2020
let full = f.area();
2121

22-
// Dynamic footer height based on the footer content and available width (generic estimator)
22+
// Compute footer height from hotkey segments and terminal width (cells).
23+
// This prevents the footer from wrapping unexpectedly when the screen is narrow.
2324
let segs = if app
2425
.overlays
2526
.expert
@@ -37,6 +38,8 @@ pub fn draw_config(
3738

3839
let layout = Layout::default()
3940
.direction(Direction::Vertical)
41+
// Reserve at least 10 rows for content; footer is exact length computed above.
42+
// Using fixed footer length avoids layout jitter across frames.
4043
.constraints([Constraint::Min(10), Constraint::Length(lines_needed)])
4144
.split(full);
4245

@@ -51,27 +54,33 @@ pub fn draw_config(
5154
main_outer,
5255
);
5356
let mut content = main_outer;
57+
// Shrink to inner content area (1-cell border on all sides).
58+
// Use saturating ops to avoid underflow on very small terminals.
5459
content.x += 1;
5560
content.y += 1;
5661
content.width = content.width.saturating_sub(2);
5762
content.height = content.height.saturating_sub(2);
5863

64+
// Heuristic height for sinks panel: items + 2 (title/padding), capped at 6 rows.
65+
// Small cap keeps the top area from starving models/languages.
5966
let sink_height: u16 = ((sinks_list.len() as u16) + 2).min(6);
6067

6168
let chunks = Layout::default()
6269
.direction(Direction::Vertical)
6370
.constraints([
6471
Constraint::Length(3), // Top info (metrics + warnings)
65-
Constraint::Length(sink_height.max(5)), // Sinks
72+
Constraint::Length(sink_height.max(5)), // Sinks (min 5 rows for readability)
6673
Constraint::Min(10), // Models + (Languages/Devices)
67-
Constraint::Length(7), // VAD + Logs
74+
Constraint::Length(7), // VAD + Logs (fixed to avoid flicker)
6875
])
6976
.split(content);
7077
// Brand overlay: render title intersecting the top frame border
7178
{
7279
use crate::tui::widgets::brand::{BrandOptions, render_brand};
7380
let brand_area = ratatui::layout::Rect {
7481
x: main_outer.x,
82+
// Lift the brand one cell above to overlap the outer border.
83+
// `saturating_sub` prevents underflow when y == 0.
7584
y: main_outer.y.saturating_sub(1),
7685
width: main_outer.width,
7786
height: 1,
@@ -100,6 +109,7 @@ pub fn draw_config(
100109
Span::raw(" "),
101110
Span::styled("time: ", Theme::label()),
102111
Span::styled(
112+
// Convert milliseconds to HH:MM:SS string; assumes ms are monotonic counters.
103113
keyless_core::utils::fmt_hms(app.lifetime_talk_ms),
104114
Theme::text_primary(),
105115
),
@@ -110,6 +120,8 @@ pub fn draw_config(
110120
// Warnings / file path (rendered below metrics)
111121
let mut warn_lines: Vec<Line> = Vec::new();
112122
if matches!(app.selections.sink_choice, SinkChoice::File) {
123+
// File sink: show current path and edit state. `file_path` is cloned to avoid
124+
// borrowing across UI composition. Encoding assumed UTF-8 for display.
113125
let mut spans = vec![
114126
Span::styled("file: ", Theme::label()),
115127
Span::styled(app.selections.file_path.clone(), Colors::text_primary()),
@@ -130,6 +142,7 @@ pub fn draw_config(
130142
));
131143
warn_lines.push(Line::from(spans));
132144
} else {
145+
// Show the most recent error (if any) and permission warnings gathered at startup.
133146
if let Some(err) = &app.overlays.error_message {
134147
warn_lines.push(Line::from(vec![
135148
Span::styled("error: ", Theme::warn()),
@@ -146,6 +159,7 @@ pub fn draw_config(
146159
}
147160
}
148161
if !warn_lines.is_empty() {
162+
// Only render the warnings box when there is content to avoid consuming vertical space.
149163
let warning_p = Paragraph::new(warn_lines);
150164
f.render_widget(warning_p, extra_rows[1]);
151165
}
@@ -160,6 +174,7 @@ pub fn draw_config(
160174
.split(chunks[2]);
161175

162176
// Left: Models
177+
// Build an O(1) membership set for installed models; allocates proportional to count.
163178
let installed_set: std::collections::HashSet<String> =
164179
app.models.discovered.iter().cloned().collect();
165180
models::render_models(
@@ -213,11 +228,13 @@ pub fn draw_config(
213228

214229
// Expert overlay (centered)
215230
if app.expert_mode() {
231+
// Render after footer so overlay sits on top of lower widgets.
216232
expert::render_expert(f, full, app);
217233
}
218234

219235
// Download overlay (centered, modal)
220236
if app.is_downloading() {
237+
// Two states: transient "loading" (no progress) vs active download.
221238
let is_loading = app
222239
.overlays
223240
.download
@@ -257,7 +274,7 @@ pub fn draw_config(
257274
])
258275
.alignment(Alignment::Center)
259276
});
260-
// center a box for message + gauge
277+
// Center a box for message + gauge. The middle 4 rows are dedicated to content.
261278
let outer = Layout::default()
262279
.direction(Direction::Vertical)
263280
.constraints([
@@ -275,10 +292,11 @@ pub fn draw_config(
275292
])
276293
.split(outer);
277294
let area = inner_cols[1];
278-
// Clear the background to make overlay opaque
295+
// Clear the background to make overlay opaque and avoid bleed-through.
279296
f.render_widget(Clear, area);
280297
f.render_widget(overlay, area);
281298
let mut msg_area = area;
299+
// Inset by 2 cells horizontally to avoid text hugging the border.
282300
msg_area.x += 2;
283301
msg_area.y += 1;
284302
msg_area.width = msg_area.width.saturating_sub(4);
@@ -289,11 +307,11 @@ pub fn draw_config(
289307
.as_ref()
290308
.map(|d| d.message.as_str())
291309
.unwrap_or("this may take a few minutes on first run...");
292-
// If text starts with percentage (e.g., "75% model..."), hide it from display
310+
// If text starts with a percentage (e.g., "75% model..."), hide it to reduce noise.
293311
let display_text = if let Some(pct_end) = text.find('%') {
294312
let before_pct = &text[..pct_end];
295313
if before_pct.trim().parse::<u16>().is_ok() {
296-
// Skip past "NN% " to hide the percentage from user
314+
// Skip past "NN% ". Assumes percentage fits in u16 and precedes '%'.
297315
text.get(pct_end + 1..).unwrap_or(text).trim_start()
298316
} else {
299317
text
@@ -309,11 +327,11 @@ pub fn draw_config(
309327
} else {
310328
Theme::text_primary()
311329
};
312-
// render primary line
330+
// Render primary line; keep same bg as overlay for a seamless modal look.
313331
let p = Paragraph::new(Line::from(vec![Span::styled(display_text, stage_style)]))
314332
.style(Style::default().bg(Colors::bg_selected()));
315333
f.render_widget(p, msg_area);
316-
// render gauge on second line if percent known
334+
// Render gauge on the second line if a percentage can be parsed.
317335
let mut gauge_area = msg_area;
318336
gauge_area.y = gauge_area.y.saturating_add(1);
319337
if let Some(pct_end) = text.find('%') {
@@ -323,6 +341,7 @@ pub fn draw_config(
323341
.gauge_style(Theme::selected())
324342
.style(Style::default().bg(Colors::bg_selected()))
325343
.ratio({
344+
// Clamp to [0,1]. Use 1.0 when nearing completion and ETA reports 0s.
326345
let r = val as f64 / 100.0;
327346
if val >= 99 && text.contains("ETA 0s") {
328347
1.0
@@ -360,6 +379,7 @@ pub fn draw_config(
360379
let area = cols[1];
361380
f.render_widget(toast, area);
362381
let mut msg = area;
382+
// One-line message centered horizontally; trim margins to avoid wrapping.
363383
msg.x += 2;
364384
msg.y += 1;
365385
msg.width = msg.width.saturating_sub(4);

0 commit comments

Comments
 (0)