fix: address PR #4 follow-ups (#5)

* fix: address PR #4 follow-ups

Honor low_motion in the default tool transcript path and align composer cursor padding with the rendered placeholder. Add focused regression tests for both behaviors.


* lint: remove redundant imports in empty_state test, reuse create_test_app

The test had inner `use` statements for Config, App, TuiOptions, and
PathBuf that duplicated the module-level test imports. It also manually
constructed App instead of calling the existing create_test_app() helper.

* fix: replace useless format!("{text}") with text.to_string() in details_affordance_line

* test: pin composer_density in cursor test to avoid sensitivity to loaded settings

Settings::load() may return a non-default composer_density on some CI
environments. Explicitly set ComposerDensity::Comfortable so the
expected cursor position is deterministic across all platforms.

* fix: make tool low_motion test robust against coarse Windows timers

Use a 2× cycle offset so the animated frame index is 2 (maximally
distant from 0), giving 1800 ms of headroom before the animation could
wrap back to index 0. The previous 1× offset left only ~15 ms of
margin, causing flaky failures on Windows where Instant resolution is
approximately 15.6 ms.

* fix: correct headroom comment in tool animation test (3600ms, not 1800ms)

* fix: resolve lint, parity, and Windows test failures

- Fix rustfmt line-length issue in history.rs tool animation test
- Settings::path() now respects DEEPSEEK_CONFIG_PATH for Windows test compat
- doctor_check_mcp_server recognizes Unix-style absolute paths on Windows
- Use checked_sub for Instant arithmetic in web_run tests to prevent
  underflow on freshly-booted Windows CI runners


* fix: expand ~ in DEEPSEEK_CONFIG_PATH when resolving settings path

---------
This commit is contained in:
Hunter Bown
2026-03-12 21:29:14 -05:00
parent b172b8d306
commit ea3cc3d0d2
6 changed files with 195 additions and 47 deletions
+1 -1
View File
@@ -525,7 +525,7 @@ fn default_requirements_path() -> Option<PathBuf> {
}
}
fn expand_path(path: &str) -> PathBuf {
pub(crate) fn expand_path(path: &str) -> PathBuf {
if let Some(stripped) = path.strip_prefix('~')
&& (stripped.is_empty() || stripped.starts_with('/') || stripped.starts_with('\\'))
&& let Some(mut home) = effective_home_dir()
+3 -1
View File
@@ -1918,7 +1918,9 @@ fn doctor_check_mcp_server(server: &McpServerConfig) -> McpServerDoctorStatus {
}
let cmd_path = Path::new(cmd);
let is_absolute = cmd_path.is_absolute();
// Also accept Unix-style `/` prefix on Windows, where Path::is_absolute()
// requires a drive letter.
let is_absolute = cmd_path.is_absolute() || cmd.starts_with('/');
if is_absolute && !cmd_path.exists() {
return McpServerDoctorStatus::Error(format!("command not found: {cmd}"));
+14 -1
View File
@@ -7,7 +7,7 @@ use std::path::PathBuf;
use anyhow::{Context, Result};
use serde::{Deserialize, Serialize};
use crate::config::normalize_model_name;
use crate::config::{expand_path, normalize_model_name};
/// User settings with defaults
#[derive(Debug, Clone, Serialize, Deserialize)]
@@ -64,6 +64,19 @@ impl Default for Settings {
impl Settings {
/// Get the settings file path
pub fn path() -> Result<PathBuf> {
// Allow tests to override the settings directory via the same env var
// used for config (DEEPSEEK_CONFIG_PATH points at config.toml; the
// settings file lives as a sibling in the same directory).
if let Ok(config_path) = std::env::var("DEEPSEEK_CONFIG_PATH") {
let config_path = config_path.trim();
if !config_path.is_empty() {
let p = expand_path(config_path);
if let Some(parent) = p.parent() {
return Ok(parent.join("settings.toml"));
}
}
}
let config_dir = dirs::config_dir()
.context("Failed to resolve config directory: not found.")?
.join("deepseek");
+16 -2
View File
@@ -1555,13 +1555,27 @@ mod tests {
let ref_id = format!("{}turn0search1", scoped_ref_prefix(namespace));
store_page(namespace, &ref_id, sample_page("https://example.com/alpha"));
with_state(|state| {
// On Windows, Instant's epoch is system boot. If the CI runner has
// been up for less than WEB_RUN_SESSION_TTL the subtraction would
// underflow, so we skip the test in that case.
let stale = WEB_RUN_SESSION_TTL + Duration::from_secs(1);
let can_test = with_state(|state| {
let session = state
.sessions
.get_mut(namespace)
.expect("session should exist");
session.last_access = Instant::now() - WEB_RUN_SESSION_TTL - Duration::from_secs(1);
match Instant::now().checked_sub(stale) {
Some(past) => {
session.last_access = past;
true
}
None => false,
}
});
if !can_test {
// System uptime shorter than session TTL; can't test eviction.
return;
}
let _ = next_turn_for_namespace("session-beta");
+46 -3
View File
@@ -153,7 +153,10 @@ impl HistoryCell {
}
lines
}
_ => self.lines(width),
HistoryCell::Tool(cell) => cell.lines_with_motion(width, options.low_motion),
HistoryCell::User { .. }
| HistoryCell::Assistant { .. }
| HistoryCell::System { .. } => self.lines(width),
}
}
@@ -1444,7 +1447,7 @@ fn status_symbol(started_at: Option<Instant>, status: ToolStatus, low_motion: bo
fn details_affordance_line(text: &str, style: Style) -> Line<'static> {
Line::from(vec![
Span::styled("", Style::default().fg(palette::TEXT_DIM)),
Span::styled(format!("{text}"), style),
Span::styled(text.to_string(), style),
])
}
@@ -1628,7 +1631,11 @@ fn thinking_state_accent(state: ThinkingVisualState) -> Color {
#[cfg(test)]
mod tests {
use super::{extract_reasoning_summary, render_thinking};
use super::{
ExecCell, ExecSource, HistoryCell, TOOL_RUNNING_SYMBOLS, TOOL_STATUS_SYMBOL_MS, ToolCell,
ToolStatus, TranscriptRenderOptions, extract_reasoning_summary, render_thinking,
};
use std::time::{Duration, Instant};
#[test]
fn extract_reasoning_summary_prefers_summary_block() {
@@ -1661,4 +1668,40 @@ mod tests {
assert!(text.contains("summary only; press v for details"));
assert!(text.contains("thinking"));
}
#[test]
fn tool_lines_with_options_respects_low_motion_in_default_path() {
// Use a 2× cycle offset so the animated frame lands on index 2,
// which is maximally far from index 0. This avoids flaky failures on
// platforms with coarse timer resolution (Windows ≈ 15.6 ms) and
// gives 3600 ms of headroom before the index could wrap back to 0
// (indices 2 → 3 → 0 requires two more full cycles).
let started_at = Some(Instant::now() - Duration::from_millis(TOOL_STATUS_SYMBOL_MS * 2));
let cell = HistoryCell::Tool(ToolCell::Exec(ExecCell {
command: "echo hi".to_string(),
status: ToolStatus::Running,
output: None,
started_at,
duration_ms: None,
source: ExecSource::Assistant,
interaction: None,
}));
let animated = cell.lines_with_options(80, TranscriptRenderOptions::default());
let low_motion = cell.lines_with_options(
80,
TranscriptRenderOptions {
low_motion: true,
..TranscriptRenderOptions::default()
},
);
let animated_symbol = animated[0].spans[0].content.trim();
let low_motion_symbol = low_motion[0].spans[0].content.trim();
// low_motion always pins to the first (static) frame.
assert_eq!(low_motion_symbol, TOOL_RUNNING_SYMBOLS[0]);
// The animated path should be on a different frame (index 2).
assert_ne!(animated_symbol, TOOL_RUNNING_SYMBOLS[0]);
}
}
+115 -39
View File
@@ -213,7 +213,7 @@ impl Renderable for ComposerWidget<'_> {
let mut input_lines = Vec::new();
if self.app.input.is_empty() {
input_lines.push(Line::from(Span::styled(
"Write a task or use /.",
COMPOSER_PLACEHOLDER,
Style::default().fg(palette::TEXT_MUTED).italic(),
)));
} else {
@@ -225,7 +225,16 @@ impl Renderable for ComposerWidget<'_> {
}
}
let top_padding = composer_vertical_padding(input_lines.len(), input_rows_budget);
// For non-empty input, input_lines.len() already reflects wrapping via
// layout_input. For the empty-input placeholder, Paragraph::wrap will
// wrap the single Line at render time, so we must estimate the wrapped
// row count ourselves to keep padding accurate on narrow widths.
let visual_rows = if self.app.input.is_empty() {
placeholder_visual_lines(content_width)
} else {
input_lines.len()
};
let top_padding = composer_top_padding(visual_rows, input_rows_budget);
let mut lines = Vec::new();
for _ in 0..top_padding {
lines.push(Line::from(""));
@@ -278,25 +287,18 @@ impl Renderable for ComposerWidget<'_> {
let input_rows_budget =
composer_input_rows_budget(inner_area.height, self.slash_menu_entries.len());
let (_visible_lines, cursor_row, cursor_col) = layout_input(
let (visible_lines, cursor_row, cursor_col) = layout_input(
&self.app.input,
self.app.cursor_position,
content_width,
input_rows_budget,
);
let top_padding = if self.app.input.is_empty() {
let empty_lines = if self.app.input_history.is_empty() && input_rows_budget > 1 {
2
} else {
1
};
composer_vertical_padding(empty_lines, input_rows_budget)
let visual_rows = if self.app.input.is_empty() {
placeholder_visual_lines(content_width)
} else {
let visible_count = wrap_input_lines(&self.app.input, content_width)
.len()
.max(1);
composer_vertical_padding(visible_count.min(input_rows_budget), input_rows_budget)
visible_lines.len()
};
let top_padding = composer_top_padding(visual_rows, input_rows_budget);
let cursor_x = area
.x
@@ -805,8 +807,16 @@ fn composer_input_rows_budget(inner_height: u16, extra_lines: usize) -> usize {
usize::from(inner_height).saturating_sub(extra_lines).max(1)
}
fn composer_vertical_padding(content_lines: usize, rows_budget: usize) -> usize {
rows_budget.saturating_sub(content_lines)
fn composer_top_padding(content_lines: usize, rows_budget: usize) -> usize {
rows_budget.saturating_sub(content_lines.clamp(1, rows_budget))
}
/// Placeholder text shown when the composer input is empty.
const COMPOSER_PLACEHOLDER: &str = "Write a task or use /.";
/// How many visual rows the empty-input placeholder occupies after wrapping.
fn placeholder_visual_lines(content_width: usize) -> usize {
wrap_text(COMPOSER_PLACEHOLDER, content_width).len().max(1)
}
fn composer_min_input_rows(density: ComposerDensity) -> usize {
@@ -1021,18 +1031,42 @@ fn wrap_text(text: &str, width: usize) -> Vec<String> {
#[cfg(test)]
mod tests {
use super::{
COMPOSER_PANEL_HEIGHT, apply_selection_to_line, composer_height, composer_max_height,
composer_min_input_rows, cursor_row_col, layout_input, pad_lines_to_bottom,
COMPOSER_PANEL_HEIGHT, ComposerWidget, Renderable, apply_selection_to_line,
composer_height, composer_max_height, composer_min_input_rows, composer_top_padding,
cursor_row_col, layout_input, pad_lines_to_bottom, placeholder_visual_lines,
should_render_empty_state, slash_completion_hints, wrap_input_lines, wrap_text,
};
use crate::config::Config;
use crate::palette;
use crate::tui::app::ComposerDensity;
use crate::tui::app::{App, ComposerDensity, TuiOptions};
use ratatui::{
layout::Rect,
style::Style,
text::{Line, Span},
};
use std::path::PathBuf;
use unicode_width::UnicodeWidthStr;
fn create_test_app() -> App {
let options = TuiOptions {
model: "deepseek-chat".to_string(),
workspace: PathBuf::from("."),
allow_shell: false,
use_alt_screen: true,
max_subagents: 1,
skills_dir: PathBuf::from("."),
memory_path: PathBuf::from("memory.md"),
notes_path: PathBuf::from("notes.txt"),
mcp_config_path: PathBuf::from("mcp.json"),
use_memory: false,
start_in_agent_mode: true,
skip_onboarding: true,
yolo: false,
resume_session_id: None,
};
App::new(options, &Config::default())
}
#[test]
fn pad_lines_to_bottom_noop_when_already_filled() {
let mut lines = vec![Line::from("one"), Line::from("two")];
@@ -1274,28 +1308,70 @@ mod tests {
}
#[test]
fn empty_state_renders_only_without_transcript_activity() {
use crate::config::Config;
use crate::tui::app::{App, TuiOptions};
use std::path::PathBuf;
fn empty_composer_cursor_matches_placeholder_padding() {
let mut app = create_test_app();
// Pin density so the test is independent of any loaded user settings.
app.composer_density = ComposerDensity::Comfortable;
let slash_menu_entries = Vec::<String>::new();
let widget = ComposerWidget::new(&app, 5, &slash_menu_entries);
let options = TuiOptions {
model: "deepseek-chat".to_string(),
workspace: PathBuf::from("."),
allow_shell: false,
use_alt_screen: true,
max_subagents: 1,
skills_dir: PathBuf::from("."),
memory_path: PathBuf::from("memory.md"),
notes_path: PathBuf::from("notes.txt"),
mcp_config_path: PathBuf::from("mcp.json"),
use_memory: false,
start_in_agent_mode: true,
skip_onboarding: true,
yolo: false,
resume_session_id: None,
// Use a wide area so the placeholder fits on one line (no wrapping).
let area = Rect {
x: 0,
y: 0,
width: 40,
height: 5,
};
let mut app = App::new(options, &Config::default());
// inner_area: {x:1, y:1, w:38, h:3} (borders shrink by 1 each side)
// input_rows_budget = 3
// placeholder_visual_lines(38) = 1 (placeholder is 22 chars, fits in 38)
// top_padding = 3 - clamp(1, 1, 3) = 2
// cursor_x = 0 + (1-0) + 0 = 1
// cursor_y = 0 + (1-0) + (2+0) = 3
assert_eq!(widget.cursor_pos(area), Some((1, 3)));
}
#[test]
fn empty_composer_cursor_accounts_for_placeholder_wrapping() {
let mut app = create_test_app();
app.composer_density = ComposerDensity::Comfortable;
let slash_menu_entries = Vec::<String>::new();
let widget = ComposerWidget::new(&app, 5, &slash_menu_entries);
// Narrow area forces the placeholder to wrap.
let area = Rect {
x: 0,
y: 0,
width: 14,
height: 5,
};
// inner_area: {x:1, y:1, w:12, h:3}
// input_rows_budget = 3
// placeholder_visual_lines(12) = 2 ("Write a task" / " or use /.")
// top_padding = 3 - clamp(2, 1, 3) = 1
// cursor_x = 0 + (1-0) + 0 = 1
// cursor_y = 0 + (1-0) + (1+0) = 2
assert_eq!(placeholder_visual_lines(12), 2);
assert_eq!(widget.cursor_pos(area), Some((1, 2)));
}
#[test]
fn composer_top_padding_uses_clamp() {
// content_lines=0 is clamped to 1
assert_eq!(composer_top_padding(0, 3), 2);
// content_lines=1
assert_eq!(composer_top_padding(1, 3), 2);
// content_lines=3 fills the budget
assert_eq!(composer_top_padding(3, 3), 0);
// content_lines > budget is clamped
assert_eq!(composer_top_padding(5, 3), 0);
}
#[test]
fn empty_state_renders_only_without_transcript_activity() {
let mut app = create_test_app();
assert!(should_render_empty_state(&app));
app.add_message(crate::tui::history::HistoryCell::User {
content: "hello".to_string(),