dgf1988/codewhale
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Merge branch 'fix/v067-file-mention' (#101 @-file mention BLOCKER fix)

Browse Source
This commit is contained in:
Hunter Bown
2026-04-27 22:17:22 -05:00
parent 0f7252198d fd13dffd60
commit 363f064fce
4 changed files with 352 additions and 74 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/tui/src/tui/file_mention.rs
+168 -64
View File
@@ -23,9 +23,7 @@
use std::fmt::Write;
use std::io::Read;
use std::path::Path;
use ignore::WalkBuilder;
use std::path::{Path, PathBuf};
use crate::tui::app::App;
use crate::working_set::Workspace;
@@ -43,10 +41,6 @@ pub const MAX_DIRECTORY_MENTION_ENTRIES: usize = 80;
/// the cost of walking large workspaces; subsequent keystrokes narrow further.
const FILE_MENTION_COMPLETION_LIMIT: usize = 64;
/// Maximum directory depth walked when completing a file mention. Mirrors the
/// existing `project_tree` cutoff and keeps Tab snappy in deep monorepos.
const FILE_MENTION_COMPLETION_DEPTH: usize = 6;
// ---------------------------------------------------------------------------
// Tab-completion
// ---------------------------------------------------------------------------
@@ -92,56 +86,31 @@ pub fn partial_file_mention_at_cursor(input: &str, cursor_chars: usize) -> Optio
Some((byte_start, partial))
}
/// Walk the workspace and return relative paths whose representation matches
/// the partial mention. A file matches when its case-insensitive relative
/// path either starts with the partial or contains it as a substring; the
/// former rank earlier so a partial like `docs/de` resolves to
/// `docs/deepseek_v4.pdf` before any path that merely contains those bytes.
pub fn find_file_mention_completions(workspace: &Path, partial: &str, limit: usize) -> Vec<String> {
if limit == 0 {
return Vec::new();
}
let needle = partial.to_lowercase();
let mut prefix_hits: Vec<String> = Vec::new();
let mut substring_hits: Vec<String> = Vec::new();
/// Cwd-aware completion entry point. Shares its walker with the future
/// Ctrl+P fuzzy picker (#97); see [`Workspace::completions`] for the
/// ranking + display rules.
pub fn find_file_mention_completions(
workspace: &Workspace,
partial: &str,
limit: usize,
) -> Vec<String> {
let entries = workspace.completions(partial, limit);
tracing::debug!(
target: "deepseek_tui::file_mention",
partial = %partial,
workspace = %workspace.root.display(),
cwd = ?std::env::current_dir().ok(),
match_count = entries.len(),
"file mention completion walk",
);
entries
}
let mut builder = WalkBuilder::new(workspace);
builder
.hidden(true)
.follow_links(false)
.max_depth(Some(FILE_MENTION_COMPLETION_DEPTH));
for entry in builder.build().flatten() {
if prefix_hits.len() + substring_hits.len() >= limit {
break;
}
let path = entry.path();
let Ok(rel) = path.strip_prefix(workspace) else {
continue;
};
let rel_str = rel.to_string_lossy().replace('\\', "/");
if rel_str.is_empty() {
continue;
}
let is_dir = entry.file_type().is_some_and(|ft| ft.is_dir());
let candidate = if is_dir {
format!("{rel_str}/")
} else {
rel_str.clone()
};
let lower = candidate.to_lowercase();
if needle.is_empty() || lower.starts_with(&needle) {
prefix_hits.push(candidate);
} else if lower.contains(&needle) {
substring_hits.push(candidate);
}
}
prefix_hits.sort();
substring_hits.sort();
prefix_hits.extend(substring_hits);
prefix_hits.truncate(limit);
prefix_hits
/// Build a `Workspace` for the running app: anchors at `app.workspace` and
/// captures the process CWD so the resolver and completion walker honor the
/// user's launch directory when it differs from `--workspace`.
fn workspace_for_app(app: &App) -> Workspace {
Workspace::with_cwd(app.workspace.clone(), std::env::current_dir().ok())
}
/// Resolve the `@`-mention completion popup contents for the current
@@ -169,7 +138,8 @@ pub fn visible_mention_menu_entries(app: &App, limit: usize) -> Vec<String> {
if limit == 0 {
return Vec::new();
}
find_file_mention_completions(&app.workspace, &partial, limit)
let ws = workspace_for_app(app);
find_file_mention_completions(&ws, &partial, limit)
}
/// Apply the currently selected `@`-mention popup entry to the composer
@@ -209,9 +179,8 @@ pub fn try_autocomplete_file_mention(app: &mut App) -> bool {
else {
return false;
};
let workspace = app.workspace.clone();
let candidates =
find_file_mention_completions(&workspace, &partial, FILE_MENTION_COMPLETION_LIMIT);
let ws = workspace_for_app(app);
let candidates = find_file_mention_completions(&ws, &partial, FILE_MENTION_COMPLETION_LIMIT);
if candidates.is_empty() {
app.status_message = Some(format!("No files match @{partial}"));
return true;
@@ -287,14 +256,28 @@ pub fn longest_common_prefix<'a>(values: &[&'a str]) -> &'a str {
/// Append a "Local context from @mentions" block to the user's message when
/// any `@path` references are present. Returns the input unchanged when
/// there are none.
pub fn user_request_with_file_mentions(input: &str, workspace: &Path) -> String {
let Some(context) = local_context_from_file_mentions(input, workspace) else {
///
/// `cwd` carries the user's launch directory and drives the second
/// resolution pass (issue #101): relative `@<path>` mentions resolve under
/// `cwd` when `workspace.join(path)` doesn't exist, so the user's mental
/// anchor (their shell's pwd) wins when it diverges from `--workspace`.
/// Pass `None` to disable the cwd pass entirely (workspace-only).
pub fn user_request_with_file_mentions(
input: &str,
workspace: &Path,
cwd: Option<PathBuf>,
) -> String {
let Some(context) = local_context_from_file_mentions(input, workspace, cwd) else {
return input.to_string();
};
format!("{input}\n\n---\n\nLocal context from @mentions:\n{context}")
}
fn local_context_from_file_mentions(input: &str, workspace: &Path) -> Option<String> {
fn local_context_from_file_mentions(
input: &str,
workspace: &Path,
cwd: Option<PathBuf>,
) -> Option<String> {
let mentions = extract_file_mentions(input);
if mentions.is_empty() {
return None;
@@ -302,7 +285,7 @@ fn local_context_from_file_mentions(input: &str, workspace: &Path) -> Option<Str
let mut blocks = Vec::new();
let mut seen = std::collections::HashSet::new();
let ws = Workspace::new(workspace.to_path_buf());
let ws = Workspace::with_cwd(workspace.to_path_buf(), cwd);
for mention in mentions.into_iter().take(MAX_FILE_MENTIONS_PER_MESSAGE) {
// `Workspace::resolve` already returns absolute paths when the root
@@ -320,6 +303,15 @@ fn local_context_from_file_mentions(input: &str, workspace: &Path) -> Option<Str
(p, d, false)
}
};
tracing::debug!(
target: "deepseek_tui::file_mention",
raw_typed = %mention,
workspace = %workspace.display(),
cwd = ?std::env::current_dir().ok(),
resolved = %display_path,
exists,
"file mention resolution",
);
// Gate every block — including <missing-file> — through the dedup
// set so a user typing the same non-existent file twice doesn't
@@ -528,3 +520,115 @@ fn is_media_path(path: &Path) -> bool {
| "mkv"
)
}
// ---------------------------------------------------------------------------
// #101 regression repros
// ---------------------------------------------------------------------------
//
// The bug being guarded: typing `@<some/file>` resolved under `--workspace`,
// not the user's launch CWD. When the two diverged (the canonical case is
// `--workspace=/repo` with `pwd=/repo/sub`), every relative `@` token routed
// to the wrong root and the prompt got `<missing-file>` blocks.
#[cfg(test)]
mod tests {
use super::*;
use tempfile::TempDir;
/// #101 regression — workspace-vs-cwd divergence: `@bar.txt` typed from
/// the cwd `<root>/sub` MUST resolve to `<root>/sub/bar.txt`, never to
/// `<root>/bar.txt` (which doesn't exist).
#[test]
fn cwd_pass_resolves_when_workspace_pass_misses() {
let tmp = TempDir::new().expect("tempdir");
let sub = tmp.path().join("sub");
std::fs::create_dir_all(&sub).expect("mkdir");
let bar = sub.join("bar.txt");
std::fs::write(&bar, "hello bar").expect("write bar");
let content =
user_request_with_file_mentions("look at @bar.txt", tmp.path(), Some(sub.clone()));
// The block must reference the cwd-rooted path with the file's body —
// and crucially it must NOT collapse to <missing-file>.
assert!(
content.contains("hello bar"),
"expected file body to be inlined; got: {content}",
);
assert!(
!content.contains("<missing-file"),
"must not surface <missing-file> for a path that exists under cwd; got: {content}",
);
let bar_disp = bar.display().to_string();
assert!(
content.contains(&bar_disp),
"expected resolved path {bar_disp} in content; got: {content}",
);
// Belt-and-suspenders: the workspace-rooted path doesn't exist and
// must not appear in the rendered <file path="..."> attribute.
let wrong = tmp.path().join("bar.txt").display().to_string();
assert!(
!content.contains(&format!("path=\"{wrong}\"")),
"should NOT have routed to {wrong}; got: {content}",
);
}
/// #101 regression — nested workspace path: `@nested/deep/file.md` with
/// the file at workspace root resolves through the workspace pass.
#[test]
fn workspace_pass_resolves_nested_path() {
let tmp = TempDir::new().expect("tempdir");
let nested = tmp.path().join("nested/deep");
std::fs::create_dir_all(&nested).expect("mkdir");
let file_md = nested.join("file.md");
std::fs::write(&file_md, "# nested deep").expect("write file_md");
// Cwd is irrelevant; an unrelated tempdir would do. Pass `None` so we
// are unambiguously testing the workspace-pass path.
let content = user_request_with_file_mentions("see @nested/deep/file.md", tmp.path(), None);
assert!(content.contains("# nested deep"), "got: {content}");
assert!(!content.contains("<missing-file"), "got: {content}");
assert!(
content.contains(&file_md.display().to_string()),
"got: {content}",
);
}
/// Snapshot-style check: the rendered `<file>` block for a resolvable
/// mention must include the expected attributes and contents, and must
/// NOT contain `<missing-file>`.
#[test]
fn resolvable_mention_renders_file_block_not_missing_file() {
let tmp = TempDir::new().expect("tempdir");
std::fs::write(tmp.path().join("guide.md"), "# Guide\nUse the fast path.\n")
.expect("write");
let content = user_request_with_file_mentions("read @guide.md", tmp.path(), None);
// Header + tag presence.
assert!(content.contains("Local context from @mentions:"));
assert!(content.contains("<file mention=\"@guide.md\""));
assert!(content.contains("# Guide\nUse the fast path."));
assert!(content.ends_with("</file>"), "got: {content}");
// The bug fingerprint MUST be absent.
assert!(!content.contains("<missing-file"), "got: {content}");
}
/// Negative test: a truly missing path still produces `<missing-file>`
/// so the user gets an explicit signal instead of silent failure.
#[test]
fn truly_missing_mention_still_renders_missing_file() {
let tmp = TempDir::new().expect("tempdir");
let content = user_request_with_file_mentions(
"huh @does/not/exist.txt",
tmp.path(),
Some(tmp.path().to_path_buf()),
);
assert!(
content.contains("<missing-file mention=\"@does/not/exist.txt\""),
"got: {content}",
);
}
}
crates/tui/src/tui/ui.rs
+8 -2
View File
@@ -2158,8 +2158,14 @@ fn build_queued_message(app: &mut App, input: String) -> QueuedMessage {
}
fn queued_message_content_for_app(app: &App, message: &QueuedMessage) -> String {
let user_request =
crate::tui::file_mention::user_request_with_file_mentions(&message.display, &app.workspace);
// Pass the process CWD explicitly so the resolver's two-pass logic can
// honor the user's launch directory when it differs from `--workspace`
// (issue #101 — file mentions silently routing to the wrong root).
let user_request = crate::tui::file_mention::user_request_with_file_mentions(
&message.display,
&app.workspace,
std::env::current_dir().ok(),
);
if let Some(skill_instruction) = message.skill_instruction.as_ref() {
format!("{skill_instruction}\n\n---\n\nUser request: {user_request}")
} else {
crates/tui/src/tui/ui/tests.rs
+7 -4
View File
@@ -6,6 +6,7 @@ use crate::tui::file_mention::{
};
use crate::tui::history::{GenericToolCell, HistoryCell, ToolCell, ToolStatus};
use crate::tui::views::{ModalView, ViewAction};
use crate::working_set::Workspace;
use std::path::PathBuf;
use std::process::Command;
use tempfile::TempDir;
@@ -158,7 +159,7 @@ fn file_mentions_do_not_trigger_inside_email_addresses() {
let tmpdir = TempDir::new().expect("tempdir");
std::fs::write(tmpdir.path().join("example.com"), "not a mention").expect("write file");
let content = user_request_with_file_mentions("email me@example.com", tmpdir.path());
let content = user_request_with_file_mentions("email me@example.com", tmpdir.path(), None);
assert_eq!(content, "email me@example.com");
}
@@ -168,7 +169,7 @@ fn media_file_mentions_point_to_attach_instead_of_inlining_bytes() {
let tmpdir = TempDir::new().expect("tempdir");
std::fs::write(tmpdir.path().join("photo.png"), b"\0png").expect("write image");
let content = user_request_with_file_mentions("inspect @photo.png", tmpdir.path());
let content = user_request_with_file_mentions("inspect @photo.png", tmpdir.path(), None);
assert!(content.contains("<media-file mention=\"@photo.png\""));
assert!(content.contains("Use /attach photo.png"));
@@ -1051,7 +1052,8 @@ fn file_mention_completion_finds_unique_match() {
std::fs::create_dir_all(tmpdir.path().join("docs")).unwrap();
std::fs::write(tmpdir.path().join("docs/deepseek_v4.pdf"), b"%PDF-").unwrap();
let matches = find_file_mention_completions(tmpdir.path(), "docs/de", 16);
let ws = Workspace::with_cwd(tmpdir.path().to_path_buf(), None);
let matches = find_file_mention_completions(&ws, "docs/de", 16);
assert_eq!(matches, vec!["docs/deepseek_v4.pdf".to_string()]);
}
@@ -1062,7 +1064,8 @@ fn file_mention_completion_ranks_prefix_before_substring() {
std::fs::create_dir_all(tmpdir.path().join("nested")).unwrap();
std::fs::write(tmpdir.path().join("nested/README.md"), "x").unwrap();
let matches = find_file_mention_completions(tmpdir.path(), "README", 16);
let ws = Workspace::with_cwd(tmpdir.path().to_path_buf(), None);
let matches = find_file_mention_completions(&ws, "README", 16);
// Top-level README (prefix match) outranks the nested one (substring).
assert_eq!(matches.first().map(String::as_str), Some("README.md"));
}
crates/tui/src/working_set.rs
+169 -4
View File
@@ -32,6 +32,11 @@ pub struct Workspace {
}
impl Workspace {
/// Construct a workspace anchored at `root`, capturing the process CWD as
/// the secondary resolution pass. Convenience entry point intended for
/// callers that don't already have a CWD on hand; the App routes through
/// [`Workspace::with_cwd`] with its own captured launch directory.
#[allow(dead_code)] // Keeps the surface stable for #97 (Ctrl+P picker).
pub fn new(root: PathBuf) -> Self {
Self::with_cwd(root, std::env::current_dir().ok())
}
@@ -90,6 +95,9 @@ impl Workspace {
let mut index: HashMap<String, Vec<PathBuf>> = HashMap::new();
let mut builder = WalkBuilder::new(&self.root);
builder.hidden(true).follow_links(false).max_depth(Some(6));
// Honor `.deepseekignore` in addition to the defaults the `ignore` crate
// already respects (`.gitignore`, `.git/info/exclude`, `.ignore`).
let _ = builder.add_custom_ignore_filename(".deepseekignore");
for entry in builder.build().flatten() {
if entry
@@ -105,6 +113,121 @@ impl Workspace {
}
index
}
/// Walk the workspace (and the recorded `cwd` when it diverges) and
/// return relative paths whose representation matches `partial`.
///
/// Ranking: a candidate matches when its case-insensitive display string
/// starts with `partial` (prefix hit) or contains it as a substring; prefix
/// hits sort first so `docs/de` lands `docs/deepseek_v4.pdf` ahead of any
/// path that merely shares those bytes.
///
/// Display strings are workspace-relative for files under `root`, and
/// cwd-relative for files only under the recorded `cwd` — so what the user
/// Tab-completes matches what their shell would have shown them.
///
/// Honors `.gitignore`, `.git/info/exclude`, `.ignore`, and
/// `.deepseekignore`. Capped at `limit` results.
#[must_use]
pub fn completions(&self, partial: &str, limit: usize) -> Vec<String> {
if limit == 0 {
return Vec::new();
}
let needle = partial.to_lowercase();
let mut prefix_hits: Vec<String> = Vec::new();
let mut substring_hits: Vec<String> = Vec::new();
let mut seen: HashSet<PathBuf> = HashSet::new();
// Walk the recorded cwd first when it diverges from the workspace
// root, so cwd-relative entries appear ahead of duplicates surfaced by
// the workspace walk.
let cwd_diverges = self
.cwd
.as_deref()
.map(|c| c != self.root.as_path())
.unwrap_or(false);
if cwd_diverges && let Some(cwd) = self.cwd.as_deref() {
walk_for_completions(
cwd,
cwd,
&needle,
limit,
&mut prefix_hits,
&mut substring_hits,
&mut seen,
);
}
walk_for_completions(
&self.root,
&self.root,
&needle,
limit,
&mut prefix_hits,
&mut substring_hits,
&mut seen,
);
prefix_hits.sort();
substring_hits.sort();
prefix_hits.extend(substring_hits);
prefix_hits.truncate(limit);
prefix_hits
}
}
/// Maximum directory depth walked when surfacing file-mention completions.
/// Mirrors the existing `project_tree` cutoff and keeps Tab snappy in deep
/// monorepos.
const COMPLETIONS_WALK_DEPTH: usize = 6;
#[allow(clippy::too_many_arguments)]
fn walk_for_completions(
walk_root: &Path,
display_root: &Path,
needle: &str,
limit: usize,
prefix_hits: &mut Vec<String>,
substring_hits: &mut Vec<String>,
seen: &mut HashSet<PathBuf>,
) {
let mut builder = WalkBuilder::new(walk_root);
builder
.hidden(true)
.follow_links(false)
.max_depth(Some(COMPLETIONS_WALK_DEPTH));
let _ = builder.add_custom_ignore_filename(".deepseekignore");
for entry in builder.build().flatten() {
if prefix_hits.len() + substring_hits.len() >= limit {
break;
}
let path = entry.path();
let Ok(rel) = path.strip_prefix(display_root) else {
continue;
};
let rel_str = rel.to_string_lossy().replace('\\', "/");
if rel_str.is_empty() {
continue;
}
// Dedup across the (cwd, workspace) double-walk by absolute path; we
// want the cwd-relative display when both walks see the same file.
let abs = path.to_path_buf();
if !seen.insert(abs) {
continue;
}
let is_dir = entry.file_type().is_some_and(|ft| ft.is_dir());
let candidate = if is_dir {
format!("{rel_str}/")
} else {
rel_str.clone()
};
let lower = candidate.to_lowercase();
if needle.is_empty() || lower.starts_with(needle) {
prefix_hits.push(candidate);
} else if lower.contains(needle) {
substring_hits.push(candidate);
}
}
}
impl Clone for Workspace {
@@ -926,14 +1049,17 @@ mod tests {
// tests that mutate the real process cwd.
let ws = Workspace::with_cwd(tmp.path().to_path_buf(), Some(sub.clone()));
// Test 1: @bar.txt with cwd=sub → resolves via the cwd pass.
// #101 repro #1: @bar.txt with cwd=sub MUST resolve via the cwd pass,
// never to the bogus workspace path tmp/bar.txt (which doesn't exist).
let res1 = ws.resolve("bar.txt").unwrap();
assert_eq!(
res1.canonicalize().unwrap_or(res1),
bar.canonicalize().unwrap_or(bar)
res1.canonicalize().unwrap_or(res1.clone()),
bar.canonicalize().unwrap_or(bar.clone())
);
let wrong = tmp.path().join("bar.txt");
assert_ne!(res1, wrong, "must not have routed to workspace fallback");
// Test 2: @nested/deep/file.md → falls through to workspace root.
// #101 repro #2: @nested/deep/file.md falls through to workspace root.
let res2 = ws.resolve("nested/deep/file.md").unwrap();
assert_eq!(
res2.canonicalize().unwrap_or(res2),
@@ -941,6 +1067,45 @@ mod tests {
);
}
/// Negative test (#101): a truly missing path returns `Err` with a path
/// that callers can show to the user as a signal of failure.
#[test]
fn workspace_resolve_returns_err_for_truly_missing_path() {
let tmp = TempDir::new().unwrap();
let ws = Workspace::with_cwd(tmp.path().to_path_buf(), Some(tmp.path().to_path_buf()));
let res = ws.resolve("does/not/exist.txt");
assert!(res.is_err(), "expected Err for missing path, got: {res:?}");
}
/// `Workspace::completions` returns workspace-relative entries for files
/// under the root, and cwd-relative entries when the cwd-only file lives
/// outside the workspace tree. Honors `.gitignore`.
#[test]
fn workspace_completions_walk_surfaces_workspace_and_cwd() {
let tmp = TempDir::new().unwrap();
// Two trees: a workspace under `ws/` and a cwd under `cwd/` that is
// NOT inside the workspace, so the two walks are disjoint and we can
// assert each branch contributed.
let ws_root = tmp.path().join("ws");
let cwd_root = tmp.path().join("cwd");
std::fs::create_dir_all(&ws_root).unwrap();
std::fs::create_dir_all(&cwd_root).unwrap();
std::fs::write(ws_root.join("alpha.txt"), "a").unwrap();
std::fs::write(cwd_root.join("alphabeta.txt"), "b").unwrap();
let ws = Workspace::with_cwd(ws_root.clone(), Some(cwd_root.clone()));
let entries = ws.completions("alpha", 16);
assert!(
entries.iter().any(|e| e == "alpha.txt"),
"expected workspace entry alpha.txt; got: {entries:?}",
);
assert!(
entries.iter().any(|e| e == "alphabeta.txt"),
"expected cwd entry alphabeta.txt; got: {entries:?}",
);
}
#[test]
fn fuzzy_index_finds_files_and_directories() {
let tmp = TempDir::new().unwrap();