From 667f70e177b3cc647f693fcc40641e5e4a44f944 Mon Sep 17 00:00:00 2001 From: Taylor Eernisse Date: Fri, 30 Jan 2026 16:54:36 -0500 Subject: [PATCH] refactor(commands): Add IngestDisplay, resolve_project, and color-aware tables Ingest: - Introduce IngestDisplay struct with show_progress/show_text booleans to decouple progress bars from text output. Replaces the robot_mode bool parameter with explicit display control, enabling sync to show progress without duplicating summary text (progress_only mode). - Use resolve_project() for --project filtering instead of LIKE queries, providing proper error messages for ambiguous or missing projects. List: - Add colored_cell() helper that checks console::colors_enabled() before applying comfy-table foreground colors, bridging the gap between the console and comfy-table crates for --color flag support. - Use resolve_project() for project filtering (exact ID match). - Improve since filter to return explicit errors instead of silently ignoring invalid values. - Improve format_relative_time for proper singular/plural forms. Search: - Validate --after/--updated-after with explicit error messages. - Handle optional title field (Option) in HydratedRow. Show: - Use resolve_project() for project disambiguation. Sync: - Thread robot_mode via SyncOptions for IngestDisplay selection. - Use IngestDisplay::progress_only() in interactive sync mode. GenerateDocs: - Use resolve_project() for --project filtering. Co-Authored-By: Claude (us.anthropic.claude-opus-4-5-20251101-v1:0) --- src/cli/commands/generate_docs.rs | 18 ++--- src/cli/commands/ingest.rs | 84 ++++++++++++++++++----- src/cli/commands/list.rs | 107 ++++++++++++++++++------------ src/cli/commands/mod.rs | 2 +- src/cli/commands/search.rs | 30 +++++++-- src/cli/commands/show.rs | 57 ++++++++-------- src/cli/commands/sync.rs | 13 +++- 7 files changed, 207 insertions(+), 104 deletions(-) diff --git a/src/cli/commands/generate_docs.rs b/src/cli/commands/generate_docs.rs index e6377c2..eb6aa3a 100644 --- a/src/cli/commands/generate_docs.rs +++ b/src/cli/commands/generate_docs.rs @@ -8,7 +8,8 @@ use tracing::info; use crate::core::db::create_connection; use crate::core::error::Result; use crate::core::paths::get_db_path; -use crate::documents::{regenerate_dirty_documents, SourceType}; +use crate::core::project::resolve_project; +use crate::documents::{SourceType, regenerate_dirty_documents}; use crate::Config; const FULL_MODE_CHUNK_SIZE: i64 = 2000; @@ -81,18 +82,7 @@ fn seed_dirty( loop { let inserted = if let Some(project) = project_filter { - // Resolve project to ID for filtering - let project_id: Option = conn - .query_row( - "SELECT id FROM projects WHERE path_with_namespace = ?1 COLLATE NOCASE", - [project], - |row| row.get(0), - ) - .ok(); - - let Some(pid) = project_id else { - break; - }; + let project_id = resolve_project(conn, project)?; conn.execute( &format!( @@ -101,7 +91,7 @@ fn seed_dirty( FROM {table} WHERE id > ?3 AND project_id = ?4 ORDER BY id LIMIT ?5 ON CONFLICT(source_type, source_id) DO NOTHING" ), - rusqlite::params![type_str, now, last_id, pid, FULL_MODE_CHUNK_SIZE], + rusqlite::params![type_str, now, last_id, project_id, FULL_MODE_CHUNK_SIZE], )? } else { conn.execute( diff --git a/src/cli/commands/ingest.rs b/src/cli/commands/ingest.rs index d8b71b7..1c9cd0f 100644 --- a/src/cli/commands/ingest.rs +++ b/src/cli/commands/ingest.rs @@ -10,6 +10,7 @@ use crate::core::db::create_connection; use crate::core::error::{LoreError, Result}; use crate::core::lock::{AppLock, LockOptions}; use crate::core::paths::get_db_path; +use crate::core::project::resolve_project; use crate::gitlab::GitLabClient; use crate::ingestion::{ IngestMrProjectResult, IngestProjectResult, ProgressEvent, ingest_project_issues_with_progress, @@ -40,6 +41,36 @@ pub struct IngestResult { pub notes_upserted: usize, } +/// Controls what interactive UI elements `run_ingest` displays. +/// +/// Separates progress indicators (spinners, bars) from text output (headers, +/// per-project summaries) so callers like `sync` can show progress without +/// duplicating summary text. +#[derive(Debug, Clone, Copy)] +pub struct IngestDisplay { + /// Show animated spinners and progress bars. + pub show_progress: bool, + /// Show text headers ("Ingesting...") and per-project summary lines. + pub show_text: bool, +} + +impl IngestDisplay { + /// Interactive mode: everything visible. + pub fn interactive() -> Self { + Self { show_progress: true, show_text: true } + } + + /// Robot/JSON mode: everything hidden. + pub fn silent() -> Self { + Self { show_progress: false, show_text: false } + } + + /// Progress only (used by sync in interactive mode). + pub fn progress_only() -> Self { + Self { show_progress: true, show_text: false } + } +} + /// Run the ingest command. pub async fn run_ingest( config: &Config, @@ -47,7 +78,7 @@ pub async fn run_ingest( project_filter: Option<&str>, force: bool, full: bool, - robot_mode: bool, + display: IngestDisplay, ) -> Result { // Validate resource type early if resource_type != "issues" && resource_type != "mrs" { @@ -86,7 +117,7 @@ pub async fn run_ingest( // If --full flag is set, reset sync cursors and discussion watermarks for a complete re-fetch if full { - if !robot_mode { + if display.show_text { println!( "{}", style("Full sync: resetting cursors to fetch all data...").yellow() @@ -139,7 +170,7 @@ pub async fn run_ingest( } else { "merge requests" }; - if !robot_mode { + if display.show_text { println!("{}", style(format!("Ingesting {type_label}...")).blue()); println!(); } @@ -147,7 +178,7 @@ pub async fn run_ingest( // Sync each project for (local_project_id, gitlab_project_id, path) in &projects { // Show spinner while fetching (only in interactive mode) - let spinner = if robot_mode { + let spinner = if !display.show_progress { ProgressBar::hidden() } else { let s = ProgressBar::new_spinner(); @@ -162,7 +193,7 @@ pub async fn run_ingest( }; // Progress bar for discussion sync (hidden until needed, or always hidden in robot mode) - let disc_bar = if robot_mode { + let disc_bar = if !display.show_progress { ProgressBar::hidden() } else { let b = ProgressBar::new(0); @@ -178,7 +209,7 @@ pub async fn run_ingest( // Create progress callback (no-op in robot mode) let spinner_clone = spinner.clone(); let disc_bar_clone = disc_bar.clone(); - let progress_callback: crate::ingestion::ProgressCallback = if robot_mode { + let progress_callback: crate::ingestion::ProgressCallback = if !display.show_progress { Box::new(|_| {}) } else { Box::new(move |event: ProgressEvent| match event { @@ -225,7 +256,7 @@ pub async fn run_ingest( disc_bar.finish_and_clear(); // Print per-project summary (only in interactive mode) - if !robot_mode { + if display.show_text { print_issue_project_summary(path, &result); } @@ -254,7 +285,7 @@ pub async fn run_ingest( disc_bar.finish_and_clear(); // Print per-project summary (only in interactive mode) - if !robot_mode { + if display.show_text { print_mr_project_summary(path, &result); } @@ -283,16 +314,39 @@ fn get_projects_to_sync( configured_projects: &[crate::core::config::ProjectConfig], filter: Option<&str>, ) -> Result> { - let mut projects = Vec::new(); + // If a filter is provided, resolve it to a specific project + if let Some(filter_str) = filter { + let project_id = resolve_project(conn, filter_str)?; - for project_config in configured_projects { - if let Some(filter_path) = filter - && !project_config.path.contains(filter_path) - { - continue; + // Verify the resolved project is in our config + let row: Option<(i64, String)> = conn + .query_row( + "SELECT gitlab_project_id, path_with_namespace FROM projects WHERE id = ?1", + [project_id], + |row| Ok((row.get(0)?, row.get(1)?)), + ) + .ok(); + + if let Some((gitlab_id, path)) = row { + // Confirm it's a configured project + if configured_projects.iter().any(|p| p.path == path) { + return Ok(vec![(project_id, gitlab_id, path)]); + } + return Err(LoreError::Other(format!( + "Project '{}' exists in database but is not in configuration", + path + ))); } - // Get project from database + return Err(LoreError::Other(format!( + "Project '{}' not found in database", + filter_str + ))); + } + + // No filter: return all configured projects + let mut projects = Vec::new(); + for project_config in configured_projects { let result: Option<(i64, i64)> = conn .query_row( "SELECT id, gitlab_project_id FROM projects WHERE path_with_namespace = ?", diff --git a/src/cli/commands/list.rs b/src/cli/commands/list.rs index ef4e2b7..ff0ccb9 100644 --- a/src/cli/commands/list.rs +++ b/src/cli/commands/list.rs @@ -6,10 +6,21 @@ use serde::Serialize; use crate::Config; use crate::core::db::create_connection; -use crate::core::error::Result; +use crate::core::error::{LoreError, Result}; use crate::core::paths::get_db_path; +use crate::core::project::resolve_project; use crate::core::time::{ms_to_iso, now_ms, parse_since}; +/// Apply foreground color to a Cell only if colors are enabled. +fn colored_cell(content: impl std::fmt::Display, color: Color) -> Cell { + let cell = Cell::new(content); + if console::colors_enabled() { + cell.fg(color) + } else { + cell + } +} + /// Issue row for display. #[derive(Debug, Serialize)] pub struct IssueListRow { @@ -232,11 +243,9 @@ fn query_issues(conn: &Connection, filters: &ListFilters) -> Result let mut params: Vec> = Vec::new(); if let Some(project) = filters.project { - // Exact match or suffix match after '/' to avoid partial matches - // e.g. "foo" matches "group/foo" but NOT "group/foobar" - where_clauses.push("(p.path_with_namespace = ? OR p.path_with_namespace LIKE ?)"); - params.push(Box::new(project.to_string())); - params.push(Box::new(format!("%/{project}"))); + let project_id = resolve_project(conn, project)?; + where_clauses.push("i.project_id = ?"); + params.push(Box::new(project_id)); } if let Some(state) = filters.state @@ -264,9 +273,13 @@ fn query_issues(conn: &Connection, filters: &ListFilters) -> Result } // Handle since filter - if let Some(since_str) = filters.since - && let Some(cutoff_ms) = parse_since(since_str) - { + if let Some(since_str) = filters.since { + let cutoff_ms = parse_since(since_str).ok_or_else(|| { + LoreError::Other(format!( + "Invalid --since value '{}'. Use relative (7d, 2w, 1m) or absolute (YYYY-MM-DD) format.", + since_str + )) + })?; where_clauses.push("i.updated_at >= ?"); params.push(Box::new(cutoff_ms)); } @@ -419,11 +432,9 @@ fn query_mrs(conn: &Connection, filters: &MrListFilters) -> Result let mut params: Vec> = Vec::new(); if let Some(project) = filters.project { - // Exact match or suffix match after '/' to avoid partial matches - // e.g. "foo" matches "group/foo" but NOT "group/foobar" - where_clauses.push("(p.path_with_namespace = ? OR p.path_with_namespace LIKE ?)"); - params.push(Box::new(project.to_string())); - params.push(Box::new(format!("%/{project}"))); + let project_id = resolve_project(conn, project)?; + where_clauses.push("m.project_id = ?"); + params.push(Box::new(project_id)); } if let Some(state) = filters.state @@ -461,9 +472,13 @@ fn query_mrs(conn: &Connection, filters: &MrListFilters) -> Result } // Handle since filter - if let Some(since_str) = filters.since - && let Some(cutoff_ms) = parse_since(since_str) - { + if let Some(since_str) = filters.since { + let cutoff_ms = parse_since(since_str).ok_or_else(|| { + LoreError::Other(format!( + "Invalid --since value '{}'. Use relative (7d, 2w, 1m) or absolute (YYYY-MM-DD) format.", + since_str + )) + })?; where_clauses.push("m.updated_at >= ?"); params.push(Box::new(cutoff_ms)); } @@ -628,10 +643,22 @@ fn format_relative_time(ms_epoch: i64) -> String { match diff { d if d < 60_000 => "just now".to_string(), d if d < 3_600_000 => format!("{} min ago", d / 60_000), - d if d < 86_400_000 => format!("{} hours ago", d / 3_600_000), - d if d < 604_800_000 => format!("{} days ago", d / 86_400_000), - d if d < 2_592_000_000 => format!("{} weeks ago", d / 604_800_000), - _ => format!("{} months ago", diff / 2_592_000_000), + d if d < 86_400_000 => { + let n = d / 3_600_000; + format!("{n} {} ago", if n == 1 { "hour" } else { "hours" }) + } + d if d < 604_800_000 => { + let n = d / 86_400_000; + format!("{n} {} ago", if n == 1 { "day" } else { "days" }) + } + d if d < 2_592_000_000 => { + let n = d / 604_800_000; + format!("{n} {} ago", if n == 1 { "week" } else { "weeks" }) + } + _ => { + let n = diff / 2_592_000_000; + format!("{n} {} ago", if n == 1 { "month" } else { "months" }) + } } } @@ -735,19 +762,19 @@ pub fn print_list_issues(result: &ListResult) { let discussions = format_discussions(issue.discussion_count, issue.unresolved_count); let state_cell = if issue.state == "opened" { - Cell::new(&issue.state).fg(Color::Green) + colored_cell(&issue.state, Color::Green) } else { - Cell::new(&issue.state).fg(Color::DarkGrey) + colored_cell(&issue.state, Color::DarkGrey) }; table.add_row(vec![ - Cell::new(format!("#{}", issue.iid)).fg(Color::Cyan), + colored_cell(format!("#{}", issue.iid), Color::Cyan), Cell::new(title), state_cell, - Cell::new(assignee).fg(Color::Magenta), - Cell::new(labels).fg(Color::Yellow), + colored_cell(assignee, Color::Magenta), + colored_cell(labels, Color::Yellow), Cell::new(discussions), - Cell::new(relative_time).fg(Color::DarkGrey), + colored_cell(relative_time, Color::DarkGrey), ]); } @@ -807,7 +834,6 @@ pub fn print_list_mrs(result: &MrListResult) { ]); for mr in &result.mrs { - // Add [DRAFT] prefix for draft MRs let title = if mr.draft { format!("[DRAFT] {}", truncate_with_ellipsis(&mr.title, 38)) } else { @@ -819,25 +845,24 @@ pub fn print_list_mrs(result: &MrListResult) { let discussions = format_discussions(mr.discussion_count, mr.unresolved_count); let state_cell = match mr.state.as_str() { - "opened" => Cell::new(&mr.state).fg(Color::Green), - "merged" => Cell::new(&mr.state).fg(Color::Magenta), - "closed" => Cell::new(&mr.state).fg(Color::Red), - "locked" => Cell::new(&mr.state).fg(Color::Yellow), - _ => Cell::new(&mr.state).fg(Color::DarkGrey), + "opened" => colored_cell(&mr.state, Color::Green), + "merged" => colored_cell(&mr.state, Color::Magenta), + "closed" => colored_cell(&mr.state, Color::Red), + "locked" => colored_cell(&mr.state, Color::Yellow), + _ => colored_cell(&mr.state, Color::DarkGrey), }; table.add_row(vec![ - Cell::new(format!("!{}", mr.iid)).fg(Color::Cyan), + colored_cell(format!("!{}", mr.iid), Color::Cyan), Cell::new(title), state_cell, - Cell::new(format!( - "@{}", - truncate_with_ellipsis(&mr.author_username, 12) - )) - .fg(Color::Magenta), - Cell::new(branches).fg(Color::Blue), + colored_cell( + format!("@{}", truncate_with_ellipsis(&mr.author_username, 12)), + Color::Magenta, + ), + colored_cell(branches, Color::Blue), Cell::new(discussions), - Cell::new(relative_time).fg(Color::DarkGrey), + colored_cell(relative_time, Color::DarkGrey), ]); } diff --git a/src/cli/commands/mod.rs b/src/cli/commands/mod.rs index 10e2f7e..cede53e 100644 --- a/src/cli/commands/mod.rs +++ b/src/cli/commands/mod.rs @@ -23,7 +23,7 @@ pub use stats::{print_stats, print_stats_json, run_stats}; pub use search::{ print_search_results, print_search_results_json, run_search, SearchCliFilters, SearchResponse, }; -pub use ingest::{print_ingest_summary, print_ingest_summary_json, run_ingest}; +pub use ingest::{IngestDisplay, print_ingest_summary, print_ingest_summary_json, run_ingest}; pub use init::{InitInputs, InitOptions, InitResult, run_init}; pub use list::{ ListFilters, MrListFilters, open_issue_in_browser, open_mr_in_browser, print_list_issues, diff --git a/src/cli/commands/search.rs b/src/cli/commands/search.rs index d865c84..2eb69c7 100644 --- a/src/cli/commands/search.rs +++ b/src/cli/commands/search.rs @@ -104,8 +104,30 @@ pub fn run_search( .map(|p| resolve_project(&conn, p)) .transpose()?; - let after = cli_filters.after.as_deref().and_then(parse_since); - let updated_after = cli_filters.updated_after.as_deref().and_then(parse_since); + let after = cli_filters + .after + .as_deref() + .map(|s| { + parse_since(s).ok_or_else(|| { + LoreError::Other(format!( + "Invalid --after value '{}'. Use relative (7d, 2w, 1m) or absolute (YYYY-MM-DD) format.", + s + )) + }) + }) + .transpose()?; + let updated_after = cli_filters + .updated_after + .as_deref() + .map(|s| { + parse_since(s).ok_or_else(|| { + LoreError::Other(format!( + "Invalid --updated-after value '{}'. Use relative (7d, 2w, 1m) or absolute (YYYY-MM-DD) format.", + s + )) + }) + }) + .transpose()?; let path = cli_filters.path.as_deref().map(|p| { if p.ends_with('/') { @@ -192,7 +214,7 @@ pub fn run_search( results.push(SearchResultDisplay { document_id: row.document_id, source_type: row.source_type.clone(), - title: row.title.clone(), + title: row.title.clone().unwrap_or_default(), url: row.url.clone(), author: row.author.clone(), created_at: row.created_at.map(ms_to_iso), @@ -219,7 +241,7 @@ pub fn run_search( struct HydratedRow { document_id: i64, source_type: String, - title: String, + title: Option, url: Option, author: Option, created_at: Option, diff --git a/src/cli/commands/show.rs b/src/cli/commands/show.rs index 012d1b6..bbdff2b 100644 --- a/src/cli/commands/show.rs +++ b/src/cli/commands/show.rs @@ -8,6 +8,7 @@ use crate::Config; use crate::core::db::create_connection; use crate::core::error::{LoreError, Result}; use crate::core::paths::get_db_path; +use crate::core::project::resolve_project; use crate::core::time::ms_to_iso; /// Merge request metadata for display. @@ -145,18 +146,20 @@ struct IssueRow { /// Find issue by iid, optionally filtered by project. fn find_issue(conn: &Connection, iid: i64, project_filter: Option<&str>) -> Result { let (sql, params): (&str, Vec>) = match project_filter { - Some(project) => ( - "SELECT i.id, i.iid, i.title, i.description, i.state, i.author_username, - i.created_at, i.updated_at, i.web_url, p.path_with_namespace - FROM issues i - JOIN projects p ON i.project_id = p.id - WHERE i.iid = ? AND (p.path_with_namespace = ? OR p.path_with_namespace LIKE ?)", - vec![ - Box::new(iid), - Box::new(project.to_string()), - Box::new(format!("%/{}", project)), - ], - ), + Some(project) => { + let project_id = resolve_project(conn, project)?; + ( + "SELECT i.id, i.iid, i.title, i.description, i.state, i.author_username, + i.created_at, i.updated_at, i.web_url, p.path_with_namespace + FROM issues i + JOIN projects p ON i.project_id = p.id + WHERE i.iid = ? AND i.project_id = ?", + vec![ + Box::new(iid), + Box::new(project_id), + ], + ) + } None => ( "SELECT i.id, i.iid, i.title, i.description, i.state, i.author_username, i.created_at, i.updated_at, i.web_url, p.path_with_namespace @@ -333,20 +336,22 @@ struct MrRow { /// Find MR by iid, optionally filtered by project. fn find_mr(conn: &Connection, iid: i64, project_filter: Option<&str>) -> Result { let (sql, params): (&str, Vec>) = match project_filter { - Some(project) => ( - "SELECT m.id, m.iid, m.title, m.description, m.state, m.draft, - m.author_username, m.source_branch, m.target_branch, - m.created_at, m.updated_at, m.merged_at, m.closed_at, - m.web_url, p.path_with_namespace - FROM merge_requests m - JOIN projects p ON m.project_id = p.id - WHERE m.iid = ? AND (p.path_with_namespace = ? OR p.path_with_namespace LIKE ?)", - vec![ - Box::new(iid), - Box::new(project.to_string()), - Box::new(format!("%/{}", project)), - ], - ), + Some(project) => { + let project_id = resolve_project(conn, project)?; + ( + "SELECT m.id, m.iid, m.title, m.description, m.state, m.draft, + m.author_username, m.source_branch, m.target_branch, + m.created_at, m.updated_at, m.merged_at, m.closed_at, + m.web_url, p.path_with_namespace + FROM merge_requests m + JOIN projects p ON m.project_id = p.id + WHERE m.iid = ? AND m.project_id = ?", + vec![ + Box::new(iid), + Box::new(project_id), + ], + ) + } None => ( "SELECT m.id, m.iid, m.title, m.description, m.state, m.draft, m.author_username, m.source_branch, m.target_branch, diff --git a/src/cli/commands/sync.rs b/src/cli/commands/sync.rs index 84ffd0d..ae8b737 100644 --- a/src/cli/commands/sync.rs +++ b/src/cli/commands/sync.rs @@ -9,7 +9,7 @@ use crate::core::error::Result; use super::embed::run_embed; use super::generate_docs::run_generate_docs; -use super::ingest::run_ingest; +use super::ingest::{IngestDisplay, run_ingest}; /// Options for the sync command. #[derive(Debug, Default)] @@ -18,6 +18,7 @@ pub struct SyncOptions { pub force: bool, pub no_embed: bool, pub no_docs: bool, + pub robot_mode: bool, } /// Result of the sync command. @@ -34,15 +35,21 @@ pub struct SyncResult { pub async fn run_sync(config: &Config, options: SyncOptions) -> Result { let mut result = SyncResult::default(); + let ingest_display = if options.robot_mode { + IngestDisplay::silent() + } else { + IngestDisplay::progress_only() + }; + // Stage 1: Ingest issues info!("Sync stage 1/4: ingesting issues"); - let issues_result = run_ingest(config, "issues", None, options.force, options.full, true).await?; + let issues_result = run_ingest(config, "issues", None, options.force, options.full, ingest_display).await?; result.issues_updated = issues_result.issues_upserted; result.discussions_fetched += issues_result.discussions_fetched; // Stage 2: Ingest MRs info!("Sync stage 2/4: ingesting merge requests"); - let mrs_result = run_ingest(config, "mrs", None, options.force, options.full, true).await?; + let mrs_result = run_ingest(config, "mrs", None, options.force, options.full, ingest_display).await?; result.mrs_updated = mrs_result.mrs_upserted; result.discussions_fetched += mrs_result.discussions_fetched;