tidaldb/.sdlc/features/p1-briefing-ux-reason-labels/review.md

Code Review: Briefing UX & Reason Labels

Summary

This feature adds a reason label system to tidalDB's query response types. Every ranked result now carries one or more ReasonLabel values explaining why it was surfaced. The implementation spans 5 modified source files, 1 new integration test file, and touches both the RETRIEVE and SEARCH pipelines.

Files Changed

File	Change
tidal/src/ranking/reason.rs	New module: ReasonCode enum (19 variants), ReasonLabel struct, select_top_reasons(), reason_for_sort(), reason_for_strategy() helpers, 7 unit tests
tidal/src/ranking/executor/mod.rs	compute_raw_score() returns (f64, Vec<(String, f64)>) tuple; score_inner and score_personalized populate signal_snapshot from boost values
tidal/src/query/executor/mod.rs	Stage 3 reason tagging block after scoring: tags strategy, sort-mode, cohort, and exploration reasons
tidal/src/query/search/executor/pipeline.rs	Search pipeline reason tagging: tags BM25 (TextRelevance), ANN (SemanticMatch), sort-mode, and scope reasons
tidal/tests/p1_reason_labels.rs	10 integration tests covering trending, hidden_gems, hot, controversial, following, weight ordering, cap at 3, signal snapshot, text search, top_week

Correctness

PASS. The implementation correctly:

1. Populates signal_snapshot during compute_raw_score() by collecting boost signal values > 0.0. This fixes the previously empty signal_snapshot field.
2. Tags reasons from two independent sources: sort mode (reason_for_sort) and candidate strategy (reason_for_strategy). Both are exhaustive match blocks with no missing arms.
3. Applies dominance threshold (0.10) and max cap (3) via select_top_reasons() before writing to RetrieveResult and SearchResultItem.
4. Tags exploration-injected candidates with ExplorationBudget using the correct heuristic (score == 0.0 and empty reasons).
5. Handles search-specific reasons: BM25 results get TextRelevance with weight proportional to BM25 score (clamped to [0.1, 1.0]), ANN results get SemanticMatch with fixed weight 0.6.
6. Tags scope-derived reasons in the search pipeline (WithinScope::Trending -> TrendingGlobal, etc.).

Architecture

PASS. The design follows the existing pipeline architecture:

• Reasons are populated during scoring, not post-hoc -- matching the spec's requirement.
• The ReasonCode enum is closed and serde-serializable with snake_case naming.
• Helper functions reason_for_sort() and reason_for_strategy() are pure mapping functions with no side effects.
• The select_top_reasons() function is a clean filter-sort-take pipeline.
• Signal snapshot collection is integrated into the existing boost computation loop with zero additional ledger reads.

Performance

PASS. Overhead is minimal:

• compute_raw_score collects snapshot values in the existing boost loop -- no additional signal reads.
• Reason tagging in Stage 3 is O(n) over scored candidates with constant-time reason construction.
• select_top_reasons() sorts a vec of at most ~5 elements per candidate.
• No heap allocations on the hot path beyond the Vec pushes (which are pre-allocated via with_capacity).

Test Coverage

PASS. Comprehensive coverage across unit and integration tests:

• 7 unit tests in ranking/reason.rs: serde roundtrip for all 19 codes, label construction, threshold filtering, cap enforcement, empty input, all-below-threshold.
• 10 integration tests in p1_reason_labels.rs: trending (TrendingGlobal), hidden_gems (HiddenGem), hot (TrendingGlobal), controversial (Controversial), following (FollowedCreator), weight ordering, max 3 cap, signal snapshot population, text search (TextRelevance), top_week (TopInWindow).
• 1352 lib tests pass with no regressions.
• Existing test suites (m3-m8 UAT, session durability, etc.) unaffected.

Issues Found

None. The implementation is clean, well-documented, and matches the spec.

Minor Observations (non-blocking)

1. ReasonLabel does not implement Serialize/Deserialize: The ReasonCode enum has serde derives, but ReasonLabel itself does not. This is fine for the engine layer (the server serializes RetrieveResult which contains the label data), but adding serde derives to ReasonLabel would make it directly serializable for debugging/logging.
2. Reason weight normalization: Reason weights are absolute values (e.g., 1.0 for sort, 0.9 for strategy, 0.8 for cohort). They are not normalized relative to the item's total score. This means weights represent "confidence of attribution" rather than "fraction of score explained." The spec does not require normalization, so this is consistent.

Verdict

APPROVE -- The implementation is complete, correct, well-tested, and matches the spec. All 7 SDLC tasks are done with full test coverage.