insights-correlations (#22)

Reviewed-on: #22

crates/application/src/deps.rs

@@ -13,6 +13,7 @@ use crate::import::deps::{
     DeleteImportProfileDeps, ExecuteImportDeps, GetMappingStageDeps, GetPreviewStageDeps,
     GetSessionStateDeps, ListImportProfilesDeps, SaveProfileDeps,
 };
+use crate::insights::deps::GetInsightsDeps;
 use crate::integrations::deps::{
     ConfirmWatchEventsDeps, DismissWatchEventsDeps, GenerateWebhookTokenDeps, GetWatchQueueDeps,
     GetWebhookTokensDeps, IngestWatchEventDeps, RevokeWebhookTokenDeps,
@@ -92,6 +93,10 @@ pub struct IntegrationsGroup {
     pub plex_parser: Arc<dyn MediaServerParser>,
 }
 
+pub struct InsightsGroup {
+    pub get: GetInsightsDeps,
+}
+
 pub struct MoviesGroup {
     pub sync_poster: SyncPosterDeps,
     pub get_movie_profile: GetMovieProfileDeps,
@@ -159,6 +164,7 @@ pub struct Deps {
     pub diary: DiaryGroup,
     pub goals: GoalsGroup,
     pub import: ImportGroup,
+    pub insights: InsightsGroup,
     pub integrations: IntegrationsGroup,
     pub movies: MoviesGroup,
     pub person: PersonGroup,

crates/application/src/diary/get_user_feed.rs

@@ -7,9 +7,6 @@ use crate::diary::deps::{GetDiaryDeps, GetUserFeedDeps};
 use crate::diary::get_diary;
 use crate::diary::queries::GetDiaryQuery;
 
-/// The RSS feed's author line — derived the same way the deleted handler code
-/// derived its page title: from the local part of the user's email, not their
-/// username.
 pub struct FeedAuthor {
     pub display_name: String,
 }

crates/application/src/diary/tests/review_logger.rs

@@ -242,8 +242,6 @@ async fn watchlist_not_present_does_not_publish_removed() {
     );
 }
 
-/// A metadata client that returns a movie with an external_metadata_id,
-/// triggering the MovieDiscovered event path.
 struct MetadataClientWithExternalId;
 
 #[async_trait]

crates/application/src/import/apply_profile.rs

@@ -5,8 +5,6 @@ use domain::{
 
 use super::{commands::ApplyImportProfileCommand, deps::ApplyProfileDeps};
 
-/// Copies the profile's field_mappings onto the session. Caller must then invoke
-/// apply_import_mapping to regenerate row_results with the new mappings.
 pub async fn execute(
     deps: &ApplyProfileDeps,
     cmd: ApplyImportProfileCommand,

crates/application/src/import/deps.rs

@@ -50,11 +50,6 @@ pub struct ListImportProfilesDeps {
     pub import_profile: Arc<dyn ImportProfileRepository>,
 }
 
-/// Backs `apply_profile_and_map`, which internally drives `apply_profile::execute`
-/// then `apply_mapping::execute` — these fields are exactly the union of
-/// `ApplyProfileDeps` and `ApplyMappingDeps`'s fields, cloned once here and used to
-/// build each nested deps struct inline at the call site (see that file's doc
-/// comment for why: no use-case signature changes, per this task's constraints).
 pub struct ApplyProfileAndMapDeps {
     pub import_profile: Arc<dyn ImportProfileRepository>,
     pub import_session: Arc<dyn ImportSessionRepository>,

crates/application/src/import/get_mapping_stage.rs

@@ -1,16 +1,8 @@
-//! The mapping-page stage gate: a session must exist and have a `parsed_file`
-//! before its columns/sample rows can be shown for field mapping. Absorbs
-//! `handlers/import.rs::get_mapping_page`'s two early-return checks (session
-//! missing, `parsed_file` absent) — both collapse to `NotFound` here since the
-//! handler redirected to the same place (`/import`) for either.
-
 use domain::{errors::DomainError, value_objects::ImportSessionId};
 use uuid::Uuid;
 
 use crate::import::deps::GetMappingStageDeps;
 
-/// Cap on sample rows shown on the mapping page — was a bare `.take(5)` in the
-/// handler.
 pub const SAMPLE_ROW_LIMIT: usize = 5;
 
 pub struct MappingStage {

crates/application/src/import/get_preview_stage.rs

@@ -1,10 +1,3 @@
-//! The preview-page stage gate: a session must have `row_results` (i.e. a
-//! mapping has already been applied) before its rows can be previewed. Serves
-//! both the HTML preview handler and the API preview handler —
-//! `handlers/import.rs::get_preview_page` and `::api_get_preview` — which
-//! render/respond to `NotYetMapped` differently (redirect vs. status code); that
-//! decision stays in the handlers, not here.
-
 use domain::{
     errors::DomainError,
     models::AnnotatedRow,
@@ -14,15 +7,6 @@ use uuid::Uuid;
 
 use crate::import::deps::GetPreviewStageDeps;
 
-/// The columns and mapped/annotated rows for a session whose mapping has
-/// already been applied. `columns` comes from the session's `parsed_file` —
-/// the HTML preview template renders it as the table header — while `rows`
-/// comes from `row_results`. Not in the brief's `PreviewStage::Ready(Vec<AnnotatedRow>)`
-/// sketch: the deleted `get_preview_page` handler code read both
-/// `session.parsed_file.columns` and `session.row_results` to render the page,
-/// so dropping `columns` here would either blank the preview table's header or
-/// force the handler to re-fetch the session itself (forbidden — that's the
-/// exact repo call this task removes). See task-2 report for detail.
 pub struct PreviewRows {
     pub columns: Vec<String>,
     pub rows: Vec<AnnotatedRow>,

crates/application/src/import/tests/apply_mapping.rs

@@ -86,8 +86,6 @@ async fn fails_when_session_not_found() {
     assert!(result.is_err());
 }
 
-/// A document parser that returns rows with specific field values for testing
-/// the mark_duplicates logic.
 struct DuplicateTestParser {
     rows: Vec<ImportRow>,
 }

crates/application/src/insights/deps.rs

@@ -0,0 +1,7 @@
+use std::sync::Arc;
+
+use domain::ports::InsightsQuery;
+
+pub struct GetInsightsDeps {
+    pub insights: Arc<dyn InsightsQuery>,
+}

crates/application/src/insights/get.rs

@@ -0,0 +1,24 @@
+//! Live Insights for the requesting user (ADR-0010): fetch the whole diary as
+//! `WatchRow`s, run the pure analyzer, hand back whatever clears the honesty
+//! thresholds. No caching, no snapshots — the same on-the-fly shape as the
+//! existing live stats.
+
+use chrono::TimeZone;
+use domain::{errors::DomainError, models::Insight, value_objects::UserId};
+
+use crate::insights::deps::GetInsightsDeps;
+
+pub async fn execute<Tz: TimeZone + Sync>(
+    deps: &GetInsightsDeps,
+    user_id: UserId,
+    tz: &Tz,
+) -> Result<Vec<Insight>, DomainError> {
+    let rows = deps.insights.get_watch_rows(&user_id).await?;
+    Ok(domain::services::insights_analyzer::build_insights(
+        &rows, tz,
+    ))
+}
+
+#[cfg(test)]
+#[path = "tests/get.rs"]
+mod tests;

crates/application/src/insights/mod.rs

@@ -0,0 +1,2 @@
+pub mod deps;
+pub mod get;

crates/application/src/insights/tests/get.rs

@@ -0,0 +1,99 @@
+use std::sync::Arc;
+
+use chrono::NaiveDate;
+use uuid::Uuid;
+
+use domain::{models::insights::WatchRow, testing::InMemoryInsightsQuery, value_objects::UserId};
+
+use super::*;
+
+fn watch(genre: &str, rating: u8, day: u32, hour: u32) -> WatchRow {
+    WatchRow {
+        movie_id: Uuid::new_v4(),
+        rating,
+        watched_at: NaiveDate::from_ymd_opt(2024, 1, day)
+            .unwrap()
+            .and_hms_opt(hour, 0, 0)
+            .unwrap(),
+        watch_medium: Some("streaming".to_string()),
+        genres: vec![genre.to_string()],
+    }
+}
+
+#[tokio::test]
+async fn returns_analyzer_output_for_the_user() {
+    let user = Uuid::new_v4();
+    let rows: Vec<WatchRow> = (0..12)
+        .map(|i| watch("Thriller", 5, 1 + i, 22))
+        .chain((0..12).map(|i| watch("Comedy", 2, 13 + i, 22)))
+        .collect();
+    let deps = GetInsightsDeps {
+        insights: InMemoryInsightsQuery::with_rows(user, rows),
+    };
+
+    let insights = execute(&deps, UserId::from_uuid(user), &chrono::Utc)
+        .await
+        .unwrap();
+    assert!(insights.iter().any(
+        |i| matches!(i, domain::models::Insight::RatingLift { genre, .. } if genre == "Thriller")
+    ));
+}
+
+#[tokio::test]
+async fn other_users_rows_are_not_seen() {
+    let user = Uuid::new_v4();
+    let other = Uuid::new_v4();
+    let rows: Vec<WatchRow> = (0..12)
+        .map(|i| watch("Thriller", 5, 1 + i, 22))
+        .chain((0..12).map(|i| watch("Comedy", 2, 13 + i, 22)))
+        .collect();
+    let deps = GetInsightsDeps {
+        insights: InMemoryInsightsQuery::with_rows(other, rows),
+    };
+
+    let insights = execute(&deps, UserId::from_uuid(user), &chrono::Utc)
+        .await
+        .unwrap();
+    assert!(insights.is_empty());
+}
+
+#[tokio::test]
+async fn empty_diary_returns_empty_list() {
+    let user = Uuid::new_v4();
+    let deps = GetInsightsDeps {
+        insights: InMemoryInsightsQuery::new(),
+    };
+
+    let insights = execute(&deps, UserId::from_uuid(user), &chrono::Utc)
+        .await
+        .unwrap();
+    assert!(insights.is_empty());
+}
+
+#[tokio::test]
+async fn port_errors_propagate() {
+    struct Failing;
+    #[async_trait::async_trait]
+    impl domain::ports::InsightsQuery for Failing {
+        async fn get_watch_rows(
+            &self,
+            _: &UserId,
+        ) -> Result<Vec<WatchRow>, domain::errors::DomainError> {
+            Err(domain::errors::DomainError::InfrastructureError(
+                "db down".into(),
+            ))
+        }
+    }
+
+    let deps = GetInsightsDeps {
+        insights: Arc::new(Failing),
+    };
+
+    let err = execute(&deps, UserId::from_uuid(Uuid::new_v4()), &chrono::Utc)
+        .await
+        .unwrap_err();
+    assert!(matches!(
+        err,
+        domain::errors::DomainError::InfrastructureError(_)
+    ));
+}

crates/application/src/lib.rs

@@ -9,6 +9,7 @@ pub mod auth;
 pub mod diary;
 pub mod goals;
 pub mod import;
+pub mod insights;
 pub mod integrations;
 pub mod movies;
 pub mod person;

crates/application/src/movies/discovery_indexer.rs

@@ -8,9 +8,6 @@ use domain::{
     ports::{EventHandler, MovieQuery, SearchCommand},
 };
 
-/// Reacts to `MovieDiscovered` and inserts a bare search index entry immediately,
-/// so movies are findable before TMDb enrichment runs.
-/// Enrichment will later overwrite this with the full document (cast, genres, etc.).
 pub struct MovieDiscoveryIndexer {
     movie_repository: Arc<dyn MovieQuery>,
     search_command: Arc<dyn SearchCommand>,

crates/application/src/movies/enrich_movie.rs

@@ -8,22 +8,18 @@ use domain::{
 use crate::movies::{commands::EnrichMovieCommand, deps::EnrichMovieDeps};
 
 pub async fn execute(deps: &EnrichMovieDeps, cmd: EnrichMovieCommand) -> Result<(), DomainError> {
-    // 1. Persist the enriched profile (also handles movie_cast, movie_crew, genres, keywords)
     deps.movie_profile.upsert(&cmd.profile).await?;
 
-    // 2. Upsert persons extracted from cast + crew (no reads — only upsert)
     let persons = extract_persons(&cmd.profile.cast, &cmd.profile.crew);
     if !persons.is_empty() {
         deps.person_command.upsert_batch(&persons).await?;
     }
 
-    // 3. Fetch the movie for the search index document
     let Some(movie) = deps.movie_query.get_movie_by_id(&cmd.movie_id).await? else {
         tracing::warn!(movie_id = %cmd.movie_id.value(), "enrich_movie: movie not found after profile upsert");
         return Ok(());
     };
 
-    // 4. Index the movie in search
     deps.search_command
         .index(IndexableDocument::Movie {
             id: cmd.movie_id.clone(),
@@ -32,7 +28,6 @@ pub async fn execute(deps: &EnrichMovieDeps, cmd: EnrichMovieCommand) -> Result<
         })
         .await?;
 
-    // 5. Index each unique person in search (no reads — persons built from in-memory data)
     for person in &persons {
         deps.search_command
             .index(IndexableDocument::Person {
@@ -50,9 +45,6 @@ pub async fn execute(deps: &EnrichMovieDeps, cmd: EnrichMovieCommand) -> Result<
     Ok(())
 }
 
-/// Build unique Person values from cast and crew.
-/// Uses deterministic UUIDv5 so the same tmdb_person_id always maps to the same PersonId.
-/// No DB reads — persons are built entirely from in-memory TMDb data.
 fn extract_persons(cast: &[CastMember], crew: &[CrewMember]) -> Vec<Person> {
     let mut seen: HashMap<u64, Person> = HashMap::new();
 

crates/application/src/movies/resolve.rs

@@ -9,9 +9,6 @@ use domain::{
 use crate::diary::commands::MovieInput;
 use crate::diary::movie_resolver::{MovieResolver, MovieResolverDeps};
 
-/// Resolves a movie from input, persists it, and publishes `MovieDiscovered` if new.
-///
-/// Returns `(movie, is_new_movie)`.
 pub async fn resolve_and_persist_movie(
     input: &MovieInput,
     movie_command: &dyn MovieCommand,

crates/application/src/services.rs

@@ -7,9 +7,6 @@ use domain::ports::{
 
 use crate::ports::ReviewLogger;
 
-/// Services the application layer needs, assembled by the composition root.
-/// Adapter-typed ports do not belong here — the AP port inversion removed the
-/// last of them; see ADR-0008.
 #[derive(Clone)]
 pub struct Services {
     pub auth: Arc<dyn AuthService>,

crates/application/src/social/local_service.rs

@@ -13,13 +13,6 @@ use domain::{
     },
 };
 
-/// The subset of `SocialCommand`/`FollowGraphQuery`/`BlockQuery` that a single
-/// instance can serve with only its own database — no ActivityPub involved.
-///
-/// This is the local half of `activitypub::CompositeSocialAdapter`, extracted
-/// here so `application` needs no dependency on the `activitypub` crate to
-/// offer it. It implements `domain::ports::LocalSocial` via that trait's
-/// blanket impl over the five ports below.
 pub struct LocalSocialService {
     user_repo: Arc<dyn UserRepository>,
     follow_command: Arc<dyn FollowCommand>,

crates/application/src/tests/services.rs

@@ -6,10 +6,6 @@ use domain::testing::{
 };
 use std::sync::Arc;
 
-/// Services must be constructible from domain ports alone — no adapter types.
-/// If this file ever needs an adapter crate import, the boundary has regressed.
-/// Asserts on Clone specifically: AppState is Clone, so a Services that clones
-/// its Arcs by value instead of sharing them would silently duplicate state.
 #[test]
 fn services_clone_shares_the_same_port_instances() {
     let logger: Arc<dyn crate::ports::ReviewLogger> = Arc::new(NoopReviewLogger);

crates/application/src/users/authorize_admin.rs

@@ -5,15 +5,6 @@ use uuid::Uuid;
 
 use crate::users::deps::AuthorizeAdminDeps;
 
-/// Whether `user_id` is an admin, for the two extractors (`AdminApiUser`,
-/// `AdminUser`) that gate admin-only routes.
-///
-/// Returns `Ok(None)` when the user row does not exist — a distinct success
-/// value, not folded into `DomainError::NotFound`, specifically so callers can
-/// tell "row missing" apart from "repository call failed" without depending on
-/// which `DomainError` variant a lookup failure happens to produce. Each
-/// extractor rejects those two cases differently (missing row is a 401/404,
-/// a repository error is a 500) and that distinction must stay exact.
 pub async fn execute(
     deps: &AuthorizeAdminDeps,
     user_id: Uuid,

crates/application/src/users/deps.rs

@@ -7,9 +7,6 @@ use domain::ports::{
 };
 use domain::value_objects::InstanceIdentity;
 
-/// The local half of the former `GetProfileDeps` split (ADR-0004). `user` and
-/// `instance` build the always-populated `ProfileIdentity`; `stats`/`diary`/
-/// `social_query` are unchanged from before the split.
 pub struct GetLocalProfileDeps {
     pub stats: Arc<dyn StatsRepository>,
     pub diary: Arc<dyn DiaryQuery>,
@@ -18,18 +15,12 @@ pub struct GetLocalProfileDeps {
     pub instance: InstanceIdentity,
 }
 
-/// The federated half of the former `GetProfileDeps` split. No `user`/`instance` —
-/// the federated handler builds identity from the resolved remote actor and never
-/// reads a local user row for this path.
 pub struct GetFederatedProfileStatsDeps {
     pub stats: Arc<dyn StatsRepository>,
     pub diary: Arc<dyn DiaryQuery>,
     pub social_query: Arc<dyn FollowGraphQuery>,
 }
 
-/// Backs page-chrome data (email/role badge, pending-follow badge). See the error
-/// policy note on `get_page_viewer::execute`: this deliberately propagates errors
-/// rather than degrading internally — the caller decides chrome degrades.
 pub struct GetPageViewerDeps {
     pub user: Arc<dyn UserRepository>,
     pub follow_graph: Arc<dyn FollowGraphQuery>,
@@ -82,9 +73,6 @@ pub struct AuthorizeAdminDeps {
     pub user: Arc<dyn UserRepository>,
 }
 
-/// `Option` mirrors `Repositories::federated_profile` — genuine optional
-/// configuration (federation on/off), not a container-shape workaround. Same
-/// exemption as `Services::person_enrichment`.
 pub struct GetFederatedProfileDeps {
     pub federated_profile: Option<Arc<dyn FederatedProfileQuery>>,
 }

crates/application/src/users/get_federated_profile.rs

@@ -4,18 +4,6 @@ use uuid::Uuid;
 
 use crate::users::deps::GetFederatedProfileDeps;
 
-/// Looks up a federated (remote) profile by synthetic user id — the fallback
-/// path `get_user_profile` (`handlers/users.rs`) takes when the local-profile
-/// lookup comes back `NotFound`.
-///
-/// Federation being disabled (`deps.federated_profile` is `None`) collapses to
-/// `Ok(None)`, the same value a present-but-empty lookup would return. That's
-/// deliberate: the handler's `if let Ok(Some(fed)) = ...` treats "port absent",
-/// "call returned `Ok(None)`", and "call returned `Err`" identically — all three
-/// fall through to a 404 — and this use case must not make any of those three
-/// distinguishable to a caller that cannot act on the difference. An `Err` from
-/// the port is propagated unchanged; the swallow stays at the handler, exactly
-/// where it already was.
 pub async fn execute(
     deps: &GetFederatedProfileDeps,
     user_id: Uuid,

crates/application/src/users/get_local_profile.rs

@@ -30,10 +30,6 @@ pub struct ProfileIdentity {
     pub actor_url: String,
     pub avatar_url: Option<String>,
     pub banner_url: Option<String>,
-    /// Only the HTML profile handler reads this (it derives its "display name"
-    /// from the email's local part, not from `username` — see
-    /// `handlers/users.rs::get_user_profile_html`). Sourced from the same local
-    /// `User` row as every other identity field.
     pub email: String,
 }
 

crates/application/src/users/get_profile_settings.rs

@@ -1,12 +1,3 @@
-//! Absorbs BOTH of `handlers/users.rs::get_profile_settings`'s repository calls:
-//! the user lookup (`repos.user.find_by_id`) and the profile_fields join
-//! (`repos.profile_fields.get_fields`) — plus the avatar/banner URL derivation via
-//! `InstanceIdentity`, following the same pattern `get_local_profile` uses.
-//!
-//! Error policy: profile fields are primary content for this page (it's the page
-//! whose entire job is editing them), not page chrome, so a failed join
-//! propagates as `Err` rather than silently degrading to an empty list.
-
 use domain::{errors::DomainError, value_objects::UserId};
 use uuid::Uuid;
 
@@ -18,10 +9,6 @@ pub struct ProfileSettings {
     pub bio: Option<String>,
     pub avatar_url: Option<String>,
     pub banner_url: Option<String>,
-    /// Not in the brief's interface sketch, but the deleted handler code read
-    /// `user.also_known_as()` and rendered it in `ProfileSettingsTemplate` — this
-    /// use case absorbs that read too, or the settings page would silently stop
-    /// showing it.
     pub also_known_as: Option<String>,
     pub fields: Vec<(String, String)>,
 }

crates/application/src/users/tests/get_federated_profile_stats.rs

@@ -3,14 +3,6 @@ use crate::users::deps::GetFederatedProfileStatsDeps;
 use crate::users::get_federated_profile_stats;
 use crate::users::queries::{GetUserProfileQuery, ProfileView};
 
-/// Mirrors exactly how `build_federated_profile_response`
-/// (`crates/presentation/src/handlers/users.rs`) calls `execute`: a `user_id` that
-/// has no row in the local `users` table (a federated/remote profile uses a
-/// synthetic id — see `FederatedProfileQuery`), `is_own_profile: false`,
-/// `include_remote: true`. Before ADR-0004's split this was the tolerant branch of
-/// the unified `get_profile`; now it's simply this function's only behavior — it
-/// never looks at the local `users` table at all, so there is nothing to be
-/// tolerant about.
 #[tokio::test]
 async fn succeeds_for_a_user_id_with_no_local_row() {
     let b = TestContextBuilder::new();

crates/application/src/users/tests/get_local_profile.rs

@@ -54,10 +54,6 @@ fn unknown_query(user_id: uuid::Uuid) -> GetUserProfileQuery {
     }
 }
 
-/// `get_local_profile` is the local-only half of the former `get_profile` split
-/// (ADR-0004's closed wart). Unlike the old tolerant behavior, a user id with no
-/// local row must be `NotFound` here — the empty-string identity sentinel this
-/// replaces cannot be represented in `LocalProfileData` at all.
 #[tokio::test]
 async fn get_local_profile_is_not_found_for_unknown_user() {
     let b = TestContextBuilder::new();
@@ -272,10 +268,6 @@ async fn populates_handle_and_actor_url_for_a_local_user() {
     let user = user_repo.find_by_email(&email).await.unwrap().unwrap();
     let uid = user.id().value();
 
-    // `InMemoryUserRepository::update_profile` now actually persists (previously a
-    // no-op stub) — use it to give this test a real display_name/bio to assert,
-    // rather than asserting `None` for a mapping that could just as easily be
-    // wrong, inverted, or hardcoded.
     let update_deps = UpdateProfileDeps {
         user: user_repo.clone(),
         object_storage: b.object_storage.clone(),

crates/application/src/users/tests/get_page_viewer.rs

@@ -34,9 +34,6 @@ async fn setup_user(b: &TestContextBuilder, email: &str, username: &str) -> uuid
     user.id().value()
 }
 
-/// `get_page_viewer` absorbs `helpers.rs::build_page_context`'s inline pending-count
-/// computation. It must reuse `FollowGraphQuery::count_pending_followers` rather than
-/// re-deriving the count from raw follow rows.
 #[tokio::test]
 async fn get_page_viewer_reports_pending_follower_count() {
     let b = TestContextBuilder::new();

crates/application/src/users/tests/get_profile_settings.rs

@@ -8,10 +8,6 @@ use crate::test_helpers::TestContextBuilder;
 use crate::users::deps::GetProfileSettingsDeps;
 use crate::users::get_profile_settings;
 
-/// `get_profile_settings` absorbs BOTH of the handler's calls: the user lookup
-/// (`repos.user.find_by_id`) and the profile_fields join
-/// (`repos.profile_fields.get_fields`). This proves the join actually moved in,
-/// not just the user lookup.
 #[tokio::test]
 async fn get_profile_settings_includes_profile_fields() {
     let b = TestContextBuilder::new();

crates/application/src/users/tests/resolve_username_to_id.rs

@@ -4,9 +4,6 @@ use crate::test_helpers::TestContextBuilder;
 use crate::users::deps::ResolveUsernameDeps;
 use crate::users::resolve_username_to_id;
 
-/// `resolve_username_to_id` absorbs `handlers/users.rs::get_user_by_username`'s
-/// `repos.user.find_by_username()` call. Unknown usernames must resolve to `None`,
-/// not an error — the handler turns `None` into a 404, same as today.
 #[tokio::test]
 async fn resolve_username_to_id_returns_none_for_unknown() {
     let b = TestContextBuilder::new();

crates/application/src/watchlist/get_watchlist_for_owner.rs

@@ -9,22 +9,11 @@ use crate::watchlist::deps::{GetWatchlistDeps, GetWatchlistForOwnerDeps};
 use crate::watchlist::get;
 use crate::watchlist::queries::GetWatchlistQuery;
 
-/// Which data source answered a watchlist page request. The former handler
-/// decided this by probing for a local user row — that decision now lives here,
-/// not in `handlers/watchlist.rs`.
 pub enum WatchlistView {
     Local(Paginated<WatchlistWithMovie>),
     Remote(Vec<RemoteWatchlistEntry>),
 }
 
-/// `limit`/`offset` are taken raw, not as a pre-built `PageParams`, on purpose:
-/// they're only validated on the local branch, by delegating to
-/// `watchlist::get::execute` (whose `PageParams::new` call is where the validation
-/// actually happens). The remote branch ignores them entirely, unconditionally
-/// fetching the full federated watchlist. The delegation call below must stay
-/// inside the `if is_local` branch — calling it before the local/remote decision
-/// would turn an out-of-range `limit` into a 400 on a remote owner's page, which
-/// never happened before and must not start happening now.
 pub async fn execute(
     deps: &GetWatchlistForOwnerDeps,
     owner_id: Uuid,
@@ -33,9 +22,6 @@ pub async fn execute(
 ) -> Result<WatchlistView, DomainError> {
     let user_id = UserId::from_uuid(owner_id);
 
-    // Matches the deleted handler's `.map(|u| u.is_some()).unwrap_or(false)`
-    // exactly: a lookup error is treated the same as "no local row", falling
-    // through to the remote arm rather than propagating.
     let is_local = deps
         .user
         .find_by_id(&user_id)
@@ -58,8 +44,6 @@ pub async fn execute(
         .await?;
         Ok(WatchlistView::Local(paginated))
     } else {
-        // Matches the deleted handler's `.unwrap_or_default()`: a federation
-        // lookup error yields an empty list, not a propagated error.
         let entries = deps
             .remote_watchlist
             .get_by_derived_uuid(owner_id)

crates/application/src/watchlist/tests/get_watchlist_for_owner.rs

@@ -44,8 +44,6 @@ async fn watchlist_for_owner_returns_local_when_user_row_exists() {
     let user = b.user_repo.find_by_email(&email).await.unwrap().unwrap();
     let uid = user.id().value();
 
-    // If the local-row decision were wrong and this reached the remote arm, the
-    // panic port below would fail the test instead of silently passing.
     let deps = GetWatchlistForOwnerDeps {
         user: b.user_repo.clone(),
         watchlist: InMemoryWatchlistRepository::new(),
@@ -75,9 +73,6 @@ async fn watchlist_for_owner_returns_remote_when_no_local_row() {
         },
     ]);
 
-    // No user row exists for `owner_id` in this fresh, empty user repo — the
-    // local-vs-remote decision must fall through to the remote arm. The panic
-    // port below proves the local arm is never touched.
     let deps = GetWatchlistForOwnerDeps {
         user: InMemoryUserRepository::new(),
         watchlist: Arc::new(PanicWatchlistRepository),
@@ -97,10 +92,6 @@ async fn watchlist_for_owner_returns_remote_when_no_local_row() {
     }
 }
 
-/// Pins a pre-existing quirk: the old handler validated `limit`/`offset` only on
-/// the local branch (inside `watchlist::get::execute`'s `PageParams::new` call) —
-/// the remote branch never looked at them at all. An out-of-range limit must
-/// still surface as a validation error on the local arm.
 #[tokio::test]
 async fn watchlist_for_owner_local_arm_validates_limit() {
     let b = TestContextBuilder::new();
@@ -126,9 +117,6 @@ async fn watchlist_for_owner_local_arm_validates_limit() {
     ));
 }
 
-/// Same quirk, other direction: an out-of-range limit is silently ignored on the
-/// remote arm, exactly as before — it must not turn into a validation error just
-/// because the use case now owns the branch decision.
 #[tokio::test]
 async fn watchlist_for_owner_remote_arm_ignores_invalid_limit() {
     let owner_id = Uuid::new_v4();

crates/application/src/wrapup/get_ready_report.rs

@@ -6,23 +6,6 @@ use domain::{
 
 use crate::wrapup::deps::GetReadyReportDeps;
 
-/// Absorbs the year -> date-range conversion and the `status == Ready` +
-/// `report.is_some()` gating both HTML wrap-up handlers used to do by hand.
-///
-/// Takes `domain::models::wrapup::WrapUpScope` directly (the same type
-/// `wrapup::handle_requested` already matches on) rather than a second,
-/// use-case-local `WrapUpScope` — there is exactly one concept here, one type.
-/// `None` can no longer be silently passed for "global": callers must pick
-/// `WrapUpScope::User(uid)` or `WrapUpScope::Global` explicitly.
-///
-/// Error shape mirrors the deleted handlers exactly:
-/// - An unrepresentable `year` (can't form a valid `NaiveDate`) is
-///   `ValidationError` — the handlers used this to return a bare 400.
-/// - Everything else — no record, a record whose `status` isn't `Ready`, a
-///   `Ready` record with `report: None`, or even a repository error — collapses
-///   to `NotFound`, matching the old handlers' `_ => return StatusCode::NOT_FOUND`
-///   catch-all (which really did swallow repo errors into a 404, not just the
-///   "not ready yet" cases; preserved here rather than tidied).
 pub async fn execute(
     deps: &GetReadyReportDeps,
     scope: WrapUpScope,

crates/application/src/wrapup/tests/get_ready_report.rs

@@ -133,10 +133,6 @@ async fn ready_report_global_scope_ignores_user_id() {
         .unwrap();
 }
 
-/// The old handlers guarded `NaiveDate::from_ymd_opt` up front and returned a bare
-/// 400 for a year that can't form a valid date — that guard moved into the use
-/// case along with the rest of the year->date-range conversion. A distinguishable
-/// error (ValidationError, not NotFound) lets the handler still map it to 400.
 #[tokio::test]
 async fn ready_report_is_validation_error_for_unrepresentable_year() {
     let repo = InMemoryWrapUpRepository::new();