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

The plan is strong, but I’d revise it in 10 places to improve correctness, scalability, and operator trust.

1. Add rename/old-path awareness (correctness gap) Analysis: right now both existing code and your plan still center on position_new_path / new_path matches (src/cli/commands/who.rs:643, src/cli/commands/who.rs:681). That misses expertise on renamed/deleted paths and under-ranks long-time owners after refactors.

@@ ## Context
-This produces two compounding problems:
+This produces three compounding problems:
@@
 2. **Reviewer inflation**: ...
+3. **Path-history blindness**: Renamed/moved files lose historical expertise because matching relies on current-path fields only.

@@ ### 3. SQL Restructure (who.rs)
-AND n.position_new_path {path_op}
+AND (n.position_new_path {path_op} OR n.position_old_path {path_op})

-AND fc.new_path {path_op}
+AND (fc.new_path {path_op} OR fc.old_path {path_op})

2. Follow rename chains for queried paths Analysis: matching old_path helps, but true continuity needs alias expansion (A→B→C). Without this, expertise before multi-hop renames is fragmented.

@@ ### 3. SQL Restructure (who.rs)
+**Path alias expansion**: Before scoring, resolve a bounded rename alias set (default max depth: 20)
+from `mr_file_changes(change_type='renamed')`. Query signals against all aliases.
+Output includes `path_aliases_used` for transparency.

3. Use hybrid SQL pre-aggregation instead of fully raw rows Analysis: the “raw row” design is simpler but will degrade on large repos with heavy DiffNote volume. Pre-aggregating to (user, mr) for MR signals and (user, mr, note_count) for note signals keeps memory/latency predictable.

@@ ### 3. SQL Restructure (who.rs)
-The SQL CTE ... removes the outer GROUP BY aggregation. Instead, it returns raw signal rows:
-SELECT username, signal, mr_id, note_id, ts FROM signals
+Use hybrid aggregation:
+- SQL returns MR-level rows for author/reviewer signals (1 row per user+MR+signal_class)
+- SQL returns note groups (1 row per user+MR with note_count, max_ts)
+- Rust applies decay + ln(1+count) + final ranking.

4. Make timestamp policy state-aware (merged vs opened) Analysis: replacing updated_at with only COALESCE(merged_at, created_at) over-decays long-running open MRs. Open MRs need recency from active lifecycle; merged MRs should anchor to merge time.

@@ ### 3. SQL Restructure (who.rs)
-Replace m.updated_at with COALESCE(m.merged_at, m.created_at)
+Use state-aware timestamp:
+activity_ts =
+  CASE
+    WHEN m.state = 'merged' THEN COALESCE(m.merged_at, m.updated_at, m.created_at, m.last_seen_at)
+    WHEN m.state = 'opened' THEN COALESCE(m.updated_at, m.created_at, m.last_seen_at)
+  END

5. Replace fixed 24m with config-driven max age Analysis: 24m is reasonable now, but brittle after tuning weights/half-lives. Tie cutoff to config so model behavior remains coherent as parameters evolve.

@@ ### 1. ScoringConfig (config.rs)
+pub max_age_days: u32,                  // default: 730 (or 1095)

@@ ### 5. Default --since Change
-Expert mode: "6m" -> "24m"
+Expert mode default window derives from `scoring.max_age_days`
+unless user passes `--since` or `--all-history`.

6. Add reproducible scoring time via --as-of Analysis: decayed ranking is time-sensitive; debugging and tests become flaky without a fixed reference clock. This improves reliability and incident triage.

@@ ## Files to Modify
-2. src/cli/commands/who.rs
+2. src/cli/commands/who.rs
+3. src/cli/mod.rs
+4. src/main.rs

@@ ### 5. Default --since Change
+Add `--as-of <RFC3339|YYYY-MM-DD>` to score at a fixed timestamp.
+`resolved_input` includes `as_of_ms` and `as_of_iso`.

7. Add explainability output (--explain-score) Analysis: decayed multi-signal ranking will be disputed without decomposition. Show components and top evidence MRs to make results actionable and debuggable.

@@ ## Rejected Ideas (with rationale)
-- **`--explain-score` flag with component breakdown**: ... deferred
+**Included in this iteration**: `--explain-score` adds per-user score components
+(`author`, `review_participated`, `review_assigned`, `notes`) plus top evidence MRs.

8. Add confidence/coverage metadata Analysis: rankings can look precise while data is incomplete (mr_file_changes gaps, sparse DiffNotes). Confidence fields prevent false certainty.

@@ ### 4. Rust-Side Aggregation (who.rs)
+Compute and emit:
+- `coverage`: {mrs_considered, mrs_with_file_changes, mrs_with_diffnotes, percent}
+- `confidence`: high|medium|low (threshold-based)

9. Add index migration for new query shapes Analysis: your new EXISTS/NOT EXISTS reviewer split and path dual-matching will need better indexes; current who indexes are not enough for author+path+time combinations.

@@ ## Files to Modify
+3. **`migrations/021_who_decay_indexes.sql`** — indexes for decay query patterns:
+   - notes(diffnote path + author + created_at + discussion_id) partial
+   - notes(old_path variant) partial
+   - mr_file_changes(project_id, new_path, merge_request_id)
+   - mr_file_changes(project_id, old_path, merge_request_id) partial
+   - merge_requests(state, merged_at, updated_at, created_at)

10. Expand tests to invariants and determinism Analysis: example-based tests are good, but ranking systems need invariant tests to avoid subtle regressions.

@@ ### 7. New Tests (TDD)
+**`test_score_monotonicity_by_age`**: same signal, older timestamp never scores higher
+**`test_row_order_independence`**: shuffled SQL row order yields identical ranking
+**`test_as_of_reproducibility`**: same data + same `--as-of` => identical output
+**`test_rename_alias_chain_scoring`**: expertise carries across A->B->C rename chain
+**`test_overlap_participated_vs_assigned_counts`**: overlap reflects split reviewer semantics

If you want, I can produce a full consolidated v2 plan doc patch (single unified diff against plans/time-decay-expert-scoring.md) rather than per-change snippets.