gitlore/plans/time-decay-expert-scoring.feedback-5.md

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.

@@ ## 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.

@@ ## 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.

@@ ## 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<String>, // 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.

@@ ### 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.

@@ ### 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.

@@ ### 1. ScoringConfig (config.rs)
+    pub excluded_username_patterns: Vec<String>, // 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.

@@ ### 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.

@@ ### 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.

@@ **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.

@@ ## 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.