diff --git a/AGENTS.md b/AGENTS.md index e797214..2315d17 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -740,3 +740,53 @@ lore -J mrs --fields iid,title,state,draft,labels # Custom field list - Use `lore --robot health` as a fast pre-flight check before queries - Use `lore robot-docs` for response schema discovery - The `-p` flag supports fuzzy project matching (suffix and substring) + +````markdown +## UBS Quick Reference for AI Agents + +UBS stands for "Ultimate Bug Scanner": **The AI Coding Agent's Secret Weapon: Flagging Likely Bugs for Fixing Early On** + +**Install:** `curl -sSL https://raw.githubusercontent.com/Dicklesworthstone/ultimate_bug_scanner/master/install.sh | bash` + +**Golden Rule:** `ubs ` before every commit. Exit 0 = safe. Exit >0 = fix & re-run. + +**Commands:** +```bash +ubs file.ts file2.py # Specific files (< 1s) — USE THIS +ubs $(git diff --name-only --cached) # Staged files — before commit +ubs --only=js,python src/ # Language filter (3-5x faster) +ubs --ci --fail-on-warning . # CI mode — before PR +ubs --help # Full command reference +ubs sessions --entries 1 # Tail the latest install session log +ubs . # Whole project (ignores things like .venv and node_modules automatically) +``` + +**Output Format:** +``` +⚠️ Category (N errors) + file.ts:42:5 – Issue description + 💡 Suggested fix +Exit code: 1 +``` +Parse: `file:line:col` → location | 💡 → how to fix | Exit 0/1 → pass/fail + +**Fix Workflow:** +1. Read finding → category + fix suggestion +2. Navigate `file:line:col` → view context +3. Verify real issue (not false positive) +4. Fix root cause (not symptom) +5. Re-run `ubs ` → exit 0 +6. Commit + +**Speed Critical:** Scope to changed files. `ubs src/file.ts` (< 1s) vs `ubs .` (30s). Never full scan for small edits. + +**Bug Severity:** +- **Critical** (always fix): Null safety, XSS/injection, async/await, memory leaks +- **Important** (production): Type narrowing, division-by-zero, resource leaks +- **Contextual** (judgment): TODO/FIXME, console logs + +**Anti-Patterns:** +- ❌ Ignore findings → ✅ Investigate each +- ❌ Full scan per edit → ✅ Scope to file +- ❌ Fix symptom (`if (x) { x.y }`) → ✅ Root cause (`x?.y`) +```` diff --git a/src/cli/autocorrect.rs b/src/cli/autocorrect.rs index 89ad7ee..2d779e0 100644 --- a/src/cli/autocorrect.rs +++ b/src/cli/autocorrect.rs @@ -177,6 +177,8 @@ const COMMAND_FLAGS: &[(&str, &[&str])] = &[ "--since", "--project", "--limit", + "--detail", + "--no-detail", ], ), ( diff --git a/src/cli/commands/who.rs b/src/cli/commands/who.rs index 6ba11f8..af5f641 100644 --- a/src/cli/commands/who.rs +++ b/src/cli/commands/who.rs @@ -6,6 +6,7 @@ use std::collections::{HashMap, HashSet}; use crate::Config; use crate::cli::WhoArgs; use crate::cli::robot::RobotMeta; +use crate::core::config::ScoringConfig; use crate::core::db::create_connection; use crate::core::error::{LoreError, Result}; use crate::core::paths::get_db_path; @@ -144,6 +145,22 @@ pub struct Expert { pub review_note_count: u32, pub author_mr_count: u32, pub last_seen_ms: i64, + /// Stable MR references like "group/project!123" + pub mr_refs: Vec, + pub mr_refs_total: u32, + pub mr_refs_truncated: bool, + /// Per-MR detail breakdown (only populated when --detail is set) + pub details: Option>, +} + +#[derive(Clone)] +pub struct ExpertMrDetail { + pub mr_ref: String, + pub title: String, + /// "R", "A", or "A+R" + pub role: String, + pub note_count: u32, + pub last_activity_ms: i64, } // --- Workload --- @@ -250,6 +267,9 @@ pub struct OverlapUser { pub mr_refs_truncated: bool, } +/// Maximum MR references to retain per user in output (shared across modes). +const MAX_MR_REFS_PER_USER: usize = 50; + // ─── Entry Point ───────────────────────────────────────────────────────────── /// Main entry point. Resolves mode + resolved inputs once, then dispatches. @@ -268,6 +288,7 @@ pub fn run_who(config: &Config, args: &WhoArgs) -> Result { .transpose()?; let mode = resolve_mode(args)?; + validate_mode_flags(&mode, args)?; // since_mode semantics: // - expert/reviews/active/overlap: default window applies if args.since is None -> "default" @@ -287,7 +308,15 @@ pub fn run_who(config: &Config, args: &WhoArgs) -> Result { WhoMode::Expert { path } => { let since_ms = resolve_since(args.since.as_deref(), "6m")?; let limit = usize::from(args.limit); - let result = query_expert(&conn, &path, project_id, since_ms, limit)?; + let result = query_expert( + &conn, + &path, + project_id, + since_ms, + limit, + &config.scoring, + args.detail, + )?; Ok(WhoRun { resolved_input: WhoResolvedInput { mode: "expert".to_string(), @@ -375,6 +404,15 @@ pub fn run_who(config: &Config, args: &WhoArgs) -> Result { } } +fn validate_mode_flags(mode: &WhoMode<'_>, args: &WhoArgs) -> Result<()> { + if args.detail && !matches!(mode, WhoMode::Expert { .. }) { + return Err(LoreError::Other( + "--detail is only supported in expert mode (`lore who --path ` or `lore who `).".to_string(), + )); + } + Ok(()) +} + // ─── Helpers ───────────────────────────────────────────────────────────────── /// Look up the project path for a resolved project ID. @@ -409,6 +447,7 @@ fn resolve_since_required(input: &str) -> Result { // ─── Path Query Construction ───────────────────────────────────────────────── /// Describes how to match a user-supplied path in SQL. +#[derive(Debug)] struct PathQuery { /// The parameter value to bind. value: String, @@ -486,30 +525,102 @@ fn build_path_query(conn: &Connection, path: &str, project_id: Option) -> R false }; - // Forced directory always wins; otherwise: exact > prefix > heuristic - let is_file = if forced_dir { - false - } else if exact_exists { - true - } else if prefix_exists { - false + // Probe 3: suffix match — user typed a bare filename or partial path that + // doesn't exist as-is. Search for full paths ending with /input (or equal to input). + // This handles "login.rs" matching "src/auth/login.rs". + let suffix_resolved = if !forced_dir && !exact_exists && !prefix_exists && looks_like_file { + suffix_probe(conn, trimmed, project_id)? } else { - looks_like_file + SuffixResult::NotAttempted }; - if is_file { - // IMPORTANT: do NOT escape for exact match (=). LIKE metacharacters - // are not special in `=`, so escaping would produce wrong values. - Ok(PathQuery { - value: trimmed.to_string(), + match suffix_resolved { + SuffixResult::Unique(full_path) => Ok(PathQuery { + value: full_path, is_prefix: false, - }) - } else { - let escaped = escape_like(trimmed); - Ok(PathQuery { - value: format!("{escaped}/%"), - is_prefix: true, - }) + }), + SuffixResult::Ambiguous(candidates) => { + let list = candidates + .iter() + .map(|p| format!(" {p}")) + .collect::>() + .join("\n"); + Err(LoreError::Ambiguous(format!( + "'{trimmed}' matches multiple paths. Use the full path or -p to scope:\n{list}" + ))) + } + SuffixResult::NotAttempted | SuffixResult::NoMatch => { + // Original logic: exact > prefix > heuristic + let is_file = if forced_dir { + false + } else if exact_exists { + true + } else if prefix_exists { + false + } else { + looks_like_file + }; + + if is_file { + Ok(PathQuery { + value: trimmed.to_string(), + is_prefix: false, + }) + } else { + let escaped = escape_like(trimmed); + Ok(PathQuery { + value: format!("{escaped}/%"), + is_prefix: true, + }) + } + } + } +} + +/// Result of a suffix probe against the DB. +enum SuffixResult { + /// Suffix probe was not attempted (conditions not met). + NotAttempted, + /// No paths matched the suffix. + NoMatch, + /// Exactly one distinct path matched — auto-resolve. + Unique(String), + /// Multiple distinct paths matched — user must disambiguate. + Ambiguous(Vec), +} + +/// Probe both notes and mr_file_changes for paths ending with the given suffix. +/// Returns up to 11 distinct candidates (enough to detect ambiguity + show a useful list). +fn suffix_probe(conn: &Connection, suffix: &str, project_id: Option) -> Result { + let escaped = escape_like(suffix); + let suffix_pat = format!("%/{escaped}"); + + let mut stmt = conn.prepare_cached( + "SELECT DISTINCT full_path FROM ( + SELECT position_new_path AS full_path FROM notes + WHERE note_type = 'DiffNote' + AND is_system = 0 + AND (position_new_path LIKE ?1 ESCAPE '\\' OR position_new_path = ?2) + AND (?3 IS NULL OR project_id = ?3) + UNION + SELECT new_path AS full_path FROM mr_file_changes + WHERE (new_path LIKE ?1 ESCAPE '\\' OR new_path = ?2) + AND (?3 IS NULL OR project_id = ?3) + ) + ORDER BY full_path + LIMIT 11", + )?; + + let candidates: Vec = stmt + .query_map(rusqlite::params![suffix_pat, suffix, project_id], |row| { + row.get(0) + })? + .collect::, _>>()?; + + match candidates.len() { + 0 => Ok(SuffixResult::NoMatch), + 1 => Ok(SuffixResult::Unique(candidates.into_iter().next().unwrap())), + _ => Ok(SuffixResult::Ambiguous(candidates)), } } @@ -523,12 +634,15 @@ fn escape_like(input: &str) -> String { // ─── Query: Expert Mode ───────────────────────────────────────────────────── +#[allow(clippy::too_many_arguments)] fn query_expert( conn: &Connection, path: &str, project_id: Option, since_ms: i64, limit: usize, + scoring: &ScoringConfig, + detail: bool, ) -> Result { let pq = build_path_query(conn, path, project_id)?; let limit_plus_one = (limit + 1) as i64; @@ -538,11 +652,15 @@ fn query_expert( // 2. DiffNote MR author — authored MR that has DiffNotes on this path // 3. File-change author — authored MR that touched this path (mr_file_changes) // 4. File-change reviewer — assigned reviewer on MR that touched this path + // Each branch now JOINs projects to produce mr_ref for aggregation. let path_op = if pq.is_prefix { "LIKE ?1 ESCAPE '\\'" } else { "= ?1" }; + let author_w = scoring.author_weight; + let reviewer_w = scoring.reviewer_weight; + let note_b = scoring.note_bonus; let sql = format!( " WITH signals AS ( @@ -552,10 +670,12 @@ fn query_expert( 'diffnote_reviewer' AS signal, m.id AS mr_id, n.id AS note_id, - n.created_at AS seen_at + n.created_at AS seen_at, + (p.path_with_namespace || '!' || CAST(m.iid AS TEXT)) AS mr_ref FROM notes n JOIN discussions d ON n.discussion_id = d.id JOIN merge_requests m ON d.merge_request_id = m.id + JOIN projects p ON m.project_id = p.id WHERE n.note_type = 'DiffNote' AND n.is_system = 0 AND n.author_username IS NOT NULL @@ -573,10 +693,12 @@ fn query_expert( 'diffnote_author' AS signal, m.id AS mr_id, NULL AS note_id, - MAX(n.created_at) AS seen_at + MAX(n.created_at) AS seen_at, + (p.path_with_namespace || '!' || CAST(m.iid AS TEXT)) AS mr_ref FROM merge_requests m JOIN discussions d ON d.merge_request_id = m.id JOIN notes n ON n.discussion_id = d.id + JOIN projects p ON m.project_id = p.id WHERE n.note_type = 'DiffNote' AND n.is_system = 0 AND m.author_username IS NOT NULL @@ -594,9 +716,11 @@ fn query_expert( 'file_author' AS signal, m.id AS mr_id, NULL AS note_id, - m.updated_at AS seen_at + m.updated_at AS seen_at, + (p.path_with_namespace || '!' || CAST(m.iid AS TEXT)) AS mr_ref FROM mr_file_changes fc JOIN merge_requests m ON fc.merge_request_id = m.id + JOIN projects p ON m.project_id = p.id WHERE m.author_username IS NOT NULL AND m.state IN ('opened','merged') AND fc.new_path {path_op} @@ -611,9 +735,11 @@ fn query_expert( 'file_reviewer' AS signal, m.id AS mr_id, NULL AS note_id, - m.updated_at AS seen_at + m.updated_at AS seen_at, + (p.path_with_namespace || '!' || CAST(m.iid AS TEXT)) AS mr_ref FROM mr_file_changes fc JOIN merge_requests m ON fc.merge_request_id = m.id + JOIN projects p ON m.project_id = p.id JOIN mr_reviewers r ON r.merge_request_id = m.id WHERE r.username IS NOT NULL AND (m.author_username IS NULL OR r.username != m.author_username) @@ -632,11 +758,12 @@ fn query_expert( MAX(seen_at) AS last_seen_at, ( (COUNT(DISTINCT CASE WHEN signal IN ('diffnote_reviewer', 'file_reviewer') - THEN mr_id END) * 20) + + THEN mr_id END) * {reviewer_w}) + (COUNT(DISTINCT CASE WHEN signal IN ('diffnote_author', 'file_author') - THEN mr_id END) * 12) + - (COUNT(CASE WHEN signal = 'diffnote_reviewer' THEN note_id END) * 1) - ) AS score + THEN mr_id END) * {author_w}) + + (COUNT(CASE WHEN signal = 'diffnote_reviewer' THEN note_id END) * {note_b}) + ) AS score, + GROUP_CONCAT(DISTINCT mr_ref) AS mr_refs_csv FROM signals GROUP BY username ORDER BY score DESC, last_seen_at DESC, username ASC @@ -650,6 +777,17 @@ fn query_expert( .query_map( rusqlite::params![pq.value, since_ms, project_id, limit_plus_one], |row| { + let mr_refs_csv: Option = row.get(6)?; + let mut mr_refs: Vec = mr_refs_csv + .as_deref() + .map(|csv| csv.split(',').map(|s| s.trim().to_string()).collect()) + .unwrap_or_default(); + mr_refs.sort(); + let mr_refs_total = mr_refs.len() as u32; + let mr_refs_truncated = mr_refs.len() > MAX_MR_REFS_PER_USER; + if mr_refs_truncated { + mr_refs.truncate(MAX_MR_REFS_PER_USER); + } Ok(Expert { username: row.get(0)?, review_mr_count: row.get(1)?, @@ -657,22 +795,221 @@ fn query_expert( author_mr_count: row.get(3)?, last_seen_ms: row.get(4)?, score: row.get(5)?, + mr_refs, + mr_refs_total, + mr_refs_truncated, + details: None, }) }, )? .collect::, _>>()?; let truncated = experts.len() > limit; - let experts: Vec = experts.into_iter().take(limit).collect(); + let mut experts: Vec = experts.into_iter().take(limit).collect(); + + // Populate per-MR detail when --detail is requested + if detail && !experts.is_empty() { + let details_map = query_expert_details(conn, &pq, &experts, since_ms, project_id)?; + for expert in &mut experts { + expert.details = details_map.get(&expert.username).cloned(); + } + } Ok(ExpertResult { - path_query: path.to_string(), + path_query: if pq.is_prefix { + // Use raw input (unescaped) for display — pq.value has LIKE escaping. + path.trim_end_matches('/').to_string() + } else { + // For exact matches (including suffix-resolved), show the resolved path. + pq.value.clone() + }, path_match: if pq.is_prefix { "prefix" } else { "exact" }.to_string(), experts, truncated, }) } +/// Query per-MR detail for a set of experts. Returns a map of username -> Vec. +fn query_expert_details( + conn: &Connection, + pq: &PathQuery, + experts: &[Expert], + since_ms: i64, + project_id: Option, +) -> Result>> { + let path_op = if pq.is_prefix { + "LIKE ?1 ESCAPE '\\'" + } else { + "= ?1" + }; + + // Build IN clause for usernames + let placeholders: Vec = experts + .iter() + .enumerate() + .map(|(i, _)| format!("?{}", i + 4)) + .collect(); + let in_clause = placeholders.join(","); + + let sql = format!( + " + WITH signals AS ( + -- 1. DiffNote reviewer + SELECT + n.author_username AS username, + 'reviewer' AS role, + m.id AS mr_id, + (p.path_with_namespace || '!' || CAST(m.iid AS TEXT)) AS mr_ref, + m.title AS title, + COUNT(*) AS note_count, + MAX(n.created_at) AS last_activity + FROM notes n + JOIN discussions d ON n.discussion_id = d.id + JOIN merge_requests m ON d.merge_request_id = m.id + JOIN projects p ON m.project_id = p.id + WHERE n.note_type = 'DiffNote' + AND n.is_system = 0 + AND n.author_username IS NOT NULL + AND (m.author_username IS NULL OR n.author_username != m.author_username) + AND m.state IN ('opened','merged') + AND n.position_new_path {path_op} + AND n.created_at >= ?2 + AND (?3 IS NULL OR n.project_id = ?3) + AND n.author_username IN ({in_clause}) + GROUP BY n.author_username, m.id + + UNION ALL + + -- 2. DiffNote MR author + SELECT + m.author_username AS username, + 'author' AS role, + m.id AS mr_id, + (p.path_with_namespace || '!' || CAST(m.iid AS TEXT)) AS mr_ref, + m.title AS title, + 0 AS note_count, + MAX(n.created_at) AS last_activity + FROM merge_requests m + JOIN discussions d ON d.merge_request_id = m.id + JOIN notes n ON n.discussion_id = d.id + JOIN projects p ON m.project_id = p.id + WHERE n.note_type = 'DiffNote' + AND n.is_system = 0 + AND m.author_username IS NOT NULL + AND m.state IN ('opened','merged') + AND n.position_new_path {path_op} + AND n.created_at >= ?2 + AND (?3 IS NULL OR n.project_id = ?3) + AND m.author_username IN ({in_clause}) + GROUP BY m.author_username, m.id + + UNION ALL + + -- 3. MR author via file changes + SELECT + m.author_username AS username, + 'author' AS role, + m.id AS mr_id, + (p.path_with_namespace || '!' || CAST(m.iid AS TEXT)) AS mr_ref, + m.title AS title, + 0 AS note_count, + m.updated_at AS last_activity + FROM mr_file_changes fc + JOIN merge_requests m ON fc.merge_request_id = m.id + JOIN projects p ON m.project_id = p.id + WHERE m.author_username IS NOT NULL + AND m.state IN ('opened','merged') + AND fc.new_path {path_op} + AND m.updated_at >= ?2 + AND (?3 IS NULL OR fc.project_id = ?3) + AND m.author_username IN ({in_clause}) + + UNION ALL + + -- 4. MR reviewer via file changes + mr_reviewers + SELECT + r.username AS username, + 'reviewer' AS role, + m.id AS mr_id, + (p.path_with_namespace || '!' || CAST(m.iid AS TEXT)) AS mr_ref, + m.title AS title, + 0 AS note_count, + m.updated_at AS last_activity + FROM mr_file_changes fc + JOIN merge_requests m ON fc.merge_request_id = m.id + JOIN projects p ON m.project_id = p.id + JOIN mr_reviewers r ON r.merge_request_id = m.id + WHERE r.username IS NOT NULL + AND (m.author_username IS NULL OR r.username != m.author_username) + AND m.state IN ('opened','merged') + AND fc.new_path {path_op} + AND m.updated_at >= ?2 + AND (?3 IS NULL OR fc.project_id = ?3) + AND r.username IN ({in_clause}) + ) + SELECT + username, + mr_ref, + title, + GROUP_CONCAT(DISTINCT role) AS roles, + SUM(note_count) AS total_notes, + MAX(last_activity) AS last_activity + FROM signals + GROUP BY username, mr_ref + ORDER BY username ASC, last_activity DESC + " + ); + + // prepare() not prepare_cached(): the IN clause varies by expert count, + // so the SQL shape changes per invocation and caching wastes memory. + let mut stmt = conn.prepare(&sql)?; + + // Build params: ?1=path, ?2=since_ms, ?3=project_id, ?4..=usernames + let mut params: Vec> = Vec::new(); + params.push(Box::new(pq.value.clone())); + params.push(Box::new(since_ms)); + params.push(Box::new(project_id)); + for expert in experts { + params.push(Box::new(expert.username.clone())); + } + let param_refs: Vec<&dyn rusqlite::types::ToSql> = params.iter().map(|p| p.as_ref()).collect(); + + let rows: Vec<(String, String, String, String, u32, i64)> = stmt + .query_map(param_refs.as_slice(), |row| { + Ok(( + row.get(0)?, + row.get(1)?, + row.get(2)?, + row.get::<_, String>(3)?, + row.get(4)?, + row.get(5)?, + )) + })? + .collect::, _>>()?; + + let mut map: HashMap> = HashMap::new(); + for (username, mr_ref, title, roles_csv, note_count, last_activity) in rows { + let has_author = roles_csv.contains("author"); + let has_reviewer = roles_csv.contains("reviewer"); + let role = match (has_author, has_reviewer) { + (true, true) => "A+R", + (true, false) => "A", + (false, true) => "R", + _ => "?", + } + .to_string(); + map.entry(username).or_default().push(ExpertMrDetail { + mr_ref, + title, + role, + note_count, + last_activity_ms: last_activity, + }); + } + + Ok(map) +} + // ─── Query: Workload Mode ─────────────────────────────────────────────────── fn query_workload( @@ -1322,7 +1659,6 @@ fn query_overlap( } // Convert accumulators to output structs - const MAX_MR_REFS_PER_USER: usize = 50; let mut users: Vec = user_map .into_values() .map(|a| { @@ -1358,7 +1694,11 @@ fn query_overlap( users.truncate(limit); Ok(OverlapResult { - path_query: path.to_string(), + path_query: if pq.is_prefix { + path.trim_end_matches('/').to_string() + } else { + pq.value.clone() + }, path_match: if pq.is_prefix { "prefix" } else { "exact" }.to_string(), users, truncated, @@ -1424,13 +1764,14 @@ fn print_expert_human(r: &ExpertResult, project_path: Option<&str>) { } println!( - " {:<16} {:>6} {:>12} {:>6} {:>12} {}", + " {:<16} {:>6} {:>12} {:>6} {:>12} {} {}", style("Username").bold(), style("Score").bold(), style("Reviewed(MRs)").bold(), style("Notes").bold(), style("Authored(MRs)").bold(), style("Last Seen").bold(), + style("MR Refs").bold(), ); for expert in &r.experts { @@ -1449,15 +1790,59 @@ fn print_expert_human(r: &ExpertResult, project_path: Option<&str>) { } else { "-".to_string() }; + let mr_str = expert + .mr_refs + .iter() + .take(5) + .cloned() + .collect::>() + .join(", "); + let overflow = if expert.mr_refs_total > 5 { + format!(" +{}", expert.mr_refs_total - 5) + } else { + String::new() + }; println!( - " {:<16} {:>6} {:>12} {:>6} {:>12} {}", + " {:<16} {:>6} {:>12} {:>6} {:>12} {:<12}{}{}", style(format!("@{}", expert.username)).cyan(), expert.score, reviews, notes, authored, - style(format_relative_time(expert.last_seen_ms)).dim(), + format_relative_time(expert.last_seen_ms), + if mr_str.is_empty() { + String::new() + } else { + format!(" {mr_str}") + }, + overflow, ); + + // Print detail sub-rows when populated + if let Some(details) = &expert.details { + const MAX_DETAIL_DISPLAY: usize = 10; + for d in details.iter().take(MAX_DETAIL_DISPLAY) { + let notes_str = if d.note_count > 0 { + format!("{} notes", d.note_count) + } else { + String::new() + }; + println!( + " {:<3} {:<30} {:>30} {:>10} {}", + style(&d.role).dim(), + d.mr_ref, + truncate_str(&format!("\"{}\"", d.title), 30), + notes_str, + style(format_relative_time(d.last_activity_ms)).dim(), + ); + } + if details.len() > MAX_DETAIL_DISPLAY { + println!( + " {}", + style(format!("+{} more", details.len() - MAX_DETAIL_DISPLAY)).dim() + ); + } + } } if r.truncated { println!( @@ -1791,6 +2176,7 @@ pub fn print_who_json(run: &WhoRun, args: &WhoArgs, elapsed_ms: u64) { "project": args.project, "since": args.since, "limit": args.limit, + "detail": args.detail, }); // Resolved/computed values -- what actually ran @@ -1844,14 +2230,29 @@ fn expert_to_json(r: &ExpertResult) -> serde_json::Value { "path_query": r.path_query, "path_match": r.path_match, "truncated": r.truncated, - "experts": r.experts.iter().map(|e| serde_json::json!({ - "username": e.username, - "score": e.score, - "review_mr_count": e.review_mr_count, - "review_note_count": e.review_note_count, - "author_mr_count": e.author_mr_count, - "last_seen_at": ms_to_iso(e.last_seen_ms), - })).collect::>(), + "experts": r.experts.iter().map(|e| { + let mut obj = serde_json::json!({ + "username": e.username, + "score": e.score, + "review_mr_count": e.review_mr_count, + "review_note_count": e.review_note_count, + "author_mr_count": e.author_mr_count, + "last_seen_at": ms_to_iso(e.last_seen_ms), + "mr_refs": e.mr_refs, + "mr_refs_total": e.mr_refs_total, + "mr_refs_truncated": e.mr_refs_truncated, + }); + if let Some(details) = &e.details { + obj["details"] = serde_json::json!(details.iter().map(|d| serde_json::json!({ + "mr_ref": d.mr_ref, + "title": d.title, + "role": d.role, + "note_count": d.note_count, + "last_activity_at": ms_to_iso(d.last_activity_ms), + })).collect::>()); + } + obj + }).collect::>(), }) } @@ -2012,6 +2413,10 @@ mod tests { conn } + fn default_scoring() -> ScoringConfig { + ScoringConfig::default() + } + fn insert_project(conn: &Connection, id: i64, path: &str) { conn.execute( "INSERT INTO projects (id, gitlab_project_id, path_with_namespace, web_url) @@ -2170,6 +2575,8 @@ mod tests { since: None, project: None, limit: 20, + detail: false, + no_detail: false, }) .unwrap(), WhoMode::Expert { .. } @@ -2186,6 +2593,8 @@ mod tests { since: None, project: None, limit: 20, + detail: false, + no_detail: false, }) .unwrap(), WhoMode::Workload { .. } @@ -2202,6 +2611,8 @@ mod tests { since: None, project: None, limit: 20, + detail: false, + no_detail: false, }) .unwrap(), WhoMode::Workload { .. } @@ -2218,6 +2629,8 @@ mod tests { since: None, project: None, limit: 20, + detail: false, + no_detail: false, }) .unwrap(), WhoMode::Reviews { .. } @@ -2234,6 +2647,8 @@ mod tests { since: None, project: None, limit: 20, + detail: false, + no_detail: false, }) .unwrap(), WhoMode::Expert { .. } @@ -2250,12 +2665,55 @@ mod tests { since: None, project: None, limit: 20, + detail: false, + no_detail: false, }) .unwrap(), WhoMode::Expert { .. } )); } + #[test] + fn test_detail_rejected_outside_expert_mode() { + let args = WhoArgs { + target: Some("asmith".to_string()), + path: None, + active: false, + overlap: None, + reviews: false, + since: None, + project: None, + limit: 20, + detail: true, + no_detail: false, + }; + let mode = resolve_mode(&args).unwrap(); + let err = validate_mode_flags(&mode, &args).unwrap_err(); + let msg = err.to_string(); + assert!( + msg.contains("--detail is only supported in expert mode"), + "unexpected error: {msg}" + ); + } + + #[test] + fn test_detail_allowed_in_expert_mode() { + let args = WhoArgs { + target: None, + path: Some("README.md".to_string()), + active: false, + overlap: None, + reviews: false, + since: None, + project: None, + limit: 20, + detail: true, + no_detail: false, + }; + let mode = resolve_mode(&args).unwrap(); + assert!(validate_mode_flags(&mode, &args).is_ok()); + } + #[test] fn test_build_path_query() { let conn = setup_test_db(); @@ -2374,9 +2832,10 @@ mod tests { "looks good", ); - let result = query_expert(&conn, "src/auth/", None, 0, 20).unwrap(); - assert_eq!(result.experts.len(), 3); // reviewer_b, reviewer_c, author_a - assert_eq!(result.experts[0].username, "reviewer_b"); // highest score + let result = + query_expert(&conn, "src/auth/", None, 0, 20, &default_scoring(), false).unwrap(); + assert_eq!(result.experts.len(), 3); // author_a, reviewer_b, reviewer_c + assert_eq!(result.experts[0].username, "author_a"); // highest score (authorship dominates) } #[test] @@ -2621,7 +3080,8 @@ mod tests { "looks good", ); - let result = query_expert(&conn, "src/auth/", None, 0, 20).unwrap(); + let result = + query_expert(&conn, "src/auth/", None, 0, 20, &default_scoring(), false).unwrap(); // author_a should appear as author only, not as reviewer let author = result .experts @@ -2701,12 +3161,14 @@ mod tests { } // limit = 2, should return truncated = true - let result = query_expert(&conn, "src/auth/", None, 0, 2).unwrap(); + let result = + query_expert(&conn, "src/auth/", None, 0, 2, &default_scoring(), false).unwrap(); assert!(result.truncated); assert_eq!(result.experts.len(), 2); // limit = 10, should return truncated = false - let result = query_expert(&conn, "src/auth/", None, 0, 10).unwrap(); + let result = + query_expert(&conn, "src/auth/", None, 0, 10, &default_scoring(), false).unwrap(); assert!(!result.truncated); } @@ -2718,7 +3180,16 @@ mod tests { insert_mr(&conn, 1, 1, 100, "file_author", "merged"); insert_file_change(&conn, 1, 1, "src/auth/login.rs", "modified"); - let result = query_expert(&conn, "src/auth/login.rs", None, 0, 20).unwrap(); + let result = query_expert( + &conn, + "src/auth/login.rs", + None, + 0, + 20, + &default_scoring(), + false, + ) + .unwrap(); assert_eq!(result.experts.len(), 1); assert_eq!(result.experts[0].username, "file_author"); assert!(result.experts[0].author_mr_count > 0); @@ -2735,7 +3206,16 @@ mod tests { insert_file_change(&conn, 1, 1, "src/auth/login.rs", "modified"); insert_reviewer(&conn, 1, "assigned_reviewer"); - let result = query_expert(&conn, "src/auth/login.rs", None, 0, 20).unwrap(); + let result = query_expert( + &conn, + "src/auth/login.rs", + None, + 0, + 20, + &default_scoring(), + false, + ) + .unwrap(); let reviewer = result .experts .iter() @@ -2765,7 +3245,16 @@ mod tests { insert_file_change(&conn, 1, 1, "src/auth/login.rs", "modified"); insert_reviewer(&conn, 1, "reviewer_b"); - let result = query_expert(&conn, "src/auth/login.rs", None, 0, 20).unwrap(); + let result = query_expert( + &conn, + "src/auth/login.rs", + None, + 0, + 20, + &default_scoring(), + false, + ) + .unwrap(); let reviewer = result .experts .iter() @@ -2789,7 +3278,8 @@ mod tests { insert_mr(&conn, 2, 1, 200, "author_a", "merged"); insert_file_change(&conn, 2, 1, "src/auth/session.rs", "added"); - let result = query_expert(&conn, "src/auth/", None, 0, 20).unwrap(); + let result = + query_expert(&conn, "src/auth/", None, 0, 20, &default_scoring(), false).unwrap(); let author = result .experts .iter() @@ -2808,7 +3298,8 @@ mod tests { insert_file_change(&conn, 1, 1, "src/auth/login.rs", "modified"); insert_file_change(&conn, 1, 1, "src/auth/session.rs", "added"); - let result = query_expert(&conn, "src/auth/", None, 0, 20).unwrap(); + let result = + query_expert(&conn, "src/auth/", None, 0, 20, &default_scoring(), false).unwrap(); assert_eq!(result.path_match, "prefix"); assert_eq!(result.experts.len(), 1); assert_eq!(result.experts[0].username, "author_a"); @@ -2861,7 +3352,16 @@ mod tests { // real_reviewer is also assigned insert_reviewer(&conn, 1, "real_reviewer"); - let result = query_expert(&conn, "src/auth/login.rs", None, 0, 20).unwrap(); + let result = query_expert( + &conn, + "src/auth/login.rs", + None, + 0, + 20, + &default_scoring(), + false, + ) + .unwrap(); // author_a should appear as author only, not reviewer let author = result @@ -2896,4 +3396,255 @@ mod tests { assert!(user.is_some()); assert_eq!(user.unwrap().review_touch_count, 0); } + + // ─── Suffix / Fuzzy Path Resolution Tests ─────────────────────────────── + + #[test] + fn test_build_path_query_suffix_resolves_bare_filename() { + // User types just "login.rs" but the DB has "src/auth/login.rs" + let conn = setup_test_db(); + insert_project(&conn, 1, "team/backend"); + insert_mr(&conn, 1, 1, 100, "author_a", "merged"); + insert_file_change(&conn, 1, 1, "src/auth/login.rs", "modified"); + + let pq = build_path_query(&conn, "login.rs", None).unwrap(); + assert_eq!(pq.value, "src/auth/login.rs"); + assert!(!pq.is_prefix); + } + + #[test] + fn test_build_path_query_suffix_resolves_partial_path() { + // User types "auth/login.rs" but full path is "src/auth/login.rs" + let conn = setup_test_db(); + insert_project(&conn, 1, "team/backend"); + insert_mr(&conn, 1, 1, 100, "author_a", "merged"); + insert_file_change(&conn, 1, 1, "src/auth/login.rs", "modified"); + + let pq = build_path_query(&conn, "auth/login.rs", None).unwrap(); + assert_eq!(pq.value, "src/auth/login.rs"); + assert!(!pq.is_prefix); + } + + #[test] + fn test_build_path_query_suffix_ambiguous_returns_error() { + // Two different files share the same filename -> Ambiguous error + let conn = setup_test_db(); + insert_project(&conn, 1, "team/backend"); + insert_mr(&conn, 1, 1, 100, "author_a", "merged"); + insert_file_change(&conn, 1, 1, "src/auth/utils.rs", "modified"); + insert_file_change(&conn, 1, 1, "src/db/utils.rs", "modified"); + + let err = build_path_query(&conn, "utils.rs", None).unwrap_err(); + let msg = err.to_string(); + assert!( + msg.contains("src/auth/utils.rs"), + "should list candidates: {msg}" + ); + assert!( + msg.contains("src/db/utils.rs"), + "should list candidates: {msg}" + ); + } + + #[test] + fn test_build_path_query_suffix_scoped_to_project() { + // Two projects have the same filename; scoping to one should resolve + let conn = setup_test_db(); + insert_project(&conn, 1, "team/backend"); + insert_project(&conn, 2, "team/frontend"); + insert_mr(&conn, 1, 1, 100, "author_a", "merged"); + insert_mr(&conn, 2, 2, 200, "author_b", "merged"); + insert_file_change(&conn, 1, 1, "src/utils.rs", "modified"); + insert_file_change(&conn, 2, 2, "lib/utils.rs", "modified"); + + // Unscoped -> ambiguous + assert!(build_path_query(&conn, "utils.rs", None).is_err()); + + // Scoped to project 1 -> resolves + let pq = build_path_query(&conn, "utils.rs", Some(1)).unwrap(); + assert_eq!(pq.value, "src/utils.rs"); + assert!(!pq.is_prefix); + } + + #[test] + fn test_build_path_query_suffix_deduplicates_across_sources() { + // Same path in both notes AND mr_file_changes -> single unique match, not ambiguous + let conn = setup_test_db(); + insert_project(&conn, 1, "team/backend"); + insert_mr(&conn, 1, 1, 100, "author_a", "merged"); + insert_file_change(&conn, 1, 1, "src/auth/login.rs", "modified"); + insert_discussion(&conn, 1, 1, Some(1), None, true, false); + insert_diffnote( + &conn, + 1, + 1, + 1, + "reviewer_a", + "src/auth/login.rs", + "review note", + ); + + let pq = build_path_query(&conn, "login.rs", None).unwrap(); + assert_eq!(pq.value, "src/auth/login.rs"); + assert!(!pq.is_prefix); + } + + #[test] + fn test_build_path_query_exact_match_still_preferred() { + // If the exact path exists in the DB, suffix should NOT be attempted + let conn = setup_test_db(); + insert_project(&conn, 1, "team/backend"); + insert_mr(&conn, 1, 1, 100, "author_a", "merged"); + insert_file_change(&conn, 1, 1, "README.md", "modified"); + insert_file_change(&conn, 1, 1, "docs/README.md", "modified"); + + // "README.md" exists as exact match -> use it directly, no ambiguity + let pq = build_path_query(&conn, "README.md", None).unwrap(); + assert_eq!(pq.value, "README.md"); + assert!(!pq.is_prefix); + } + + #[test] + fn test_expert_scoring_weights_are_configurable() { + // With reviewer-heavy weights, reviewer should rank above author. + // With author-heavy weights (default), author should rank above reviewer. + let conn = setup_test_db(); + insert_project(&conn, 1, "team/backend"); + insert_mr(&conn, 1, 1, 100, "the_author", "merged"); + insert_file_change(&conn, 1, 1, "src/app.rs", "modified"); + insert_discussion(&conn, 1, 1, Some(1), None, true, false); + insert_diffnote(&conn, 1, 1, 1, "the_reviewer", "src/app.rs", "lgtm"); + + // Default weights: author=25, reviewer=10 → author wins + let result = + query_expert(&conn, "src/app.rs", None, 0, 20, &default_scoring(), false).unwrap(); + assert_eq!(result.experts[0].username, "the_author"); + + // Custom weights: flip so reviewer dominates + let flipped = ScoringConfig { + author_weight: 5, + reviewer_weight: 30, + note_bonus: 1, + }; + let result = query_expert(&conn, "src/app.rs", None, 0, 20, &flipped, false).unwrap(); + assert_eq!(result.experts[0].username, "the_reviewer"); + } + + #[test] + fn test_expert_mr_refs() { + let conn = setup_test_db(); + insert_project(&conn, 1, "team/backend"); + insert_mr(&conn, 1, 1, 891, "author_a", "merged"); + insert_mr(&conn, 2, 1, 847, "author_a", "merged"); + insert_discussion(&conn, 1, 1, Some(1), None, true, false); + insert_discussion(&conn, 2, 1, Some(2), None, true, false); + insert_diffnote(&conn, 1, 1, 1, "reviewer_b", "src/auth/login.rs", "note1"); + insert_diffnote(&conn, 2, 2, 1, "reviewer_b", "src/auth/login.rs", "note2"); + + let result = + query_expert(&conn, "src/auth/", None, 0, 20, &default_scoring(), false).unwrap(); + + // reviewer_b should have MR refs + let reviewer = result + .experts + .iter() + .find(|e| e.username == "reviewer_b") + .unwrap(); + assert!(reviewer.mr_refs.contains(&"team/backend!847".to_string())); + assert!(reviewer.mr_refs.contains(&"team/backend!891".to_string())); + assert_eq!(reviewer.mr_refs_total, 2); + assert!(!reviewer.mr_refs_truncated); + + // author_a should also have MR refs + let author = result + .experts + .iter() + .find(|e| e.username == "author_a") + .unwrap(); + assert!(author.mr_refs.contains(&"team/backend!847".to_string())); + assert!(author.mr_refs.contains(&"team/backend!891".to_string())); + assert_eq!(author.mr_refs_total, 2); + } + + #[test] + fn test_expert_mr_refs_multi_project() { + let conn = setup_test_db(); + insert_project(&conn, 1, "team/backend"); + insert_project(&conn, 2, "team/frontend"); + insert_mr(&conn, 1, 1, 100, "author_a", "opened"); + insert_mr(&conn, 2, 2, 100, "author_a", "opened"); // Same iid, different project + insert_discussion(&conn, 1, 1, Some(1), None, true, false); + insert_discussion(&conn, 2, 2, Some(2), None, true, false); + insert_diffnote(&conn, 1, 1, 1, "reviewer_x", "src/auth/login.rs", "review"); + insert_diffnote(&conn, 2, 2, 2, "reviewer_x", "src/auth/login.rs", "review"); + + let result = + query_expert(&conn, "src/auth/", None, 0, 20, &default_scoring(), false).unwrap(); + let reviewer = result + .experts + .iter() + .find(|e| e.username == "reviewer_x") + .unwrap(); + // Should have two distinct refs despite same iid + assert!(reviewer.mr_refs.contains(&"team/backend!100".to_string())); + assert!(reviewer.mr_refs.contains(&"team/frontend!100".to_string())); + assert_eq!(reviewer.mr_refs_total, 2); + } + + #[test] + fn test_expert_detail_mode() { + let conn = setup_test_db(); + insert_project(&conn, 1, "team/backend"); + insert_mr(&conn, 1, 1, 891, "author_a", "merged"); + insert_mr(&conn, 2, 1, 902, "author_a", "merged"); + insert_discussion(&conn, 1, 1, Some(1), None, true, false); + insert_discussion(&conn, 2, 1, Some(2), None, true, false); + insert_diffnote(&conn, 1, 1, 1, "reviewer_b", "src/auth/login.rs", "note1"); + insert_diffnote(&conn, 2, 1, 1, "reviewer_b", "src/auth/login.rs", "note2"); + insert_diffnote(&conn, 3, 2, 1, "reviewer_b", "src/auth/session.rs", "note3"); + + // Without detail: details should be None + let result = + query_expert(&conn, "src/auth/", None, 0, 20, &default_scoring(), false).unwrap(); + for expert in &result.experts { + assert!(expert.details.is_none()); + } + + // With detail: details should be populated + let result = + query_expert(&conn, "src/auth/", None, 0, 20, &default_scoring(), true).unwrap(); + let reviewer = result + .experts + .iter() + .find(|e| e.username == "reviewer_b") + .unwrap(); + let details = reviewer.details.as_ref().unwrap(); + assert!(!details.is_empty()); + + // All detail entries should have role "R" for reviewer + for d in details { + assert!( + d.role == "R" || d.role == "A+R", + "role should be R or A+R, got {}", + d.role + ); + assert!(d.mr_ref.starts_with("team/backend!")); + } + + // author_a should have detail entries with role "A" + let author = result + .experts + .iter() + .find(|e| e.username == "author_a") + .unwrap(); + let author_details = author.details.as_ref().unwrap(); + assert!(!author_details.is_empty()); + for d in author_details { + assert!( + d.role == "A" || d.role == "A+R", + "role should be A or A+R, got {}", + d.role + ); + } + } } diff --git a/src/cli/mod.rs b/src/cli/mod.rs index c395c58..ed04919 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -751,6 +751,13 @@ pub struct WhoArgs { help_heading = "Output" )] pub limit: u16, + + /// Show per-MR detail breakdown (expert mode only) + #[arg(long, help_heading = "Output", overrides_with = "no_detail")] + pub detail: bool, + + #[arg(long = "no-detail", hide = true, overrides_with = "detail")] + pub no_detail: bool, } #[derive(Parser)] diff --git a/src/core/config.rs b/src/core/config.rs index b1b323b..153fcf3 100644 --- a/src/core/config.rs +++ b/src/core/config.rs @@ -146,6 +146,32 @@ impl Default for LoggingConfig { } } +#[derive(Debug, Clone, Deserialize)] +#[serde(default)] +pub struct ScoringConfig { + /// Points per MR where the user authored code touching the path. + #[serde(rename = "authorWeight")] + pub author_weight: i64, + + /// Points per MR where the user reviewed code touching the path. + #[serde(rename = "reviewerWeight")] + pub reviewer_weight: i64, + + /// Bonus points per individual inline review comment (DiffNote). + #[serde(rename = "noteBonus")] + pub note_bonus: i64, +} + +impl Default for ScoringConfig { + fn default() -> Self { + Self { + author_weight: 25, + reviewer_weight: 10, + note_bonus: 1, + } + } +} + #[derive(Debug, Clone, Deserialize)] pub struct Config { pub gitlab: GitLabConfig, @@ -162,6 +188,9 @@ pub struct Config { #[serde(default)] pub logging: LoggingConfig, + + #[serde(default)] + pub scoring: ScoringConfig, } impl Config { @@ -207,10 +236,31 @@ impl Config { }); } + validate_scoring(&config.scoring)?; + Ok(config) } } +fn validate_scoring(scoring: &ScoringConfig) -> Result<()> { + if scoring.author_weight < 0 { + return Err(LoreError::ConfigInvalid { + details: "scoring.authorWeight must be >= 0".to_string(), + }); + } + if scoring.reviewer_weight < 0 { + return Err(LoreError::ConfigInvalid { + details: "scoring.reviewerWeight must be >= 0".to_string(), + }); + } + if scoring.note_bonus < 0 { + return Err(LoreError::ConfigInvalid { + details: "scoring.noteBonus must be >= 0".to_string(), + }); + } + Ok(()) +} + #[derive(Debug, serde::Serialize)] pub struct MinimalConfig { pub gitlab: MinimalGitLabConfig, @@ -236,3 +286,81 @@ impl serde::Serialize for ProjectConfig { state.end() } } + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::TempDir; + + fn write_config(dir: &TempDir, scoring_json: &str) -> std::path::PathBuf { + let path = dir.path().join("config.json"); + let config = format!( + r#"{{ + "gitlab": {{ + "baseUrl": "https://gitlab.example.com", + "tokenEnvVar": "GITLAB_TOKEN" + }}, + "projects": [ + {{ "path": "group/project" }} + ], + "scoring": {scoring_json} +}}"# + ); + fs::write(&path, config).unwrap(); + path + } + + #[test] + fn test_load_rejects_negative_author_weight() { + let dir = TempDir::new().unwrap(); + let path = write_config( + &dir, + r#"{ + "authorWeight": -1, + "reviewerWeight": 10, + "noteBonus": 1 +}"#, + ); + let err = Config::load_from_path(&path).unwrap_err(); + let msg = err.to_string(); + assert!( + msg.contains("scoring.authorWeight"), + "unexpected error: {msg}" + ); + } + + #[test] + fn test_load_rejects_negative_reviewer_weight() { + let dir = TempDir::new().unwrap(); + let path = write_config( + &dir, + r#"{ + "authorWeight": 25, + "reviewerWeight": -1, + "noteBonus": 1 +}"#, + ); + let err = Config::load_from_path(&path).unwrap_err(); + let msg = err.to_string(); + assert!( + msg.contains("scoring.reviewerWeight"), + "unexpected error: {msg}" + ); + } + + #[test] + fn test_load_rejects_negative_note_bonus() { + let dir = TempDir::new().unwrap(); + let path = write_config( + &dir, + r#"{ + "authorWeight": 25, + "reviewerWeight": 10, + "noteBonus": -1 +}"#, + ); + let err = Config::load_from_path(&path).unwrap_err(); + let msg = err.to_string(); + assert!(msg.contains("scoring.noteBonus"), "unexpected error: {msg}"); + } +}