5.2 KiB
5.2 KiB
Code Review: Feedback Loop UX
Files Changed
| File | Change | Lines |
|---|---|---|
tidal/src/db/feedback.rs |
NEW: FeedbackAction, FeedbackState, submit_feedback, feedback_state, try_negative_preference_update, 21 unit tests | 594 |
tidal/src/db/mod.rs |
Added pub mod feedback; declaration, skip_counter field on TidalDb struct, skip_counter init in from_config() |
+3 |
tidal/src/db/from_parts.rs |
Added skip_counter: dashmap::DashMap::new() in from_parts() |
+1 |
tidal/src/entities/user_state.rs |
Added remove_like() and remove_save() methods |
+12 |
tidal/src/lib.rs |
Added pub use db::feedback::{FeedbackAction, FeedbackState}; |
+1 |
tidal/tests/p1_feedback_loop.rs |
NEW: 10 integration tests | 246 |
tidal/src/schema/validation/policies.rs |
Added Default impl for AgentPolicy (pre-existing fix) |
+9 |
tidal/src/session/state.rs |
Added overrides_rejected, default_profile fields (pre-existing fix) |
+4 |
tidal/src/session/serde/start_record.rs |
Added missing field inits (pre-existing fix) | +2 |
tidal/src/db/sessions.rs |
Added missing field inits (pre-existing fix) | +2 |
tidal/src/db/session_restore.rs |
Added missing field inits (pre-existing fix) | +2 |
Review Checklist
Correctness
- All 11 FeedbackAction variants dispatch correctly. Each variant in the match arm performs the exact side effects specified in the design dispatch table.
- Signal-writing actions use the correct signal type names. Like -> "like", Hide -> "hide", Dislike -> "dislike", MuteCreator -> "block", NotInterested -> "skip".
- Undo actions reverse the correct state. Unlike removes from liked bitmap. Unhide removes from hard-neg AND hidden-items. UndoDislike removes from hard-neg. UnmuteCreator removes from blocked-creators. Unsave removes from saved bitmap.
- Unhide writes a durable signal (weight=-1.0) for audit trail and replication, as specified.
- Skip escalation threshold is correct.
SKIP_ESCALATION_THRESHOLD = 3. Usessaturating_addto prevent u8 overflow. - Negative preference update uses correct damping. Dislike: 0.3x, NotInterested: 0.1x. Base damping 0.1 matches
PreferenceVectors::DAMPING. - feedback_state distinguishes Hide from Dislike. Hide = in hidden_items bitmap. Dislike = in hard_neg but NOT in hidden_items.
require_writeableguard prevents writes on follower nodes.
API Design
- FeedbackAction derives Copy. All variants are data-free or contain
u64, making the enum trivially copyable. This avoids unnecessary cloning on the call site. - FeedbackState derives Default. All-false default is the correct semantic for "no feedback recorded."
- Public API surface is minimal. Only
submit_feedback,feedback_state,FeedbackAction, andFeedbackStateare public.try_negative_preference_updateis private. - Method signatures match the spec.
submit_feedback(user_id: u64, entity_id: EntityId, action: FeedbackAction, timestamp: Timestamp) -> Result<()>.
Safety
#[allow(clippy::cast_possible_truncation)]onentity_id.as_u64() as u32. This is an existing pattern throughout the codebase (EntityId is u64 but bitmap indexes use u32).#[allow(clippy::struct_excessive_bools)]on FeedbackState. Justified: 5 independent boolean UI toggle states are the natural representation.- No panics in production paths. All fallible operations return
Resultor uselet...elsefor early returns.try_negative_preference_updatesilently returns on missing storage/embedding (graceful degradation). - DashMap concurrency is safe. All accesses use the DashMap API correctly (entry/get_mut/contains patterns). No manual locking.
Performance
- No heap allocation on undo paths. Unlike, UndoDislike, UnmuteCreator, Unsave only perform DashMap removals.
- Skip counter uses u8 (not u64) to minimize memory per entry.
- feedback_state is read-only with no I/O (4 DashMap reads, ~400ns total).
- No blocking I/O in any path except the WAL write in signal-writing actions (existing cost).
Test Coverage
- 21 unit tests cover all action variants, undo paths, skip escalation, edge cases (undo on untouched items, coexisting states).
- 10 integration tests verify roundtrip behavior, multi-user independence, preference vector updates, combined actions.
- Full lib suite passes (1345 tests, 0 failures).
Style
- Follows existing codebase patterns. Module structure matches
db/signals.rs,db/sessions.rs. Naming matches existing conventions. - Doc comments on all public items.
submit_feedback,feedback_state,FeedbackAction,FeedbackStateall have rustdoc with examples and error conditions. - No dead code. All types and methods are exercised by tests.
- cargo fmt clean. No formatting issues.
- cargo clippy clean for all feedback module files (0 warnings).
Issues Found
None. The implementation matches the spec and design exactly. All acceptance criteria are met.
Verdict
APPROVED. Clean implementation with comprehensive test coverage. Ready for QA verification.