**Best Revisions To Strengthen The Plan** 1. **[Critical] Replace one-hop rename matching with canonical path identities** Analysis and rationale: Current `old_path OR new_path` fixes direct renames, but it still breaks on rename chains (`a.rs -> b.rs -> c.rs`) and split/move patterns. A canonical `path_identity` graph built from `mr_file_changes(old_path,new_path)` gives stable identity over time, which is the right architectural boundary for expertise history. ```diff @@ ## Context -- Match both old and new paths in all signal queries AND path resolution probes so expertise survives file renames +- Build canonical path identities from rename edges and score by identity, not raw path strings, so expertise survives multi-hop renames and moves @@ ## Files to Modify -2. **`src/cli/commands/who.rs`** — Core changes: +2. **`src/cli/commands/who.rs`** — Core changes: ... - - Match both `new_path` and `old_path` in all signal queries (rename awareness) + - Resolve queried paths to `path_identity_id` and match all aliases in that identity set +4. **`src/core/path_identity.rs`** — New module: + - Build/maintain rename graph from `mr_file_changes` + - Resolve path -> identity + aliases for probes/scoring ``` 2. **[Critical] Shift scoring input from runtime CTE joins to a normalized `expertise_events` table** Analysis and rationale: Your SQL is correct but complex and expensive at query time. Precomputing normalized events at ingestion gives simpler, faster, and more reliable scoring queries; it also enables model versioning/backfills without touching raw MR/note tables each request. ```diff @@ ## Files to Modify -3. **`src/core/db.rs`** — Add migration for indexes supporting the new query shapes +3. **`src/core/db.rs`** — Add migrations for: + - `expertise_events` table (normalized scoring events) + - supporting indexes +4. **`src/core/ingest/expertise_events.rs`** — New: + - Incremental upsert of events during sync/ingest @@ ## SQL Restructure (who.rs) -The SQL uses CTE-based dual-path matching and hybrid aggregation... +Runtime SQL reads precomputed `expertise_events` filtered by path identity + time window. +Heavy joins/aggregation move to ingest-time normalization. ``` 3. **[High] Upgrade reviewer engagement model beyond char-count threshold** Analysis and rationale: `min_note_chars` is a useful guardrail but brittle (easy to game, penalizes concise high-quality comments). Add explicit review-state signals (`approved`, `changes_requested`) and trivial-comment pattern filtering to better capture real reviewer expertise. ```diff @@ ## Scoring Formula -| **Reviewer Participated** (left DiffNote on MR/path) | 10 | 90 days | +| **Reviewer Participated** (substantive DiffNote and/or formal review action) | 10 | 90 days | +| **Review Decision: changes_requested** | 6 | 120 days | +| **Review Decision: approved** | 4 | 75 days | @@ ### 1. ScoringConfig (config.rs) pub reviewer_min_note_chars: u32, + pub reviewer_trivial_note_patterns: Vec, // default: ["lgtm","+1","nit","ship it","👍"] + pub review_approved_weight: i64, // default: 4 + pub review_changes_requested_weight: i64, // default: 6 ``` 4. **[High] Make temporal semantics explicit and deterministic** Analysis and rationale: `--as-of` is good, but day parsing and boundary semantics can still cause subtle reproducibility issues. Define window as `[since_ms, as_of_ms)` and parse `YYYY-MM-DD` as end-of-day UTC (or explicit timezone) so user expectations match outputs. ```diff @@ ### 5a. Reproducible Scoring via `--as-of` -- All event selection is bounded by `[since_ms, as_of_ms]` +- All event selection is bounded by `[since_ms, as_of_ms)` (exclusive upper bound) +- `YYYY-MM-DD` is interpreted as `23:59:59.999Z` unless `--timezone` is provided +- Robot output includes `window_start_iso`, `window_end_iso`, `window_end_exclusive: true` ``` 5. **[High] Replace fixed default `--since 24m` with contribution-floor auto cutoff** Analysis and rationale: A static window is simple but often over-scans data. Compute a model-derived horizon from a minimum contribution floor (for example `0.01` points) per signal; this keeps results equivalent while reducing query cost. ```diff @@ ### 5. Default --since Change -Expert mode: `"6m"` -> `"24m"` +Expert mode default: `--since auto` +`auto` computes earliest relevant timestamp from configured weights/half-lives and `min_contribution_floor` +Add config: `min_contribution_floor` (default: 0.01) +`--since` still overrides, `--all-history` still bypasses cutoff ``` 6. **[High] Add bot/service-account filtering now (not later)** Analysis and rationale: Bot activity can materially distort expertise rankings in real repos. This is low implementation cost with high quality gain and should be in v1 of the scoring revamp, not deferred. ```diff @@ ### 1. ScoringConfig (config.rs) + pub excluded_username_patterns: Vec, // default: ["bot","\\[bot\\]","service-account","ci-"] @@ ### 2. SQL Restructure (who.rs) +Apply username exclusion in all signal sources unless `--include-bots` is set @@ ### 5b. Score Explainability via `--explain-score` +Add `filtered_events` counts in robot output metadata ``` 7. **[Medium] Enforce deterministic floating-point accumulation** Analysis and rationale: Even with small sets, unordered `HashMap` iteration can cause tiny platform-dependent ranking differences near ties. Sorting contributions and using Neumaier summation removes nondeterminism and stabilizes tests/CI. ```diff @@ ### 4. Rust-Side Aggregation (who.rs) -Compute score as `f64`. +Compute score as `f64` using deterministic contribution ordering: +1) sort by (username, signal, mr_id, ts) +2) sum with Neumaier compensation +Tie-break remains `(raw_score DESC, last_seen DESC, username ASC)` ``` 8. **[Medium] Strengthen explainability with evidence, not just totals** Analysis and rationale: Component totals help, but disputes usually need “why this user got this score now.” Add compact top evidence rows per component (`mr_id`, `ts`, `raw_contribution`) behind an optional mode. ```diff @@ ### 5b. Score Explainability via `--explain-score` -Component breakdown only (4 floats per user). +Add `--explain-score=summary|full`: +`summary`: current 4-component totals +`full`: adds top N evidence rows per component (default N=3) +Robot output includes per-evidence `mr_id`, `signal`, `ts`, `contribution` ``` 9. **[Medium] Make query plan strategy explicit: `UNION ALL` default for dual-path scans** Analysis and rationale: You currently treat `UNION ALL` as fallback if planner regresses. For SQLite, OR-across-indexed-columns regressions are common enough that defaulting to branch-split queries is often more predictable. ```diff @@ **Index optimization fallback (UNION ALL split)** -Start with the simpler `OR` approach and only switch to `UNION ALL` if query plans confirm degradation. +Use `UNION ALL` + dedup as default for dual-path matching. +Keep `OR` variant as optional strategy flag for benchmarking/regression checks. ``` 10. **[Medium] Add explicit performance SLO + benchmark gate** Analysis and rationale: This plan is query-heavy and ranking-critical; add measurable performance budgets so future edits do not silently degrade UX. Include synthetic fixture benchmarks for exact, prefix, and suffix path modes. ```diff @@ ## Verification +8. Performance regression gate: + - `cargo bench --bench who_expert_scoring` + - Dataset tiers: 100k, 1M, 5M notes + - SLOs: p95 exact path < 150ms, prefix < 250ms, suffix < 400ms on reference hardware + - Fail CI if regression > 20% vs stored baseline ``` If you want, I can produce a single consolidated “iteration 5” plan document with these changes already merged into your current structure.