From 8442bcf367c61ba66b2bb21b2ccd6a10cea824e6 Mon Sep 17 00:00:00 2001 From: teernisse Date: Wed, 18 Feb 2026 13:28:37 -0500 Subject: [PATCH] feat(trace,file-history): add tracing instrumentation and diagnostic hints MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add structured tracing spans to trace and file-history pipelines so debug logging (-vv) shows path resolution counts, MR match counts, and discussion counts at each stage. This makes empty-result debugging straightforward. Add a hints field to TraceResult and FileHistoryResult that carries machine-readable diagnostic strings explaining *why* results may be empty (e.g., "Run 'lore sync' to fetch MR file changes"). The CLI renders these as info lines; robot mode includes them in JSON when non-empty. Also: fix filter_map(Result::ok) → collect:: in trace.rs (same pattern fixed in prior commit for file_history/path_resolver), and switch conn.prepare → conn.prepare_cached for the MR query. Co-Authored-By: Claude Opus 4.6 --- src/cli/commands/file_history.rs | 101 +++++++++++++++++++++++++++---- src/cli/commands/trace.rs | 17 ++++-- src/core/trace.rs | 101 +++++++++++++++++++++++++++---- 3 files changed, 192 insertions(+), 27 deletions(-) diff --git a/src/cli/commands/file_history.rs b/src/cli/commands/file_history.rs index a3894cd..588e87e 100644 --- a/src/cli/commands/file_history.rs +++ b/src/cli/commands/file_history.rs @@ -1,5 +1,7 @@ use serde::Serialize; +use tracing::info; + use crate::Config; use crate::cli::render::{self, Icons, Theme}; use crate::core::db::create_connection; @@ -46,6 +48,9 @@ pub struct FileHistoryResult { pub discussions: Vec, pub total_mrs: usize, pub paths_searched: usize, + /// Diagnostic hints explaining why results may be empty. + #[serde(skip_serializing_if = "Vec::is_empty")] + pub hints: Vec, } /// Run the file-history query. @@ -77,6 +82,11 @@ pub fn run_file_history( let paths_searched = all_paths.len(); + info!( + paths = paths_searched, + renames_followed, "file-history: resolved {} path(s) for '{}'", paths_searched, path + ); + // Build placeholders for IN clause let placeholders: Vec = (0..all_paths.len()) .map(|i| format!("?{}", i + 2)) @@ -135,14 +145,31 @@ pub fn run_file_history( web_url: row.get(8)?, }) })? - .filter_map(std::result::Result::ok) - .collect(); + .collect::, _>>()?; let total_mrs = merge_requests.len(); + info!( + mr_count = total_mrs, + "file-history: found {} MR(s) touching '{}'", total_mrs, path + ); + // Optionally fetch DiffNote discussions on this file let discussions = if include_discussions && !merge_requests.is_empty() { - fetch_file_discussions(&conn, &all_paths, project_id)? + let discs = fetch_file_discussions(&conn, &all_paths, project_id)?; + info!( + discussion_count = discs.len(), + "file-history: found {} discussion(s)", + discs.len() + ); + discs + } else { + Vec::new() + }; + + // Build diagnostic hints when no results found + let hints = if total_mrs == 0 { + build_file_history_hints(&conn, project_id, &all_paths)? } else { Vec::new() }; @@ -155,6 +182,7 @@ pub fn run_file_history( discussions, total_mrs, paths_searched, + hints, }) } @@ -179,8 +207,7 @@ fn fetch_file_discussions( JOIN discussions d ON d.id = n.discussion_id \ WHERE n.position_new_path IN ({in_clause}) {project_filter} \ AND n.is_system = 0 \ - ORDER BY n.created_at DESC \ - LIMIT 50" + ORDER BY n.created_at DESC" ); let mut stmt = conn.prepare(&sql)?; @@ -210,12 +237,57 @@ fn fetch_file_discussions( created_at_iso: ms_to_iso(created_at), }) })? - .filter_map(std::result::Result::ok) - .collect(); + .collect::, _>>()?; Ok(discussions) } +/// Build diagnostic hints explaining why a file-history query returned no results. +fn build_file_history_hints( + conn: &rusqlite::Connection, + project_id: Option, + paths: &[String], +) -> Result> { + let mut hints = Vec::new(); + + // Check if mr_file_changes has ANY rows for this project + let has_file_changes: bool = if let Some(pid) = project_id { + conn.query_row( + "SELECT EXISTS(SELECT 1 FROM mr_file_changes WHERE project_id = ?1 LIMIT 1)", + rusqlite::params![pid], + |row| row.get(0), + )? + } else { + conn.query_row( + "SELECT EXISTS(SELECT 1 FROM mr_file_changes LIMIT 1)", + [], + |row| row.get(0), + )? + }; + + if !has_file_changes { + hints.push( + "No MR file changes have been synced yet. Run 'lore sync' to fetch file change data." + .to_string(), + ); + return Ok(hints); + } + + // File changes exist but none match these paths + let path_list = paths + .iter() + .map(|p| format!("'{p}'")) + .collect::>() + .join(", "); + hints.push(format!( + "Searched paths [{}] were not found in MR file changes. \ + The file may predate the sync window or use a different path.", + path_list + )); + + Ok(hints) +} + // ── Human output ──────────────────────────────────────────────────────────── pub fn print_file_history(result: &FileHistoryResult) { @@ -250,10 +322,16 @@ pub fn print_file_history(result: &FileHistoryResult) { Icons::info(), Theme::dim().render("No merge requests found touching this file.") ); - println!( - " {}", - Theme::dim().render("Hint: Run 'lore sync' to fetch MR file changes.") - ); + if !result.renames_followed && result.rename_chain.len() == 1 { + println!( + " {} Searched: {}", + Icons::info(), + Theme::dim().render(&result.rename_chain[0]) + ); + } + for hint in &result.hints { + println!(" {} {}", Icons::info(), Theme::dim().render(hint)); + } println!(); return; } @@ -327,6 +405,7 @@ pub fn print_file_history_json(result: &FileHistoryResult, elapsed_ms: u64) { "total_mrs": result.total_mrs, "renames_followed": result.renames_followed, "paths_searched": result.paths_searched, + "hints": if result.hints.is_empty() { None } else { Some(&result.hints) }, } }); diff --git a/src/cli/commands/trace.rs b/src/cli/commands/trace.rs index e072901..e56ee07 100644 --- a/src/cli/commands/trace.rs +++ b/src/cli/commands/trace.rs @@ -50,17 +50,23 @@ pub fn print_trace(result: &TraceResult) { ); } + // Show searched paths when there are renames but no chains if result.trace_chains.is_empty() { println!( "\n {} {}", Icons::info(), Theme::dim().render("No trace chains found for this file.") ); - println!( - " {}", - Theme::dim() - .render("Hint: Run 'lore sync' to fetch MR file changes and cross-references.") - ); + if !result.renames_followed && result.resolved_paths.len() == 1 { + println!( + " {} Searched: {}", + Icons::info(), + Theme::dim().render(&result.resolved_paths[0]) + ); + } + for hint in &result.hints { + println!(" {} {}", Icons::info(), Theme::dim().render(hint)); + } println!(); return; } @@ -195,6 +201,7 @@ pub fn print_trace_json(result: &TraceResult, elapsed_ms: u64, line_requested: O "elapsed_ms": elapsed_ms, "total_chains": result.total_chains, "renames_followed": result.renames_followed, + "hints": if result.hints.is_empty() { None } else { Some(&result.hints) }, } }); diff --git a/src/core/trace.rs b/src/core/trace.rs index 8cf319f..ef30e1c 100644 --- a/src/core/trace.rs +++ b/src/core/trace.rs @@ -1,4 +1,5 @@ use serde::Serialize; +use tracing::info; use super::error::Result; use super::file_history::resolve_rename_chain; @@ -51,6 +52,9 @@ pub struct TraceResult { pub renames_followed: bool, pub trace_chains: Vec, pub total_chains: usize, + /// Diagnostic hints explaining why results may be empty. + #[serde(skip_serializing_if = "Vec::is_empty")] + pub hints: Vec, } /// Run the trace query: file -> MR -> issue chain. @@ -75,6 +79,14 @@ pub fn run_trace( (vec![path.to_string()], false) }; + info!( + paths = all_paths.len(), + renames_followed, + "trace: resolved {} path(s) for '{}'", + all_paths.len(), + path + ); + // Build placeholders for IN clause let placeholders: Vec = (0..all_paths.len()) .map(|i| format!("?{}", i + 2)) @@ -100,7 +112,7 @@ pub fn run_trace( all_paths.len() + 2 ); - let mut stmt = conn.prepare(&mr_sql)?; + let mut stmt = conn.prepare_cached(&mr_sql)?; let mut params: Vec> = Vec::new(); params.push(Box::new(project_id.unwrap_or(0))); @@ -137,8 +149,14 @@ pub fn run_trace( web_url: row.get(8)?, }) })? - .filter_map(std::result::Result::ok) - .collect(); + .collect::, _>>()?; + + info!( + mr_count = mr_rows.len(), + "trace: found {} MR(s) touching '{}'", + mr_rows.len(), + path + ); // Step 2: For each MR, find linked issues + optional discussions let mut trace_chains = Vec::with_capacity(mr_rows.len()); @@ -152,6 +170,16 @@ pub fn run_trace( Vec::new() }; + info!( + mr_iid = mr.iid, + issues = issues.len(), + discussions = discussions.len(), + "trace: MR !{}: {} issue(s), {} discussion(s)", + mr.iid, + issues.len(), + discussions.len() + ); + trace_chains.push(TraceChain { mr_iid: mr.iid, mr_title: mr.title.clone(), @@ -168,12 +196,20 @@ pub fn run_trace( let total_chains = trace_chains.len(); + // Build diagnostic hints when no results found + let hints = if total_chains == 0 { + build_trace_hints(conn, project_id, &all_paths)? + } else { + Vec::new() + }; + Ok(TraceResult { path: path.to_string(), resolved_paths: all_paths, renames_followed, trace_chains, total_chains, + hints, }) } @@ -191,7 +227,7 @@ fn fetch_linked_issues(conn: &rusqlite::Connection, mr_id: i64) -> Result = stmt .query_map(rusqlite::params![mr_id], |row| { Ok(TraceIssue { @@ -202,8 +238,7 @@ fn fetch_linked_issues(conn: &rusqlite::Connection, mr_id: i64) -> Result, _>>()?; Ok(issues) } @@ -225,11 +260,10 @@ fn fetch_trace_discussions( WHERE d.merge_request_id = ?1 \ AND n.position_new_path IN ({in_clause}) \ AND n.is_system = 0 \ - ORDER BY n.created_at DESC \ - LIMIT 20" + ORDER BY n.created_at DESC" ); - let mut stmt = conn.prepare(&sql)?; + let mut stmt = conn.prepare_cached(&sql)?; let mut params: Vec> = Vec::new(); params.push(Box::new(mr_id)); @@ -251,12 +285,57 @@ fn fetch_trace_discussions( created_at_iso: ms_to_iso(created_at), }) })? - .filter_map(std::result::Result::ok) - .collect(); + .collect::, _>>()?; Ok(discussions) } +/// Build diagnostic hints explaining why a trace query returned no results. +fn build_trace_hints( + conn: &rusqlite::Connection, + project_id: Option, + paths: &[String], +) -> Result> { + let mut hints = Vec::new(); + + // Check if mr_file_changes has ANY rows for this project + let has_file_changes: bool = if let Some(pid) = project_id { + conn.query_row( + "SELECT EXISTS(SELECT 1 FROM mr_file_changes WHERE project_id = ?1 LIMIT 1)", + rusqlite::params![pid], + |row| row.get(0), + )? + } else { + conn.query_row( + "SELECT EXISTS(SELECT 1 FROM mr_file_changes LIMIT 1)", + [], + |row| row.get(0), + )? + }; + + if !has_file_changes { + hints.push( + "No MR file changes have been synced yet. Run 'lore sync' to fetch file change data." + .to_string(), + ); + return Ok(hints); + } + + // File changes exist but none match these paths + let path_list = paths + .iter() + .map(|p| format!("'{p}'")) + .collect::>() + .join(", "); + hints.push(format!( + "Searched paths [{}] were not found in MR file changes. \ + The file may predate the sync window or use a different path.", + path_list + )); + + Ok(hints) +} + #[cfg(test)] #[path = "trace_tests.rs"] mod tests;